diff --git a/Cargo.lock b/Cargo.lock index f28eedfa56..2985a654f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4169,6 +4169,7 @@ dependencies = [ name = "tracing-utils" version = "0.1.0" dependencies = [ + "hyper", "opentelemetry", "opentelemetry-otlp", "opentelemetry-semantic-conventions", diff --git a/compute_tools/src/http/api.rs b/compute_tools/src/http/api.rs index f2a49f332c..74d733424d 100644 --- a/compute_tools/src/http/api.rs +++ b/compute_tools/src/http/api.rs @@ -8,11 +8,17 @@ use hyper::service::{make_service_fn, service_fn}; use hyper::{Body, Method, Request, Response, Server, StatusCode}; use serde_json; use tracing::{error, info}; +use tracing_utils::http::OtelName; use crate::compute::ComputeNode; // Service function to handle all available routes. -async fn routes(req: Request, compute: Arc) -> Response { +async fn routes(req: Request, compute: &Arc) -> Response { + // + // NOTE: The URI path is currently included in traces. That's OK because + // it doesn't contain any variable parts or sensitive information. But + // please keep that in mind if you change the routing here. + // match (req.method(), req.uri().path()) { // Serialized compute state. (&Method::GET, "/status") => { @@ -30,7 +36,7 @@ async fn routes(req: Request, compute: Arc) -> Response (&Method::POST, "/check_writability") => { info!("serving /check_writability POST request"); - let res = crate::checker::check_writability(&compute).await; + let res = crate::checker::check_writability(compute).await; match res { Ok(_) => Response::new(Body::from("true")), Err(e) => Response::new(Body::from(e.to_string())), @@ -56,7 +62,19 @@ async fn serve(state: Arc) { async move { Ok::<_, Infallible>(service_fn(move |req: Request| { let state = state.clone(); - async move { Ok::<_, Infallible>(routes(req, state).await) } + async move { + Ok::<_, Infallible>( + // NOTE: We include the URI path in the string. It + // doesn't contain any variable parts or sensitive + // information in this API. + tracing_utils::http::tracing_handler( + req, + |req| routes(req, &state), + OtelName::UriPath, + ) + .await, + ) + } })) } }); diff --git a/libs/tracing-utils/Cargo.toml b/libs/tracing-utils/Cargo.toml index 9d5cfb0d56..8c3d3f9063 100644 --- a/libs/tracing-utils/Cargo.toml +++ b/libs/tracing-utils/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true license.workspace = true [dependencies] +hyper.workspace = true opentelemetry = { workspace = true, features=["rt-tokio"] } opentelemetry-otlp = { workspace = true, default_features=false, features = ["http-proto", "trace", "http", "reqwest-client"] } opentelemetry-semantic-conventions.workspace = true diff --git a/libs/tracing-utils/src/http.rs b/libs/tracing-utils/src/http.rs new file mode 100644 index 0000000000..3f80f49de1 --- /dev/null +++ b/libs/tracing-utils/src/http.rs @@ -0,0 +1,96 @@ +//! Tracing wrapper for Hyper HTTP server + +use hyper::HeaderMap; +use hyper::{Body, Request, Response}; +use std::future::Future; +use tracing::Instrument; +use tracing_opentelemetry::OpenTelemetrySpanExt; + +/// Configuration option for what to use as the "otel.name" field in the traces. +pub enum OtelName<'a> { + /// Use a constant string + Constant(&'a str), + + /// Use the path from the request. + /// + /// That's very useful information, but is not appropriate if the + /// path contains parameters that differ on ever request, or worse, + /// sensitive information like usernames or email addresses. + /// + /// See + UriPath, +} + +/// Handle an incoming HTTP request using the given handler function, +/// with OpenTelemetry tracing. +/// +/// This runs 'handler' on the request in a new span, with fields filled in +/// from the request. Notably, if the request contains tracing information, +/// it is propagated to the span, so that this request is traced as part of +/// the same trace. +/// +/// XXX: Usually, this is handled by existing libraries, or built +/// directly into HTTP servers. However, I couldn't find one for Hyper, +/// so I had to write our own. OpenTelemetry website has a registry of +/// instrumentation libraries at: +/// https://opentelemetry.io/registry/?language=rust&component=instrumentation +/// If a Hyper crate appears, consider switching to that. +pub async fn tracing_handler( + req: Request, + handler: F, + otel_name: OtelName<'_>, +) -> Response +where + F: Fn(Request) -> R, + R: Future>, +{ + // Create a tracing span, with context propagated from the incoming + // request if any. + // + // See list of standard fields defined for HTTP requests at + // https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/semantic_conventions/http.md + // We only fill in a few of the most useful ones here. + let otel_name = match otel_name { + OtelName::Constant(s) => s, + OtelName::UriPath => req.uri().path(), + }; + + let span = tracing::info_span!( + "http request", + otel.name= %otel_name, + http.method = %req.method(), + http.status_code = tracing::field::Empty, + ); + let parent_ctx = extract_remote_context(req.headers()); + span.set_parent(parent_ctx); + + // Handle the request within the span + let response = handler(req).instrument(span.clone()).await; + + // Fill in the fields from the response code + let status = response.status(); + span.record("http.status_code", status.as_str()); + span.record( + "otel.status_code", + if status.is_success() { "OK" } else { "ERROR" }, + ); + + response +} + +// Extract remote tracing context from the HTTP headers +fn extract_remote_context(headers: &HeaderMap) -> opentelemetry::Context { + struct HeaderExtractor<'a>(&'a HeaderMap); + + impl<'a> opentelemetry::propagation::Extractor for HeaderExtractor<'a> { + fn get(&self, key: &str) -> Option<&str> { + self.0.get(key).and_then(|value| value.to_str().ok()) + } + + fn keys(&self) -> Vec<&str> { + self.0.keys().map(|value| value.as_str()).collect() + } + } + let extractor = HeaderExtractor(headers); + opentelemetry::global::get_text_map_propagator(|propagator| propagator.extract(&extractor)) +} diff --git a/libs/tracing-utils/src/lib.rs b/libs/tracing-utils/src/lib.rs index 179f54c659..de0e2ad799 100644 --- a/libs/tracing-utils/src/lib.rs +++ b/libs/tracing-utils/src/lib.rs @@ -40,6 +40,8 @@ use opentelemetry_otlp::{OTEL_EXPORTER_OTLP_ENDPOINT, OTEL_EXPORTER_OTLP_TRACES_ pub use tracing_opentelemetry::OpenTelemetryLayer; +pub mod http; + /// Set up OpenTelemetry exporter, using configuration from environment variables. /// /// `service_name` is set as the OpenTelemetry 'service.name' resource (see @@ -141,6 +143,11 @@ fn init_tracing_internal(service_name: String) -> opentelemetry::sdk::trace::Tra } } + // Propagate trace information in the standard W3C TraceContext format. + opentelemetry::global::set_text_map_propagator( + opentelemetry::sdk::propagation::TraceContextPropagator::new(), + ); + opentelemetry_otlp::new_pipeline() .tracing() .with_exporter(exporter)