Merge branch 'main' into cijothomas/design-internalog

Author: cijothomas

opentelemetry-appender-tracing/CHANGELOG.md

@@ -5,6 +5,32 @@
 Fixes [1682](https://github.com/open-telemetry/opentelemetry-rust/issues/1682).
 "spec_unstable_logs_enabled" feature now do not suppress logs for other layers.
 
+The special treatment of the "message" field has been extended when recording
+string values. With this change, when a log is emitted with a field named
+"message" (and string value), its value is directly assigned to the LogRecord’s
+body rather than being stored as an attribute named "message". This offers a
+slight performance improvement over previous.
+
+For example, the below will now produce LogRecord with the message value
+populated as LogRecord's body:
+
+```rust
+error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]", message = "This is an example message");
+```
+
+Previously, Body was only populated when the below style was used.
+
+```rust
+error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]", "This is an example message");
+```
+
+This style, while slightly slower, should still be used when the value is not a
+simple string, but require format arguments as in the below example.
+
+```rust
+error!(name: "my-event-name", target: "my-system", event_id = 20, user_name = "otel", user_email = "[email protected]", "This is an example message with format arguments {} and {}", "foo", "bar");
+```
+
 ## 0.28.1
 
 Released 2025-Feb-12

opentelemetry-appender-tracing/src/layer.rs

@@ -88,15 +88,14 @@ impl<LR: LogRecord> tracing::field::Visit for EventVisitor<'_, LR> {
         if is_duplicated_metadata(field.name()) {
             return;
         }
-        //TODO: Consider special casing "message" to populate body and document
-        // to users to use message field for log message, to avoid going to the
-        // record_debug, which has dyn dispatch, string allocation and
-        // formatting cost.
-
         //TODO: Fix heap allocation. Check if lifetime of &str can be used
         // to optimize sync exporter scenario.
-        self.log_record
-            .add_attribute(Key::new(field.name()), AnyValue::from(value.to_owned()));
+        if field.name() == "message" {
+            self.log_record.set_body(AnyValue::from(value.to_owned()));
+        } else {
+            self.log_record
+                .add_attribute(Key::new(field.name()), AnyValue::from(value.to_owned()));
+        }
     }
 
     fn record_bool(&mut self, field: &tracing_core::Field, value: bool) {

opentelemetry-otlp/src/exporter/http/mod.rs

@@ -120,7 +120,7 @@ impl HttpExporterBuilder {
             .or(env::var(OTEL_EXPORTER_OTLP_TIMEOUT).ok())
         {
             Some(val) => match val.parse() {
-                Ok(seconds) => Duration::from_millis(seconds),
+                Ok(milli_seconds) => Duration::from_millis(milli_seconds),
                 Err(_) => self.exporter_config.timeout,
             },
             None => self.exporter_config.timeout,

opentelemetry-otlp/src/exporter/http/trace.rs

@@ -9,7 +9,7 @@ use opentelemetry_sdk::{
 };
 
 impl SpanExporter for OtlpHttpClient {
-    async fn export(&mut self, batch: Vec<SpanData>) -> OTelSdkResult {
+    async fn export(&self, batch: Vec<SpanData>) -> OTelSdkResult {
         let client = match self
             .client
             .lock()

opentelemetry-otlp/src/exporter/mod.rs

@@ -52,7 +52,7 @@ const OTEL_EXPORTER_OTLP_PROTOCOL_HTTP_JSON: &str = "http/json";
 /// Max waiting time for the backend to process each signal batch, defaults to 10 seconds.
 pub const OTEL_EXPORTER_OTLP_TIMEOUT: &str = "OTEL_EXPORTER_OTLP_TIMEOUT";
 /// Default max waiting time for the backend to process each signal batch.
-pub const OTEL_EXPORTER_OTLP_TIMEOUT_DEFAULT: u64 = 10000;
+pub const OTEL_EXPORTER_OTLP_TIMEOUT_DEFAULT: Duration = Duration::from_millis(10000);
 
 // Endpoints per protocol https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md
 #[cfg(feature = "grpc-tonic")]
@@ -87,7 +87,7 @@ impl Default for ExportConfig {
             // don't use default_endpoint(protocol) here otherwise we
             // won't know if user provided a value
             protocol,
-            timeout: Duration::from_millis(OTEL_EXPORTER_OTLP_TIMEOUT_DEFAULT),
+            timeout: OTEL_EXPORTER_OTLP_TIMEOUT_DEFAULT,
         }
     }
 }

opentelemetry-otlp/src/exporter/tonic/mod.rs

@@ -200,7 +200,7 @@ impl TonicExporterBuilder {
             .or(env::var(OTEL_EXPORTER_OTLP_TIMEOUT).ok())
         {
             Some(val) => match val.parse() {
-                Ok(seconds) => Duration::from_millis(seconds),
+                Ok(milli_seconds) => Duration::from_millis(milli_seconds),
                 Err(_) => config.timeout,
             },
             None => config.timeout,

opentelemetry-otlp/src/exporter/tonic/trace.rs

@@ -1,4 +1,5 @@
 use core::fmt;
+use tokio::sync::Mutex;
 
 use opentelemetry::otel_debug;
 use opentelemetry_proto::tonic::collector::trace::v1::{
@@ -23,7 +24,7 @@ pub(crate) struct TonicTracesClient {
 
 struct ClientInner {
     client: TraceServiceClient<Channel>,
-    interceptor: BoxInterceptor,
+    interceptor: Mutex<BoxInterceptor>,
 }
 
 impl fmt::Debug for TonicTracesClient {
@@ -50,26 +51,27 @@ impl TonicTracesClient {
         TonicTracesClient {
             inner: Some(ClientInner {
                 client,
-                interceptor,
+                interceptor: Mutex::new(interceptor),
             }),
             resource: Default::default(),
         }
     }
 }
 
 impl SpanExporter for TonicTracesClient {
-    async fn export(&mut self, batch: Vec<SpanData>) -> OTelSdkResult {
-        let (mut client, metadata, extensions) = match &mut self.inner {
+    async fn export(&self, batch: Vec<SpanData>) -> OTelSdkResult {
+        let (mut client, metadata, extensions) = match &self.inner {
             Some(inner) => {
-                let (m, e, _) = match inner.interceptor.call(Request::new(())) {
-                    Ok(res) => res.into_parts(),
-                    Err(e) => return Err(OTelSdkError::InternalFailure(e.to_string())),
-                };
+                let (m, e, _) = inner
+                    .interceptor
+                    .lock()
+                    .await // tokio::sync::Mutex doesn't return a poisoned error, so we can safely use the interceptor here
+                    .call(Request::new(()))
+                    .map_err(|e| OTelSdkError::InternalFailure(format!("error: {:?}", e)))?
+                    .into_parts();
                 (inner.client.clone(), m, e)
             }
-            None => {
-                return Err(OTelSdkError::AlreadyShutdown);
-            }
+            None => return Err(OTelSdkError::AlreadyShutdown),
         };
 
         let resource_spans = group_spans_by_resource_and_scope(batch, &self.resource);

opentelemetry-otlp/src/span.rs

@@ -141,8 +141,8 @@ impl SpanExporter {
 }
 
 impl opentelemetry_sdk::trace::SpanExporter for SpanExporter {
-    async fn export(&mut self, batch: Vec<SpanData>) -> OTelSdkResult {
-        match &mut self.client {
+    async fn export(&self, batch: Vec<SpanData>) -> OTelSdkResult {
+        match &self.client {
             #[cfg(feature = "grpc-tonic")]
             SupportedTransportClient::Tonic(client) => client.export(batch).await,
             #[cfg(any(feature = "http-proto", feature = "http-json"))]

opentelemetry-sdk/CHANGELOG.md

@@ -2,13 +2,48 @@
 
 ## vNext
 
-- Calls to `MeterProviderBuilder::with_resource`, `TracerProviderBuilder::with_resource`, 
+- Calls to `MeterProviderBuilder::with_resource`, `TracerProviderBuilder::with_resource`,
   `LoggerProviderBuilder::with_resource` are now additive ([#2677](https://github.com/open-telemetry/opentelemetry-rust/pull/2677)).
-- *Breaking*: Make `force_flush()` in `PushMetricExporter` synchronous
 - Moved `ExportError` trait from `opentelemetry::trace::ExportError` to `opentelemetry_sdk::export::ExportError`
 - Moved `TraceError` enum from `opentelemetry::trace::TraceError` to `opentelemetry_sdk::trace::TraceError`
 - Moved `TraceResult` type alias from `opentelemetry::trace::TraceResult` to `opentelemetry_sdk::trace::TraceResult`
+- *Breaking*: Make `force_flush()` in `PushMetricExporter` synchronous
+- **Breaking Change:** Updated the `SpanExporter` trait method signature:
+
+```rust
+  fn export(&mut self, batch: Vec<SpanData>) -> BoxFuture<'static, OTelSdkResult>;
+```
+
+  to
+
+```rust
+  fn export(
+    &mut self,
+    batch: Vec<SpanData>,
+) -> impl std::future::Future<Output = OTelSdkResult> + Send;
+```
 
+  This affects anyone who writes custom exporters, as custom implementations of SpanExporter
+  should now define export as an `async fn`:
+
+```rust
+  impl trace::SpanExporter for CustomExporter {
+    async fn export(&mut self, batch: Vec<trace::SpanData>) -> OTelSdkResult {
+        // Implementation here
+    }
+}
+```
+- **Breaking** The SpanExporter::export() method no longer requires a mutable reference to self.
+  Before: 
+  ```rust
+    async fn export(&mut self, batch: Vec<SpanData>) -> OTelSdkResult 
+  ```
+  After: 
+  ```rust 
+    async fn export(&self, batch: Vec<SpanData>) -> OTelSdkResult  
+  ```
+  Custom exporters will need to internally synchronize any mutable state, if applicable.
+  
 ## 0.28.0
 
 Released 2025-Feb-10

opentelemetry-sdk/benches/context.rs

@@ -136,7 +136,7 @@ fn parent_sampled_tracer(inner_sampler: Sampler) -> (SdkTracerProvider, BoxedTra
 struct NoopExporter;
 
 impl SpanExporter for NoopExporter {
-    async fn export(&mut self, _spans: Vec<SpanData>) -> OTelSdkResult {
+    async fn export(&self, _spans: Vec<SpanData>) -> OTelSdkResult {
         Ok(())
     }
 }