From da277a4e5945e98827866fb407b7aa1d28a1d4d8 Mon Sep 17 00:00:00 2001 From: thewoolleyman Date: Tue, 14 Jul 2026 03:22:42 +0200 Subject: [PATCH] feat(cli): add opt-in OTLP/HTTP export for tracing spans Adds an additive, opt-in OTLP/HTTP exporter for fabro's existing tracing spans. It activates only when an OTLP endpoint env var is set (OTEL_EXPORTER_OTLP_ENDPOINT / OTEL_EXPORTER_OTLP_TRACES_ENDPOINT); when unset, the tracing stack is byte-for-byte the current fmt-only setup and nothing is exported. No new instrumentation -- pure transport, so the spans fabro already logs also reach an OTLP collector when one is configured. Honors the standard OTLP env vars (endpoint, protocol, headers, service.name defaulting to "fabro"). Default protocol is http/json; http/protobuf via OTEL_EXPORTER_OTLP_PROTOCOL. Reuses the reqwest already present in the lockfile -- no new reqwest version is introduced. Co-Authored-By: Claude Opus 4.8 (1M context) --- Cargo.lock | 141 ++++++++++++++ Cargo.toml | 12 ++ lib/crates/fabro-cli/Cargo.toml | 4 + lib/crates/fabro-cli/src/logging.rs | 10 + lib/crates/fabro-cli/src/main.rs | 4 + lib/crates/fabro-cli/src/otel.rs | 246 ++++++++++++++++++++++++ lib/crates/fabro-static/src/env_vars.rs | 14 ++ 7 files changed, 431 insertions(+) create mode 100644 lib/crates/fabro-cli/src/otel.rs diff --git a/Cargo.lock b/Cargo.lock index 8be18d81ad..4cca8d38f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2451,6 +2451,9 @@ dependencies = [ "nix 0.30.1", "object_store", "openssl", + "opentelemetry", + "opentelemetry-otlp", + "opentelemetry_sdk", "paste", "predicates", "progenitor-client", @@ -2476,6 +2479,7 @@ dependencies = [ "toml_edit", "tracing", "tracing-appender", + "tracing-opentelemetry", "tracing-subscriber", "ulid", "walkdir", @@ -5722,6 +5726,81 @@ dependencies = [ "vcpkg", ] +[[package]] +name = "opentelemetry" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aaf416e4cb72756655126f7dd7bb0af49c674f4c1b9903e80c009e0c37e552e6" +dependencies = [ + "futures-core", + "futures-sink", + "js-sys", + "pin-project-lite", + "thiserror 2.0.18", + "tracing", +] + +[[package]] +name = "opentelemetry-http" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50f6639e842a97dbea8886e3439710ae463120091e2e064518ba8e716e6ac36d" +dependencies = [ + "async-trait", + "bytes", + "http 1.4.0", + "opentelemetry", + "reqwest 0.12.28", +] + +[[package]] +name = "opentelemetry-otlp" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dbee664a43e07615731afc539ca60c6d9f1a9425e25ca09c57bc36c87c55852b" +dependencies = [ + "http 1.4.0", + "opentelemetry", + "opentelemetry-http", + "opentelemetry-proto", + "opentelemetry_sdk", + "prost", + "reqwest 0.12.28", + "serde_json", + "thiserror 2.0.18", +] + +[[package]] +name = "opentelemetry-proto" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2e046fd7660710fe5a05e8748e70d9058dc15c94ba914e7c4faa7c728f0e8ddc" +dependencies = [ + "base64", + "hex", + "opentelemetry", + "opentelemetry_sdk", + "prost", + "serde", + "tonic", +] + +[[package]] +name = "opentelemetry_sdk" +version = "0.30.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "11f644aa9e5e31d11896e024305d7e3c98a88884d9f8919dbf37a9991bc47a4b" +dependencies = [ + "futures-channel", + "futures-executor", + "futures-util", + "opentelemetry", + "percent-encoding", + "rand 0.9.4", + "serde_json", + "thiserror 2.0.18", +] + [[package]] name = "option-ext" version = "0.2.0" @@ -6221,6 +6300,29 @@ dependencies = [ "syn 2.0.117", ] +[[package]] +name = "prost" +version = "0.13.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2796faa41db3ec313a31f7624d9286acf277b52de526150b7e69f3debf891ee5" +dependencies = [ + "bytes", + "prost-derive", +] + +[[package]] +name = "prost-derive" +version = "0.13.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a56d757972c98b346a9b766e3f02746cde6dd1cd1d1d563472929fdd74bec4d" +dependencies = [ + "anyhow", + "itertools", + "proc-macro2", + "quote", + "syn 2.0.117", +] + [[package]] name = "psl-types" version = "2.0.11" @@ -8300,6 +8402,27 @@ version = "1.0.6+spec-1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ab16f14aed21ee8bfd8ec22513f7287cd4a91aa92e44edfe2c17ddd004e92607" +[[package]] +name = "tonic" +version = "0.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e581ba15a835f4d9ea06c55ab1bd4dce26fc53752c69a04aac00703bfb49ba9" +dependencies = [ + "async-trait", + "base64", + "bytes", + "http 1.4.0", + "http-body 1.0.1", + "http-body-util", + "percent-encoding", + "pin-project", + "prost", + "tokio-stream", + "tower-layer", + "tower-service", + "tracing", +] + [[package]] name = "tower" version = "0.5.3" @@ -8407,6 +8530,24 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-opentelemetry" +version = "0.31.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddcf5959f39507d0d04d6413119c04f33b623f4f951ebcbdddddfad2d0623a9c" +dependencies = [ + "js-sys", + "once_cell", + "opentelemetry", + "opentelemetry_sdk", + "smallvec", + "tracing", + "tracing-core", + "tracing-log", + "tracing-subscriber", + "web-time", +] + [[package]] name = "tracing-subscriber" version = "0.3.22" diff --git a/Cargo.toml b/Cargo.toml index b98efe3790..a66eb33919 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -64,6 +64,18 @@ git2 = { version = "0.20", default-features = false, features = ["vendored-libgi tracing = "0.1" tracing-subscriber = { version = "0.3", features = ["fmt", "env-filter"] } tracing-appender = "0.2" +# Opt-in OTLP/HTTP export of fabro's existing `tracing` spans (see +# fabro-cli/src/otel.rs). Additive and inert unless an OTLP endpoint env is set. +# Uses the BLOCKING OTLP client (the crate's own default) so the SDK batch +# processor's dedicated export thread needs no async runtime. NOTE: +# opentelemetry-otlp's `http-proto`/`http-json` features enable `metrics` +# unconditionally, so the trace+metrics SDK is compiled regardless — a trace-only +# reduction is not achievable while using the HTTP exporter. `opentelemetry-otlp` +# keeps `default-features = false` only to drop its unused OTLP logs exporter. +opentelemetry = "0.30" +opentelemetry_sdk = "0.30" +opentelemetry-otlp = { version = "0.30", default-features = false, features = ["trace", "http-proto", "http-json", "reqwest-blocking-client"] } +tracing-opentelemetry = "0.31" rmcp = { version = "1.4", default-features = false } walkdir = "2" regex = "1" diff --git a/lib/crates/fabro-cli/Cargo.toml b/lib/crates/fabro-cli/Cargo.toml index 6f14e385e5..224f3fd2cf 100644 --- a/lib/crates/fabro-cli/Cargo.toml +++ b/lib/crates/fabro-cli/Cargo.toml @@ -66,6 +66,10 @@ tokio-tungstenite.workspace = true tracing.workspace = true tracing-subscriber.workspace = true tracing-appender.workspace = true +opentelemetry.workspace = true +opentelemetry_sdk.workspace = true +opentelemetry-otlp.workspace = true +tracing-opentelemetry.workspace = true chrono = { workspace = true, features = ["serde"] } dirs.workspace = true fs2.workspace = true diff --git a/lib/crates/fabro-cli/src/logging.rs b/lib/crates/fabro-cli/src/logging.rs index 81c7be1572..6a691fa05e 100644 --- a/lib/crates/fabro-cli/src/logging.rs +++ b/lib/crates/fabro-cli/src/logging.rs @@ -22,6 +22,8 @@ use tracing_subscriber::registry::LookupSpan; use tracing_subscriber::util::SubscriberInitExt; use tracing_subscriber::{EnvFilter, fmt}; +use crate::otel; + const LOG_RETENTION_DAYS: u32 = 7; #[derive(Clone, Debug, PartialEq, Eq)] @@ -362,6 +364,8 @@ where .with_target(true) .with_ansi(false), ) + // Additive OTLP export: no-op unless an OTLP endpoint env is set. + .with(otel::otel_layer()) .init(); } @@ -384,6 +388,8 @@ where .with_ansi(ansi) .event_format(TtyLogFormat::new(ansi)), ) + // Additive OTLP export: no-op unless an OTLP endpoint env is set. + .with(otel::otel_layer()) .init(); } @@ -409,6 +415,8 @@ fn init_worker_subscriber( .with_target(true) .with_ansi(false), ) + // Additive OTLP export: no-op unless an OTLP endpoint env is set. + .with(otel::otel_layer()) .init(); } @@ -441,6 +449,8 @@ fn init_worker_stdout_subscriber( .with_target(true) .with_ansi(false), ) + // Additive OTLP export: no-op unless an OTLP endpoint env is set. + .with(otel::otel_layer()) .init(); } diff --git a/lib/crates/fabro-cli/src/main.rs b/lib/crates/fabro-cli/src/main.rs index 9579f8e792..771b93be2c 100644 --- a/lib/crates/fabro-cli/src/main.rs +++ b/lib/crates/fabro-cli/src/main.rs @@ -11,6 +11,7 @@ mod landing; mod local_server; mod logging; mod manifest_args; +mod otel; mod server_client; mod server_runs; mod shared; @@ -113,6 +114,9 @@ async fn main() { } } fabro_telemetry::shutdown(); + // Best-effort final drain of the OTLP span batch on normal exit (no-op when + // OTLP is not configured; the batch processor also exports periodically). + otel::shutdown(); if let Err(err) = result { let json_mode = raw_args.iter().any(|a| a == "--json"); diff --git a/lib/crates/fabro-cli/src/otel.rs b/lib/crates/fabro-cli/src/otel.rs new file mode 100644 index 0000000000..549dffc1fe --- /dev/null +++ b/lib/crates/fabro-cli/src/otel.rs @@ -0,0 +1,246 @@ +//! OTLP/HTTP export for fabro's `tracing` spans (opt-in observability). +//! +//! This is an ADDITIVE, opt-in path: it activates ONLY when an OTLP endpoint +//! env var is set (`OTEL_EXPORTER_OTLP_ENDPOINT` or +//! `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`). When unset, [`otel_layer`] returns +//! `None`, the tracing stack is exactly the existing `fmt`-only configuration, +//! and nothing is exported. When an endpoint IS configured, fabro's existing +//! `tracing` spans — the same span tree that structures its log output — are +//! bridged to OTLP. This adds NO new instrumentation points to fabro's code; +//! the `tracing-opentelemetry` layer maps those spans to OTel using standard +//! semantic conventions (e.g. error events -> span status), and the optional +//! synthetic extras (busy/idle timing, thread, source location) are disabled +//! ([`otel_layer`]) so exported spans stay close to fabro's actual spans. There +//! is no behavior change when disabled. A malformed endpoint fails the exporter +//! build and disables export (see [`build_provider`]). +//! +//! Standard OTLP env vars are honored: `OTEL_EXPORTER_OTLP_ENDPOINT` / +//! `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT`, `OTEL_EXPORTER_OTLP_PROTOCOL` / +//! `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL`, `OTEL_EXPORTER_OTLP_HEADERS`, +//! `OTEL_SERVICE_NAME` (names via `fabro_static::EnvVars`). Endpoint and +//! protocol are resolved here (the per-signal `TRACES_*` var wins) and set +//! programmatically; headers/timeouts are read by the exporter itself; service +//! name defaults to `fabro`. +//! +//! Protocol: `opentelemetry-otlp` 0.30 over HTTP supports BOTH `http/json` and +//! `http/protobuf` (the `http-json` + `http-proto` features are enabled). The +//! default is `http/protobuf` (the OTLP spec default); set +//! `OTEL_EXPORTER_OTLP_PROTOCOL=http/json` to switch. (`grpc` is not built here +//! and falls back to the protobuf default.) + +#![expect( + clippy::disallowed_methods, + reason = "intentional process-env lookup facade: reads the OTLP/OTEL env vars (names via fabro_static::EnvVars) to configure the exporter" +)] +#![expect( + clippy::print_stderr, + reason = "OTLP setup runs before the tracing subscriber is initialized, so stderr is the only diagnostic sink when the exporter fails to build or is disabled" +)] + +use std::sync::OnceLock; + +use fabro_static::EnvVars; +use opentelemetry::trace::TracerProvider as _; +use opentelemetry_otlp::{Protocol, SpanExporter, WithExportConfig as _}; +use opentelemetry_sdk::Resource; +use opentelemetry_sdk::trace::{SdkTracer, SdkTracerProvider}; +use tracing::Subscriber; +use tracing_subscriber::registry::LookupSpan; + +/// Built once on first [`otel_layer`] call. `Some(None)` means "no endpoint +/// configured / exporter build failed" (OTLP disabled); `Some(Some(provider))` +/// holds the live provider so [`shutdown`] can drain the batch on exit. +static PROVIDER: OnceLock> = OnceLock::new(); + +/// Resolve the traces endpoint from the environment, or `None` to disable +/// export. We resolve it ourselves (rather than letting the exporter read the +/// env) and pass it programmatically so a malformed endpoint fails the build +/// instead of silently falling back to the exporter's localhost default. +fn resolve_endpoint() -> Option { + resolve_endpoint_from( + std::env::var(EnvVars::OTEL_EXPORTER_OTLP_TRACES_ENDPOINT) + .ok() + .as_deref(), + std::env::var(EnvVars::OTEL_EXPORTER_OTLP_ENDPOINT) + .ok() + .as_deref(), + ) +} + +/// Standard OTLP endpoint precedence for the traces signal: a non-empty +/// per-signal endpoint is used as-is; otherwise a non-empty base endpoint gets +/// `/v1/traces` appended. Empty / whitespace-only values are treated as unset. +/// Returns `None` when neither is set (export disabled — no localhost +/// fallback). +fn resolve_endpoint_from(traces: Option<&str>, base: Option<&str>) -> Option { + fn non_empty(value: Option<&str>) -> Option<&str> { + value.map(str::trim).filter(|trimmed| !trimmed.is_empty()) + } + if let Some(endpoint) = non_empty(traces) { + return Some(endpoint.to_string()); + } + let base = non_empty(base)?; + Some(format!("{}/v1/traces", base.trim_end_matches('/'))) +} + +/// Read the OTLP protocol from the environment. Per the OTLP spec the +/// per-signal `OTEL_EXPORTER_OTLP_TRACES_PROTOCOL` takes precedence over the +/// base `OTEL_EXPORTER_OTLP_PROTOCOL` (symmetric with the endpoint precedence); +/// empty/whitespace values are treated as unset. +fn protocol_from_env() -> Protocol { + fn non_empty(name: &str) -> Option { + std::env::var(name).ok().filter(|v| !v.trim().is_empty()) + } + let raw = non_empty(EnvVars::OTEL_EXPORTER_OTLP_TRACES_PROTOCOL) + .or_else(|| non_empty(EnvVars::OTEL_EXPORTER_OTLP_PROTOCOL)); + parse_protocol(raw.as_deref()).unwrap_or_else(|| { + // A set-but-unrecognized value (e.g. `grpc`) warrants a one-time warning; + // an unset value silently defaults to the OTLP spec default. + if let Some(value) = raw.as_deref() { + eprintln!( + "otel: unsupported OTLP protocol {:?}, using http/protobuf", + value.trim() + ); + } + Protocol::HttpBinary + }) +} + +/// Pure mapping of a protocol value to a transport, or `None` for an unset or +/// unrecognized value (the caller then defaults to `http/protobuf`). +/// `http/json` and `http/protobuf` are the only recognized values; leading / +/// trailing whitespace is ignored. +fn parse_protocol(raw: Option<&str>) -> Option { + match raw.map(str::trim) { + Some("http/json") => Some(Protocol::HttpJson), + Some("http/protobuf") => Some(Protocol::HttpBinary), + _ => None, + } +} + +fn build_provider() -> Option { + let endpoint = resolve_endpoint()?; + + // The SDK batch processor spawns a background export thread and `expect`s + // that spawn to succeed; isolate any panic during exporter/provider + // construction so a failure disables export instead of aborting fabro. + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + build_provider_inner(&endpoint) + })) + .unwrap_or_else(|_| { + eprintln!("otel: panic while building OTLP provider, disabling export"); + None + }) +} + +fn build_provider_inner(endpoint: &str) -> Option { + // The endpoint is resolved + validated by `resolve_endpoint` and passed + // programmatically so a malformed value fails `build()` (→ disabled) rather + // than silently falling back to localhost. Headers are still read from the + // standard env vars by the exporter builder; we pin the protocol. + let exporter = match SpanExporter::builder() + .with_http() + .with_protocol(protocol_from_env()) + .with_endpoint(endpoint) + .build() + { + Ok(exporter) => exporter, + Err(err) => { + eprintln!("otel: failed to build OTLP span exporter, disabling export: {err}"); + return None; + } + }; + + let service_name = + std::env::var(EnvVars::OTEL_SERVICE_NAME).unwrap_or_else(|_| "fabro".to_string()); + let resource = Resource::builder().with_service_name(service_name).build(); + + let provider = SdkTracerProvider::builder() + .with_batch_exporter(exporter) + .with_resource(resource) + .build(); + Some(provider) +} + +/// A `tracing_opentelemetry` layer wired to an OTLP/HTTP exporter, or `None` +/// when OTLP is not configured. Added to each subscriber alongside the +/// existing `fmt` layer; `None` is a no-op layer. +pub(crate) fn otel_layer() -> Option> +where + S: Subscriber + for<'a> LookupSpan<'a>, +{ + let provider = PROVIDER.get_or_init(build_provider).as_ref()?; + let tracer = provider.tracer("fabro"); + Some( + tracing_opentelemetry::layer() + .with_tracer(tracer) + // Minimize attributes the bridge synthesizes beyond fabro's own span + // fields: no busy/idle timing, thread, or source-code location. (The + // standard tracing -> OTel error/status mapping is left enabled.) + .with_tracked_inactivity(false) + .with_threads(false) + .with_location(false), + ) +} + +/// Best-effort final flush + shutdown of the OTLP provider so the current batch +/// drains on a normal exit. No-op when OTLP was never configured. Call once, +/// late in `main`, right beside fabro's own `fabro_telemetry::shutdown()` and +/// following the same convention: fabro drains telemetry on the normal-exit +/// path, and its `std::process::exit` bailout sites do not — the batch +/// processor exports periodically, so an abrupt exit at most drops the last +/// un-flushed batch. +pub(crate) fn shutdown() { + if let Some(Some(provider)) = PROVIDER.get() { + // `shutdown` already drains the batch processor; a separate `force_flush` + // first would double the worst-case exit stall against an unreachable + // collector. + let _ = provider.shutdown(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_protocol_recognizes_json_and_protobuf() { + assert!(matches!( + parse_protocol(Some("http/json")), + Some(Protocol::HttpJson) + )); + assert!(matches!( + parse_protocol(Some(" http/json ")), + Some(Protocol::HttpJson) + )); + assert!(matches!( + parse_protocol(Some("http/protobuf")), + Some(Protocol::HttpBinary) + )); + // Unrecognized / unset → None (the caller defaults to protobuf). + assert!(parse_protocol(Some("grpc")).is_none()); + assert!(parse_protocol(Some("")).is_none()); + assert!(parse_protocol(None).is_none()); + } + + #[test] + fn resolve_endpoint_precedence_and_disabled() { + // Per-signal traces endpoint wins and is used as-is. + assert_eq!( + resolve_endpoint_from(Some("http://h:4318/v1/traces"), Some("http://base:4318")), + Some("http://h:4318/v1/traces".to_string()) + ); + // Base endpoint gets `/v1/traces` appended (trailing slash normalized). + assert_eq!( + resolve_endpoint_from(None, Some("http://base:4318")), + Some("http://base:4318/v1/traces".to_string()) + ); + assert_eq!( + resolve_endpoint_from(None, Some("http://base:4318/")), + Some("http://base:4318/v1/traces".to_string()) + ); + // Empty / whitespace / unset → disabled (no localhost fallback). + assert_eq!(resolve_endpoint_from(Some(" "), Some("")), None); + assert_eq!(resolve_endpoint_from(None, None), None); + } +} diff --git a/lib/crates/fabro-static/src/env_vars.rs b/lib/crates/fabro-static/src/env_vars.rs index cd16892b26..15c5beb583 100644 --- a/lib/crates/fabro-static/src/env_vars.rs +++ b/lib/crates/fabro-static/src/env_vars.rs @@ -136,6 +136,15 @@ impl EnvVars { pub const TWIN_OPENAI_REQUIRE_AUTH: &'static str = "TWIN_OPENAI_REQUIRE_AUTH"; pub const USER: &'static str = "USER"; pub const ZDOTDIR: &'static str = "ZDOTDIR"; + + // Observability / OTLP export + pub const OTEL_EXPORTER_OTLP_ENDPOINT: &'static str = "OTEL_EXPORTER_OTLP_ENDPOINT"; + pub const OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: &'static str = + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT"; + pub const OTEL_EXPORTER_OTLP_PROTOCOL: &'static str = "OTEL_EXPORTER_OTLP_PROTOCOL"; + pub const OTEL_EXPORTER_OTLP_TRACES_PROTOCOL: &'static str = + "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL"; + pub const OTEL_SERVICE_NAME: &'static str = "OTEL_SERVICE_NAME"; } #[cfg(test)] @@ -270,6 +279,11 @@ mod tests { EnvVars::TWIN_OPENAI_REQUIRE_AUTH, EnvVars::USER, EnvVars::ZDOTDIR, + EnvVars::OTEL_EXPORTER_OTLP_ENDPOINT, + EnvVars::OTEL_EXPORTER_OTLP_TRACES_ENDPOINT, + EnvVars::OTEL_EXPORTER_OTLP_PROTOCOL, + EnvVars::OTEL_EXPORTER_OTLP_TRACES_PROTOCOL, + EnvVars::OTEL_SERVICE_NAME, ]; for value in values {