Skip to content

Commit d9092b3

Browse files
DeepfreezechillBrian KrafftCopilot
authored
Epic 6.1: Observability - Metrics, Tracing, Health (#50)
* feat: Epic 6.1 — Observability (metrics, tracing, health) Add openspace/observability/ package with Prometheus metrics, structured tracing, and health aggregation. Instrument grounding execution loop. Add 3 MCP tools: health_check, get_metrics, get_execution_traces. - MetricsRegistry: 10 Prometheus metrics (execution, skill, evolution, tool) - ExecutionTracer: span trees with ring buffer, trace_async decorator - HealthAggregator: probe-based status (healthy/degraded/unhealthy) - 48 new observability tests, 1813 total passed - Update test expectations for 7 MCP tools (was 4) - Add prometheus-client to requirements.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address 5-reviewer findings for Epic 6.1 Security: - Hash instruction in trace attributes (no raw text leak) - Sanitize error returns: type(e).__name__ instead of str(e) - Remove raw instruction from error dict Correctness: - Move trace/metrics inside try block (prevent gauge/trace leak) - Add execution_latency.observe() on both success and error paths - Fix trace_async to set root span status='error' on exceptions - Preserve span status in finish_trace() (don't overwrite error→ok) Thread safety: - Add threading.Lock to ExecutionTracer ring buffer - Switch ring buffer from list to collections.deque(maxlen=N) Bounds: - Add max_spans_per_trace (500) to prevent unbounded span growth - Add _sanitize_label helper for Prometheus label values Health: - Cache SkillStore in health probe (no new instance per check) - Remove hardcoded tool count from health probe detail Tests (14 new, 1827 total passed): - 3 untested metrics: execution_iterations, skill_misses, skill_search_latency - In-flight gauge lifecycle (inc/dec on success + error) - track_execution latency recording - trace_async child-span path + error status propagation - Thread safety concurrent finish_trace - Health: 50% boundary, probe exception, unregister noop - Span auto-create trace, finish_trace with no active trace - Rename stale test_registers_four_tools → test_registers_seven_tools Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix: address R2 adversarial findings — isolate observability from execution Critical (P0→fixed): - Observability errors can no longer discard valid execution results (all metric/trace calls wrapped in isolated try/except blocks) - Import-time crash guard: graceful fallback if prometheus_client missing - _gauge_incremented flag prevents negative gauge values Security: - Strict label sanitization: regex [a-zA-Z0-9_-]{1,32} with unknown fallback - Health probe errors use opaque 'not available' (no type(e).__name__ leak) - Health probe metadata removed (no internal skill count exposure) Reliability: - Health probes now timeout after 5s via ThreadPoolExecutor - start_trace() auto-finishes orphaned active trace (re-entrancy safe) - Span.finish() is idempotent (no double-finish corruption) - max_spans guard returns sentinel span (no parent pollution) Tests: 1,828 passed (+1 probe timeout test) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Brian Krafft <bkrafft@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8653cb9 commit d9092b3

15 files changed

Lines changed: 1452 additions & 40 deletions

RUNBOOK.yaml

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
schema_version: '1.0'
22
lock:
3-
active_claim: '5.11'
3+
active_claim: '6.1'
44
claimed_by: 2026-04-06-e9b4572b
5-
claimed_at: '2026-04-06T21:30:00Z'
5+
claimed_at: '2026-04-06T23:24:00Z'
66
review:
77
max_rounds: 3
88
required_gates:
@@ -352,9 +352,9 @@ epics:
352352
- id: '5.1'
353353
phase: P5
354354
title: Extract EvolutionModels
355-
status: pending
355+
status: done
356356
branch: null
357-
pr: null
357+
pr: 39
358358
review_round: 0
359359
source_file: openspace/skill_engine/evolver.py
360360
target_module: openspace/skill_engine/evolution/models.py
@@ -368,9 +368,9 @@ epics:
368368
- id: '5.2'
369369
phase: P5
370370
title: Extract EvolutionOrchestrator
371-
status: pending
371+
status: done
372372
branch: null
373-
pr: null
373+
pr: 40
374374
review_round: 0
375375
source_file: openspace/skill_engine/evolver.py
376376
target_module: openspace/skill_engine/evolution/orchestrator.py
@@ -384,9 +384,9 @@ epics:
384384
- id: '5.3'
385385
phase: P5
386386
title: Extract AnalysisEvolution + ToolDegradation + MetricMonitor
387-
status: pending
387+
status: done
388388
branch: null
389-
pr: null
389+
pr: 41
390390
review_round: 0
391391
source_file: openspace/skill_engine/evolver.py
392392
target_module: openspace/skill_engine/evolution/triggers/
@@ -400,9 +400,9 @@ epics:
400400
- id: '5.4'
401401
phase: P5
402402
title: Extract EvolutionConfirmation
403-
status: pending
403+
status: done
404404
branch: null
405-
pr: null
405+
pr: 42
406406
review_round: 0
407407
source_file: openspace/skill_engine/evolver.py
408408
target_module: openspace/skill_engine/evolution/confirmation.py
@@ -416,9 +416,9 @@ epics:
416416
- id: '5.5'
417417
phase: P5
418418
title: Extract FixEngine + DerivedCapturedEngine
419-
status: pending
419+
status: done
420420
branch: null
421-
pr: null
421+
pr: 43
422422
review_round: 0
423423
source_file: openspace/skill_engine/evolver.py
424424
target_module: openspace/skill_engine/evolution/engines/
@@ -432,9 +432,9 @@ epics:
432432
- id: '5.6'
433433
phase: P5
434434
title: "P5a integration \xE2\u20AC\u201D evolver.py deletion"
435-
status: pending
435+
status: done
436436
branch: null
437-
pr: null
437+
pr: 44
438438
review_round: 0
439439
source_file: openspace/skill_engine/evolver.py
440440
target_module: null
@@ -449,9 +449,9 @@ epics:
449449
- id: '5.7'
450450
phase: P5
451451
title: Extract SkillContextManager + MessageSafety
452-
status: pending
452+
status: done
453453
branch: null
454-
pr: null
454+
pr: 45
455455
review_round: 0
456456
source_file: openspace/agents/grounding_agent.py
457457
target_module: openspace/agents/grounding/
@@ -465,9 +465,9 @@ epics:
465465
- id: '5.8'
466466
phase: P5
467467
title: Extract AgentExecutionLoop + PromptBuilder
468-
status: pending
468+
status: done
469469
branch: null
470-
pr: null
470+
pr: 46
471471
review_round: 0
472472
source_file: openspace/agents/grounding_agent.py
473473
target_module: openspace/agents/grounding/
@@ -481,9 +481,9 @@ epics:
481481
- id: '5.9'
482482
phase: P5
483483
title: Extract ToolCatalog + VisualAnalysis + WorkspaceInspector
484-
status: pending
484+
status: done
485485
branch: null
486-
pr: null
486+
pr: 47
487487
review_round: 0
488488
source_file: openspace/agents/grounding_agent.py
489489
target_module: openspace/agents/grounding/
@@ -497,9 +497,9 @@ epics:
497497
- id: '5.10'
498498
phase: P5
499499
title: Extract ResultFormatter + Telemetry
500-
status: pending
500+
status: done
501501
branch: null
502-
pr: null
502+
pr: 48
503503
review_round: 0
504504
source_file: openspace/agents/grounding_agent.py
505505
target_module: openspace/agents/grounding/result_telemetry.py
@@ -513,9 +513,9 @@ epics:
513513
- id: '5.11'
514514
phase: P5
515515
title: "P5b integration \xE2\u20AC\u201D grounding_agent.py deletion"
516-
status: claimed
516+
status: done
517517
branch: null
518-
pr: null
518+
pr: 49
519519
review_round: 0
520520
source_file: openspace/agents/grounding_agent.py
521521
target_module: null
@@ -531,7 +531,7 @@ epics:
531531
- id: '6.1'
532532
phase: P6
533533
title: Observability (metrics, tracing, health)
534-
status: pending
534+
status: claimed
535535
branch: null
536536
pr: null
537537
review_round: 0

openspace/agents/grounding/execution.py

Lines changed: 80 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,22 @@
33
Implements the multi-round LLM iteration with tool calling,
44
skill-context stripping, message truncation, and result building.
55
Extracted from grounding_agent.py (Epic 5.8).
6+
Instrumented with observability (Epic 6.1).
67
"""
78

89
from __future__ import annotations
910

1011
import copy
12+
import hashlib
13+
import time
1114
from typing import Any, Dict, List, Optional
1215

16+
try:
17+
from openspace.observability.metrics import metrics as _metrics
18+
from openspace.observability.tracing import tracer as _tracer
19+
except ImportError:
20+
_metrics = None # type: ignore[assignment]
21+
_tracer = None # type: ignore[assignment]
1322
from openspace.prompts import GroundingAgentPrompts
1423
from openspace.utils.logging import Logger
1524

@@ -47,6 +56,10 @@ async def process(agent, context: Dict[str, Any]) -> Dict[str, Any]:
4756

4857
logger.info(f"Grounding Agent: Processing instruction at step {agent.step}")
4958

59+
# ── Observability: agent name sanitized for Prometheus labels ─────
60+
_agent_label = _sanitize_label(agent._name)
61+
_exec_start = time.monotonic()
62+
5063
# Existing workspace files check
5164
workspace_info = await agent._check_workspace_artifacts(context)
5265
if workspace_info["has_files"]:
@@ -96,6 +109,20 @@ async def process(agent, context: Dict[str, Any]) -> Dict[str, Any]:
96109
tools=tools,
97110
)
98111

112+
# ── Observability setup (isolated — must not kill execution) ──────
113+
_gauge_incremented = False
114+
try:
115+
_instr_hash = hashlib.sha256(instruction.encode()).hexdigest()[:12]
116+
_tracer.start_trace(
117+
"grounding.process",
118+
instruction_hash=_instr_hash,
119+
instruction_length=len(instruction),
120+
)
121+
_metrics.execution_in_flight.labels(agent=_agent_label).inc()
122+
_gauge_incremented = True
123+
except Exception:
124+
logger.debug("Observability setup failed", exc_info=True)
125+
99126
try:
100127
while current_iteration < max_iterations:
101128
current_iteration += 1
@@ -274,14 +301,47 @@ async def process(agent, context: Dict[str, Any]) -> Dict[str, Any]:
274301
f"Grounding Agent: Execution completed with status: "
275302
f"{result.get('status')}"
276303
)
304+
305+
# ── Observability: record success (isolated — must not discard result)
306+
try:
307+
_elapsed = time.monotonic() - _exec_start
308+
_metrics.execution_latency.labels(agent=_agent_label, status="success").observe(_elapsed)
309+
_metrics.execution_iterations.labels(agent=_agent_label).observe(current_iteration)
310+
_metrics.execution_total.labels(agent=_agent_label, status="success").inc()
311+
if _gauge_incremented:
312+
_metrics.execution_in_flight.labels(agent=_agent_label).dec()
313+
_gauge_incremented = False
314+
root_span = _tracer.current_span()
315+
if root_span:
316+
root_span.attributes["iterations"] = current_iteration
317+
root_span.attributes["status"] = result.get("status", "unknown")
318+
_tracer.finish_trace()
319+
except Exception:
320+
logger.debug("Observability recording failed", exc_info=True)
321+
277322
return result
278323

279324
except Exception as e:
280325
logger.error(f"Grounding Agent: Execution failed: {e}")
326+
327+
# ── Observability: record error (isolated — must not mask result)
328+
try:
329+
_elapsed = time.monotonic() - _exec_start
330+
_metrics.execution_latency.labels(agent=_agent_label, status="error").observe(_elapsed)
331+
_metrics.execution_total.labels(agent=_agent_label, status="error").inc()
332+
if _gauge_incremented:
333+
_metrics.execution_in_flight.labels(agent=_agent_label).dec()
334+
root_span = _tracer.current_span()
335+
if root_span:
336+
root_span.add_event("error", error_type=type(e).__name__)
337+
root_span.status = "error"
338+
_tracer.finish_trace()
339+
except Exception:
340+
logger.debug("Observability recording failed", exc_info=True)
341+
281342
result = {
282-
"error": str(e),
343+
"error": type(e).__name__,
283344
"status": "error",
284-
"instruction": instruction,
285345
"iteration": current_iteration,
286346
}
287347
agent.increment_step()
@@ -323,3 +383,21 @@ def _build_retrieved_tools_list(
323383

324384
retrieved_tools_list.append(tool_info)
325385
return retrieved_tools_list
386+
387+
388+
import re
389+
390+
# ── Label sanitization ───────────────────────────────────────────────
391+
392+
# Prometheus label values must be bounded and safe. Strict allowlist
393+
# prevents cardinality bombs and scraper injection.
394+
_LABEL_MAX_LEN = 32
395+
_LABEL_SAFE_RE = re.compile(r"[^a-zA-Z0-9_\-]")
396+
397+
398+
def _sanitize_label(value: str) -> str:
399+
"""Sanitize a string for use as a Prometheus label value."""
400+
if not value:
401+
return "unknown"
402+
clean = _LABEL_SAFE_RE.sub("_", value[:_LABEL_MAX_LEN])
403+
return clean or "unknown"

openspace/mcp/server.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,9 +143,45 @@ def create_mcp_app():
143143

144144
app = FastMCP("OpenSpace", **kwargs)
145145
register_handlers(app)
146+
147+
# Register default health probes (Epic 6.1)
148+
_register_health_probes()
149+
146150
return app
147151

148152

153+
def _register_health_probes() -> None:
154+
"""Register default health probes for core subsystems."""
155+
from openspace.observability.health import HealthProbe, health
156+
157+
def _probe_skill_store() -> HealthProbe:
158+
try:
159+
from openspace.skill_engine.store import SkillStore
160+
161+
if not hasattr(_probe_skill_store, "_cached"):
162+
_probe_skill_store._cached = SkillStore()
163+
store = _probe_skill_store._cached
164+
count = len(store.list_skills()) if hasattr(store, "list_skills") else 0
165+
return HealthProbe(ok=True, detail=f"{count} skills")
166+
except Exception:
167+
return HealthProbe(ok=False, detail="not available")
168+
169+
def _probe_grounding() -> HealthProbe:
170+
try:
171+
from openspace.agents.grounding_agent import GroundingAgent
172+
173+
return HealthProbe(ok=True, detail="module loaded")
174+
except Exception:
175+
return HealthProbe(ok=False, detail="not available")
176+
177+
def _probe_mcp_tools() -> HealthProbe:
178+
return HealthProbe(ok=True, detail="tools registered")
179+
180+
health.register("skill_store", _probe_skill_store)
181+
health.register("grounding_engine", _probe_grounding)
182+
health.register("mcp_tools", _probe_mcp_tools)
183+
184+
149185
# ---------------------------------------------------------------------------
150186
# 4. Server entry point
151187
# ---------------------------------------------------------------------------

openspace/mcp/tool_handlers.py

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -785,6 +785,50 @@ async def upload_skill(
785785
return handle_mcp_exception(e, tool_name="upload_skill", error_code=EXECUTION_ERROR)
786786

787787

788+
# ---------------------------------------------------------------------------
789+
# Observability tools (Epic 6.1)
790+
# ---------------------------------------------------------------------------
791+
792+
793+
async def health_check() -> str:
794+
"""Check OpenSpace system health.
795+
796+
Returns structured health status including all registered subsystem
797+
probes (skill store, LLM connectivity, grounding engine, etc.).
798+
"""
799+
from openspace.observability.health import health
800+
801+
result = health.check()
802+
return json.dumps(result, indent=2)
803+
804+
805+
async def get_metrics() -> str:
806+
"""Get Prometheus-compatible metrics.
807+
808+
Returns all OpenSpace metrics in Prometheus exposition format, suitable
809+
for scraping by Prometheus, Grafana Agent, or similar tools.
810+
"""
811+
from openspace.observability.metrics import metrics
812+
813+
return metrics.render().decode("utf-8")
814+
815+
816+
async def get_execution_traces(limit: int = 5) -> str:
817+
"""Get recent execution traces for debugging.
818+
819+
Returns the last N execution traces with span trees, timings, and
820+
metadata for debugging multi-step grounding workflows.
821+
822+
Args:
823+
limit: Maximum number of traces to return (default: 5, max: 50)
824+
"""
825+
from openspace.observability.tracing import tracer
826+
827+
limit = min(max(1, limit), 50)
828+
traces = tracer.recent_traces[-limit:]
829+
return json.dumps([t.to_dict() for t in traces], indent=2, default=str)
830+
831+
788832
# ---------------------------------------------------------------------------
789833
# Registration
790834
# ---------------------------------------------------------------------------
@@ -797,3 +841,7 @@ def register_handlers(mcp) -> None:
797841
mcp.tool()(search_skills)
798842
mcp.tool()(fix_skill)
799843
mcp.tool()(upload_skill)
844+
# Observability (Epic 6.1)
845+
mcp.tool()(health_check)
846+
mcp.tool()(get_metrics)
847+
mcp.tool()(get_execution_traces)

0 commit comments

Comments
 (0)