From 7848755c71216e1ea255d5af3926b2a9154ea867 Mon Sep 17 00:00:00 2001 From: Zhongyang Wu Date: Tue, 21 Nov 2023 00:26:46 -0800 Subject: [PATCH] fix(api): move composite propagator to API (#1373) Move `TextMapCompositePropagator` from `opentelemetry-sdk` to `opentelemetry` crate. Fixes #1013 --- opentelemetry-sdk/CHANGELOG.md | 1 + opentelemetry-sdk/src/propagation/mod.rs | 2 - opentelemetry/CHANGELOG.md | 4 +- opentelemetry/Cargo.toml | 3 + .../src/propagation/composite.rs | 217 ++++++++---------- opentelemetry/src/propagation/mod.rs | 172 ++------------ .../src/propagation/text_map_propagator.rs | 12 +- 7 files changed, 132 insertions(+), 279 deletions(-) rename {opentelemetry-sdk => opentelemetry}/src/propagation/composite.rs (59%) diff --git a/opentelemetry-sdk/CHANGELOG.md b/opentelemetry-sdk/CHANGELOG.md index 3a604cc610..1b5e0b76f9 100644 --- a/opentelemetry-sdk/CHANGELOG.md +++ b/opentelemetry-sdk/CHANGELOG.md @@ -21,6 +21,7 @@ `SpanData` now stores `events` as `SpanEvents` instead of `EvictedQueue` where `SpanEvents` is a struct with a `Vec` of events and `dropped_count`. +- **Breaking** Remove `TextMapCompositePropagator` [#1373](https://github.com/open-telemetry/opentelemetry-rust/pull/1373). Use `TextMapCompositePropagator` in opentelemetry API. - [#1375](https://github.com/open-telemetry/opentelemetry-rust/pull/1375/) Fix metric collections during PeriodicReader shutdown diff --git a/opentelemetry-sdk/src/propagation/mod.rs b/opentelemetry-sdk/src/propagation/mod.rs index d0b88076d4..6cfb5d07bd 100644 --- a/opentelemetry-sdk/src/propagation/mod.rs +++ b/opentelemetry-sdk/src/propagation/mod.rs @@ -1,8 +1,6 @@ //! OpenTelemetry Propagators mod baggage; -mod composite; mod trace_context; pub use baggage::BaggagePropagator; -pub use composite::TextMapCompositePropagator; pub use trace_context::TraceContextPropagator; diff --git a/opentelemetry/CHANGELOG.md b/opentelemetry/CHANGELOG.md index e1b57c8105..0e7aebd62f 100644 --- a/opentelemetry/CHANGELOG.md +++ b/opentelemetry/CHANGELOG.md @@ -4,13 +4,15 @@ ### Changed -Modified `AnyValue.Map` to be backed by `HashMap` instead of custom `OrderMap`, +- Modified `AnyValue.Map` to be backed by `HashMap` instead of custom `OrderMap`, which internally used `IndexMap`. There was no requirement to maintain the order of entries, so moving from `IndexMap` to `HashMap` offers slight performance gains, and avoids `IndexMap` dependency. This affects `body` and `attributes` of `LogRecord`. [#1353](https://github.com/open-telemetry/opentelemetry-rust/pull/1353) +- Add `TextMapCompositePropagator` [#1373](https://github.com/open-telemetry/opentelemetry-rust/pull/1373) + ### Removed - Removed `OrderMap` type as there was no requirement to use this over regular diff --git a/opentelemetry/Cargo.toml b/opentelemetry/Cargo.toml index 39ec98087f..957d2d6a49 100644 --- a/opentelemetry/Cargo.toml +++ b/opentelemetry/Cargo.toml @@ -38,3 +38,6 @@ metrics = [] testing = ["trace", "metrics"] logs = [] logs_level_enabled = ["logs"] + +[dev-dependencies] +opentelemetry_sdk = { path = "../opentelemetry-sdk" } # for documentation tests diff --git a/opentelemetry-sdk/src/propagation/composite.rs b/opentelemetry/src/propagation/composite.rs similarity index 59% rename from opentelemetry-sdk/src/propagation/composite.rs rename to opentelemetry/src/propagation/composite.rs index 218b5399ae..db638449e1 100644 --- a/opentelemetry-sdk/src/propagation/composite.rs +++ b/opentelemetry/src/propagation/composite.rs @@ -1,10 +1,17 @@ -use opentelemetry::{ +//! # Composite Propagator +//! +//! A utility over multiple `Propagator`s to group multiple Propagators from different cross-cutting +//! concerns in order to leverage them as a single entity. +//! +//! Each composite Propagator will implement a specific Propagator type, such as TextMapPropagator, +//! as different Propagator types will likely operate on different data types. +use crate::{ propagation::{text_map_propagator::FieldIter, Extractor, Injector, TextMapPropagator}, Context, }; use std::collections::HashSet; -/// Composite propagator +/// Composite propagator for [`TextMapPropagator`]s. /// /// A propagator that chains multiple [`TextMapPropagator`] propagators together, /// injecting or extracting by their respective HTTP header names. @@ -12,19 +19,18 @@ use std::collections::HashSet; /// Injection and extraction from this propagator will preserve the order of the /// injectors and extractors passed in during initialization. /// -/// [`TextMapPropagator`]: opentelemetry::propagation::TextMapPropagator -/// /// # Examples /// /// ``` /// use opentelemetry::{ /// baggage::BaggageExt, -/// propagation::TextMapPropagator, +/// propagation::{TextMapPropagator, TextMapCompositePropagator}, +/// /// trace::{TraceContextExt, Tracer, TracerProvider}, /// Context, KeyValue, /// }; /// use opentelemetry_sdk::propagation::{ -/// BaggagePropagator, TextMapCompositePropagator, TraceContextPropagator, +/// BaggagePropagator, TraceContextPropagator, /// }; /// use opentelemetry_sdk::trace as sdktrace; /// use std::collections::HashMap; @@ -67,7 +73,7 @@ pub struct TextMapCompositePropagator { impl TextMapCompositePropagator { /// Constructs a new propagator out of instances of [`TextMapPropagator`]. /// - /// [`TextMapPropagator`]: opentelemetry::propagation::TextMapPropagator + /// [`TextMapPropagator`]: TextMapPropagator pub fn new(propagators: Vec>) -> Self { let mut fields = HashSet::new(); for propagator in &propagators { @@ -107,31 +113,31 @@ impl TextMapPropagator for TextMapCompositePropagator { } } -#[cfg(all(test, feature = "testing", feature = "trace"))] +#[cfg(all(test, feature = "trace"))] mod tests { - use crate::propagation::{TextMapCompositePropagator, TraceContextPropagator}; + use crate::baggage::BaggageExt; + use crate::propagation::TextMapCompositePropagator; use crate::testing::trace::TestSpan; - use opentelemetry::{ + use crate::{ propagation::{text_map_propagator::FieldIter, Extractor, Injector, TextMapPropagator}, trace::{SpanContext, SpanId, TraceContextExt, TraceFlags, TraceId, TraceState}, - Context, + Context, KeyValue, }; use std::collections::HashMap; - use std::str::FromStr; - /// Dummy propagator for testing - /// - /// The format we are using is {trace id(in base10 u128)}-{span id(in base10 u64)}-{flag(in u8)} + /// A test propagator that injects and extracts a single header. #[derive(Debug)] struct TestPropagator { - fields: [String; 1], + header: &'static str, + fields: Vec, // used by fields method } impl TestPropagator { #[allow(unreachable_pub)] - pub fn new() -> Self { + pub fn new(header: &'static str) -> Self { TestPropagator { - fields: ["testheader".to_string()], + header, + fields: vec![header.to_string()], } } } @@ -140,69 +146,59 @@ mod tests { fn inject_context(&self, cx: &Context, injector: &mut dyn Injector) { let span = cx.span(); let span_context = span.span_context(); - injector.set( - "testheader", - format!( - "{:x}-{:x}-{:02x}", - span_context.trace_id(), - span_context.span_id(), - span_context.trace_flags() - ), - ) + match self.header { + "span-id" => injector.set(self.header, format!("{:x}", span_context.span_id())), + "baggage" => injector.set(self.header, cx.baggage().to_string()), + _ => {} + } } fn extract_with_context(&self, cx: &Context, extractor: &dyn Extractor) -> Context { - let span = if let Some(val) = extractor.get("testheader") { - let parts = val.split_terminator('-').collect::>(); - if parts.len() != 3 { - SpanContext::empty_context() - } else { - SpanContext::new( - TraceId::from_u128(u128::from_str(parts[0]).unwrap_or(0)), - SpanId::from_u64(u64::from_str(parts[1]).unwrap_or(0)), - TraceFlags::new(u8::from_str(parts[2]).unwrap_or(0)), - true, - TraceState::default(), - ) - } - } else { - SpanContext::empty_context() - }; - - cx.with_remote_span_context(span) + match (self.header, extractor.get(self.header)) { + ("span-id", Some(val)) => cx.with_remote_span_context(SpanContext::new( + TraceId::from_u128(1), + SpanId::from_u64(u64::from_str_radix(val, 16).unwrap()), + TraceFlags::default(), + false, + TraceState::default(), + )), + ("baggage", Some(_)) => cx.with_baggage(vec![KeyValue::new("baggagekey", "value")]), + _ => cx.clone(), + } } fn fields(&self) -> FieldIter<'_> { - FieldIter::new(&self.fields) + FieldIter::new(self.fields.as_slice()) } } + fn setup() -> Context { + let mut cx = Context::default(); + cx = cx.with_span(TestSpan(SpanContext::new( + TraceId::from_u128(1), + SpanId::from_u64(11), + TraceFlags::default(), + true, + TraceState::default(), + ))); + // setup for baggage propagator + cx.with_baggage(vec![KeyValue::new("baggagekey", "value")]) + } + fn test_data() -> Vec<(&'static str, &'static str)> { - vec![ - ("testheader", "1-1-00"), - ( - "traceparent", - "00-00000000000000000000000000000001-0000000000000001-00", - ), - ] + vec![("span-id", "b"), ("baggage", "baggagekey=value")] } #[test] fn zero_propogators_are_noop() { + // setup let composite_propagator = TextMapCompositePropagator::new(vec![]); + let cx = setup(); - let cx = Context::default().with_span(TestSpan(SpanContext::new( - TraceId::from_u128(1), - SpanId::from_u64(1), - TraceFlags::default(), - false, - TraceState::default(), - ))); let mut injector = HashMap::new(); composite_propagator.inject_context(&cx, &mut injector); assert_eq!(injector.len(), 0); - for (header_name, header_value) in test_data() { let mut extractor = HashMap::new(); extractor.insert(header_name.to_string(), header_value.to_string()); @@ -218,20 +214,12 @@ mod tests { #[test] fn inject_multiple_propagators() { - let test_propagator = TestPropagator::new(); - let trace_context = TraceContextPropagator::new(); let composite_propagator = TextMapCompositePropagator::new(vec![ - Box::new(test_propagator), - Box::new(trace_context), + Box::new(TestPropagator::new("span-id")), + Box::new(TestPropagator::new("baggage")), ]); - let cx = Context::default().with_span(TestSpan(SpanContext::new( - TraceId::from_u128(1), - SpanId::from_u64(1), - TraceFlags::default(), - false, - TraceState::default(), - ))); + let cx = setup(); let mut injector = HashMap::new(); composite_propagator.inject_context(&cx, &mut injector); @@ -242,64 +230,59 @@ mod tests { #[test] fn extract_multiple_propagators() { - let test_propagator = TestPropagator::new(); - let trace_context = TraceContextPropagator::new(); let composite_propagator = TextMapCompositePropagator::new(vec![ - Box::new(test_propagator), - Box::new(trace_context), + Box::new(TestPropagator::new("span-id")), + Box::new(TestPropagator::new("baggage")), ]); + let mut extractor = HashMap::new(); for (header_name, header_value) in test_data() { - let mut extractor = HashMap::new(); extractor.insert(header_name.to_string(), header_value.to_string()); - assert_eq!( - composite_propagator - .extract(&extractor) - .span() - .span_context(), - &SpanContext::new( - TraceId::from_u128(1), - SpanId::from_u64(1), - TraceFlags::default(), - true, - TraceState::default(), - ) - ); } + let cx = composite_propagator.extract(&extractor); + assert_eq!( + cx.span().span_context(), + &SpanContext::new( + TraceId::from_u128(1), + SpanId::from_u64(11), + TraceFlags::default(), + false, + TraceState::default(), + ) + ); + assert_eq!(cx.baggage().to_string(), "baggagekey=value",); } #[test] fn test_get_fields() { - let test_propagator = TestPropagator::new(); - let b3_fields = test_propagator - .fields() - .map(|s| s.to_string()) - .collect::>(); - - let trace_context = TraceContextPropagator::new(); - let trace_context_fields = trace_context - .fields() - .map(|s| s.to_string()) - .collect::>(); + let test_cases = vec![ + // name, header_name, expected_result + // ("single propagator", vec!["span-id"], vec!["span-id"]), + ( + "multiple propagators with order", + vec!["span-id", "baggage"], + vec!["baggage", "span-id"], + ), + ]; - let composite_propagator = TextMapCompositePropagator::new(vec![ - Box::new(test_propagator), - Box::new(trace_context), - ]); + for test_case in test_cases { + let test_propagators = test_case + .1 + .into_iter() + .map(|name| { + Box::new(TestPropagator::new(name)) as Box + }) + .collect(); - let mut fields = composite_propagator - .fields() - .map(|s| s.to_string()) - .collect::>(); - fields.sort(); + let composite_propagator = TextMapCompositePropagator::new(test_propagators); - let mut expected = vec![b3_fields, trace_context_fields] - .into_iter() - .flatten() - .collect::>(); - expected.sort(); - expected.dedup(); + let mut fields = composite_propagator + .fields() + .map(|s| s.to_string()) + .collect::>(); + fields.sort(); - assert_eq!(fields, expected); + assert_eq!(fields, test_case.2); + } } } diff --git a/opentelemetry/src/propagation/mod.rs b/opentelemetry/src/propagation/mod.rs index 8fec86f9da..7701b3a75a 100644 --- a/opentelemetry/src/propagation/mod.rs +++ b/opentelemetry/src/propagation/mod.rs @@ -1,170 +1,30 @@ //! # OpenTelemetry Propagator interface +//! Cross-cutting concerns send their state to the next process using Propagators, which are defined +//! as objects used to read and write context data to and from messages exchanged by the applications. //! -//! Propagators API consists of two main formats: +//! `Propagator`s leverage the [`Context`] to inject and extract data for each cross-cutting concern, +//! such as `TraceContext` and [`Baggage`]. //! -//! - `BinaryFormat` is used to serialize and deserialize a value -//! into a binary representation. -//! - `TextMapFormat` is used to inject and extract a value as -//! text into injectors and extractors that travel in-band across process boundaries. +//! The Propagators API is expected to be leveraged by users writing instrumentation libraries. //! -//! Deserializing must set `is_remote` to true on the returned -//! `SpanContext`. +//! Currently, the following `Propagator` types are supported: +//! - [`TextMapPropagator`], inject values into and extracts values from carriers as string key/value pairs //! -//! ## Binary Format +//! A binary Propagator type will be added in +//! the future, See [tracking issues](https://github.com/open-telemetry/opentelemetry-specification/issues/437)). //! -//! `BinaryFormat` is a formatter to serialize and deserialize a value -//! into a binary format. -//! -//! `BinaryFormat` MUST expose the APIs that serializes values into bytes, -//! and deserializes values from bytes. -//! -//! ### ToBytes -//! -//! Serializes the given value into the on-the-wire representation. -//! -//! Required arguments: -//! -//! - the value to serialize, can be `SpanContext` or `DistributedContext`. -//! -//! Returns the on-the-wire byte representation of the value. -//! -//! ### FromBytes -//! -//! Creates a value from the given on-the-wire encoded representation. -//! -//! If the value could not be parsed, the underlying implementation -//! SHOULD decide to return ether an empty value, an invalid value, or -//! a valid value. -//! -//! Required arguments: -//! -//! - on-the-wire byte representation of the value. -//! -//! Returns a value deserialized from bytes. -//! -//! ## TextMap Format -//! -//! `TextMapFormat` is a formatter that injects and extracts a value -//! as text into injectors and extractors that travel in-band across process boundaries. -//! -//! Encoding is expected to conform to the HTTP Header Field semantics. -//! Values are often encoded as RPC/HTTP request headers. -//! -//! The carrier of propagated data on both the client (injector) and -//! server (extractor) side is usually a http request. Propagation is -//! usually implemented via library-specific request interceptors, where -//! the client-side injects values and the server-side extracts them. -//! -//! `TextMapFormat` MUST expose the APIs that injects values into injectors, -//! and extracts values from extractors. -//! -//! ### Fields -//! -//! The propagation fields defined. If your injector is reused, you should -//! delete the fields here before calling `inject`. -//! -//! For example, if the injector is a single-use or immutable request object, -//! you don't need to clear fields as they couldn't have been set before. -//! If it is a mutable, retryable object, successive calls should clear -//! these fields first. -//! -//! The use cases of this are: -//! -//! - allow pre-allocation of fields, especially in systems like gRPC -//! Metadata -//! - allow a single-pass over an iterator -//! -//! Returns list of fields that will be used by this formatter. -//! -//! ### Inject -//! -//! Injects the value downstream. For example, as http headers. -//! -//! Required arguments: -//! -//! - the `SpanContext` to be injected. -//! - the injector that holds propagation fields. For example, an outgoing -//! message or http request. -//! - the `Setter` invoked for each propagation key to add or remove. -//! -//! #### Setter argument -//! -//! Setter is an argument in `Inject` that puts value into given field. -//! -//! `Setter` allows a `TextMapFormat` to set propagated fields into a -//! injector. -//! -//! `Setter` MUST be stateless and allowed to be saved as a constant to -//! avoid runtime allocations. One of the ways to implement it is `Setter` -//! class with `Put` method as described below. -//! -//! ##### Put -//! -//! Replaces a propagated field with the given value. -//! -//! Required arguments: -//! -//! - the injector holds propagation fields. For example, an outgoing message -//! or http request. -//! - the key of the field. -//! - the value of the field. -//! -//! The implementation SHOULD preserve casing (e.g. it should not transform -//! `Content-Type` to `content-type`) if the used protocol is case insensitive, -//! otherwise it MUST preserve casing. -//! -//! ### Extract -//! -//! Extracts the value from upstream. For example, as http headers. -//! -//! If the value could not be parsed, the underlying implementation will -//! decide to return an object representing either an empty value, an invalid -//! value, or a valid value. -//! -//! Required arguments: -//! -//! - the extractor holds propagation fields. For example, an outgoing message -//! or http request. -//! - the instance of `Getter` invoked for each propagation key to get. -//! -//! Returns the non-null extracted value. -//! -//! #### Getter argument -//! -//! Getter is an argument in `Extract` that get value from given field -//! -//! `Getter` allows a `TextMapFormat` to read propagated fields from a -//! extractor. -//! -//! `Getter` MUST be stateless and allowed to be saved as a constant to avoid -//! runtime allocations. One of the ways to implement it is `Getter` class -//! with `Get` method as described below. -//! -//! ##### Get -//! -//! The Get function MUST return the first value of the given propagation -//! key or return `None` if the key doesn't exist. -//! -//! Required arguments: -//! -//! - the extractor of propagation fields, such as an HTTP request. -//! - the key of the field. -//! -//! The `get` function is responsible for handling case sensitivity. If -//! the getter is intended to work with an HTTP request object, the getter -//! MUST be case insensitive. To improve compatibility with other text-based -//! protocols, text format implementations MUST ensure to always use the -//! canonical casing for their attributes. NOTE: Canonical casing for HTTP -//! headers is usually title case (e.g. `Content-Type` instead of `content-type`). -//! -//! ##### Keys -//! -//! The Keys function returns a vector of the propagation keys. +//! `Propagator`s uses [`Injector`] and [`Extractor`] to read and write context data to and from messages. +//! Each specific Propagator type defines its expected carrier type, such as a string map or a byte array. //! +//! [`Baggage`]: crate::baggage::Baggage +//! [`Context`]: crate::Context + use std::collections::HashMap; +pub mod composite; pub mod text_map_propagator; +pub use composite::TextMapCompositePropagator; pub use text_map_propagator::TextMapPropagator; /// Injector provides an interface for adding fields from an underlying struct like `HashMap` diff --git a/opentelemetry/src/propagation/text_map_propagator.rs b/opentelemetry/src/propagation/text_map_propagator.rs index 4307905a30..c078e49456 100644 --- a/opentelemetry/src/propagation/text_map_propagator.rs +++ b/opentelemetry/src/propagation/text_map_propagator.rs @@ -1,7 +1,13 @@ -//! # Text Propagator +//! # TextMapPropagator //! -//! `TextMapPropagator` is a formatter to serialize and deserialize a value into a -//! text format. +//! [`TextMapPropagator`] performs the injection and extraction of a cross-cutting concern value as +//! string key/values pairs into carriers that travel in-band across process boundaries. +//! +//! The carrier of propagated data on both the client (injector) and server (extractor) side is +//! usually an HTTP request. +//! +//! In order to increase compatibility, the key/value pairs MUST only consist of US-ASCII characters +//! that make up valid HTTP header fields as per RFC 7230. use crate::{ propagation::{Extractor, Injector}, Context,