feat(otel): tracing facade — unified span+log subscriber, diagnostics migration, log CI assertions - #350
Conversation
|
@codex Please review — PR 3 of the otel stack (base: #348): tracing facade per decision Q6. Unified subscriber: stderr fmt layer (always on for visibility), Note for reviewers: Posted on behalf of @schickling
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 757a8291c9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
308cf0c to
506171d
Compare
506171d to
6568b7b
Compare
5f6363b to
b34aaef
Compare
b34aaef to
48cd14b
Compare
… migration, log CI assertions PR3 per interview decision Q6 (docs/vrs/06-observability/open-questions.md): - Telemetry::init now installs one tracing subscriber: a stderr fmt layer runs unconditionally so migrated diagnostics stay visible with or without an endpoint (documented deviation from PR1's zero-output unset case; RUST_LOG overrides the INFO default), plus — behind the endpoint guard — a tracing-opentelemetry span layer and an opentelemetry-appender-tracing bridge over a new SDK logger provider sharing endpoint/protocol/resource. experimental_use_tracing_span_context stamps active span ids onto records. shutdown flushes logger+meter+tracer together. - st2.reconcile_pass spans move to tracing::info_span! at all three sites (up_once, up_loop_until pass, reconcile_pass_specs_with_sessions); names and st2.host/st2.crash_loops/st2.unparked attributes unchanged; metric recording calls untouched. Each pass emits a deterministic INFO record (target st2, "reconcile pass complete", result=pass|fail). - Non-user-facing eprintln! diagnostics migrate to tracing::warn!/error! with unchanged text: crash-loop give-ups and supervisor notification, park-channel setup, catalog watching, ding transport ambiguity, driver session degradation, harness-state floor writes. CLI command output, boot banners, and validation reports stay println!/eprintln!. - tests/otel_export.rs additionally asserts logs.ndjson carries the completion record (body/severity) correlated to the captured st2.reconcile_pass span via trace/span ids; existing span+metric assertions unchanged. 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
Two review findings on the log-bridge PR: 1. The EnvFilter was attached to the base subscriber, so RUST_LOG=warn silently disabled INFO span/log export even with an endpoint set. The stderr fmt layer now carries the filter alone. The OpenTelemetry layers carry their own static filter, independent of RUST_LOG, that silences only exporter-internal targets (opentelemetry*): the SDK's BatchLogProcessor emits tracing events from inside its own emit(), and feeding those back through the unfiltered log bridge recursed until the export thread overflowed its stack (gdb-proven). App signals export unfiltered. 2. A metric exporter build failure early-returned before logger setup, killing logs along with metrics. Metric setup is now best-effort: warn and continue to the logger provider. 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
…ws tty tracing_subscriber's fmt layer enables ANSI whenever the 'ansi' feature is on, regardless of destination, so redirected stderr carried SGR escapes where the pre-migration eprintln! path emitted plain bytes. Gate the layer on std::io::stderr().is_terminal(). 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
…anly 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@e4789b0
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@e4789b0
5ed906f to
80c7a28
Compare
What
PR3 of the otel stack (#347 → #348 → #350): adopts the
tracingfacade so spans and logs share one subscriber, migrates supervisor-relevant diagnostics onto it, and extends the otelite CI gate to assert log delivery.src/telemetry.rs): a human-readable stderr fmt layer runs unconditionally, and behind theOTEL_EXPORTER_OTLP_ENDPOINTguard atracing-opentelemetrylayer exports spans whileopentelemetry-appender-tracingexports logs via a new SDK logger provider sharing endpoint/protocol/resource with tracer+meter.experimental_use_tracing_span_contextstamps active span ids onto records.Telemetry::shutdownflushes logger+meter+tracer together.st2.reconcile_passsites (up_once,up_loop_untilpass,reconcile_pass_specs_with_sessions) now usetracing::info_span!; name andst2.host/st2.crash_loops/st2.unparkedattributes unchanged; metric calls untouched.st2, messagereconcile pass complete, fieldresult=pass|fail.println!/eprintln!.Documented deviation from PR1's literal zero-output unset-endpoint behavior: the stderr fmt layer stays installed without an endpoint so migrated diagnostics remain visible (level default INFO;
RUST_LOGoverrides).Design
Per interview decision Q6 recorded in
docs/vrs/06-observability/open-questions.md: tracing facade adopted (tracing-opentelemetry+opentelemetry-appender-tracing), journald/dual-emit alternatives declined. Spec updated to landed reality in this PR.Verification
cargo buildgreencargo test --lib: 471 passedcargo test --test otel_export(withST2_OTELITE_BIN): asserts spans (st2.reconcile_pass,st2-cliresource) + metrics (reconcile_passes_total, duration histogram,result=pass) + NEW:logs.ndjsoncarries thereconcile pass completeINFO record with severity number 9 correlated to the captured span via trace/span ids (32/16-hex)Posted on behalf of @schickling
agent_identityagent_personaagent_supervisoragent_toolagent_tool_versionagent_runtimetooling_profile