feat(monitoring): gateway health & diagnostics OTLP export - #64536
Conversation
|
Good catches, all four are real. Fixing now:
One commit, then re-running tests + the live OTLP smoke. |
|
All four fixed in 8708d62.
Re-verified: 36 monitoring tests + 210 gateway/plugins green, ruff clean, live OTLP smoke exit 0 with all three routes and log bodies arriving pre-redacted ( |
22b1066 to
b934b72
Compare
|
Rebased and completed at Verification:
Local Docker was unavailable, so the production image was not built on this host. A Dockerfile regression test verifies the locked emitter |
|
@jquesnelle, this branch has been rebased and materially tightened since your previous review. I resolved the two open threads after re-verifying them on the pushed head: the dead telemetry config and legacy fallback are gone, and the install ID is minted once and persisted through This iteration also:
Verification is green: 381 focused tests, independent security re-review, exact-head tmux protobuf capture across all three OTLP routes, and all 37 GitHub checks including amd64 and arm64 Docker builds. |
135a106 to
c8cbb92
Compare
Gateway diagnostic enrichment follow-upThis follow-up enriches the content-free OTLP diagnostic stream without reintroducing rendered log messages or adding a static summary catalog. What changedSource logger becomes the OpenTelemetry instrumentation scopeGateway diagnostics now preserve a validated, source-controlled Python logger name on the internal event and use it as the OTel instrumentation scope: This provides code-level attribution for logs and subsystem routes while keeping the existing coarse aggregation fields stable. The scope boundary is deliberately constrained:
Repository inspection found gateway logger declarations are static literals or module-derived Relay diagnostics retain stable subsystem aggregation
This fixes the prior flattening to the generic Broader network failure classificationCommon Python and transport connection failures now normalize to: The classifier covers forms such as connection refusal, failed connect calls, unreachable hosts or networks, and name-resolution failures. Authentication and timeout classification retain precedence, and the exported values remain fixed low-cardinality constants. Privacy boundaryThe diagnostic stream remains content-free by design:
diagnostic_detail: redacted_messageThat future mode is not implemented here and must not weaken the default content-free path. Compatibility
Executed verificationAt the current PR head:
The smoke input included a synthetic email address, private path, endpoint, token-shaped value, and rendered connection message. None appeared in the exported OTLP payload. |
Add a built-in telemetry system that records what the agent does — workflows,
model calls, tool calls, errors — to the local machine, powers `/insights`, and
can export to an operator-chosen destination. Default-on locally; nothing leaves
the machine unless the user exports it or opts into the aggregate plane.
Three planes with a hard wall between them:
- local: full-fidelity observability (real model/provider/tool names), on by
default, never leaves the machine.
- aggregate: opt-in metadata, default off. No uploader ships — consent is
recorded via telemetry.consent_state, and `preview` shows what would be
produced, computed locally.
- trajectories: full message content, opt-in, exported only to the operator's
own destination.
Mechanism:
- Bundled `telemetry` plugin registers observational lifecycle hooks
(on_session_start / post_api_request / post_tool_call / on_session_finalize).
No core call sites are edited; hooks already carry the data.
- Fire-and-forget emitter: emit() returns in microseconds, never blocks or
raises into a model/tool call. A daemon thread writes events to an
append-only JSONL log and the tel_* tables in state.db (its own sqlite
connection, separate from SessionDB).
- tel_runs / tel_model_calls / tel_tool_calls live in the declarative
SCHEMA_SQL and are reconciled automatically; SCHEMA_VERSION 16 -> 17.
- metrics derives rollups for /usage and /insights; rollup builds per-run
summaries for `hermes telemetry preview`.
Consent is config, not a parallel command surface. The config file is the root
of trust: set telemetry.consent_state with `hermes config set`, or pin any
telemetry.* key (including allow_aggregate) via managed scope, which overrides
the user's value per key. `hermes telemetry` exposes only what config cannot:
status (report), preview (query), and export.
Export:
- exporter_bulk writes telemetry (and, when the trajectories plane is enabled,
session content) to ndjson/json.
- otlp_exporter streams spans to a configured OpenTelemetry Collector over
OTLP/HTTP. The SDK is an optional extra (hermes-agent[otlp]), lazily
installed via tools.lazy_deps on first use.
- Secrets are always redacted on every export path
(redact_sensitive_text(force=True)); content export is gated by the
trajectories plane, and PII scrubbing follows telemetry.content_redaction.
OTLP auth headers reference environment variable names, never inline values.
No outbound emission to Nous. The aggregate uploader is intentionally not built.
policy.resolve() / TelemetryDecision was a read-only projection used only by
`hermes telemetry status` for display. The actual behavior gates already read
telemetry.* straight from config: the emitter (whether to write) and the plugin
loader (whether to auto-load) each call .get("local", True) on the loaded config,
never through policy.
Make config the single chokepoint the status command reads too: it now resolves
local/allow_aggregate/consent_state inline from the loaded config, the same way
the other gates do. policy.py keeps only what config can't express on its own —
the consent constants, ensure_install_id(), and may_upload_aggregate(config) as a
pure function (the gate a future uploader must consult). resolve() and the
TelemetryDecision dataclass are removed; policy.py drops 107 -> 70 lines.
No behavior change: status renders identically, and the default-on local plane is
still defaulted in DEFAULT_CONFIG plus a fail-safe .get(..., True) at each gate.
Rename the telemetry tiers away from the borrowed control-plane/data-plane jargon to plain language, across code, CLI output, config, and docs: - "local plane" -> "local telemetry" - "aggregate plane" -> "aggregate metrics" - "trajectories plane" -> "trajectories" / "telemetry.trajectories" - "three planes with a hard wall" -> "three settings, isolated from each other" User-facing `hermes telemetry status` now reads "Local telemetry: on" / "Aggregate metrics: off" / "Content export: off (trajectories disabled)". The OTLP resource attribute key telemetry.plane is renamed to telemetry.scope (wire-level identifier; nothing consumes it yet). No behavior change — wording only. Status renders identically apart from the labels; tests updated to match the new strings.
The existing hook tests call the plugin's _on_* callbacks directly, which passes
even if the bundled plugin stops auto-loading or a hook name drifts from what core
fires — real runs would go dark while the suite stays green.
Add test_plugin_e2e.py, which drives the real dispatch chain through public entry
points only (discover_plugins -> invoke_hook -> registered callback -> emitter ->
tel_* tables), exactly as core does:
- one completed turn produces tel_runs / tel_model_calls / tel_tool_calls rows
with real provider/model/tool values and correct counts;
- telemetry.local=false means the plugin does not load and nothing is written.
Verified robust against test ordering (singleton resets for the plugin manager and
the emitter in the fixture).
Aggregate metrics are derived from the local tel_* tables — they're a coarsened
view of local data, not an independent capture path. With telemetry.local=false
nothing is written, so an aggregate opt-in had nothing to aggregate, yet
may_upload_aggregate() returned True and `status` showed "Aggregate metrics: on".
The config could claim a state it couldn't fulfill.
Gate aggregate on local being enabled:
- may_upload_aggregate() now requires local_enabled AND allow_aggregate AND
consent_state == aggregate.
- `telemetry status` computes aggregate_enabled the same way and, when consent is
aggregate but local is off, prints "inert: local telemetry is off — nothing to
aggregate" instead of the opt-in hint.
Happy path is unchanged (local on + consent aggregate -> on). Adds policy and CLI
tests for the inert combo.
The subagent_start/stop hooks are registered but no-op. The prior comment implied subagents need no handling because they inherit via contextvars — misleading, since a delegated child runs on a separate thread with its own session id and trace. Clarify the real situation: a subagent's model/tool calls are already captured as their own tel_runs row via the child's run_conversation, so nothing is lost. These hooks are reserved for recording parent->child lineage (needs a tel_runs.parent_run_id column), deferred until a consumer needs the delegation tree. Comment-only.
Addresses the review on NousResearch#51714: the trace/span layer was declared but unwired — tel_spans was never written, call rows had no timestamp, and nothing set parent lineage, so the store was metrics-only and couldn't reconstruct a trace. Wire the span layer (keeping the praised star-schema shape): - New SpanEvent (span_id/trace_id/run_id/parent_span_id/name/kind/start_ns/end_ns) mapped into tel_spans via the emitter's _TABLE_COLUMNS. - The plugin mints a root span per run and, on each model/tool call, emits a SpanEvent (timing + parent = the run's root) keyed by the SAME span_id as the detail row, so tel_model_calls / tel_tool_calls JOIN to their span. - Call hooks fire on completion, so end_ns = now and start_ns is reconstructed from the measured latency/duration. The run's root span is emitted at finalize with the true run start/end. Result: tel_spans is a connected, single-trace_id, run -> calls tree a desktop waterfall (or any reader) can render directly, ordered by start_ns. Existing metrics rows (tel_runs/model_calls/tool_calls) are unchanged. OTLP: spans now flow to the exporter with their trace/parent/timing attributes. The exporter still emits one OTel span per event rather than reconstructing OTel SpanContexts into a connected trace tree; that projection is left for a follow-up and the module docstring now says so plainly instead of over-claiming. Adds test_spans_trace.py (connected-tree + detail-row JOIN) over the real dispatch path. Accurate (pre-hook) start times, real OTLP SpanContexts, and subagent cross-run lineage remain follow-ups.
…itten Self-review after the NousResearch#51714 feedback found the reviewer's dead-table finding was not isolated — the schema advertised far more than the code populates, and our own tests hid it by hand-feeding fields production never sends. Make the surface honest by subtraction. Schema (10 tel_* tables -> 5): - Delete tel_gateway_events, tel_cron_events, tel_skill_events, tel_memory_events, tel_feedback_events — declared, never written, never read. - Drop columns nothing populates: tel_runs.{profile_id,estimated_cost_usd, cost_status}; tel_model_calls.{ttft_ms,estimated_cost_usd,cost_status, cost_source,end_reason,retry_count}; tel_tool_calls.{backend,retry_count, approval}; tel_spans.attrs_json. Cost duplicated the existing sessions billing columns and was always NULL here. - events.py / emitter _TABLE_COLUMNS / OTLP _span_attrs / rollup / preview display all trimmed to match. Correctness: - end_reason no longer hardcodes "completed". Production finalize callers pass `reason` (shutdown/session_expired/session_reset); _coarse_end_reason now reads it and maps accordingly. - Fix a latent bug the trim exposed: the model_call hook passed end_reason= to ModelCallEvent, which the @_safe wrapper was silently swallowing — so tel_model_calls dropped every row in real runs. Now writes correctly. Tests: - Stop hand-feeding estimated_cost_usd / turn_exit_reason that no production call site sends. Finalize is now driven with the real `reason` kwarg, and assertions cover only fields that are actually populated. This is what let the model_call drop hide — the suite graded on a fictional contract. Net: a smaller system that does what it says. Verified end-to-end over the real dispatch path (runs + connected span tree + model/tool rows populate; dead tables gone). 160 telemetry/state/insights tests green.
Match the docs to the code after the dead-schema cut and span layer:
- List the actual tel_* tables (runs, spans, model_calls, tool_calls,
error_events) instead of a vague "indexed tel_* tables".
- Add a "Traces and spans" section: a run = one session, each call is a child
span under the run root in tel_spans keyed by span_id, reconstructable as a
connected run -> calls tree. Note subagent cross-run lineage isn't recorded.
- Fix stale "tool failure rates by category" -> "by tool" (categories were
removed; insights groups by raw tool name).
- OTLP: state plainly that events export as per-event spans and the tel_spans
parent/timing linkage isn't reconstructed into connected SpanContexts yet,
matching the exporter's own docstring.
- README: "telemetry plane" -> "telemetry system" (stale rename miss); mention
spans.
Config reference verified to match DEFAULT_CONFIG exactly (9 keys).
…gnostics export Salvages the event-spine foundation from feat/telemetry-observability (emitter, typed events, OTLP streaming, redaction — authorship preserved in the preceding commits) and scopes it to the plane enterprise operators need today: gateway Service Health Monitoring plus redacted Operational Diagnostics, exported over OTLP. Dropped from the salvaged branch, deliberately: - run/model/tool trajectory capture (plugins/telemetry hooks, tel_spans) - the local JSONL + state.db tel_* store (monitoring is egress, not storage) - usage rollups/metrics, /insights integration, bulk export - hermes telemetry CLI (replaced by hermes monitoring status) Those planes — shared client usage metrics and enterprise trace telemetry — are being designed on the NeMo Relay integration with distinct consent, policy, and export boundaries; this keeps the monitoring plane content-free and independently enableable. Renames agent/telemetry -> agent/monitoring, config telemetry.* -> monitoring.*, and pins the otlp extra at OpenTelemetry 1.39.1 (matching uv.lock; 1.30.0 conflicts with mistralai>=2.4 on opentelemetry-api).
…g, single redaction path - Cut the leftover telemetry.* DEFAULT_CONFIG block (nothing reads it) and the legacy telemetry-key fallback in policy.py. - install_id: persist the minted UUID back to config.yaml on first use so service.instance.id survives gateway restarts (fail-open when the write is not possible); regression test covers the restart path. - Remove the no-op gateway_health_export.redaction config keys. Redaction is always-on by design and deliberately not configurable; status output now says so. - Collapse redaction to one unconditional secrets+PII scrub: drop the none/pii content modes (they served the dropped trajectories plane) and fold gateway_health.py's duplicate bearer/token/email/phone regex layer into agent/monitoring/redaction.py.
c8cbb92 to
58a7491
Compare
Update: fleet and cron operational health addedForce-pushed the branch onto current What is now covered
Team Gateway's authoritative state for shared connectors such as its Slack app remains explicitly out of scope. Team Gateway has its own OTEL egress and should own those signals. Generic operations guidance
The actual dashboard and alert definitions remain deployment-owned artifacts rather than Hermes core code. Verification
Tracking: NS-621 |
…ree) The cron health snapshot failure path logged only at DEBUG, so a cron telemetry regression would silently drop all hermes.cron.* metrics while gateway health metrics kept flowing. Promote to WARNING with the exception *type* name only (no message text, preserving the no-raw-error contract); keep the traceback on DEBUG. (cherry picked from commit 1c9a3e737b7c57fa9886bc927f1623753a9a811b)
active_agents counts foreground turns + cron + API runs but never the backgrounded delegate_task subagents / terminal(background) processes / kanban workers tracked only for scale-to-zero. Emit them as a distinct content-free gauge so the fleet dashboard can show subagent/background load per peer. Best-effort, sums async_delegation.active_count + process_registry.count_running; 0 if a source is unavailable. (cherry picked from commit 1b7ec684e253651a5aa5760b3bd527a3073ce207)
…n guide Document hermes.gateway.background_work in the export table, and add a 'Maintaining and extending this plane' section: the content-free invariant, and per-change checklists for adding a metric, a new subsystem, extending the error-class/status/source/state enums, and adding a content-free span attribute. Each calls out the layers that silently drop an undeclared signal (observable metric_names registration, the emitter keep_by_kind allowlist, the closed enums, and any collector name/keep_keys allowlist) plus whole-chain verification.
…egistration invariant Extend the cron-export test to assert background_work membership (behavior contract, not a frozen list), and add a regression guard that every gauge emitted in the runtime snapshot is also registered in the observable metric_names list — the silent-drop trap the extension guide documents.
…e batches) A delegate_task fan-out batch occupies ONE async-pool slot by design, so active_count() (unit/slot count) reports a 3-task batch as 1 — which undercounts real concurrent subagent load on the background_work metric. Add active_task_count() that expands a running batch to its child count (N-task batch -> N, single -> 1) and switch the background_work reader to it. active_count() is unchanged (capacity semantics preserved). Adds a contract test for the unit-vs-task distinction and documents both counts.
…t count) Complements the task-granular background_work with the async-delegation UNIT count (each dispatch/batch = 1), recovering the pool-slot semantics active_count() gives. Together: background_work = real concurrent subagent load (batch expanded), background_delegations = slot pressure to alert against delegation.max_concurrent_children. Registered in metric_names, documented, and covered by a task-vs-unit contract test.
|
Rechecked this branch against NVIDIA's #67607 after the cron-health additions. A narrow semantic overlap now exists for LLM-backed cron runs: #67607 classifies them as generic The implementations and operational contracts remain separate. This PR reads the scheduler and durable cron execution ledger, so it also covers scheduler freshness, overdue and catch-up state, interrupted executions, exact delivery outcomes, bounded cron error classes, and script-only I updated the PR body to document the overlap and the current boundary explicitly. Both signals can describe the same LLM-backed run without duplicating ownership: this PR answers whether expected automation is healthy now, while #67607 counts aggregate task activity. |
# Conflicts: # cron/executions.py # cron/jobs.py
…iagnostics-monitoring # Conflicts: # uv.lock
…eway-health-diagnostics feat(monitoring): gateway health & diagnostics OTLP export
…eway-health-diagnostics feat(monitoring): gateway health & diagnostics OTLP export
Summary
Adds an explicit, off-by-default Gateway Health and Diagnostics exporter for operator-owned OTLP infrastructure. It reports whether each Hermes gateway, locally owned platform adapter, scheduler, cron execution, and background-work surface is healthy without exporting user or agent content.
This is the Service Health Monitoring and Operational Diagnostics plane. Product usage analytics, governance/audit records, vendor quality reports, and detailed execution trajectories remain separate.
What ships
Gateway and platform health
hermes.gateway.up,state,busy,drainable,restart_requested, andactive_agentshermes.platform.upandhermes.platform.degradedwith bounded platform state and error-code dimensionsservice.instance.idBackground work and capacity
hermes.gateway.background_workcounts detached work omitted byactive_agents: backgrounded subagent tasks, terminal background processes, and Kanban workershermes.gateway.background_delegationscounts dispatch units so operators can compare pool-slot pressure againstdelegation.max_concurrent_childrenCron scheduler health
Cron execution and delivery health
claimed,running,completed,failed, and recoveredunknownlifecycle statesdelivered,failed,suppressed, ornot_configuredno_agentjobs through the shared execution ledgerOTLP runtime and operator surface
/v1/metrics/v1/traces/v1/logshermes monitoring statusdocs/observability/monitoring.mdPrivacy and security boundary
The exporter is content-free by construction:
A shared connector owned by another service must publish its own authoritative health. Hermes reports only the gateway, local adapters, and Relay transport it owns.
Relationship to NeMo Relay shared metrics
PR #67607 now has a narrow semantic intersection with this PR: an LLM-backed cron run is also a generic
scheduled_task, so its local shared-metrics package can count task start/finish, aggregate outcome, duration bucket, and model/tool/retry buckets.The implementations and products remain separate:
AIAgenttask boundary for private aggregate activity metrics stored under the local profile; it does not report scheduler freshness, overdue jobs, delivery outcomes, cron error classes, orno_agentexecutionBoth can observe the same LLM-backed scheduled run without duplicating ownership. One answers whether expected automation is healthy now; the other counts aggregate task activity.
Configuration
Verification
git diff --checkgates passed during the verified iterations/v1/metrics,/v1/traces, and/v1/logsmainLocal smoke