Skip to content

Commit 766137a

Browse files
feat(otel): RED-minimal metrics + spec-path spans
Meter provider alongside the tracer in Telemetry::init (same endpoint guard, resource, and OTLP/HTTP-JSON wire); shutdown flushes both. Instruments per interview decision Q5, labels only from bounded enums: reconcile_passes_total{result}, task_launches_total{driver}, task_reaps_total{driver}, hook_invocations_total{hook,event}, message_deliveries_total{result}, crash_loops_total, reconcile_pass_duration_seconds, session_start_duration_seconds. All record calls early-out unless a provider is installed. The single-file spec path (reconcile_pass_specs_with_sessions) now emits the same st2.reconcile_pass root span as the catalog paths. otelite e2e asserts metric rows land in metrics.ndjson. agent-identity: unknown agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.0.3 agent-runtime: OMP 18.0.3 tooling-profile: dotfiles@929dc21
1 parent 58b5931 commit 766137a

9 files changed

Lines changed: 393 additions & 47 deletions

File tree

docs/vrs/06-observability/open-questions.md

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,10 @@
22

33
Kept minimal; each blocks exactly one delivery slice, not the tree.
44

5-
- **PR2 metric set.** Resolved by interview (decision record Q5): the RED-minimal
6-
set — counters `reconcile_passes_total{result}`, `task_launches_total{driver}`,
7-
`task_reaps_total{driver}`, `hook_invocations_total{hook,event}`,
8-
`message_deliveries_total{result}`, `crash_loops_total`; histograms
9-
`reconcile_pass_duration_seconds`, `session_start_duration_seconds`. Labels only
10-
from bounded enums (result/driver/hook); ids stay in span attributes.
11-
- **`st2 up <spec>` span coverage.** The catalog reconcile paths are instrumented
12-
(`st2.reconcile_pass` at the `up_loop_until` loop pass and in `up_once`), but the single-file
13-
spec path (`reconcile_pass_specs` / `reconcile_pass_specs_with_sessions`, `src/run.rs`) emits
14-
no spans yet. Same span shape applies; folded into PR2, which needs a pass over that call
15-
chain for metrics anyway.
5+
- **PR2 metric set.** Landed: the RED-minimal set from interview decision Q5, with names,
6+
types, and label enums as specified in [spec.md](spec.md) (`src/metrics.rs`); the
7+
`st2 up <spec>` span coverage folded in as planned — all three reconcile-pass sites now
8+
emit `st2.reconcile_pass` plus the pass counter and duration histogram.
169
- **Remaining R04 resource attributes.** Resolved by source read (dotfiles dev3
1710
`monitoring.nix` transform block): the platform edge stamps `service.namespace`,
1811
`sk.site`, `sk.role`, `deployment.environment.name` where absent, and the central

docs/vrs/06-observability/spec.md

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -73,20 +73,43 @@ Instrumented in PR1, one root span per unit of work:
7373

7474
Not yet instrumented (follow-ups, not PR1 scope):
7575

76-
- Provider session lifecycles (claude / codex / opencode spawn, attach, teardown), exec sidecars
77-
(`src/exec_backend.rs`), and hooks (`src/hooks.rs`).
78-
- The `st2 up <spec>` path (`reconcile_pass_specs` and friends) emits no spans yet — tracked as
79-
an [open question](open-questions.md).
76+
- Provider session lifecycles (claude / codex / opencode spawn, attach, teardown) beyond the
77+
PR2 launch/reap counters, exec sidecars (`src/exec_backend.rs`).
8078

8179
Span names follow the central `01-conventions` rules (`span.label` discipline included). Names are
8280
registered st2-side; this list plus PR2's metric set is that registry's seed.
8381

8482
## Metrics (PR2)
8583

