Merge branch 'main' into protocol-display-fromstr

Author: lalitb

opentelemetry-appender-tracing/benches/log-attributes.rs

@@ -248,10 +248,11 @@ fn create_benchmark(c: &mut Criterion, num_attributes: usize) {
 }
 
 fn criterion_benchmark(c: &mut Criterion) {
+    create_benchmark(c, 2);
     // Run benchmarks for 0 to 12 attributes
-    for num_attributes in 0..=12 {
-        create_benchmark(c, num_attributes);
-    }
+    // for num_attributes in 0..=12 {
+    //     create_benchmark(c, 2);
+    // }
 }
 
 #[cfg(not(target_os = "windows"))]

opentelemetry-appender-tracing/benches/logs.rs

@@ -10,7 +10,17 @@
     | noop_layer_disabled         | 12 ns       |
     | noop_layer_enabled          | 25 ns       |
     | ot_layer_disabled           | 19 ns       |
-    | ot_layer_enabled            | 196 ns      |
+    | ot_layer_enabled            | 167 ns      |
+
+    Hardware: Apple M4 Pro
+    Total Number of Cores:	14 (10 performance and 4 efficiency)
+    | Test                        | Average time|
+    |-----------------------------|-------------|
+    | log_no_subscriber           | 285 ps      |
+    | noop_layer_disabled         | 8 ns       |
+    | noop_layer_enabled          | 14 ns       |
+    | ot_layer_disabled           | 12 ns       |
+    | ot_layer_enabled            | 186 ns      |
 */
 
 use criterion::{criterion_group, criterion_main, Criterion};
