diff --git a/Cargo.lock b/Cargo.lock index a781fdd..f4c3056 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2124,6 +2124,7 @@ dependencies = [ "tokio", "tokio-util", "tracing", + "tracing-actix-web", "tracing-opentelemetry", "tracing-subscriber", "tzf-rs", @@ -2543,6 +2544,12 @@ version = "0.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "13d2233c9842d08cfe13f9eac96e207ca6a2ea10b80259ebe8ad0268be27d2af" +[[package]] +name = "mutually_exclusive_features" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e94e1e6445d314f972ff7395df2de295fe51b71821694f0b0e1e79c4f12c8577" + [[package]] name = "native-tls" version = "0.2.18" @@ -4359,6 +4366,21 @@ dependencies = [ "tracing-core", ] +[[package]] +name = "tracing-actix-web" +version = "0.7.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3133a9a17a910bb708dcdcf1ff89d3dd98d412938b5e017c6e42de11b63ef164" +dependencies = [ + "actix-web", + "mutually_exclusive_features", + "opentelemetry", + "pin-project-lite", + "tracing", + "tracing-opentelemetry", + "uuid", +] + [[package]] name = "tracing-attributes" version = "0.1.31" diff --git a/Cargo.toml b/Cargo.toml index d84ef80..0f00fcf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,6 +45,7 @@ env_logger = "0.11.5" tracing = "0.1" tracing-subscriber = { version = "0.3", default-features = false, features = ["std", "registry", "env-filter", "fmt", "ansi"] } tracing-opentelemetry = "0.32" +tracing-actix-web = { version = "0.7", features = ["opentelemetry_0_31"] } actix-web-prom = "0.9.0" prometheus = "0.13" lazy_static = "1.5" diff --git a/src/main.rs b/src/main.rs index 0b7bc43..e46f478 100644 --- a/src/main.rs +++ b/src/main.rs @@ -326,6 +326,7 @@ fn main() -> std::io::Result<()> { App::new() .wrap(middleware::Logger::default()) .wrap(cors) + .wrap(tracing_actix_web::TracingLogger::default()) .service( web::resource("/login") .wrap(Governor::new(&governor_conf)) diff --git a/src/otel.rs b/src/otel.rs index 9384abf..3384c3b 100644 --- a/src/otel.rs +++ b/src/otel.rs @@ -134,7 +134,20 @@ pub fn start_llm_request_span( span } +/// The OpenTelemetry [`Context`] to parent this request's operation spans +/// under. Prefers the request's root span — created by the +/// `tracing-actix-web` middleware, which has already extracted any incoming +/// W3C `traceparent` — so the per-request operation span nests *under* the +/// root span (one trace tree) instead of duplicating it as a sibling. Falls +/// back to parsing the request's `traceparent` header when no root span is +/// active (e.g. a handler invoked directly in a test without the middleware). pub fn extract_context_from_request(req: &HttpRequest) -> Context { + use tracing_opentelemetry::OpenTelemetrySpanExt as _; + let current = tracing::Span::current(); + let cx = current.context(); + if cx.span().span_context().is_valid() { + return cx; + } let propagator = TraceContextPropagator::new(); propagator.extract(&HeaderExtractor(req.headers())) } diff --git a/tests/otel_traceparent.rs b/tests/otel_traceparent.rs new file mode 100644 index 0000000..e8605ea --- /dev/null +++ b/tests/otel_traceparent.rs @@ -0,0 +1,111 @@ +//! Verifies the per-request root-span middleware (#139 / map #135). +//! +//! Two things are pinned here: +//! +//! 1. **Traceparent extraction.** An incoming W3C `traceparent` header is +//! picked up by the `tracing-actix-web` middleware (via the global text-map +//! propagator installed in `otel::init_tracing`), so the request's root span +//! is parented onto the caller's span — the whole request shares the +//! caller's `trace_id`. +//! +//! 2. **No duplicate request span.** A handler that parents its operation span +//! through `otel::extract_context_from_request` nests it *under* the +//! middleware's root span (one tree) rather than re-parsing the header into +//! a second, sibling request span. +//! +//! One process-global subscriber + propagator per test binary (one-shot), so +//! this file holds a single test. + +use actix_web::{web, App, HttpRequest, HttpResponse}; +use opentelemetry::trace::{Tracer, TracerProvider}; +use opentelemetry_sdk::propagation::TraceContextPropagator; +use opentelemetry_sdk::trace::{InMemorySpanExporter, SdkTracerProvider, SpanData}; +use tracing_subscriber::layer::SubscriberExt; +use tracing_subscriber::registry::Registry; +use tracing_subscriber::util::SubscriberInitExt; + +const TRACE_ID: &str = "0af7651916cd43dd8448eb211c80319c"; +const REMOTE_PARENT: &str = "b7ad6b7169203331"; + +/// Mirrors the app's handler pattern: parent the operation span via +/// `extract_context_from_request` (which sources the middleware's root span). +async fn operation_handler(req: HttpRequest) -> HttpResponse { + let cx = image_api::otel::extract_context_from_request(&req); + let tracer = image_api::otel::global_tracer(); + let _span = tracer.start_with_context("test.operation", &cx); + HttpResponse::Ok().finish() +} + +fn find<'a>(spans: &'a [SpanData], name: &str) -> Option<&'a SpanData> { + spans.iter().find(|s| s.name.as_ref() == name) +} + +#[actix_web::test] +async fn traceparent_header_nests_request_spans() { + // In-memory exporter + provider; install as the global provider so the + // handler's `global_tracer()` and the middleware's layer share it. + let exporter = InMemorySpanExporter::default(); + let provider = SdkTracerProvider::builder() + .with_simple_exporter(exporter.clone()) + .build(); + opentelemetry::global::set_tracer_provider(provider.clone()); + // The middleware's `set_otel_parent` extracts via the global propagator. + opentelemetry::global::set_text_map_propagator(TraceContextPropagator::new()); + // Subscriber with the OTel layer (mirrors `otel::init_tracing`). + let tracer = provider.tracer("traceparent-test"); + Registry::default() + .with(tracing_opentelemetry::OpenTelemetryLayer::new(tracer)) + .try_init() + .expect("global subscriber not yet set"); + + let app = App::new() + .wrap(tracing_actix_web::TracingLogger::default()) + .service(web::resource("/op").route(web::get().to(operation_handler))); + let srv = actix_web::test::init_service(app).await; + + let req = actix_web::test::TestRequest::get() + .uri("/op") + .insert_header(( + "traceparent", + format!("00-{TRACE_ID}-{REMOTE_PARENT}-01"), + )) + .to_request(); + let _res = actix_web::test::call_and_read_body(&srv, req).await; + + let spans = exporter.get_finished_spans().expect("in-memory exporter"); + + // The middleware's root span is the one whose parent is remote (the + // caller's span from the traceparent header). + let root = spans + .iter() + .find(|s| s.parent_span_is_remote) + .expect("a root span parented on the remote traceparent span"); + // The handler's operation span. + let op = find(&spans, "test.operation").expect("the handler's operation span"); + + // (1) The whole request shares the caller's trace_id. + assert_eq!( + root.span_context.trace_id().to_string(), + TRACE_ID, + "root span must carry the incoming traceparent's trace_id" + ); + assert_eq!( + op.span_context.trace_id().to_string(), + TRACE_ID, + "operation span must be in the same trace" + ); + + // (1) The root span is parented on the caller's (remote) span. + assert_eq!( + root.parent_span_id.to_string(), + REMOTE_PARENT, + "root span parent must be the traceparent's parent span id" + ); + + // (2) The operation span nests under the root span (child, not sibling). + assert_eq!( + op.parent_span_id.to_string(), + root.span_context.span_id().to_string(), + "operation span must be a child of the middleware root span, not a sibling" + ); +}