86-
Exact metric set is open ([open-questions](open-questions.md)). Shape: counters for reconcile
87-
passes, spawns, reaps, hook runs, and errors by kind; histograms for reconcile-pass duration and
88-
provider-session start latency. Same resource attributes, same endpoint, batch exporter shared
89-
with traces.
84+
Landed RED-minimal set per interview decision Q5; every label value comes from a bounded enum,
85+
and identifiers never become metric labels (ids stay in span attributes). `src/metrics.rs` owns
86+
the instruments; every record call early-outs unless a meter provider is installed.
87+
88+
| Instrument | Type | Labels |
89+
| --- | --- | --- |
90+
| `reconcile_passes_total` | counter | `result` = `pass` \| `fail` |
91+
| `task_launches_total` | counter | `driver` = `codex` \| `claude` \| `opencode` \| `pi` \| `exec` \| `other` |
92+
| `task_reaps_total` | counter | `driver` (same enum as launches) |
93+
| `hook_invocations_total` | counter | `hook` = registry name (`claude-observe`), `event` = bounded Claude hook-event set, unknown → `other` |
94+
| `message_deliveries_total` | counter | `result` = `pass` \| `fail` |
95+
| `crash_loops_total` | counter ||
96+
| `reconcile_pass_duration_seconds` | histogram ||
97+
| `session_start_duration_seconds` | histogram ||
98+
99+
Scope notes: passes are counted at all three `st2.reconcile_pass` sites (catalog loop pass,
100+
one-shot up, and the single-file spec path — `reconcile_pass_specs_with_sessions`, which now
101+
emits the same root span shape); `fail` means the pass collected errors. Reaps count the
102+
restart path in the launch loop, where driver context exists. Deliveries cover bus deliveries
103+
onto a recipient inbox (`deliver_record`, send + retry paths); ding/native transport outcomes
104+
are separate follow-ups. Hook invocations are observed at the single in-process application
105+
point (`st2 driver claude-observe`); hook scripts the harnesses execute directly are not
106+
visible to st2.
107+
108+
The meter provider shares PR1's plumbing: `Telemetry::init` installs an `SdkMeterProvider`
109+
with a `PeriodicReader` + OTLP/HTTP-JSON metric exporter behind the same
110+
`OTEL_EXPORTER_OTLP_ENDPOINT` guard and resource; unset → no provider and the global meter is
111+
a silent no-op (R02 zero-overhead). `Telemetry::shutdown` force-flushes metric points alongside
112+
spans so short-lived CLI runs deliver them.
90113

91114
## Log bridge (PR3)
92115

src/claude_session.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ pub fn run_observe(
7676
runtime_id: Option<&str>,
7777
event: &str,
7878
) -> Result<()> {
79+
crate::metrics::record_hook_invocation("claude-observe", event);
7980
let agent_dir = message::resolve_agent_dir(catalog_root, identity, &crate::run::detect_host())?
8081
.with_context(|| format!("Claude driver agent '{identity}' is not declared"))?;
8182
let mut raw = String::new();

src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ pub mod host_lock;
2828
pub mod isolate;
2929
pub mod materialize;
3030
pub mod message;
31+
pub mod metrics;
3132
pub mod opencode_session;
3233
pub mod park;
3334
pub mod pi_channel;

src/message.rs

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1844,15 +1844,23 @@ fn deliver_record(recipient: &DeliveryEndpoint, record: &SentRecord) -> anyhow::
18441844
let (inbox, archive) = recipient.boxes()?;
18451845
let archived = archive.join(&record.filename);
18461846
if archived.is_file() {
1847-
anyhow::ensure!(
1848-
fs::read_to_string(&archived)? == record.rendered_message,
1849-
"archived message differs from pending send {}",
1850-
record.filename
1851-
);
1847+
if fs::read_to_string(&archived)? != record.rendered_message {
1848+
crate::metrics::record_message_delivery(true);
1849+
anyhow::bail!("archived message differs from pending send {}", record.filename);
1850+
}
1851+
crate::metrics::record_message_delivery(false);
18521852
return Ok(());
18531853
}
1854-
materialize_message_once(&inbox, &record.filename, &record.rendered_message)?;
1855-
Ok(())
1854+
match materialize_message_once(&inbox, &record.filename, &record.rendered_message) {
1855+
Ok(_) => {
1856+
crate::metrics::record_message_delivery(false);
1857+
Ok(())
1858+
}
1859+
Err(error) => {
1860+
crate::metrics::record_message_delivery(true);
1861+
Err(error)
1862+
}
1863+
}
18561864
}
18571865

