fix(api): move composite propagator to API (#1373)

Move `TextMapCompositePropagator` from `opentelemetry-sdk` to `opentelemetry` crate. 

Fixes #1013

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
 

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;

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

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

opentelemetry-sdk/src/propagation/composite.rs → opentelemetry/src/propagation/composite.rs

@@ -1,30 +1,36 @@
-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.
 ///
 /// 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<Box<dyn TextMapPropagator + Send + Sync>>) -> 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<String>, // 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::<Vec<&str>>();
-                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::<Vec<String>>();
-
-        let trace_context = TraceContextPropagator::new();
-        let trace_context_fields = trace_context
-            .fields()
-            .map(|s| s.to_string())
-            .collect::<Vec<String>>();
+        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<dyn TextMapPropagator + Send + Sync>
+                })
+                .collect();
 
-        let mut fields = composite_propagator
-            .fields()
-            .map(|s| s.to_string())
-            .collect::<Vec<String>>();
-        fields.sort();
+            let composite_propagator = TextMapCompositePropagator::new(test_propagators);
 
-        let mut expected = vec![b3_fields, trace_context_fields]
-            .into_iter()
-            .flatten()
-            .collect::<Vec<String>>();
-        expected.sort();
-        expected.dedup();
+            let mut fields = composite_propagator
+                .fields()
+                .map(|s| s.to_string())
+                .collect::<Vec<String>>();
+            fields.sort();
 
-        assert_eq!(fields, expected);
+            assert_eq!(fields, test_case.2);
+        }
     }
 }