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 {