From 557791651a340f1242cd16f63e7bacf5ef073afa Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Fri, 18 Nov 2022 16:35:06 +0100 Subject: [PATCH 01/11] add support of experimental_expose_trace_id Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/plugins/telemetry/apollo.rs | 7 ++++ apollo-router/src/plugins/telemetry/config.rs | 16 ++++++++ apollo-router/src/plugins/telemetry/mod.rs | 21 ++++++++++ .../src/plugins/telemetry/tracing/apollo.rs | 2 + .../telemetry/tracing/apollo_telemetry.rs | 32 ++++++++++----- .../src/services/supergraph_service.rs | 40 ++++++++++++------- apollo-router/src/testdata/jaeger.router.yaml | 3 ++ apollo-router/tests/common.rs | 28 ++++++------- apollo-router/tests/jaeger_test.rs | 7 +++- apollo-router/tests/zipkin_test.rs | 3 +- 10 files changed, 119 insertions(+), 40 deletions(-) diff --git a/apollo-router/src/plugins/telemetry/apollo.rs b/apollo-router/src/plugins/telemetry/apollo.rs index 25237d4acf..b38757fd91 100644 --- a/apollo-router/src/plugins/telemetry/apollo.rs +++ b/apollo-router/src/plugins/telemetry/apollo.rs @@ -12,6 +12,7 @@ use serde::Deserialize; use serde::Serialize; use url::Url; +use super::config::ExposeTraceId; use super::metrics::apollo::studio::ContextualizedStats; use super::metrics::apollo::studio::SingleStats; use super::metrics::apollo::studio::SingleStatsReport; @@ -79,6 +80,11 @@ pub(crate) struct Config { // The purpose is to allow is to pass this in to the plugin. #[schemars(skip)] pub(crate) schema_id: String, + + // Skipped because only useful at runtime, it's a copy of the configuration in tracing config + #[schemars(skip)] + #[serde(skip)] + pub(crate) expose_trace_id: ExposeTraceId, } fn apollo_key() -> Option { @@ -122,6 +128,7 @@ impl Default for Config { field_level_instrumentation_sampler: Some(SamplerOption::TraceIdRatioBased(0.01)), send_headers: ForwardHeaders::None, send_variable_values: ForwardValues::None, + expose_trace_id: ExposeTraceId::default(), } } } diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index 9cbcc182c9..2f9430b8d4 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -2,6 +2,7 @@ use std::borrow::Cow; use std::collections::BTreeMap; +use axum::headers::HeaderName; use opentelemetry::sdk::Resource; use opentelemetry::Array; use opentelemetry::KeyValue; @@ -11,6 +12,7 @@ use serde::Deserialize; use super::metrics::MetricsAttributesConf; use super::*; +use crate::plugin::serde::deserialize_option_header_name; use crate::plugins::telemetry::metrics; #[derive(thiserror::Error, Debug)] @@ -78,6 +80,9 @@ pub(crate) struct MetricsCommon { #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Tracing { + /// A way to expose trace id in response headers + #[serde(default, rename = "experimental_expose_trace_id")] + pub(crate) expose_trace_id: ExposeTraceId, pub(crate) propagation: Option, pub(crate) trace_config: Option, pub(crate) otlp: Option, @@ -86,6 +91,17 @@ pub(crate) struct Tracing { pub(crate) datadog: Option, } +#[derive(Clone, Default, Debug, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields, rename_all = "snake_case")] +pub(crate) struct ExposeTraceId { + /// Expose the trace_id in response headers + pub(crate) enabled: bool, + /// Choose the header name to expose trace_id (default: apollo-trace-id) + #[schemars(with = "String")] + #[serde(deserialize_with = "deserialize_option_header_name")] + pub(crate) header_name: Option, +} + #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Propagation { diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index aa0d921c9a..84d5159cd2 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -109,6 +109,8 @@ const CLIENT_VERSION: &str = "apollo_telemetry::client_version"; const ATTRIBUTES: &str = "apollo_telemetry::metrics_attributes"; const SUBGRAPH_ATTRIBUTES: &str = "apollo_telemetry::subgraph_metrics_attributes"; pub(crate) const STUDIO_EXCLUDE: &str = "apollo_telemetry::studio::exclude"; +pub(crate) const EXPOSE_TRACE_ID_HEADER_NAME: &str = + "apollo_telemetry::expose_trace_id::header_name"; pub(crate) const FTV1_DO_NOT_SAMPLE: &str = "apollo_telemetry::studio::ftv1_do_not_sample"; const DEFAULT_SERVICE_NAME: &str = "apollo-router"; @@ -213,6 +215,22 @@ impl Plugin for Telemetry { let metrics = metrics.clone(); let sender = metrics_sender.clone(); let start = Instant::now(); + // To expose trace_id or not + let expose_trace_id_header = config + .tracing + .as_ref() + .and_then(|t| { + t.expose_trace_id + .enabled + .then(|| t.expose_trace_id.header_name.clone()) + }) + .flatten(); + if let Some(expose_trace_id_header) = expose_trace_id_header { + let _ = ctx.insert( + EXPOSE_TRACE_ID_HEADER_NAME, + expose_trace_id_header.to_string(), + ); + } async move { let mut result: Result = fut.await; result = Self::update_otel_metrics( @@ -360,6 +378,9 @@ impl Telemetry { .apollo .as_mut() .expect("telemetry apollo config must be present"); + if let Some(tracing_conf) = &config.tracing { + apollo.expose_trace_id = tracing_conf.expose_trace_id.clone(); + } // If we have key and graph ref but no endpoint we start embedded spaceport let spaceport = match apollo { diff --git a/apollo-router/src/plugins/telemetry/tracing/apollo.rs b/apollo-router/src/plugins/telemetry/tracing/apollo.rs index 788999d692..969c1a2247 100644 --- a/apollo-router/src/plugins/telemetry/tracing/apollo.rs +++ b/apollo-router/src/plugins/telemetry/tracing/apollo.rs @@ -21,11 +21,13 @@ impl TracingConfigurator for Config { schema_id, buffer_size, field_level_instrumentation_sampler, + expose_trace_id, .. } => { tracing::debug!("configuring exporter to Studio"); let exporter = apollo_telemetry::Exporter::builder() + .expose_trace_id_config(expose_trace_id.clone()) .trace_config(trace_config.clone()) .endpoint(endpoint.clone()) .apollo_key(key) diff --git a/apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs b/apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs index 41800f6578..a3374af172 100644 --- a/apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs +++ b/apollo-router/src/plugins/telemetry/tracing/apollo_telemetry.rs @@ -57,6 +57,7 @@ const FTV1_DO_NOT_SAMPLE_REASON: Key = Key::from_static_str("ftv1.do_not_sample_ const SUBGRAPH_NAME: Key = Key::from_static_str("apollo.subgraph.name"); const CLIENT_NAME: Key = Key::from_static_str("client.name"); const CLIENT_VERSION: Key = Key::from_static_str("client.version"); +pub(crate) const DEFAULT_TRACE_ID_HEADER_NAME: &str = "apollo-trace-id"; #[derive(Error, Debug)] pub(crate) enum Error { @@ -86,6 +87,7 @@ pub(crate) enum Error { #[derive(Derivative)] #[derivative(Debug)] pub(crate) struct Exporter { + expose_trace_id_config: config::ExposeTraceId, trace_config: config::Trace, spans_by_parent_id: LruCache>, endpoint: Url, @@ -111,6 +113,7 @@ enum TreeData { impl Exporter { #[builder] pub(crate) fn new( + expose_trace_id_config: config::ExposeTraceId, trace_config: config::Trace, endpoint: Url, apollo_key: String, @@ -123,6 +126,7 @@ impl Exporter { let apollo_exporter = ApolloExporter::new(&endpoint, &apollo_key, &apollo_graph_ref, &schema_id)?; Ok(Self { + expose_trace_id_config, spans_by_parent_id: LruCache::new(buffer_size), trace_config, endpoint, @@ -427,18 +431,28 @@ impl Exporter { .map(|(header_name, value)| (header_name.to_lowercase(), Values { value })) .collect(); // For now, only trace_id - // let mut response_headers = HashMap::with_capacity(1); - // FIXME: uncomment later - // response_headers.insert( - // String::from("apollo_trace_id"), - // Values { - // value: vec![span.span_context.trace_id().to_string()], - // }, - // ); + let response_headers = if self.expose_trace_id_config.enabled { + let mut res = HashMap::with_capacity(1); + res.insert( + self.expose_trace_id_config + .header_name + .as_ref() + .map(|h| h.to_string()) + .unwrap_or_else(|| DEFAULT_TRACE_ID_HEADER_NAME.to_string()), + Values { + value: vec![span.span_context.trace_id().to_string()], + }, + ); + + res + } else { + HashMap::new() + }; + Http { method: method.into(), request_headers, - response_headers: HashMap::new(), + response_headers, status_code: 0, } } diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index 8c00aa8941..debcdf3c85 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -1,11 +1,14 @@ //! Implements the router phase of the request lifecycle. +use std::str::FromStr; use std::sync::Arc; use std::task::Poll; +use axum::headers::HeaderName; use futures::future::BoxFuture; use futures::stream::StreamExt; use futures::TryFutureExt; +use http::HeaderValue; use http::StatusCode; use indexmap::IndexMap; use multimap::MultiMap; @@ -31,6 +34,7 @@ use crate::graphql; use crate::graphql::IntoGraphQLErrors; use crate::introspection::Introspection; use crate::plugin::DynPlugin; +use crate::plugins::telemetry::EXPOSE_TRACE_ID_HEADER_NAME; use crate::plugins::traffic_shaping::TrafficShaping; use crate::plugins::traffic_shaping::APOLLO_TRAFFIC_SHAPING; use crate::query_planner::BridgeQueryPlanner; @@ -38,6 +42,7 @@ use crate::query_planner::CachingQueryPlanner; use crate::router_factory::Endpoint; use crate::router_factory::SupergraphServiceFactory; use crate::services::layers::ensure_query_presence::EnsureQueryPresence; +use crate::tracer::TraceId; use crate::Configuration; use crate::Context; use crate::ExecutionRequest; @@ -100,8 +105,8 @@ where let schema = self.schema.clone(); let context_cloned = req.context.clone(); - let fut = - service_call(planning, execution, schema, req).or_else(|error: BoxError| async move { + let fut = service_call(planning, execution, schema, req) + .or_else(|error: BoxError| async move { let errors = vec![crate::error::Error { message: error.to_string(), extensions: serde_json_bytes::json!({ @@ -119,20 +124,25 @@ where .context(context_cloned) .build() .expect("building a response like this should not fail")) + }) + .and_then(|mut res| async move { + if let Some(header_name_str) = res + .context + .get::<_, String>(EXPOSE_TRACE_ID_HEADER_NAME) + .ok() + .flatten() + { + if let Some(trace_id) = TraceId::maybe_new().map(|t| t.to_string()) { + let header_name = HeaderName::from_str(&header_name_str); + let header_value = HeaderValue::from_str(trace_id.as_str()); + if let (Ok(header_name), Ok(header_value)) = (header_name, header_value) { + res.response.headers_mut().insert(header_name, header_value); + } + } + } + + Ok(res) }); - // FIXME: Enable it later - // .and_then(|mut res| async move { - // if let Some(trace_id) = TraceId::maybe_new().map(|t| t.to_string()) { - // let header_value = HeaderValue::from_str(trace_id.as_str()); - // if let Ok(header_value) = header_value { - // res.response - // .headers_mut() - // .insert(HeaderName::from_static("apollo_trace_id"), header_value); - // } - // } - - // Ok(res) - // }); Box::pin(fut) } diff --git a/apollo-router/src/testdata/jaeger.router.yaml b/apollo-router/src/testdata/jaeger.router.yaml index 641e32ca47..d7112b23ad 100644 --- a/apollo-router/src/testdata/jaeger.router.yaml +++ b/apollo-router/src/testdata/jaeger.router.yaml @@ -1,5 +1,8 @@ telemetry: tracing: + experimental_expose_trace_id: + enabled: true + header_name: apollo-custom-trace-id trace_config: service_name: router jaeger: diff --git a/apollo-router/tests/common.rs b/apollo-router/tests/common.rs index 609b4b607a..2803d4ca91 100644 --- a/apollo-router/tests/common.rs +++ b/apollo-router/tests/common.rs @@ -7,6 +7,8 @@ use std::process::Command; use std::process::Stdio; use std::time::Duration; +use axum::response::Response; +use bytes::Bytes; use http::header::ACCEPT; use http::header::CONTENT_TYPE; use http::Method; @@ -17,6 +19,7 @@ use opentelemetry::global; use opentelemetry::propagation::TextMapPropagator; use opentelemetry::sdk::trace::Tracer; use opentelemetry_http::HttpClient; +use serde_json::json; use serde_json::Value; use tower::BoxError; use tracing::info_span; @@ -65,8 +68,8 @@ impl TracingTest { Self { test_config_location: test_config_location.clone(), router: Command::new(router_location) - .stdout(Stdio::null()) - .stderr(Stdio::null()) + .stdout(Stdio::inherit()) + .stderr(Stdio::inherit()) .args([ "--hr", "--config", @@ -89,20 +92,20 @@ impl TracingTest { Ok(()) } - pub async fn run_query(&self) -> String { + pub async fn run_query(&self) -> (String, reqwest::Response) { let client = reqwest::Client::new(); let id = Uuid::new_v4().to_string(); let span = info_span!("client_request", unit_test = id.as_str()); let _span_guard = span.enter(); for _i in 0..100 { - let mut request = Request::builder() - .method(Method::POST) + let mut request = client + .post("http://localhost:4000") .header(CONTENT_TYPE, "application/json") .header("apollographql-client-name", "custom_name") .header("apollographql-client-version", "1.0") - .uri(Uri::from_static("http://localhost:4000")) - .body(r#"{"query":"{topProducts{name}}","variables":{}}"#.into()) + .json(&json!({"query":"{topProducts{name}}","variables":{}})) + .build() .unwrap(); global::get_text_map_propagator(|propagator| { @@ -112,16 +115,13 @@ impl TracingTest { ) }); request.headers_mut().remove(ACCEPT); - match client.send(request).await { + match client.execute(request).await { Ok(result) => { - tracing::debug!( - "got {}", - String::from_utf8(result.body().to_vec()).unwrap_or_default() - ); - return id; + tracing::debug!("got {result:?}"); + return (id, result); } Err(e) => { - tracing::debug!("query failed: {}", e); + println!("query failed: {}", e); } } tokio::time::sleep(Duration::from_millis(100)).await; diff --git a/apollo-router/tests/jaeger_test.rs b/apollo-router/tests/jaeger_test.rs index 6f7335c008..a92f20f864 100644 --- a/apollo-router/tests/jaeger_test.rs +++ b/apollo-router/tests/jaeger_test.rs @@ -42,7 +42,12 @@ async fn test_jaeger_tracing() -> Result<(), BoxError> { tokio::task::spawn(subgraph()); for _ in 0..10 { - let id = router.run_query().await; + let (id, result) = router.run_query().await; + assert!(result + .headers() + .get("apollo-custom-trace-id") + .unwrap() + .is_empty()); query_jaeger_for_trace(id).await?; router.touch_config()?; tokio::time::sleep(Duration::from_millis(100)).await; diff --git a/apollo-router/tests/zipkin_test.rs b/apollo-router/tests/zipkin_test.rs index c6387c61b7..9b7557c714 100644 --- a/apollo-router/tests/zipkin_test.rs +++ b/apollo-router/tests/zipkin_test.rs @@ -16,7 +16,8 @@ async fn test_tracing() -> Result<(), BoxError> { opentelemetry_zipkin::Propagator::new(), Path::new("zipkin.router.yaml"), ); - router.run_query().await; + let (_, response) = router.run_query().await; + assert!(response.headers().get("apollo-trace-id").is_none()); Ok(()) } From 0d601accd8b2b81692cd0b24ca72012dc443a094 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Fri, 18 Nov 2022 18:09:31 +0100 Subject: [PATCH 02/11] add support of custom header propagator for trace_id Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/axum_factory/utils.rs | 4 +- apollo-router/src/plugins/telemetry/config.rs | 4 + apollo-router/src/plugins/telemetry/mod.rs | 85 ++++++++++++++++++- apollo-router/tests/common.rs | 6 -- 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/apollo-router/src/axum_factory/utils.rs b/apollo-router/src/axum_factory/utils.rs index 8ba5e6cdfb..d855f1afa8 100644 --- a/apollo-router/src/axum_factory/utils.rs +++ b/apollo-router/src/axum_factory/utils.rs @@ -238,7 +238,9 @@ impl MakeSpan for PropagatingMakeSpan { // If there was no span from the request then it will default to the NOOP span. // Attaching the NOOP span has the effect of preventing further tracing. - if context.span().span_context().is_valid() { + if context.span().span_context().is_valid() + || context.span().span_context().trace_id() != opentelemetry::trace::TraceId::INVALID + { // We have a valid remote span, attach it to the current thread before creating the root span. let _context_guard = context.attach(); tracing::span!( diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index 2f9430b8d4..1ae0f58923 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -105,6 +105,10 @@ pub(crate) struct ExposeTraceId { #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Propagation { + /// Select a custom header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id) + #[schemars(with = "String")] + #[serde(deserialize_with = "deserialize_option_header_name")] + pub(crate) custom_header: Option, pub(crate) baggage: Option, pub(crate) trace_context: Option, pub(crate) jaeger: Option, diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 84d5159cd2..422f6d5f95 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -27,13 +27,20 @@ use http::HeaderValue; use multimap::MultiMap; use once_cell::sync::OnceCell; use opentelemetry::global; +use opentelemetry::propagation::text_map_propagator::FieldIter; +use opentelemetry::propagation::Extractor; +use opentelemetry::propagation::Injector; use opentelemetry::propagation::TextMapPropagator; use opentelemetry::sdk::propagation::BaggagePropagator; use opentelemetry::sdk::propagation::TextMapCompositePropagator; use opentelemetry::sdk::propagation::TraceContextPropagator; use opentelemetry::sdk::trace::Builder; +use opentelemetry::trace::SpanContext; +use opentelemetry::trace::SpanId; use opentelemetry::trace::SpanKind; use opentelemetry::trace::TraceContextExt; +use opentelemetry::trace::TraceFlags; +use opentelemetry::trace::TraceState; use opentelemetry::trace::TracerProvider; use opentelemetry::KeyValue; use rand::Rng; @@ -113,6 +120,7 @@ pub(crate) const EXPOSE_TRACE_ID_HEADER_NAME: &str = "apollo_telemetry::expose_trace_id::header_name"; pub(crate) const FTV1_DO_NOT_SAMPLE: &str = "apollo_telemetry::studio::ftv1_do_not_sample"; const DEFAULT_SERVICE_NAME: &str = "apollo-router"; +const GLOBAL_TRACER_NAME: &str = "apollo-router"; static TELEMETRY_LOADED: OnceCell = OnceCell::new(); static TELEMETRY_REFCOUNT: AtomicU8 = AtomicU8::new(0); @@ -416,7 +424,7 @@ impl Telemetry { let tracer_provider = Self::create_tracer_provider(&config)?; let tracer = tracer_provider.versioned_tracer( - "apollo-router", + GLOBAL_TRACER_NAME, Some(env!("CARGO_PKG_VERSION")), None, ); @@ -556,6 +564,11 @@ impl Telemetry { if propagation.datadog.unwrap_or_default() || tracing.datadog.is_some() { propagators.push(Box::new(opentelemetry_datadog::DatadogPropagator::default())); } + if let Some(custom_header) = &propagation.custom_header { + propagators.push(Box::new(CustomTraceIdPropagator::new( + custom_header.to_string(), + ))); + } TextMapCompositePropagator::new(propagators) } @@ -1266,6 +1279,76 @@ impl ApolloFtv1Handler { } } +/// CustomTraceIdPropagator to set custom trace_id for our tracing system +/// coming from headers +#[derive(Debug)] +struct CustomTraceIdPropagator { + header_name: String, + fields: [String; 1], +} + +impl CustomTraceIdPropagator { + fn new(header_name: String) -> Self { + Self { + fields: [header_name.clone()], + header_name, + } + } + + fn extract_span_context(&self, extractor: &dyn Extractor) -> Option { + let trace_id = extractor.get(&self.header_name)?; + + opentelemetry::global::tracer_provider().versioned_tracer( + GLOBAL_TRACER_NAME, + Some(env!("CARGO_PKG_VERSION")), + None, + ); + // extract trace id + let trace_id = match opentelemetry::trace::TraceId::from_hex(trace_id) { + Ok(trace_id) => trace_id, + Err(err) => { + ::tracing::error!("cannot generate custom trace_id: {err}"); + return None; + } + }; + + SpanContext::new( + trace_id, + SpanId::INVALID, + TraceFlags::default().with_sampled(true), + true, + TraceState::default(), + ) + .into() + } +} + +impl TextMapPropagator for CustomTraceIdPropagator { + fn inject_context(&self, cx: &opentelemetry::Context, injector: &mut dyn Injector) { + let span = cx.span(); + let span_context = span.span_context(); + if span_context.is_valid() { + let header_value = format!("{}", span_context.trace_id()); + injector.set(&self.header_name, header_value); + } + } + + fn extract_with_context( + &self, + cx: &opentelemetry::Context, + extractor: &dyn Extractor, + ) -> opentelemetry::Context { + cx.with_remote_span_context( + self.extract_span_context(extractor) + .unwrap_or_else(SpanContext::empty_context), + ) + } + + fn fields(&self) -> FieldIter<'_> { + FieldIter::new(self.fields.as_ref()) + } +} + // // Please ensure that any tests added to the tests module use the tokio multi-threaded test executor. // diff --git a/apollo-router/tests/common.rs b/apollo-router/tests/common.rs index 2803d4ca91..5c7be812c5 100644 --- a/apollo-router/tests/common.rs +++ b/apollo-router/tests/common.rs @@ -7,18 +7,12 @@ use std::process::Command; use std::process::Stdio; use std::time::Duration; -use axum::response::Response; -use bytes::Bytes; use http::header::ACCEPT; use http::header::CONTENT_TYPE; -use http::Method; -use http::Request; -use http::Uri; use jsonpath_lib::Selector; use opentelemetry::global; use opentelemetry::propagation::TextMapPropagator; use opentelemetry::sdk::trace::Tracer; -use opentelemetry_http::HttpClient; use serde_json::json; use serde_json::Value; use tower::BoxError; From b136e0dd7834f41addab5854065d79747ac7ca7f Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Mon, 21 Nov 2022 17:07:46 +0100 Subject: [PATCH 03/11] add docs Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/configuration/mod.rs | 15 +++++++ apollo-router/src/plugins/telemetry/config.rs | 2 + apollo-router/src/plugins/telemetry/mod.rs | 40 ++++++++++--------- .../src/services/supergraph_service.rs | 27 +------------ docs/source/configuration/tracing.mdx | 32 +++++++++++---- 5 files changed, 64 insertions(+), 52 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 22e5d6ebfe..3cd895f9fc 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -207,6 +207,21 @@ impl Configuration { self.apollo_plugins .plugins .insert("include_subgraph_errors".to_string(), json!({"all": true})); + // Enable experimental_expose_trace_id + self.apollo_plugins + .plugins + .get_mut("telemetry") + .expect("telemetry plugin must be initialized at this point") + .as_object_mut() + .expect("configuration for telemetry must be an object") + .entry("tracing") + .and_modify(|e| { + e.as_object_mut() + .expect("configuration for telemetry.tracing must be an object") + .entry("experimental_expose_trace_id") + .and_modify(|e| *e = json!({"enabled": true, "header_name": null})) + .or_insert_with(|| json!({"enabled": true, "header_name": null})); + }); self.supergraph.introspection = true; self.sandbox.enabled = true; self.homepage.enabled = false; diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index 1ae0f58923..29a8ce7708 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -80,6 +80,8 @@ pub(crate) struct MetricsCommon { #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Tracing { + // TODO: when deleting the `experimental_` prefix, check the usage when enabling dev mode + // When deleting, put a #[serde(alias = "experimental_expose_trace_id")] if we don't want to break things /// A way to expose trace id in response headers #[serde(default, rename = "experimental_expose_trace_id")] pub(crate) expose_trace_id: ExposeTraceId, diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 422f6d5f95..3807360b05 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -16,6 +16,7 @@ use ::tracing::info_span; use ::tracing::subscriber::set_global_default; use ::tracing::Span; use ::tracing::Subscriber; +use axum::headers::HeaderName; use futures::future::ready; use futures::future::BoxFuture; use futures::stream::once; @@ -94,6 +95,7 @@ use crate::spaceport::server::ReportSpaceport; use crate::spaceport::StatsContext; use crate::subgraph::Request; use crate::subgraph::Response; +use crate::tracer::TraceId; use crate::Context; use crate::ExecutionRequest; use crate::ListenAddr; @@ -116,11 +118,10 @@ const CLIENT_VERSION: &str = "apollo_telemetry::client_version"; const ATTRIBUTES: &str = "apollo_telemetry::metrics_attributes"; const SUBGRAPH_ATTRIBUTES: &str = "apollo_telemetry::subgraph_metrics_attributes"; pub(crate) const STUDIO_EXCLUDE: &str = "apollo_telemetry::studio::exclude"; -pub(crate) const EXPOSE_TRACE_ID_HEADER_NAME: &str = - "apollo_telemetry::expose_trace_id::header_name"; pub(crate) const FTV1_DO_NOT_SAMPLE: &str = "apollo_telemetry::studio::ftv1_do_not_sample"; const DEFAULT_SERVICE_NAME: &str = "apollo-router"; const GLOBAL_TRACER_NAME: &str = "apollo-router"; +const DEFAULT_EXPOSE_TRACE_ID_HEADER: &str = "apollo-trace-id"; static TELEMETRY_LOADED: OnceCell = OnceCell::new(); static TELEMETRY_REFCOUNT: AtomicU8 = AtomicU8::new(0); @@ -195,13 +196,15 @@ impl Plugin for Telemetry { let metrics_sender = self.apollo_metrics_sender.clone(); let metrics = BasicMetrics::new(&self.meter_provider); let config = Arc::new(self.config.clone()); + let config_map_res_first = config.clone(); let config_map_res = config.clone(); ServiceBuilder::new() .instrument(Self::supergraph_service_span( self.field_level_instrumentation_ratio, config.apollo.clone().unwrap_or_default(), )) - .map_response(|resp: SupergraphResponse| { + .map_response(move |mut resp: SupergraphResponse| { + let config = config_map_res_first.clone(); if let Ok(Some(usage_reporting)) = resp.context.get::<_, UsageReporting>(USAGE_REPORTING) { @@ -211,6 +214,20 @@ impl Plugin for Telemetry { &usage_reporting.stats_report_key.as_str(), ); } + // To expose trace_id or not + let expose_trace_id_header = config.tracing.as_ref().and_then(|t| { + t.expose_trace_id.enabled.then(|| { + t.expose_trace_id.header_name.clone().unwrap_or_else(|| { + HeaderName::from_static(DEFAULT_EXPOSE_TRACE_ID_HEADER) + }) + }) + }); + if let (Some(header_name), Some(trace_id)) = ( + expose_trace_id_header, + TraceId::maybe_new().and_then(|t| HeaderValue::from_str(&t.to_string()).ok()), + ) { + resp.response.headers_mut().append(header_name, trace_id); + } resp }) .map_future_with_request_data( @@ -223,22 +240,7 @@ impl Plugin for Telemetry { let metrics = metrics.clone(); let sender = metrics_sender.clone(); let start = Instant::now(); - // To expose trace_id or not - let expose_trace_id_header = config - .tracing - .as_ref() - .and_then(|t| { - t.expose_trace_id - .enabled - .then(|| t.expose_trace_id.header_name.clone()) - }) - .flatten(); - if let Some(expose_trace_id_header) = expose_trace_id_header { - let _ = ctx.insert( - EXPOSE_TRACE_ID_HEADER_NAME, - expose_trace_id_header.to_string(), - ); - } + async move { let mut result: Result = fut.await; result = Self::update_otel_metrics( diff --git a/apollo-router/src/services/supergraph_service.rs b/apollo-router/src/services/supergraph_service.rs index debcdf3c85..072f581658 100644 --- a/apollo-router/src/services/supergraph_service.rs +++ b/apollo-router/src/services/supergraph_service.rs @@ -1,14 +1,11 @@ //! Implements the router phase of the request lifecycle. -use std::str::FromStr; use std::sync::Arc; use std::task::Poll; -use axum::headers::HeaderName; use futures::future::BoxFuture; use futures::stream::StreamExt; use futures::TryFutureExt; -use http::HeaderValue; use http::StatusCode; use indexmap::IndexMap; use multimap::MultiMap; @@ -34,7 +31,6 @@ use crate::graphql; use crate::graphql::IntoGraphQLErrors; use crate::introspection::Introspection; use crate::plugin::DynPlugin; -use crate::plugins::telemetry::EXPOSE_TRACE_ID_HEADER_NAME; use crate::plugins::traffic_shaping::TrafficShaping; use crate::plugins::traffic_shaping::APOLLO_TRAFFIC_SHAPING; use crate::query_planner::BridgeQueryPlanner; @@ -42,7 +38,6 @@ use crate::query_planner::CachingQueryPlanner; use crate::router_factory::Endpoint; use crate::router_factory::SupergraphServiceFactory; use crate::services::layers::ensure_query_presence::EnsureQueryPresence; -use crate::tracer::TraceId; use crate::Configuration; use crate::Context; use crate::ExecutionRequest; @@ -105,8 +100,8 @@ where let schema = self.schema.clone(); let context_cloned = req.context.clone(); - let fut = service_call(planning, execution, schema, req) - .or_else(|error: BoxError| async move { + let fut = + service_call(planning, execution, schema, req).or_else(|error: BoxError| async move { let errors = vec![crate::error::Error { message: error.to_string(), extensions: serde_json_bytes::json!({ @@ -124,24 +119,6 @@ where .context(context_cloned) .build() .expect("building a response like this should not fail")) - }) - .and_then(|mut res| async move { - if let Some(header_name_str) = res - .context - .get::<_, String>(EXPOSE_TRACE_ID_HEADER_NAME) - .ok() - .flatten() - { - if let Some(trace_id) = TraceId::maybe_new().map(|t| t.to_string()) { - let header_name = HeaderName::from_str(&header_name_str); - let header_value = HeaderValue::from_str(trace_id.as_str()); - if let (Ok(header_name), Ok(header_value)) = (header_name, header_value) { - res.response.headers_mut().insert(header_name, header_value); - } - } - } - - Ok(res) }); Box::pin(fut) diff --git a/docs/source/configuration/tracing.mdx b/docs/source/configuration/tracing.mdx index c7641baef5..20c5bf2160 100644 --- a/docs/source/configuration/tracing.mdx +++ b/docs/source/configuration/tracing.mdx @@ -7,16 +7,16 @@ import { Link } from "gatsby"; The Apollo Router supports [OpenTelemetry](https://opentelemetry.io/), with exporters for: -* [Jaeger](https://www.jaegertracing.io/) -* [Zipkin](https://zipkin.io/) -* [Datadog](https://www.datadoghq.com/) -* [OpenTelemetry Protocol (OTLP)](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md) over HTTP or gRPC +- [Jaeger](https://www.jaegertracing.io/) +- [Zipkin](https://zipkin.io/) +- [Datadog](https://www.datadoghq.com/) +- [OpenTelemetry Protocol (OTLP)](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md) over HTTP or gRPC The Apollo Router generates spans that include the various phases of serving a request and associated dependencies. This is useful for showing how response time is affected by: -* Sub-request response times -* Query shape (sub-request dependencies) -* Apollo Router post-processing +- Sub-request response times +- Query shape (sub-request dependencies) +- Apollo Router post-processing Span data is sent to a collector such as [Jaeger](https://www.jaegertracing.io/), which can assemble spans into a gantt chart for analysis. @@ -81,9 +81,26 @@ telemetry: # https://zipkin.io/ (compliant with opentracing) zipkin: false + # If you have your own way to generate a trace id and you want to pass it via a custom header + custom_header: "x-request-id" ``` + Specifying explicit propagation is generally only required if you're using an exporter that supports multiple trace ID formats (e.g., OpenTelemetry Collector, Jaeger, or OpenTracing compatible exporters). +### Trace ID + +If you want to expose in response headers the generated trace ID or the one you provided using propagation headers you can use this configuration: + +```yaml title="router.yaml" +telemetry: + tracing: + experimental_expose_trace_id: + enabled: true # default: false + header_name: "my-trace-id" # default: "apollo-trace-id" +``` + +Using this configuration you will have a response header called `my-trace-id` containing the trace ID. It could help you to debug a specific query if you want to grep your log with this trace id to have more context. + ## Using Datadog The Apollo Router can be configured to connect to either the default agent address or a URL. @@ -109,7 +126,6 @@ telemetry: agent: # Either 'default' or a URL endpoint: docker_jaeger:14268 - ``` ### Collector config From 2130697d613e8bb0033241b071f08e99f9758642 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 22 Nov 2022 10:50:48 +0100 Subject: [PATCH 04/11] fix tests Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- apollo-router/src/configuration/mod.rs | 8 +++++++ ...nfiguration__tests__schema_generation.snap | 24 +++++++++++++++++++ apollo-router/src/plugins/telemetry/config.rs | 4 ++-- apollo-router/src/router.rs | 16 +++++++++---- 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 3cd895f9fc..77f74a196d 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -221,6 +221,14 @@ impl Configuration { .entry("experimental_expose_trace_id") .and_modify(|e| *e = json!({"enabled": true, "header_name": null})) .or_insert_with(|| json!({"enabled": true, "header_name": null})); + }) + .or_insert_with(|| { + json!({ + "experimental_expose_trace_id": { + "enabled": true, + "header_name": null + } + }) }); self.supergraph.introspection = true; self.sandbox.enabled = true; diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 8a249cfcc5..a734f8acc7 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -1702,6 +1702,25 @@ expression: "&schema" "additionalProperties": false, "nullable": true }, + "experimental_expose_trace_id": { + "description": "A way to expose trace id in response headers", + "type": "object", + "required": [ + "enabled" + ], + "properties": { + "enabled": { + "description": "Expose the trace_id in response headers", + "type": "boolean" + }, + "header_name": { + "description": "Choose the header name to expose trace_id (default: apollo-trace-id)", + "type": "string", + "nullable": true + } + }, + "additionalProperties": false + }, "jaeger": { "type": "object", "oneOf": [ @@ -1843,6 +1862,11 @@ expression: "&schema" "type": "boolean", "nullable": true }, + "custom_header": { + "description": "Select a custom header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id)", + "type": "string", + "nullable": true + }, "datadog": { "type": "boolean", "nullable": true diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index 29a8ce7708..13942063c8 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -99,7 +99,7 @@ pub(crate) struct ExposeTraceId { /// Expose the trace_id in response headers pub(crate) enabled: bool, /// Choose the header name to expose trace_id (default: apollo-trace-id) - #[schemars(with = "String")] + #[schemars(with = "Option")] #[serde(deserialize_with = "deserialize_option_header_name")] pub(crate) header_name: Option, } @@ -108,7 +108,7 @@ pub(crate) struct ExposeTraceId { #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Propagation { /// Select a custom header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id) - #[schemars(with = "String")] + #[schemars(with = "Option")] #[serde(deserialize_with = "deserialize_option_header_name")] pub(crate) custom_header: Option, pub(crate) baggage: Option, diff --git a/apollo-router/src/router.rs b/apollo-router/src/router.rs index b671274241..88d583f5c4 100644 --- a/apollo-router/src/router.rs +++ b/apollo-router/src/router.rs @@ -743,10 +743,18 @@ mod tests { #[tokio::test(flavor = "multi_thread")] async fn config_dev_mode_without_file() { - let mut stream = - ConfigurationSource::from(Configuration::builder().dev(true).build().unwrap()) - .into_stream() - .boxed(); + let telemetry_configuration = serde_json::json!({ + "telemetry": {} + }); + let mut stream = ConfigurationSource::from( + Configuration::builder() + .apollo_plugin("telemetry", telemetry_configuration) + .dev(true) + .build() + .unwrap(), + ) + .into_stream() + .boxed(); let cfg = match stream.next().await.unwrap() { UpdateConfiguration(configuration) => configuration, From 79ed3337f77fd58043f46b60445313b2068beb3c Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 22 Nov 2022 11:22:16 +0100 Subject: [PATCH 05/11] changelog Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- NEXT_CHANGELOG.md | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index f8b89474ea..f1cd7c560c 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -27,6 +27,24 @@ By [@USERNAME](https://github.com/USERNAME) in https://github.com/apollographql/ ## ❗ BREAKING ❗ ## 🚀 Features +### Add configuration for trace ID ([Issue #2080](https://github.com/apollographql/router/issues/2080)) + +If you want to expose in response headers the generated trace ID or the one you provided using propagation headers you can use this configuration: + +```yaml title="router.yaml" +telemetry: + tracing: + experimental_expose_trace_id: + enabled: true # default: false + header_name: "my-trace-id" # default: "apollo-trace-id" + propagation: + custom_header: "x-request-id" # Specify your own trace_id with a custom header in request headers +``` + +Using this configuration you will have a response header called `my-trace-id` containing the trace ID. It could help you to debug a specific query if you want to grep your log with this trace id to have more context. + +By [@bnjjj](https://github.com/bnjjj) in https://github.com/apollographql/router/pull/2131 + ### Add a supergraph configmap option to the helm chart ([PR #2119](https://github.com/apollographql/router/pull/2119)) Adds the capability to create a configmap containing your supergraph schema. Here's an example of how you could make use of this from your values.yaml and with the `helm` install command. From 77cecffcbec9eee4b095d24fc2ad8a2ebc190302 Mon Sep 17 00:00:00 2001 From: Coenen Benjamin Date: Wed, 23 Nov 2022 11:58:43 +0100 Subject: [PATCH 06/11] Update apollo-router/src/plugins/telemetry/mod.rs Co-authored-by: Geoffroy Couprie --- apollo-router/src/plugins/telemetry/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 3807360b05..4d92996f61 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -217,9 +217,9 @@ impl Plugin for Telemetry { // To expose trace_id or not let expose_trace_id_header = config.tracing.as_ref().and_then(|t| { t.expose_trace_id.enabled.then(|| { - t.expose_trace_id.header_name.clone().unwrap_or_else(|| { + t.expose_trace_id.header_name.clone().unwrap_or( HeaderName::from_static(DEFAULT_EXPOSE_TRACE_ID_HEADER) - }) + ) }) }); if let (Some(header_name), Some(trace_id)) = ( From 3effab5b590c8e9ae060cd17ecfab9a8b6cc3f7d Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:05:03 +0100 Subject: [PATCH 07/11] change naming Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- NEXT_CHANGELOG.md | 4 ++-- apollo-router/src/configuration/mod.rs | 6 +++--- ...__configuration__tests__schema_generation.snap | 4 ++-- apollo-router/src/plugins/telemetry/config.rs | 10 +++++----- apollo-router/src/plugins/telemetry/mod.rs | 15 ++++++++------- apollo-router/src/testdata/jaeger.router.yaml | 2 +- docs/source/configuration/tracing.mdx | 4 ++-- 7 files changed, 23 insertions(+), 22 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index f1cd7c560c..4af94bdcec 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -34,11 +34,11 @@ If you want to expose in response headers the generated trace ID or the one you ```yaml title="router.yaml" telemetry: tracing: - experimental_expose_trace_id: + experimental_response_trace_id: enabled: true # default: false header_name: "my-trace-id" # default: "apollo-trace-id" propagation: - custom_header: "x-request-id" # Specify your own trace_id with a custom header in request headers + from_request_header: "x-request-id" # Specify your own trace_id with a custom header in request headers ``` Using this configuration you will have a response header called `my-trace-id` containing the trace ID. It could help you to debug a specific query if you want to grep your log with this trace id to have more context. diff --git a/apollo-router/src/configuration/mod.rs b/apollo-router/src/configuration/mod.rs index 77f74a196d..409aee2429 100644 --- a/apollo-router/src/configuration/mod.rs +++ b/apollo-router/src/configuration/mod.rs @@ -207,7 +207,7 @@ impl Configuration { self.apollo_plugins .plugins .insert("include_subgraph_errors".to_string(), json!({"all": true})); - // Enable experimental_expose_trace_id + // Enable experimental_response_trace_id self.apollo_plugins .plugins .get_mut("telemetry") @@ -218,13 +218,13 @@ impl Configuration { .and_modify(|e| { e.as_object_mut() .expect("configuration for telemetry.tracing must be an object") - .entry("experimental_expose_trace_id") + .entry("experimental_response_trace_id") .and_modify(|e| *e = json!({"enabled": true, "header_name": null})) .or_insert_with(|| json!({"enabled": true, "header_name": null})); }) .or_insert_with(|| { json!({ - "experimental_expose_trace_id": { + "experimental_response_trace_id": { "enabled": true, "header_name": null } diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index a734f8acc7..78fd0f35ce 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -1702,7 +1702,7 @@ expression: "&schema" "additionalProperties": false, "nullable": true }, - "experimental_expose_trace_id": { + "experimental_response_trace_id": { "description": "A way to expose trace id in response headers", "type": "object", "required": [ @@ -1862,7 +1862,7 @@ expression: "&schema" "type": "boolean", "nullable": true }, - "custom_header": { + "from_request_header": { "description": "Select a custom header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id)", "type": "string", "nullable": true diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index 13942063c8..106729c6d0 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -81,10 +81,10 @@ pub(crate) struct MetricsCommon { #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Tracing { // TODO: when deleting the `experimental_` prefix, check the usage when enabling dev mode - // When deleting, put a #[serde(alias = "experimental_expose_trace_id")] if we don't want to break things + // When deleting, put a #[serde(alias = "experimental_response_trace_id")] if we don't want to break things /// A way to expose trace id in response headers - #[serde(default, rename = "experimental_expose_trace_id")] - pub(crate) expose_trace_id: ExposeTraceId, + #[serde(default, rename = "experimental_response_trace_id")] + pub(crate) response_trace_id: ExposeTraceId, pub(crate) propagation: Option, pub(crate) trace_config: Option, pub(crate) otlp: Option, @@ -107,10 +107,10 @@ pub(crate) struct ExposeTraceId { #[derive(Clone, Default, Debug, Deserialize, JsonSchema)] #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Propagation { - /// Select a custom header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id) + /// Select a custom request header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id) #[schemars(with = "Option")] #[serde(deserialize_with = "deserialize_option_header_name")] - pub(crate) custom_header: Option, + pub(crate) from_request_header: Option, pub(crate) baggage: Option, pub(crate) trace_context: Option, pub(crate) jaeger: Option, diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 4d92996f61..31d3e14eb8 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -216,10 +216,11 @@ impl Plugin for Telemetry { } // To expose trace_id or not let expose_trace_id_header = config.tracing.as_ref().and_then(|t| { - t.expose_trace_id.enabled.then(|| { - t.expose_trace_id.header_name.clone().unwrap_or( - HeaderName::from_static(DEFAULT_EXPOSE_TRACE_ID_HEADER) - ) + t.response_trace_id.enabled.then(|| { + t.response_trace_id + .header_name + .clone() + .unwrap_or(HeaderName::from_static(DEFAULT_EXPOSE_TRACE_ID_HEADER)) }) }); if let (Some(header_name), Some(trace_id)) = ( @@ -389,7 +390,7 @@ impl Telemetry { .as_mut() .expect("telemetry apollo config must be present"); if let Some(tracing_conf) = &config.tracing { - apollo.expose_trace_id = tracing_conf.expose_trace_id.clone(); + apollo.expose_trace_id = tracing_conf.response_trace_id.clone(); } // If we have key and graph ref but no endpoint we start embedded spaceport @@ -566,9 +567,9 @@ impl Telemetry { if propagation.datadog.unwrap_or_default() || tracing.datadog.is_some() { propagators.push(Box::new(opentelemetry_datadog::DatadogPropagator::default())); } - if let Some(custom_header) = &propagation.custom_header { + if let Some(from_request_header) = &propagation.from_request_header { propagators.push(Box::new(CustomTraceIdPropagator::new( - custom_header.to_string(), + from_request_header.to_string(), ))); } diff --git a/apollo-router/src/testdata/jaeger.router.yaml b/apollo-router/src/testdata/jaeger.router.yaml index d7112b23ad..dd5310d2a0 100644 --- a/apollo-router/src/testdata/jaeger.router.yaml +++ b/apollo-router/src/testdata/jaeger.router.yaml @@ -1,6 +1,6 @@ telemetry: tracing: - experimental_expose_trace_id: + experimental_response_trace_id: enabled: true header_name: apollo-custom-trace-id trace_config: diff --git a/docs/source/configuration/tracing.mdx b/docs/source/configuration/tracing.mdx index 20c5bf2160..5a6da1a48b 100644 --- a/docs/source/configuration/tracing.mdx +++ b/docs/source/configuration/tracing.mdx @@ -82,7 +82,7 @@ telemetry: zipkin: false # If you have your own way to generate a trace id and you want to pass it via a custom header - custom_header: "x-request-id" + from_request_header: "x-request-id" ``` Specifying explicit propagation is generally only required if you're using an exporter that supports multiple trace ID formats (e.g., OpenTelemetry Collector, Jaeger, or OpenTracing compatible exporters). @@ -94,7 +94,7 @@ If you want to expose in response headers the generated trace ID or the one you ```yaml title="router.yaml" telemetry: tracing: - experimental_expose_trace_id: + experimental_response_trace_id: enabled: true # default: false header_name: "my-trace-id" # default: "apollo-trace-id" ``` From fad11d210623f8461087999c80a3386b86767b4c Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 23 Nov 2022 15:49:04 +0100 Subject: [PATCH 08/11] fix test Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- ...outer__configuration__tests__schema_generation.snap | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 78fd0f35ce..9c3dd9152a 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -1862,15 +1862,15 @@ expression: "&schema" "type": "boolean", "nullable": true }, - "from_request_header": { - "description": "Select a custom header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id)", - "type": "string", - "nullable": true - }, "datadog": { "type": "boolean", "nullable": true }, + "from_request_header": { + "description": "Select a custom request header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id)", + "type": "string", + "nullable": true + }, "jaeger": { "type": "boolean", "nullable": true From b687058b7b265d1bef0ed243adc4d22388bcbe95 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Wed, 23 Nov 2022 17:57:45 +0100 Subject: [PATCH 09/11] add disclaimer on experimental feature Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- docs/source/configuration/tracing.mdx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/source/configuration/tracing.mdx b/docs/source/configuration/tracing.mdx index 5a6da1a48b..4c308188fb 100644 --- a/docs/source/configuration/tracing.mdx +++ b/docs/source/configuration/tracing.mdx @@ -89,6 +89,9 @@ Specifying explicit propagation is generally only required if you're using an ex ### Trace ID +> This is part of an experimental feature, it means any time until it's stabilized (without the prefix `experimental_`) we might change the configuration shape or adding/removing features. +> If you want to give feedback or participate in that feature feel free to join [this discussion on GitHub](https://github.com/apollographql/router/discussions/2147). + If you want to expose in response headers the generated trace ID or the one you provided using propagation headers you can use this configuration: ```yaml title="router.yaml" From bf5ff08da7ca3aff8199b877f0da363ec89cc253 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 24 Nov 2022 11:07:47 +0100 Subject: [PATCH 10/11] change configuration shape Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- NEXT_CHANGELOG.md | 4 +++- ...nfiguration__tests__schema_generation.snap | 20 ++++++++++++++----- apollo-router/src/plugins/telemetry/config.rs | 14 ++++++++++--- apollo-router/src/plugins/telemetry/mod.rs | 2 +- apollo-router/tests/jaeger_test.rs | 2 +- docs/source/configuration/tracing.mdx | 5 +++-- 6 files changed, 34 insertions(+), 13 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index acee8fa578..894f06a659 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -38,7 +38,9 @@ telemetry: enabled: true # default: false header_name: "my-trace-id" # default: "apollo-trace-id" propagation: - from_request_header: "x-request-id" # Specify your own trace_id with a custom header in request headers + # If you have your own way to generate a trace id and you want to pass it via a custom request header + request: + header_name: my-trace-id ``` Using this configuration you will have a response header called `my-trace-id` containing the trace ID. It could help you to debug a specific query if you want to grep your log with this trace id to have more context. diff --git a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap index 9c3dd9152a..d715a3d6bb 100644 --- a/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap +++ b/apollo-router/src/configuration/snapshots/apollo_router__configuration__tests__schema_generation.snap @@ -1866,15 +1866,25 @@ expression: "&schema" "type": "boolean", "nullable": true }, - "from_request_header": { - "description": "Select a custom request header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id)", - "type": "string", - "nullable": true - }, "jaeger": { "type": "boolean", "nullable": true }, + "request": { + "description": "Select a custom request header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id)", + "type": "object", + "required": [ + "header_name" + ], + "properties": { + "header_name": { + "description": "Choose the header name to expose trace_id (default: apollo-trace-id)", + "type": "string" + } + }, + "additionalProperties": false, + "nullable": true + }, "trace_context": { "type": "boolean", "nullable": true diff --git a/apollo-router/src/plugins/telemetry/config.rs b/apollo-router/src/plugins/telemetry/config.rs index 106729c6d0..1a93e24a6a 100644 --- a/apollo-router/src/plugins/telemetry/config.rs +++ b/apollo-router/src/plugins/telemetry/config.rs @@ -12,6 +12,7 @@ use serde::Deserialize; use super::metrics::MetricsAttributesConf; use super::*; +use crate::plugin::serde::deserialize_header_name; use crate::plugin::serde::deserialize_option_header_name; use crate::plugins::telemetry::metrics; @@ -108,9 +109,7 @@ pub(crate) struct ExposeTraceId { #[serde(deny_unknown_fields, rename_all = "snake_case")] pub(crate) struct Propagation { /// Select a custom request header to set your own trace_id (header value must be convertible from hexadecimal to set a correct trace_id) - #[schemars(with = "Option")] - #[serde(deserialize_with = "deserialize_option_header_name")] - pub(crate) from_request_header: Option, + pub(crate) request: Option, pub(crate) baggage: Option, pub(crate) trace_context: Option, pub(crate) jaeger: Option, @@ -118,6 +117,15 @@ pub(crate) struct Propagation { pub(crate) zipkin: Option, } +#[derive(Clone, Debug, Deserialize, JsonSchema)] +#[serde(deny_unknown_fields, rename_all = "snake_case")] +pub(crate) struct PropagationRequestTraceId { + /// Choose the header name to expose trace_id (default: apollo-trace-id) + #[schemars(with = "String")] + #[serde(deserialize_with = "deserialize_header_name")] + pub(crate) header_name: HeaderName, +} + #[derive(Default, Debug, Clone, Deserialize, JsonSchema)] #[serde(deny_unknown_fields)] #[non_exhaustive] diff --git a/apollo-router/src/plugins/telemetry/mod.rs b/apollo-router/src/plugins/telemetry/mod.rs index 31d3e14eb8..ae861c1432 100644 --- a/apollo-router/src/plugins/telemetry/mod.rs +++ b/apollo-router/src/plugins/telemetry/mod.rs @@ -567,7 +567,7 @@ impl Telemetry { if propagation.datadog.unwrap_or_default() || tracing.datadog.is_some() { propagators.push(Box::new(opentelemetry_datadog::DatadogPropagator::default())); } - if let Some(from_request_header) = &propagation.from_request_header { + if let Some(from_request_header) = &propagation.request.as_ref().map(|r| &r.header_name) { propagators.push(Box::new(CustomTraceIdPropagator::new( from_request_header.to_string(), ))); diff --git a/apollo-router/tests/jaeger_test.rs b/apollo-router/tests/jaeger_test.rs index a92f20f864..76b65412a7 100644 --- a/apollo-router/tests/jaeger_test.rs +++ b/apollo-router/tests/jaeger_test.rs @@ -43,7 +43,7 @@ async fn test_jaeger_tracing() -> Result<(), BoxError> { for _ in 0..10 { let (id, result) = router.run_query().await; - assert!(result + assert!(!result .headers() .get("apollo-custom-trace-id") .unwrap() diff --git a/docs/source/configuration/tracing.mdx b/docs/source/configuration/tracing.mdx index 4c308188fb..3a6cb28853 100644 --- a/docs/source/configuration/tracing.mdx +++ b/docs/source/configuration/tracing.mdx @@ -81,8 +81,9 @@ telemetry: # https://zipkin.io/ (compliant with opentracing) zipkin: false - # If you have your own way to generate a trace id and you want to pass it via a custom header - from_request_header: "x-request-id" + # If you have your own way to generate a trace id and you want to pass it via a custom request header + request: + header_name: my-trace-id ``` Specifying explicit propagation is generally only required if you're using an exporter that supports multiple trace ID formats (e.g., OpenTelemetry Collector, Jaeger, or OpenTracing compatible exporters). From 7e691eff0b6f6fbf1491cdfbfc4c89230a087318 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Thu, 24 Nov 2022 14:40:41 +0100 Subject: [PATCH 11/11] undo formatting in tracing.mdx Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- docs/source/configuration/tracing.mdx | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/source/configuration/tracing.mdx b/docs/source/configuration/tracing.mdx index 3a6cb28853..8a3a7972fc 100644 --- a/docs/source/configuration/tracing.mdx +++ b/docs/source/configuration/tracing.mdx @@ -7,16 +7,16 @@ import { Link } from "gatsby"; The Apollo Router supports [OpenTelemetry](https://opentelemetry.io/), with exporters for: -- [Jaeger](https://www.jaegertracing.io/) -- [Zipkin](https://zipkin.io/) -- [Datadog](https://www.datadoghq.com/) -- [OpenTelemetry Protocol (OTLP)](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md) over HTTP or gRPC +* [Jaeger](https://www.jaegertracing.io/) +* [Zipkin](https://zipkin.io/) +* [Datadog](https://www.datadoghq.com/) +* [OpenTelemetry Protocol (OTLP)](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/otlp.md) over HTTP or gRPC The Apollo Router generates spans that include the various phases of serving a request and associated dependencies. This is useful for showing how response time is affected by: -- Sub-request response times -- Query shape (sub-request dependencies) -- Apollo Router post-processing +* Sub-request response times +* Query shape (sub-request dependencies) +* Apollo Router post-processing Span data is sent to a collector such as [Jaeger](https://www.jaegertracing.io/), which can assemble spans into a gantt chart for analysis.