18581866
fn key_path(root: &Path, to: &str, key: &str) -> anyhow::Result<PathBuf> {

src/metrics.rs

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
//! RED-minimal metric set for st2, per the interview Q5 decision recorded in
2+
//! `docs/vrs/06-observability/open-questions.md`.
3+
//!
4+
//! Every label value comes from a bounded enum (`result`, `driver`, hook registry name +
5+
//! normalized event); identifiers never become metric labels — those live in span attributes.
6+
//!
7+
//! Zero-overhead no-op unless a real meter provider is installed by
8+
//! [`crate::telemetry::Telemetry::init`]: every record function checks [`enabled`] first and
9+
//! returns before touching any instrument or allocating a label string. With no provider
10+
//! installed, `opentelemetry::global` hands out a silent no-op meter anyway — this early-out
11+
//! just keeps the disabled case allocation-free.
12+
13+
use std::sync::LazyLock;
14+
use std::sync::atomic::{AtomicBool, Ordering};
15+
use std::time::Duration;
16+
17+
use opentelemetry::global;
18+
use opentelemetry::metrics::{Counter, Histogram, Meter};
19+
20+
static ENABLED: AtomicBool = AtomicBool::new(false);
21+
22+
/// Whether a real meter provider is installed. False → recording is a free no-op.
23+
pub fn enabled() -> bool {
24+
ENABLED.load(Ordering::Relaxed)
25+
}
26+
27+
/// Flip recording on when [`crate::telemetry::Telemetry::init`] installs the provider.
28+
pub fn set_enabled(enabled: bool) {
29+
ENABLED.store(enabled, Ordering::Relaxed);
30+
}
31+
32+
static METER: LazyLock<Meter> = LazyLock::new(|| global::meter("st2"));
33+
34+
static RECONCILE_PASSES: LazyLock<Counter<u64>> = LazyLock::new(|| {
35+
METER
36+
.u64_counter("reconcile_passes_total")
37+
.with_description("Reconcile passes by outcome")
38+
.with_unit("1")
39+
.build()
40+
});
41+
static RECONCILE_PASS_DURATION: LazyLock<Histogram<f64>> = LazyLock::new(|| {
42+
METER
43+
.f64_histogram("reconcile_pass_duration_seconds")
44+
.with_description("Wall-clock duration of one reconcile pass")
45+
.with_unit("s")
46+
.build()
47+
});
48+
static SESSION_START_DURATION: LazyLock<Histogram<f64>> = LazyLock::new(|| {
49+
METER
50+
.f64_histogram("session_start_duration_seconds")
51+
.with_description("Latency of one task session spawn")
52+
.with_unit("s")
53+
.build()
54+
});
55+
static TASK_LAUNCHES: LazyLock<Counter<u64>> = LazyLock::new(|| {
56+
METER
57+
.u64_counter("task_launches_total")
58+
.with_description("Task sessions launched, by driver")
59+
.with_unit("1")
60+
.build()
61+
});
62+
static TASK_REAPS: LazyLock<Counter<u64>> = LazyLock::new(|| {
63+
METER
64+
.u64_counter("task_reaps_total")
65+
.with_description("Dead sessions reaped for restart, by driver")
66+
.with_unit("1")
67+
.build()
68+
});
69+
static HOOK_INVOCATIONS: LazyLock<Counter<u64>> = LazyLock::new(|| {
70+
METER
71+
.u64_counter("hook_invocations_total")
72+
.with_description("Lifecycle hook invocations applied in-process, by hook and event")
73+
.with_unit("1")
74+
.build()
75+
});
76+
static MESSAGE_DELIVERIES: LazyLock<Counter<u64>> = LazyLock::new(|| {
77+
METER
78+
.u64_counter("message_deliveries_total")
79+
.with_description("Bus deliveries onto a recipient inbox, by outcome")
80+
.with_unit("1")
81+
.build()
82+
});
83+
static CRASH_LOOPS: LazyLock<Counter<u64>> = LazyLock::new(|| {
84+
METER
85+
.u64_counter("crash_loops_total")
86+
.with_description("Tasks parked as crash-looping past their restart budget")
87+
.with_unit("1")
88+
.build()
89+
});
90+
91+
/// One reconcile pass finished. `failed` = the pass collected errors.
92+
pub fn record_reconcile_pass(duration: Duration, failed: bool) {
93+
if !enabled() {
94+
return;
95+
}
96+
RECONCILE_PASS_DURATION.record(duration.as_secs_f64(), &[]);
97+
RECONCILE_PASSES.add(
98+
1,
99+
&[opentelemetry::KeyValue::new("result", if failed { "fail" } else { "pass" })],
100+
);
101+
}
102+
103+
/// One task session spawn succeeded.
104+
pub fn record_session_start(duration: Duration, driver: &'static str) {
105+
if !enabled() {
106+
return;
107+
}
108+
SESSION_START_DURATION.record(duration.as_secs_f64(), &[]);
109+
TASK_LAUNCHES.add(1, &[opentelemetry::KeyValue::new("driver", driver)]);
110+
}
111+
112+
/// One dead session was reaped so its replacement can start.
113+
pub fn record_task_reap(driver: &'static str) {
114+
if !enabled() {
115+
return;
116+
}
117+
TASK_REAPS.add(1, &[opentelemetry::KeyValue::new("driver", driver)]);
118+
}
119+
120+
/// One lifecycle-hook invocation reached its single in-process application point.
121+
/// Unknown event names collapse to `other` so the label stays bounded.
122+
pub fn record_hook_invocation(hook: &'static str, event: &str) {
123+
if !enabled() {
124+
return;
125+
}
126+
HOOK_INVOCATIONS.add(
127+
1,
128+
&[
129+
opentelemetry::KeyValue::new("hook", hook),
130+
opentelemetry::KeyValue::new("event", normalize_hook_event(event)),
131+
],
132+
);
133+
}
134+
135+
/// One bus delivery attempt onto a recipient inbox finished. `failed` = the attempt errored.
136+
pub fn record_message_delivery(failed: bool) {
137+
if !enabled() {
138+
return;
139+
}
140+
MESSAGE_DELIVERIES.add(
141+
1,
142+
&[opentelemetry::KeyValue::new("result", if failed { "fail" } else { "pass" })],
143+
);
144+
}
145+
146+
/// A task was parked as crash-looping past its restart budget.
147+
pub fn record_crash_loop() {
148+
if !enabled() {
149+
return;
150+
}
151+
CRASH_LOOPS.add(1, &[]);
152+
}
153+
154+
/// The bounded Claude hook-event vocabulary st2 applies; anything else is `other`.
155+
fn normalize_hook_event(event: &str) -> &'static str {
156+
match event {
157+
"SessionStart" => "SessionStart",
158+
"UserPromptSubmit" => "UserPromptSubmit",
159+
"PreToolUse" => "PreToolUse",
160+
"PostToolUse" => "PostToolUse",
161+
"PermissionRequest" => "PermissionRequest",
162+
"Stop" => "Stop",
163+
"SubagentStop" => "SubagentStop",
164+
"PreCompact" => "PreCompact",
165+
"Notification" => "Notification",
166+
_ => "other",
167+
}
168+
}
169+
170+
#[cfg(test)]
171+
mod tests {
172+
use super::*;
173+
174+
#[test]
175+
fn disabled_by_default_and_recording_is_a_no_op() {
176+
// No meter provider installed in unit tests: enabled() is false and every record call
177+
// must return without panicking or touching instruments.
178+
assert!(!enabled());
179+
set_enabled(false);
180+
record_reconcile_pass(Duration::from_millis(5), false);
181+
record_session_start(Duration::from_millis(5), "exec");
182+
record_task_reap("codex");
183+
record_hook_invocation("claude-observe", "SomethingUnheardOf");
184+
record_message_delivery(true);
185+
record_crash_loop();
186+
assert!(!enabled());
187+
}
188+
189+
#[test]
190+
fn unknown_hook_events_collapse_to_other() {
191+
assert_eq!(normalize_hook_event("SessionStart"), "SessionStart");
192+
assert_eq!(normalize_hook_event("TotallyNewEvent"), "other");
193+
}
194+
}

0 commit comments

Comments
 (0)