Merge branch 'main' into cijothomas/addscott-approver

Author: cijothomas

docs/design/logs.md

@@ -280,6 +280,22 @@ this crate.
 ## Performance
 
 // Call out things done specifically for performance
+// Rough draft
+
+1. `LogRecord` is stack allocated and not Boxed unless required by the component
+needing to store it beyond the logging call. (eg: BatchProcessor)
+2. LogRecords's Attribute storage is specially designed struct, that holds up to
+five attributes in stack.
+3. When passing `LogRecord`s to processor, a mutable ref is passed. This allows
+calling multiple processors one after another, without the need for cloning.
+4. `Logger` provides a `Enabled` check which can optimize performance when
+no-one is interested in the log. The check is passed from `Logger` to the
+processor, which may consult its exporter to make the decision. An example use
+case - an ETW or user-events exporter can check for the presence of listener and
+convey that decision back to logger, allowing appender to avoid even the cost of
+creating a `LogRecord` in the first place if there is no listener. This check is
+done for each log emission, and can react dynamically to changes in interest, by
+enabling/disabling ETW/user-event listener.
 
 ### Perf test - benchmarks
 
@@ -289,6 +305,23 @@ this crate.
 
 // Share ~~ numbers
 
+## Internal logs
+
+OTel itself is instrumented with `tracing` crate to emit internal logs about its
+operations. This is feature gated under "internal-logs", and is enabled by
+default for all components. The `opentelemetry` provide few helper macros
+`otel_warn` etc., which in turn invokes various `tracing` macros like `warn!`
+etc. The cargo package name will be set as `target` when using `tracing`. For
+example, logs from `opentelemetry-otlp` will have target set to
+"opentelemetry-otlp".
+
+The helper macros are part of public API, so can be used by anyone. But it is
+only meant for OTel components itself and anyone writing extensions like custom
+Exporters etc.
+
+// TODO: Document the principles followed when selecting severity for internal
+logs // TODO: Document how this can cause circular loop and plans to address it.
+
 ## Summary
 
 - OpenTelemetry Logs does not provide a user-facing logging API.

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/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

@@ -33,7 +33,17 @@
     }
 }
 ```
-
+- **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(())
     }
 }

opentelemetry-sdk/benches/span_builder.rs

@@ -65,7 +65,7 @@ fn not_sampled_provider() -> (sdktrace::SdkTracerProvider, sdktrace::SdkTracer)
 struct NoopExporter;
 
 impl SpanExporter for NoopExporter {
-    async fn export(&mut self, _spans: Vec<SpanData>) -> OTelSdkResult {
+    async fn export(&self, _spans: Vec<SpanData>) -> OTelSdkResult {
         Ok(())
     }
 }

opentelemetry-sdk/benches/trace.rs

@@ -59,7 +59,7 @@ fn criterion_benchmark(c: &mut Criterion) {
 struct VoidExporter;
 
 impl SpanExporter for VoidExporter {
-    async fn export(&mut self, _spans: Vec<SpanData>) -> OTelSdkResult {
+    async fn export(&self, _spans: Vec<SpanData>) -> OTelSdkResult {
         Ok(())
     }
 }

opentelemetry-sdk/src/testing/trace/span_exporters.rs

@@ -41,7 +41,7 @@ pub struct TokioSpanExporter {
 }
 
 impl SpanExporter for TokioSpanExporter {
-    async fn export(&mut self, batch: Vec<SpanData>) -> OTelSdkResult {
+    async fn export(&self, batch: Vec<SpanData>) -> OTelSdkResult {
         batch.into_iter().try_for_each(|span_data| {
             self.tx_export
                 .send(span_data)
@@ -110,7 +110,7 @@ impl NoopSpanExporter {
 }
 
 impl SpanExporter for NoopSpanExporter {
-    async fn export(&mut self, _: Vec<SpanData>) -> OTelSdkResult {
+    async fn export(&self, _: Vec<SpanData>) -> OTelSdkResult {
         Ok(())
     }
 }

opentelemetry-sdk/src/trace/export.rs

@@ -28,7 +28,7 @@ pub trait SpanExporter: Send + Sync + Debug {
     /// Any retry logic that is required by the exporter is the responsibility
     /// of the exporter.
     fn export(
-        &mut self,
+        &self,
         batch: Vec<SpanData>,
     ) -> impl std::future::Future<Output = OTelSdkResult> + Send;