observability: expose durable factory metrics#41
Conversation
There was a problem hiding this comment.
Review: observability — expose durable factory metrics
CI conclusion: ✅ success — 30 test files, 303 tests passed, typecheck + typecheck:test clean, pnpm audit --prod --audit-level high reports no known vulnerabilities, dependency review passed.
What's here
This PR adds a complete Prometheus observability layer to the durable factory:
- Counters & histograms for every committed lifecycle event: event admission, execution creation/outcome/duration, sandbox provisioning, outbox publication, schedule occurrences/admission delay, checkpoint advances, revision resolutions, GitHub effects, and worker loop failures.
- PostgreSQL-derived gauges (executions by state, overdue executions, outbox pending/age, schedule lateness/missed intervals, checkpoint age, active sandbox workloads, pending revisions) collected at scrape time via a single bounded aggregate query.
- Resilient collection: 5 s in-process cache, 2 s AbortSignal timeout, last successful snapshot preserved on failure,
agentbay_observability_collector_upandagentbay_observability_snapshot_age_secondsfor health visibility. - Separate internal metrics server on its own port — not routed through Ingress.
- Helm resources: all off by default;
podAnnotations,ServiceMonitor,PrometheusRule, and Grafana dashboard ConfigMap each independently opt-in. Static render tests validate the default-off and fully-enabled renders. - Label cardinality discipline enforced throughout: bounded control-plane values only (tenant, state, result, event_type, profile_id, binding_id, trigger_id, topic, operation, component). Execution IDs, SHAs, claim names, PR numbers, prompts, and error strings are absent from all labels.
Correctness observations
registerDatabaseMetrics concurrency: The collecting ??= pattern correctly coalesces concurrent scrape calls against one in-flight query and resets the slot in .finally(). No race.
countMissedCronIntervals caps at 10 000 iterations — protects against a misconfigured expression with an extremely fine interval being queried against a stale next_fire_at. Appropriate guard.
metricsPort optional type: readNumber(env.AGENTBAY_METRICS_PORT, 9090) always returns a number, so the ?? 9090 fallback in index.ts is redundant but harmless. The ?: number declaration in Config is slightly inaccurate (it will never be undefined in practice) but causes no runtime issue.
databaseMetricsTextForTest exported from production module: The helper leaks a test-facing export from metrics.ts, but it carries no test-framework imports so it doesn't inflate the production bundle or introduce a hard dependency. Low concern.
Shutdown: Both server and metricsServer are awaited in parallel via Promise.all — correct.
Grafana dashboard: The JSON is minimal but well-formed and uses bounded labels (profile_id, topic, trigger_id, binding_id) matching the label set of the corresponding metrics.
No blocking issues found. The implementation is carefully scoped, security-conscious, and thoroughly tested.
Summary
Security and reliability
Verification