feat(otel): RED-minimal metrics + spec-path spans

Cargo.toml

@@ -38,7 +38,7 @@ tempfile = "3"
 toml = "0.9"
 opentelemetry = "0.30"
 opentelemetry-otlp = { version = "0.30", default-features = false, features = ["http-json", "reqwest-blocking-client", "internal-logs"] }
-opentelemetry_sdk = "0.30"
+opentelemetry_sdk = { version = "0.30", features = ["spec_unstable_metrics_views"] }
 tungstenite = "0.30"
 
 [dev-dependencies]

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

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

docs/vrs/06-observability/spec.md

@@ -44,7 +44,7 @@ One module, `src/telemetry.rs`, owns init and teardown via `Telemetry::init(unit
 - **Protocol**: HTTP JSON (`http-json` + protobuf-free wire), batch exporter, targeting the local
   Alloy forwarder at `127.0.0.1:4318` by convention.
 - **Resource**: `service.name` = `st2-<unit>` selected per entrypoint (below; `src/main.rs`
-  passes `supervisor` or `cli`), `service.version` from `crate::version::machine_version`, and
+  passes `supervisor`, `hook`, or `cli`), `service.version` from `crate::version::machine_version`, and
   `host.name` from the existing host detection. The remaining R04 fleet attributes
   (`service.namespace`,
   `service.instance.id`, `sk.site`, `sk.role`, `deployment.environment.name`) are not set yet —
@@ -59,7 +59,7 @@ One module, `src/telemetry.rs`, owns init and teardown via `Telemetry::init(unit
 | --- | --- |
 | Supervisor loop (`st2 up` daemon / systemd unit) | `st2-supervisor` |
 | One-shot CLI invocations | `st2-cli` |
-| Hook executions (`src/hooks.rs`) — not instrumented yet; planned value | `st2-hook` |
+| Hook executions (`st2 driver claude-observe`; other hook surfaces not instrumented yet) | `st2-hook` |
 
 ## Trace roots
 
@@ -73,20 +73,52 @@ Instrumented in PR1, one root span per unit of work:
 
 Not yet instrumented (follow-ups, not PR1 scope):
 
-- Provider session lifecycles (claude / codex / opencode spawn, attach, teardown), exec sidecars
-  (`src/exec_backend.rs`), and hooks (`src/hooks.rs`).
-- The `st2 up <spec>` path (`reconcile_pass_specs` and friends) emits no spans yet — tracked as
-  an [open question](open-questions.md).
+- Provider session lifecycles (claude / codex / opencode spawn, attach, teardown) beyond the
+  PR2 launch/reap counters, exec sidecars (`src/exec_backend.rs`).
 
 Span names follow the central `01-conventions` rules (`span.label` discipline included). Names are
 registered st2-side; this list plus PR2's metric set is that registry's seed.
 
 ## Metrics (PR2)
 
-Exact metric set is open ([open-questions](open-questions.md)). Shape: counters for reconcile
-passes, spawns, reaps, hook runs, and errors by kind; histograms for reconcile-pass duration and
-provider-session start latency. Same resource attributes, same endpoint, batch exporter shared
-with traces.
+Landed RED-minimal set per interview decision Q5; every label value comes from a bounded enum,
+and identifiers never become metric labels (ids stay in span attributes). `src/metrics.rs` owns
+the instruments; every record call early-outs unless a meter provider is installed.
+
+| Instrument | Type | Labels |
+| --- | --- | --- |
+| `reconcile_passes_total` | counter | `result` = `pass` \| `fail` |
+| `task_launches_total` | counter | `driver` = `codex` \| `claude` \| `opencode` \| `pi` \| `omp` \| `exec` \| `other` |
+| `task_reaps_total` | counter | `driver` (same enum as launches) |
+| `hook_invocations_total` | counter | `hook` = registry name (`claude-observe`), `event` = bounded Claude hook-event set, unknown → `other` |
+| `message_deliveries_total` | counter | `result` = `pass` \| `fail` |
+| `crash_loops_total` | counter | — |
+| `reconcile_pass_duration_seconds` | histogram | — |
+| `session_start_duration_seconds` | histogram | — |
+
+The duration histograms use seconds-scale explicit buckets (`1ms … 10s`, see
+`DURATION_BUCKET_BOUNDARIES` in `src/telemetry.rs`) instead of the SDK's millisecond-tuned
+defaults, so sub-second passes and spawns stay distinguishable.
+
+Scope notes: passes are counted at all three `st2.reconcile_pass` sites (catalog loop pass,
+one-shot up, and the single-file spec path — `reconcile_pass_specs_with_sessions`, which now
+emits the same root span shape); `fail` means the pass collected errors. Reaps count the
+restart path in the launch loop, where driver context exists. Deliveries cover bus deliveries
+onto a recipient inbox (`deliver_record`, send + retry paths); ding/native transport outcomes
+are separate follow-ups. Hook invocations are observed at the single in-process application
+point (`st2 driver claude-observe`); hook scripts the harnesses execute directly are not
+visible to st2. The `driver` label is a closed enum resolved by precedence: `exec` task kind
+first, then a typed driver declaration, then an observational argv/shell token heuristic
+(alphanumeric tokens matched in launch order: `codex`, `claude`, `opencode`, `omp`, `pi`; anything
+else → `other`). Because the heuristic inspects arbitrary user work, a hand-authored seat may
+be labeled by what its command line merely mentions — the label is diagnostic only and never
+influences reconcile decisions.
+
+The meter provider shares PR1's plumbing: `Telemetry::init` installs an `SdkMeterProvider`
+with a `PeriodicReader` + OTLP/HTTP-JSON metric exporter behind the same
+`OTEL_EXPORTER_OTLP_ENDPOINT` guard and resource; unset → no provider and the global meter is
+a silent no-op (R02 zero-overhead). `Telemetry::shutdown` force-flushes metric points alongside
+spans so short-lived CLI runs deliver them.
 
 ## Log bridge (PR3)
 

src/claude_session.rs

@@ -78,6 +78,9 @@ pub fn run_observe(
 ) -> Result<()> {
     let agent_dir = message::resolve_agent_dir(catalog_root, identity, &crate::run::detect_host())?
         .with_context(|| format!("Claude driver agent '{identity}' is not declared"))?;
+    // Counted only once the invocation has its application target: a hook for an undeclared
+    // agent errors out before any state is applied and must not inflate `hook_invocations_total`.
+    crate::metrics::record_hook_invocation("claude-observe", event);
     let mut raw = String::new();
     let _ = std::io::stdin().read_to_string(&mut raw);
     let payload = serde_json::from_str(&raw).unwrap_or(serde_json::Value::Null);

src/lib.rs

@@ -29,6 +29,7 @@ pub mod isolate;
 pub mod materialize;
 pub mod message;
 pub mod omp_session;
+pub mod metrics;
 pub mod opencode_session;
 pub mod park;
 pub mod pi_channel;

src/main.rs

@@ -992,12 +992,21 @@ fn main() -> Result<()> {
         command,
     } = Cli::parse();
 
-    let mut telemetry =
-        st2::telemetry::Telemetry::init(if matches!(command, Command::Up { once: false, .. }) {
+    let mut telemetry = st2::telemetry::Telemetry::init(
+        if matches!(command, Command::Up { once: false, .. }) {
             "supervisor"
+        } else if matches!(
+            command,
+            // Hook executions are their own process unit: `st2 driver claude-observe` runs per
+            // Claude hook event and records hook_invocations_total, which the documented
+            // process-unit contract assigns to `st2-hook`, not `st2-cli`.
+            Command::Driver(DriverCmd::ClaudeObserve { .. })
+        ) {
+            "hook"
         } else {
             "cli"
-        });
+        },
+    );
     let result = dispatch(command, catalog_path.as_deref());
     telemetry.shutdown();
     result

src/message.rs

@@ -1844,15 +1844,32 @@ fn deliver_record(recipient: &DeliveryEndpoint, record: &SentRecord) -> anyhow::
     let (inbox, archive) = recipient.boxes()?;
     let archived = archive.join(&record.filename);
     if archived.is_file() {
-        anyhow::ensure!(
-            fs::read_to_string(&archived)? == record.rendered_message,
-            "archived message differs from pending send {}",
-            record.filename
-        );
+        let same = match fs::read_to_string(&archived) {
+            Ok(content) => content == record.rendered_message,
+            // A read failure is a failed delivery too: report it before propagating so
+            // message_deliveries_total{result="fail"} covers filesystem errors, not just mismatches.
+            Err(error) => {
+                crate::metrics::record_message_delivery(true);
+                return Err(error.into());
+            }
+        };
+        if !same {
+            crate::metrics::record_message_delivery(true);
+            anyhow::bail!("archived message differs from pending send {}", record.filename);
+        }
+        crate::metrics::record_message_delivery(false);
         return Ok(());
     }
-    materialize_message_once(&inbox, &record.filename, &record.rendered_message)?;
-    Ok(())
+    match materialize_message_once(&inbox, &record.filename, &record.rendered_message) {
+        Ok(_) => {
+            crate::metrics::record_message_delivery(false);
+            Ok(())
+        }
+        Err(error) => {
+            crate::metrics::record_message_delivery(true);
+            Err(error)
+        }
+    }
 }
 
 fn key_path(root: &Path, to: &str, key: &str) -> anyhow::Result<PathBuf> {

src/metrics.rs

@@ -0,0 +1,194 @@
+//! RED-minimal metric set for st2, per the interview Q5 decision recorded in
+//! `docs/vrs/06-observability/open-questions.md`.
+//!
+//! Every label value comes from a bounded enum (`result`, `driver`, hook registry name +
+//! normalized event); identifiers never become metric labels — those live in span attributes.
+//!
+//! Zero-overhead no-op unless a real meter provider is installed by
+//! [`crate::telemetry::Telemetry::init`]: every record function checks [`enabled`] first and
+//! returns before touching any instrument or allocating a label string. With no provider
+//! installed, `opentelemetry::global` hands out a silent no-op meter anyway — this early-out
+//! just keeps the disabled case allocation-free.
+
+use std::sync::LazyLock;
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::time::Duration;
+
+use opentelemetry::global;
+use opentelemetry::metrics::{Counter, Histogram, Meter};
+
+static ENABLED: AtomicBool = AtomicBool::new(false);
+
+/// Whether a real meter provider is installed. False → recording is a free no-op.
+pub fn enabled() -> bool {
+    ENABLED.load(Ordering::Relaxed)
+}
+
+/// Flip recording on when [`crate::telemetry::Telemetry::init`] installs the provider.
+pub fn set_enabled(enabled: bool) {
+    ENABLED.store(enabled, Ordering::Relaxed);
+}
+
+static METER: LazyLock<Meter> = LazyLock::new(|| global::meter("st2"));
+
+static RECONCILE_PASSES: LazyLock<Counter<u64>> = LazyLock::new(|| {
+    METER
+        .u64_counter("reconcile_passes_total")
+        .with_description("Reconcile passes by outcome")
+        .with_unit("1")
+        .build()
+});
+static RECONCILE_PASS_DURATION: LazyLock<Histogram<f64>> = LazyLock::new(|| {
+    METER
+        .f64_histogram("reconcile_pass_duration_seconds")
+        .with_description("Wall-clock duration of one reconcile pass")
+        .with_unit("s")
+        .build()
+});
+static SESSION_START_DURATION: LazyLock<Histogram<f64>> = LazyLock::new(|| {
+    METER
+        .f64_histogram("session_start_duration_seconds")
+        .with_description("Latency of one task session spawn")
+        .with_unit("s")
+        .build()
+});
+static TASK_LAUNCHES: LazyLock<Counter<u64>> = LazyLock::new(|| {
+    METER
+        .u64_counter("task_launches_total")
+        .with_description("Task sessions launched, by driver")
+        .with_unit("1")
+        .build()
+});
+static TASK_REAPS: LazyLock<Counter<u64>> = LazyLock::new(|| {
+    METER
+        .u64_counter("task_reaps_total")
+        .with_description("Dead sessions reaped for restart, by driver")
+        .with_unit("1")
+        .build()
+});
+static HOOK_INVOCATIONS: LazyLock<Counter<u64>> = LazyLock::new(|| {
+    METER
+        .u64_counter("hook_invocations_total")
+        .with_description("Lifecycle hook invocations applied in-process, by hook and event")
+        .with_unit("1")
+        .build()
+});
+static MESSAGE_DELIVERIES: LazyLock<Counter<u64>> = LazyLock::new(|| {
+    METER
+        .u64_counter("message_deliveries_total")
+        .with_description("Bus deliveries onto a recipient inbox, by outcome")
+        .with_unit("1")
+        .build()
+});
+static CRASH_LOOPS: LazyLock<Counter<u64>> = LazyLock::new(|| {
+    METER
+        .u64_counter("crash_loops_total")
+        .with_description("Tasks parked as crash-looping past their restart budget")
+        .with_unit("1")
+        .build()
+});
+
+/// One reconcile pass finished. `failed` = the pass collected errors.
+pub fn record_reconcile_pass(duration: Duration, failed: bool) {
+    if !enabled() {
+        return;
+    }
+    RECONCILE_PASS_DURATION.record(duration.as_secs_f64(), &[]);
+    RECONCILE_PASSES.add(
+        1,
+        &[opentelemetry::KeyValue::new("result", if failed { "fail" } else { "pass" })],
+    );
+}
+
+/// One task session spawn succeeded.
+pub fn record_session_start(duration: Duration, driver: &'static str) {
+    if !enabled() {
+        return;
+    }
+    SESSION_START_DURATION.record(duration.as_secs_f64(), &[]);
+    TASK_LAUNCHES.add(1, &[opentelemetry::KeyValue::new("driver", driver)]);
+}
+
+/// One dead session was reaped so its replacement can start.
+pub fn record_task_reap(driver: &'static str) {
+    if !enabled() {
+        return;
+    }
+    TASK_REAPS.add(1, &[opentelemetry::KeyValue::new("driver", driver)]);
+}
+
+/// One lifecycle-hook invocation reached its single in-process application point.
+/// Unknown event names collapse to `other` so the label stays bounded.
+pub fn record_hook_invocation(hook: &'static str, event: &str) {
+    if !enabled() {
+        return;
+    }
+    HOOK_INVOCATIONS.add(
+        1,
+        &[
+            opentelemetry::KeyValue::new("hook", hook),
+            opentelemetry::KeyValue::new("event", normalize_hook_event(event)),
+        ],
+    );
+}
+
+/// One bus delivery attempt onto a recipient inbox finished. `failed` = the attempt errored.
+pub fn record_message_delivery(failed: bool) {
+    if !enabled() {
+        return;
+    }
+    MESSAGE_DELIVERIES.add(
+        1,
+        &[opentelemetry::KeyValue::new("result", if failed { "fail" } else { "pass" })],
+    );
+}
+
+/// A task was parked as crash-looping past its restart budget.
+pub fn record_crash_loop() {
+    if !enabled() {
+        return;
+    }
+    CRASH_LOOPS.add(1, &[]);
+}
+
+/// The bounded Claude hook-event vocabulary st2 applies; anything else is `other`.
+fn normalize_hook_event(event: &str) -> &'static str {
+    match event {
+        "SessionStart" => "SessionStart",
+        "UserPromptSubmit" => "UserPromptSubmit",
+        "PreToolUse" => "PreToolUse",
+        "PostToolUse" => "PostToolUse",
+        "PermissionRequest" => "PermissionRequest",
+        "Stop" => "Stop",
+        "SubagentStop" => "SubagentStop",
+        "PreCompact" => "PreCompact",
+        "Notification" => "Notification",
+        _ => "other",
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn disabled_by_default_and_recording_is_a_no_op() {
+        // No meter provider installed in unit tests: enabled() is false and every record call
+        // must return without panicking or touching instruments.
+        assert!(!enabled());
+        set_enabled(false);
+        record_reconcile_pass(Duration::from_millis(5), false);
+        record_session_start(Duration::from_millis(5), "exec");
+        record_task_reap("codex");
+        record_hook_invocation("claude-observe", "SomethingUnheardOf");
+        record_message_delivery(true);
+        record_crash_loop();
+        assert!(!enabled());
+    }
+
+    #[test]
+    fn unknown_hook_events_collapse_to_other() {
+        assert_eq!(normalize_hook_event("SessionStart"), "SessionStart");
+        assert_eq!(normalize_hook_event("TotallyNewEvent"), "other");
+    }
+}