From b8b9400858f469c438bcf00fae6cd4b980dc1350 Mon Sep 17 00:00:00 2001 From: Conrad Ludgate Date: Tue, 20 Jun 2023 18:07:12 +0100 Subject: [PATCH] disable context propagation (#94) * disable context propagation * bump version * removes leftover feature * fix new test * add back disabled test --- .github/workflows/ci.yml | 4 + reqwest-tracing/CHANGELOG.md | 6 ++ reqwest-tracing/Cargo.toml | 5 +- reqwest-tracing/src/lib.rs | 9 ++- reqwest-tracing/src/middleware.rs | 11 ++- reqwest-tracing/src/otel.rs | 77 ++++++++++++++----- .../src/reqwest_otel_span_builder.rs | 35 +++++++++ 7 files changed, 119 insertions(+), 28 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2cf36c1..6e6c697 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,6 +18,7 @@ jobs: - opentelemetry_0_16 - opentelemetry_0_17 - opentelemetry_0_18 + - opentelemetry_0_19 steps: - name: Checkout repository uses: actions/checkout@v2 @@ -44,6 +45,7 @@ jobs: - opentelemetry_0_16 - opentelemetry_0_17 - opentelemetry_0_18 + - opentelemetry_0_19 steps: - name: Checkout repository uses: actions/checkout@v2 @@ -90,6 +92,7 @@ jobs: - opentelemetry_0_16 - opentelemetry_0_17 - opentelemetry_0_18 + - opentelemetry_0_19 steps: - name: Checkout repository uses: actions/checkout@v2 @@ -118,6 +121,7 @@ jobs: - opentelemetry_0_16 - opentelemetry_0_17 - opentelemetry_0_18 + - opentelemetry_0_19 steps: - name: Checkout repository uses: actions/checkout@v2 diff --git a/reqwest-tracing/CHANGELOG.md b/reqwest-tracing/CHANGELOG.md index 432fcc7..08f120e 100644 --- a/reqwest-tracing/CHANGELOG.md +++ b/reqwest-tracing/CHANGELOG.md @@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.4.5] - 2023-06-20 + +### Added +- A new extension `DisableOtelPropagation` which stops opentelemetry contexts propagating +- Support for opentelemetry 0.19 + ## [0.4.4] - 2023-05-15 ### Added diff --git a/reqwest-tracing/Cargo.toml b/reqwest-tracing/Cargo.toml index 1493a0f..c5c85f7 100644 --- a/reqwest-tracing/Cargo.toml +++ b/reqwest-tracing/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "reqwest-tracing" -version = "0.4.4" +version = "0.4.5" authors = ["Rodrigo Gryzinski "] edition = "2018" description = "Opentracing middleware for reqwest." @@ -16,6 +16,7 @@ opentelemetry_0_15 = ["opentelemetry_0_15_pkg", "tracing-opentelemetry_0_14_pkg" opentelemetry_0_16 = ["opentelemetry_0_16_pkg", "tracing-opentelemetry_0_16_pkg"] opentelemetry_0_17 = ["opentelemetry_0_17_pkg", "tracing-opentelemetry_0_17_pkg"] opentelemetry_0_18 = ["opentelemetry_0_18_pkg", "tracing-opentelemetry_0_18_pkg"] +opentelemetry_0_19 = ["opentelemetry_0_19_pkg", "tracing-opentelemetry_0_19_pkg"] [dependencies] @@ -34,12 +35,14 @@ opentelemetry_0_15_pkg = { package = "opentelemetry", version = "0.15.0", option opentelemetry_0_16_pkg = { package = "opentelemetry", version = "0.16.0", optional = true } opentelemetry_0_17_pkg = { package = "opentelemetry", version = "0.17.0", optional = true } opentelemetry_0_18_pkg = { package = "opentelemetry", version = "0.18.0", optional = true } +opentelemetry_0_19_pkg = { package = "opentelemetry", version = "0.19.0", optional = true } tracing-opentelemetry_0_12_pkg = { package = "tracing-opentelemetry",version = "0.12.0", optional = true } tracing-opentelemetry_0_13_pkg = { package = "tracing-opentelemetry", version = "0.13.0", optional = true } tracing-opentelemetry_0_14_pkg = { package = "tracing-opentelemetry",version = "0.14.0", optional = true } tracing-opentelemetry_0_16_pkg = { package = "tracing-opentelemetry",version = "0.16.0", optional = true } tracing-opentelemetry_0_17_pkg = { package = "tracing-opentelemetry",version = "0.17.0", optional = true } tracing-opentelemetry_0_18_pkg = { package = "tracing-opentelemetry",version = "0.18.0", optional = true } +tracing-opentelemetry_0_19_pkg = { package = "tracing-opentelemetry",version = "0.19.0", optional = true } [target.'cfg(target_arch = "wasm32")'.dependencies] getrandom = { version = "0.2.0", features = ["js"] } diff --git a/reqwest-tracing/src/lib.rs b/reqwest-tracing/src/lib.rs index 562da88..bcb25d8 100644 --- a/reqwest-tracing/src/lib.rs +++ b/reqwest-tracing/src/lib.rs @@ -90,16 +90,17 @@ mod middleware; feature = "opentelemetry_0_16", feature = "opentelemetry_0_17", feature = "opentelemetry_0_18", + feature = "opentelemetry_0_19", ))] mod otel; mod reqwest_otel_span_builder; pub use middleware::TracingMiddleware; pub use reqwest_otel_span_builder::{ default_on_request_end, default_on_request_failure, default_on_request_success, - default_span_name, DefaultSpanBackend, OtelName, OtelPathNames, ReqwestOtelSpanBackend, - SpanBackendWithUrl, ERROR_CAUSE_CHAIN, ERROR_MESSAGE, HTTP_HOST, HTTP_METHOD, HTTP_SCHEME, - HTTP_STATUS_CODE, HTTP_URL, HTTP_USER_AGENT, NET_HOST_PORT, OTEL_KIND, OTEL_NAME, - OTEL_STATUS_CODE, + default_span_name, DefaultSpanBackend, DisableOtelPropagation, OtelName, OtelPathNames, + ReqwestOtelSpanBackend, SpanBackendWithUrl, ERROR_CAUSE_CHAIN, ERROR_MESSAGE, HTTP_HOST, + HTTP_METHOD, HTTP_SCHEME, HTTP_STATUS_CODE, HTTP_URL, HTTP_USER_AGENT, NET_HOST_PORT, + OTEL_KIND, OTEL_NAME, OTEL_STATUS_CODE, }; #[doc(hidden)] diff --git a/reqwest-tracing/src/middleware.rs b/reqwest-tracing/src/middleware.rs index 4c77f2b..0d5283b 100644 --- a/reqwest-tracing/src/middleware.rs +++ b/reqwest-tracing/src/middleware.rs @@ -45,8 +45,6 @@ where let request_span = ReqwestOtelSpan::on_request_start(&req, extensions); let outcome_future = async { - // Adds tracing headers to the given request to propagate the OpenTelemetry context to downstream revivers of the request. - // Spans added by downstream consumers will be part of the same trace. #[cfg(any( feature = "opentelemetry_0_13", feature = "opentelemetry_0_14", @@ -54,8 +52,15 @@ where feature = "opentelemetry_0_16", feature = "opentelemetry_0_17", feature = "opentelemetry_0_18", + feature = "opentelemetry_0_19", ))] - let req = crate::otel::inject_opentelemetry_context_into_request(req); + let req = if !extensions.contains::() { + // Adds tracing headers to the given request to propagate the OpenTelemetry context to downstream revivers of the request. + // Spans added by downstream consumers will be part of the same trace. + crate::otel::inject_opentelemetry_context_into_request(req) + } else { + req + }; // Run the request let outcome = next.run(req, extensions).await; diff --git a/reqwest-tracing/src/otel.rs b/reqwest-tracing/src/otel.rs index a57e02d..616ae22 100644 --- a/reqwest-tracing/src/otel.rs +++ b/reqwest-tracing/src/otel.rs @@ -21,6 +21,9 @@ use opentelemetry_0_17_pkg as opentelemetry; #[cfg(feature = "opentelemetry_0_18")] use opentelemetry_0_18_pkg as opentelemetry; +#[cfg(feature = "opentelemetry_0_19")] +use opentelemetry_0_19_pkg as opentelemetry; + #[cfg(feature = "opentelemetry_0_13")] pub use tracing_opentelemetry_0_12_pkg as tracing_opentelemetry; @@ -39,6 +42,9 @@ pub use tracing_opentelemetry_0_17_pkg as tracing_opentelemetry; #[cfg(feature = "opentelemetry_0_18")] pub use tracing_opentelemetry_0_18_pkg as tracing_opentelemetry; +#[cfg(feature = "opentelemetry_0_19")] +pub use tracing_opentelemetry_0_19_pkg as tracing_opentelemetry; + use opentelemetry::global; use opentelemetry::propagation::Injector; use tracing_opentelemetry::OpenTelemetrySpanExt; @@ -80,10 +86,13 @@ impl<'a> Injector for RequestCarrier<'a> { #[cfg(test)] mod test { + use std::sync::OnceLock; + use super::*; - use crate::TracingMiddleware; + use crate::{DisableOtelPropagation, TracingMiddleware}; use opentelemetry::sdk::propagation::TraceContextPropagator; - use reqwest_middleware::ClientBuilder; + use reqwest::Response; + use reqwest_middleware::{ClientBuilder, ClientWithMiddleware, Extension}; use tracing::{info_span, Instrument, Level}; #[cfg(any( feature = "opentelemetry_0_13", @@ -99,17 +108,22 @@ mod test { use tracing_subscriber_0_3::{filter, layer::SubscriberExt, Registry}; use wiremock::{matchers::any, Mock, MockServer, ResponseTemplate}; - #[tokio::test] - async fn tracing_middleware_propagates_otel_data_even_when_the_span_is_disabled() { - let tracer = opentelemetry::sdk::export::trace::stdout::new_pipeline() - .with_writer(std::io::sink()) - .install_simple(); - let telemetry = tracing_opentelemetry::layer().with_tracer(tracer); - let subscriber = Registry::default() - .with(filter::Targets::new().with_target("reqwest_tracing::otel::test", Level::DEBUG)) - .with(telemetry); - tracing::subscriber::set_global_default(subscriber).unwrap(); - global::set_text_map_propagator(TraceContextPropagator::new()); + async fn make_echo_request_in_otel_context(client: ClientWithMiddleware) -> Response { + static TELEMETRY: OnceLock<()> = OnceLock::new(); + + TELEMETRY.get_or_init(|| { + let tracer = opentelemetry::sdk::export::trace::stdout::new_pipeline() + .with_writer(std::io::sink()) + .install_simple(); + let telemetry = tracing_opentelemetry::layer().with_tracer(tracer); + let subscriber = Registry::default() + .with( + filter::Targets::new().with_target("reqwest_tracing::otel::test", Level::DEBUG), + ) + .with(telemetry); + tracing::subscriber::set_global_default(subscriber).unwrap(); + global::set_text_map_propagator(TraceContextPropagator::new()); + }); // Mock server - sends all request headers back in the response let server = MockServer::start().await; @@ -124,17 +138,40 @@ mod test { .mount(&server) .await; - let client = ClientBuilder::new(reqwest::Client::new()) - .with(TracingMiddleware::default()) - .build(); - - let resp = client + client .get(server.uri()) .send() .instrument(info_span!("some_span")) .await - .unwrap(); + .unwrap() + } - assert!(resp.headers().contains_key("traceparent")); + #[tokio::test] + async fn tracing_middleware_propagates_otel_data_even_when_the_span_is_disabled() { + let client = ClientBuilder::new(reqwest::Client::new()) + .with(TracingMiddleware::default()) + .build(); + + let resp = make_echo_request_in_otel_context(client).await; + + assert!( + resp.headers().contains_key("traceparent"), + "by default, the tracing middleware will propagate otel contexts" + ); + } + + #[tokio::test] + async fn context_no_propagated() { + let client = ClientBuilder::new(reqwest::Client::new()) + .with_init(Extension(DisableOtelPropagation)) + .with(TracingMiddleware::default()) + .build(); + + let resp = make_echo_request_in_otel_context(client).await; + + assert!( + !resp.headers().contains_key("traceparent"), + "request should not contain traceparent if context propagation is disabled" + ); } } diff --git a/reqwest-tracing/src/reqwest_otel_span_builder.rs b/reqwest-tracing/src/reqwest_otel_span_builder.rs index 83605dc..6e7729f 100644 --- a/reqwest-tracing/src/reqwest_otel_span_builder.rs +++ b/reqwest-tracing/src/reqwest_otel_span_builder.rs @@ -276,6 +276,41 @@ impl OtelPathNames { } } +/// `DisableOtelPropagation` disables opentelemetry header propagation, while still tracing the HTTP request. +/// +/// By default, the [`TracingMiddleware`](super::TracingMiddleware) middleware will also propagate any opentelemtry +/// contexts to the server. For any external facing requests, this can be problematic and it should be disabled. +/// +/// Usage: +/// ```no_run +/// # use reqwest_middleware::Result; +/// use reqwest_middleware::{ClientBuilder, Extension}; +/// use reqwest_tracing::{ +/// TracingMiddleware, DisableOtelPropagation +/// }; +/// # async fn example() -> Result<()> { +/// let reqwest_client = reqwest::Client::builder().build().unwrap(); +/// let client = ClientBuilder::new(reqwest_client) +/// // Inserts the extension before the request is started +/// .with_init(Extension(DisableOtelPropagation)) +/// // Makes use of that extension to specify the otel name +/// .with(TracingMiddleware::default()) +/// .build(); +/// +/// let resp = client.get("https://truelayer.com").send().await.unwrap(); +/// +/// // Or specify it on the individual request (will take priority) +/// let resp = client.post("https://api.truelayer.com/payment") +/// .with_extension(DisableOtelPropagation) +/// .send() +/// .await +/// .unwrap(); +/// # Ok(()) +/// # } +/// ``` +#[derive(Clone)] +pub struct DisableOtelPropagation; + /// Removes the username and/or password parts of the url, if present. fn remove_credentials(url: &Url) -> Cow<'_, str> { if !url.username().is_empty() || url.password().is_some() {