@@ -94,7 +104,7 @@ fn benchmark_no_subscriber(c: &mut Criterion) {
     c.bench_function("log_no_subscriber", |b| {
         b.iter(|| {
             error!(
-                name = "CheckoutFailed",
+                name : "CheckoutFailed",
                 book_id = "12345",
                 book_title = "Rust Programming Adventures",
                 message = "Unable to process checkout."
@@ -120,7 +130,7 @@ fn benchmark_with_ot_layer(c: &mut Criterion, enabled: bool, bench_name: &str) {
         c.bench_function(bench_name, |b| {
             b.iter(|| {
                 error!(
-                    name = "CheckoutFailed",
+                    name : "CheckoutFailed",
                     book_id = "12345",
                     book_title = "Rust Programming Adventures",
                     message = "Unable to process checkout."
@@ -137,10 +147,10 @@ fn benchmark_with_noop_layer(c: &mut Criterion, enabled: bool, bench_name: &str)
         c.bench_function(bench_name, |b| {
             b.iter(|| {
                 error!(
-                    name = "CheckoutFailed",
+                    name : "CheckoutFailed",
                     book_id = "12345",
                     book_title = "Rust Programming Adventures",
-                    "Unable to process checkout."
+                    message = "Unable to process checkout."
                 );
             });
         });

opentelemetry-otlp/tests/integration_test/tests/metrics.rs

@@ -69,11 +69,11 @@ mod metrictests {
         );
 
         // In tokio::current_thread flavor, shutdown must be done in a separate thread
-        let shutdown_resut = Handle::current()
+        let shutdown_result = Handle::current()
             .spawn_blocking(move || meter_provider.shutdown())
             .await
             .unwrap();
-        assert!(shutdown_resut.is_ok());
+        assert!(shutdown_result.is_ok());
         // We still need to sleep, to give otel-collector a chance to flush to disk
         std::thread::sleep(SLEEP_DURATION);
 
@@ -102,8 +102,8 @@ mod metrictests {
             ],
         );
 
-        let shutdown_resut = meter_provider.shutdown();
-        assert!(shutdown_resut.is_ok());
+        let shutdown_result = meter_provider.shutdown();
+        assert!(shutdown_result.is_ok());
         // We still need to sleep, to give otel-collector a chance to flush to disk
         std::thread::sleep(SLEEP_DURATION);
 
@@ -129,8 +129,8 @@ mod metrictests {
             ],
         );
 
-        let shutdown_resut = meter_provider.shutdown();
-        assert!(shutdown_resut.is_ok());
+        let shutdown_result = meter_provider.shutdown();
+        assert!(shutdown_result.is_ok());
         // We still need to sleep, to give otel-collector a chance to flush to disk
         std::thread::sleep(SLEEP_DURATION);
 

opentelemetry-sdk/src/propagation/baggage.rs

@@ -30,7 +30,7 @@ fn baggage_fields() -> &'static [String; 1] {
 /// # Examples
 ///
 /// ```
-/// use opentelemetry::{baggage::BaggageExt, KeyValue, propagation::TextMapPropagator};
+/// use opentelemetry::{baggage::{Baggage, BaggageExt}, propagation::TextMapPropagator};
 /// use opentelemetry_sdk::propagation::BaggagePropagator;
 /// use std::collections::HashMap;
 ///
@@ -48,14 +48,17 @@ fn baggage_fields() -> &'static [String; 1] {
 /// }
 ///
 /// // Add new baggage
-/// let cx_with_additions = cx.with_baggage(vec![KeyValue::new("server_id", 42)]);
+/// let mut baggage = Baggage::new();
+/// let _ = baggage.insert("server_id", "42");
+///
+/// let cx_with_additions = cx.with_baggage(baggage);
 ///
 /// // Inject baggage into http request
 /// propagator.inject_context(&cx_with_additions, &mut headers);
 ///
 /// let header_value = headers.get("baggage").expect("header is injected");
-/// assert!(header_value.contains("user_id=1"), "still contains previous name-value");
-/// assert!(header_value.contains("server_id=42"), "contains new name-value pair");
+/// assert!(!header_value.contains("user_id=1"), "still contains previous name-value");
+/// assert!(header_value.contains("server_id=42"), "does not contain new name-value pair");
 /// ```
 ///
 /// [W3C Baggage]: https://w3c.github.io/baggage
@@ -98,7 +101,7 @@ impl TextMapPropagator for BaggagePropagator {
     /// Extracts a `Context` with baggage values from a `Extractor`.
     fn extract_with_context(&self, cx: &Context, extractor: &dyn Extractor) -> Context {
         if let Some(header_value) = extractor.get(BAGGAGE_HEADER) {
-            let baggage = header_value.split(',').flat_map(|context_value| {
+            let baggage = header_value.split(',').filter_map(|context_value| {
                 if let Some((name_and_value, props)) = context_value
                     .split(';')
                     .collect::<Vec<&str>>()

opentelemetry/CHANGELOG.md

@@ -7,11 +7,13 @@
 - *Breaking* Moved `TraceResult` type alias from `opentelemetry::trace::TraceResult` to `opentelemetry_sdk::trace::TraceResult`
 - {PLACEHOLDER} - Remove the above completely. // TODO fill this when changes are actually in.
 - Bug Fix: `InstrumentationScope` implementation for `PartialEq` and `Hash` fixed to include Attributes also.
-- *Breaking* Changed value type of `Baggage` from `Value` to `StringValue`
-- Updated `Baggage` constants to reflect latest standard (`MAX_KEY_VALUE_PAIRS` - 180 -> 64, `MAX_BYTES_FOR_ONE_PAIR` - removed) and increased insert performance see #[2284](https://github.com/open-telemetry/opentelemetry-rust/pull/2284).
-- *Breaking* Align `Baggage.remove()` signature with `.get()` to take the key as a reference
+- **Breaking changes for baggage users**: [#2717](https://github.com/open-telemetry/opentelemetry-rust/issues/2717)
+  - Changed value type of `Baggage` from `Value` to `StringValue`
+  - Updated `Baggage` constants to reflect latest standard (`MAX_KEY_VALUE_PAIRS` - 180 -> 64, `MAX_BYTES_FOR_ONE_PAIR` - removed) and increased insert performance see #[2284](https://github.com/open-telemetry/opentelemetry-rust/pull/2284).
+  - Align `Baggage.remove()` signature with `.get()` to take the key as a reference
+  - `Baggage` can't be retrieved from the `Context` directly anymore and needs to be accessed via `context.baggage()`
+  - `with_baggage()` and `current_with_baggage()` override any existing `Baggage` in the `Context`
 - Changed `Context` to use a stack to properly handle out of order dropping of `ContextGuard`. This imposes a limit of `65535` nested contexts on a single thread. See #[2378](https://github.com/open-telemetry/opentelemetry-rust/pull/2284) and #[1887](https://github.com/open-telemetry/opentelemetry-rust/issues/1887).
-
 - Added additional `name: Option<&str>` parameter to the `event_enabled` method
   on the `Logger` trait. This allows implementations (SDK, processor, exporters)
   to leverage this additional information to determine if an event is enabled.

opentelemetry/src/baggage.rs

@@ -273,6 +273,16 @@ impl FromIterator<KeyValueMetadata> for Baggage {
     }
 }
 
+impl<I> From<I> for Baggage
+where
+    I: IntoIterator,
+    I::Item: Into<KeyValueMetadata>,
+{
+    fn from(value: I) -> Self {
+        value.into_iter().map(Into::into).collect()
+    }
+}
+
 fn encode(s: &str) -> String {
     let mut encoded_string = String::with_capacity(s.len());
 
@@ -312,39 +322,46 @@ pub trait BaggageExt {
     /// # Examples
     ///
     /// ```
-    /// use opentelemetry::{baggage::BaggageExt, Context, KeyValue, StringValue};
+    /// use opentelemetry::{baggage::{Baggage, BaggageExt}, Context, KeyValue, StringValue};
+    ///
+    /// // Explicit `Baggage` creation
+    /// let mut baggage = Baggage::new();
+    /// let _ = baggage.insert("my-name", "my-value");
+    ///
+    /// let cx = Context::map_current(|cx| {
+    ///     cx.with_baggage(baggage)
+    /// });
     ///
+    /// // Passing an iterator
     /// let cx = Context::map_current(|cx| {
-    ///     cx.with_baggage(vec![KeyValue::new("my-name", "my-value")])
+    ///     cx.with_baggage([KeyValue::new("my-name", "my-value")])
     /// });
     ///
     /// assert_eq!(
     ///     cx.baggage().get("my-name"),
     ///     Some(&StringValue::from("my-value")),
     /// )
     /// ```
-    fn with_baggage<T: IntoIterator<Item = I>, I: Into<KeyValueMetadata>>(
-        &self,
-        baggage: T,
-    ) -> Self;
+    fn with_baggage<T: Into<Baggage>>(&self, baggage: T) -> Self;
 
     /// Returns a clone of the current context with the included name/value pairs.
     ///
     /// # Examples
     ///
     /// ```
-    /// use opentelemetry::{baggage::BaggageExt, Context, KeyValue, StringValue};
+    /// use opentelemetry::{baggage::{Baggage, BaggageExt}, Context, StringValue};
     ///
-    /// let cx = Context::current_with_baggage(vec![KeyValue::new("my-name", "my-value")]);
+    /// let mut baggage = Baggage::new();
+    /// let _ = baggage.insert("my-name", "my-value");
+    ///
+    /// let cx = Context::current_with_baggage(baggage);
     ///
     /// assert_eq!(
     ///     cx.baggage().get("my-name"),
     ///     Some(&StringValue::from("my-value")),
     /// )
     /// ```
-    fn current_with_baggage<T: IntoIterator<Item = I>, I: Into<KeyValueMetadata>>(
-        baggage: T,
-    ) -> Self;
+    fn current_with_baggage<T: Into<Baggage>>(baggage: T) -> Self;
 
     /// Returns a clone of the given context with no baggage.
     ///
@@ -364,33 +381,26 @@ pub trait BaggageExt {
     fn baggage(&self) -> &Baggage;
 }
 
-impl BaggageExt for Context {
-    fn with_baggage<T: IntoIterator<Item = I>, I: Into<KeyValueMetadata>>(
-        &self,
-        baggage: T,
-    ) -> Self {
-        let old = self.baggage();
-        let mut merged = Baggage {
-            inner: old.inner.clone(),
-            kv_content_len: old.kv_content_len,
-        };
-        for kvm in baggage.into_iter().map(|kv| kv.into()) {
-            merged.insert_with_metadata(kvm.key, kvm.value, kvm.metadata);
-        }
+/// Solely used to store `Baggage` in the `Context` without allowing direct access
+#[derive(Debug)]
+struct BaggageContextValue(Baggage);
 
-        self.with_value(merged)
+impl BaggageExt for Context {
+    fn with_baggage<T: Into<Baggage>>(&self, baggage: T) -> Self {
+        self.with_value(BaggageContextValue(baggage.into()))
     }
 
-    fn current_with_baggage<T: IntoIterator<Item = I>, I: Into<KeyValueMetadata>>(kvs: T) -> Self {
-        Context::map_current(|cx| cx.with_baggage(kvs))
+    fn current_with_baggage<T: Into<Baggage>>(baggage: T) -> Self {
+        Context::map_current(|cx| cx.with_baggage(baggage))
     }
 
     fn with_cleared_baggage(&self) -> Self {
         self.with_value(Baggage::new())
     }
 
     fn baggage(&self) -> &Baggage {
-        self.get::<Baggage>().unwrap_or(get_default_baggage())
+        self.get::<BaggageContextValue>()
+            .map_or(get_default_baggage(), |b| &b.0)
     }
 }
 

stress/Cargo.toml

@@ -45,7 +45,7 @@ lazy_static = { workspace = true }
 num_cpus = { workspace = true }
 opentelemetry = { path = "../opentelemetry", features = ["metrics", "logs", "trace", "spec_unstable_logs_enabled"] }
 opentelemetry_sdk = { path = "../opentelemetry-sdk", features = ["metrics", "logs", "trace", "spec_unstable_logs_enabled"] }
-opentelemetry-appender-tracing = { workspace = true }
+opentelemetry-appender-tracing = { workspace = true, features = ["spec_unstable_logs_enabled"] }
 rand = { workspace = true, features = ["small_rng", "os_rng"] }
 tracing = { workspace = true, features = ["std"]}
 tracing-subscriber = { workspace = true, features = ["registry", "std"] }

stress/src/logs.rs

@@ -6,7 +6,12 @@
     ~31 M/sec
 
     Hardware: AMD EPYC 7763 64-Core Processor - 2.44 GHz, 16vCPUs,
-    ~40 M /sec
+    ~44 M /sec
+
+    Hardware: Apple M4 Pro
+    Total Number of Cores:	14 (10 performance and 4 efficiency)
+    ~50 M/sec
+    ~1.1 B/sec (when disabled)
 */
 
 use opentelemetry::InstrumentationScope;
@@ -32,6 +37,7 @@ impl LogExporter for MockLogExporter {
 #[derive(Debug)]
 pub struct MockLogProcessor {
     exporter: MockLogExporter,
+    enabled: bool,
 }
 
 impl LogProcessor for MockLogProcessor {
@@ -51,13 +57,26 @@ impl LogProcessor for MockLogProcessor {
     fn shutdown(&self) -> OTelSdkResult {
         Ok(())
     }
+
+    fn event_enabled(
+        &self,
+        _level: opentelemetry::logs::Severity,
+        _target: &str,
+        _name: Option<&str>,
+    ) -> bool {
+        self.enabled
+    }
 }
 
 fn main() {
+    // change this to false to test the throughput when enabled is false.
+    let enabled = true;
+
     // LoggerProvider with a no-op processor.
     let provider: SdkLoggerProvider = SdkLoggerProvider::builder()
         .with_log_processor(MockLogProcessor {
             exporter: MockLogExporter {},
+            enabled,
         })
         .build();
 
@@ -69,7 +88,7 @@ fn main() {
 
 fn test_log() {
     error!(
-        name = "CheckoutFailed",
+        name : "CheckoutFailed",
         book_id = "12345",
         book_title = "Rust Programming Adventures",
         message = "Unable to process checkout."