diff --git a/Cargo.lock b/Cargo.lock index 3ae582a12a..297bf89fc2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3861,6 +3861,7 @@ dependencies = [ "oauth2", "openshell-bootstrap", "openshell-core", + "openshell-otel", "openshell-policy", "openshell-providers", "openshell-sdk", diff --git a/architecture/gateway.md b/architecture/gateway.md index f7c80d1ed8..c519a84c6e 100644 --- a/architecture/gateway.md +++ b/architecture/gateway.md @@ -698,7 +698,11 @@ gRPC failure recording. The `tower_http` `TraceLayer` in `multiplex.rs` opens a span per inbound request, and that span continues incoming W3C trace context when present or starts a new -trace otherwise. It is named for the RPC and carries the request ID that also +trace otherwise. The CLI supplies that inbound context passively: its gateway +gRPC channel carries an `EnvTraceContextInterceptor` (`openshell-otel`) that +forwards `TRACEPARENT`/`TRACESTATE` from the environment as request metadata, so +a CI pipeline's trace parents the gateway span without any collector or provider +on the CLI side. It is named for the RPC and carries the request ID that also appears in the gateway's logs — the identifier that lets an operator pivot between a trace and its log lines. Store and compute-driver spans become children of the request span. Reconciliation, provider refresh, and diff --git a/crates/openshell-cli/Cargo.toml b/crates/openshell-cli/Cargo.toml index 4b96253310..894bae63a9 100644 --- a/crates/openshell-cli/Cargo.toml +++ b/crates/openshell-cli/Cargo.toml @@ -17,6 +17,7 @@ path = "src/main.rs" [dependencies] openshell-bootstrap = { path = "../openshell-bootstrap" } openshell-core = { path = "../openshell-core", default-features = false } +openshell-otel = { path = "../openshell-otel" } openshell-policy = { path = "../openshell-policy" } openshell-providers = { path = "../openshell-providers" } openshell-sdk = { path = "../openshell-sdk" } diff --git a/crates/openshell-cli/src/tls.rs b/crates/openshell-cli/src/tls.rs index c24b84c7dc..26d1c7a6fa 100644 --- a/crates/openshell-cli/src/tls.rs +++ b/crates/openshell-cli/src/tls.rs @@ -6,6 +6,7 @@ use openshell_core::auth::EdgeAuthInterceptor; use openshell_core::net::set_tcp_nodelay_best_effort; use openshell_core::proto::inference_client::InferenceClient; use openshell_core::proto::open_shell_client::OpenShellClient; +use openshell_otel::EnvTraceContextInterceptor; use rustls::{ RootCertStore, client::danger::{HandshakeSignatureValid, ServerCertVerified, ServerCertVerifier}, @@ -25,9 +26,31 @@ use tracing::debug; use url::{Host, Url}; /// Concrete gRPC client type used by all commands. -pub type GrpcClient = OpenShellClient>; +pub type GrpcClient = OpenShellClient>; /// Concrete inference client type. -pub type GrpcInferenceClient = InferenceClient>; +pub type GrpcInferenceClient = InferenceClient>; + +/// Interceptor stack applied to every gateway gRPC request. +/// +/// Runs authentication header injection first, then passive W3C trace-context +/// forwarding so a CI pipeline's `TRACEPARENT` extends into the gateway's +/// spans. tonic allows a single interceptor per channel, so the two concerns +/// are composed here. +#[derive(Clone)] +pub struct GatewayInterceptor { + auth: EdgeAuthInterceptor, + trace: EnvTraceContextInterceptor, +} + +impl tonic::service::Interceptor for GatewayInterceptor { + fn call( + &mut self, + request: tonic::Request<()>, + ) -> std::result::Result, tonic::Status> { + let request = self.auth.call(request)?; + self.trace.call(request) + } +} #[derive(Clone, Debug, Default)] pub struct TlsOptions { @@ -453,17 +476,22 @@ pub async fn build_channel(server: &str, tls: &TlsOptions) -> Result { /// Build a gRPC [`OpenShellClient`]. /// -/// When `tls.edge_token` is set, the returned client is wrapped with an -/// interceptor that injects authentication headers on every request. -/// Otherwise, standard mTLS is used (interceptor is a no-op). +/// The returned client carries a [`GatewayInterceptor`] that injects +/// authentication headers (when a token is set) and passively forwards the +/// `TRACEPARENT`/`TRACESTATE` trace context from the environment on every +/// request. With no token and no trace context, the interceptor is a no-op. pub async fn grpc_client(server: &str, tls: &TlsOptions) -> Result { let channel = build_channel(server, tls).await?; let interceptor = interceptor_from_tls(tls)?; Ok(OpenShellClient::with_interceptor(channel, interceptor)) } -fn interceptor_from_tls(tls: &TlsOptions) -> Result { - EdgeAuthInterceptor::new(tls.oidc_token.as_deref(), tls.edge_token.as_deref()) +fn interceptor_from_tls(tls: &TlsOptions) -> Result { + let auth = EdgeAuthInterceptor::new(tls.oidc_token.as_deref(), tls.edge_token.as_deref())?; + Ok(GatewayInterceptor { + auth, + trace: EnvTraceContextInterceptor::from_env(), + }) } pub async fn grpc_inference_client(server: &str, tls: &TlsOptions) -> Result { diff --git a/crates/openshell-otel/src/lib.rs b/crates/openshell-otel/src/lib.rs index 81d9f7bf01..335b365c65 100644 --- a/crates/openshell-otel/src/lib.rs +++ b/crates/openshell-otel/src/lib.rs @@ -6,6 +6,10 @@ mod grpc; mod propagation; +pub use grpc::RecordGrpcFailure; +pub use propagation::{ + EnvTraceContextInterceptor, HeaderMapExtractor, MetadataMapInjector, TraceContextInterceptor, +}; pub use grpc::{RecordGrpcFailure, RecordGrpcStatus}; pub use propagation::{HeaderMapExtractor, MetadataMapInjector, TraceContextInterceptor}; diff --git a/crates/openshell-otel/src/propagation.rs b/crates/openshell-otel/src/propagation.rs index 8deb2ffe20..5a8e585c0e 100644 --- a/crates/openshell-otel/src/propagation.rs +++ b/crates/openshell-otel/src/propagation.rs @@ -4,6 +4,7 @@ //! W3C trace-context propagation for HTTP and tonic transports. use http::HeaderMap; +use opentelemetry::Context; use opentelemetry::propagation::{Extractor, Injector, TextMapPropagator}; use opentelemetry_sdk::propagation::TraceContextPropagator; use tracing_opentelemetry::OpenTelemetrySpanExt as _; @@ -70,9 +71,108 @@ impl tonic::service::Interceptor for TraceContextInterceptor { } } +/// Reads W3C trace-context propagation fields from process environment +/// variables (`TRACEPARENT` and `TRACESTATE`). +/// +/// CI systems (GitHub Actions, GitLab CI, Jenkins OpenTelemetry plugins) +/// export these variables to propagate the pipeline trace to child processes. +/// Both upper-case and lower-case spellings are accepted; blank values are +/// treated as unset. +#[derive(Debug, Clone, Default)] +struct EnvTraceContext { + traceparent: Option, + tracestate: Option, +} + +impl EnvTraceContext { + fn from_env() -> Self { + Self { + traceparent: read_env("TRACEPARENT"), + tracestate: read_env("TRACESTATE"), + } + } +} + +/// Read an environment variable by its upper-case name, falling back to the +/// lower-case spelling, trimming whitespace and discarding blank values. +fn read_env(upper: &str) -> Option { + std::env::var(upper) + .ok() + .or_else(|| std::env::var(upper.to_ascii_lowercase()).ok()) + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) +} + +impl Extractor for EnvTraceContext { + fn get(&self, key: &str) -> Option<&str> { + match key { + "traceparent" => self.traceparent.as_deref(), + "tracestate" => self.tracestate.as_deref(), + _ => None, + } + } + + fn keys(&self) -> Vec<&str> { + let mut keys = Vec::new(); + if self.traceparent.is_some() { + keys.push("traceparent"); + } + if self.tracestate.is_some() { + keys.push("tracestate"); + } + keys + } +} + +/// Passively forwards a W3C trace context captured from the process +/// environment onto outbound tonic requests. +/// +/// Unlike [`TraceContextInterceptor`], this does not require an active +/// OpenTelemetry span or an `SdkTracerProvider`; it reads `TRACEPARENT` +/// (and optionally `TRACESTATE`) once at construction and injects them as +/// gRPC metadata. This lets short-lived clients (e.g. the CLI on a CI runner) +/// extend a pipeline trace to the gateway without any collector configuration. +/// +/// When `TRACEPARENT` is unset or invalid the captured context carries no +/// valid span, so injection is a no-op. +#[derive(Debug, Clone)] +pub struct EnvTraceContextInterceptor { + context: Context, +} + +impl EnvTraceContextInterceptor { + /// Capture the W3C trace context from the process environment. + #[must_use] + pub fn from_env() -> Self { + Self { + context: context_from(&EnvTraceContext::from_env()), + } + } +} + +/// Extract a [`Context`] from an [`Extractor`] using a fresh base context, so +/// the result depends only on the extractor and not on any ambient context. +fn context_from(extractor: &impl Extractor) -> Context { + TraceContextPropagator::new().extract_with_context(&Context::new(), extractor) +} + +impl tonic::service::Interceptor for EnvTraceContextInterceptor { + fn call( + &mut self, + mut request: tonic::Request<()>, + ) -> Result, tonic::Status> { + TraceContextPropagator::new().inject_context( + &self.context, + &mut MetadataMapInjector::new(request.metadata_mut()), + ); + Ok(request) + } +} + #[cfg(test)] mod tests { use super::*; + use tonic::service::Interceptor as _; #[test] fn header_map_extractor_reads_valid_headers() { @@ -96,4 +196,58 @@ mod tests { Some("value") ); } + + fn interceptor_with(extractor: EnvTraceContext) -> EnvTraceContextInterceptor { + EnvTraceContextInterceptor { + context: context_from(&extractor), + } + } + + #[test] + fn env_interceptor_forwards_valid_traceparent() { + let traceparent = "00-4bf92f3577b34da6a3ce929d0e0e4736-00f067aa0ba902b7-01"; + let mut interceptor = interceptor_with(EnvTraceContext { + traceparent: Some(traceparent.to_string()), + tracestate: Some("vendor=value".to_string()), + }); + + let request = interceptor.call(tonic::Request::new(())).unwrap(); + + assert_eq!( + request + .metadata() + .get("traceparent") + .and_then(|value| value.to_str().ok()), + Some(traceparent) + ); + assert_eq!( + request + .metadata() + .get("tracestate") + .and_then(|value| value.to_str().ok()), + Some("vendor=value") + ); + } + + #[test] + fn env_interceptor_is_noop_without_traceparent() { + let mut interceptor = interceptor_with(EnvTraceContext::default()); + + let request = interceptor.call(tonic::Request::new(())).unwrap(); + + assert!(request.metadata().get("traceparent").is_none()); + assert!(request.metadata().get("tracestate").is_none()); + } + + #[test] + fn env_interceptor_is_noop_for_invalid_traceparent() { + let mut interceptor = interceptor_with(EnvTraceContext { + traceparent: Some("not-a-valid-traceparent".to_string()), + tracestate: None, + }); + + let request = interceptor.call(tonic::Request::new(())).unwrap(); + + assert!(request.metadata().get("traceparent").is_none()); + } }