Skip to content

Commit 483b420

Browse files
stefanobaghinoscottgerringcijothomas
authored
fix: handle shutdown in logs exporter (#3255)
Co-authored-by: Scott Gerring <[email protected]> Co-authored-by: Cijo Thomas <[email protected]>
1 parent 53c9f47 commit 483b420

File tree

3 files changed

+28
-28
lines changed

3 files changed

+28
-28
lines changed

opentelemetry-otlp/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- Add partial success response handling for OTLP exporters (traces, metrics, logs) per OTLP spec. Exporters now log warnings when the server returns partial success responses with rejected items and error messages. [#865](https://github.com/open-telemetry/opentelemetry-rust/issues/865)
66
- Refactor `internal-logs` feature in `opentelemetry-otlp` to reduce unnecessary dependencies[3191](https://github.com/open-telemetry/opentelemetry-rust/pull/3192)
7+
- Fixed [#2777](https://github.com/open-telemetry/opentelemetry rust/issues/2777) to properly handle `shutdown_with_timeout()` when using `grpc-tonic`.
78

89
## 0.31.0
910

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

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,8 @@ use opentelemetry_proto::tonic::collector::logs::v1::{
55
};
66
use opentelemetry_sdk::error::{OTelSdkError, OTelSdkResult};
77
use opentelemetry_sdk::logs::{LogBatch, LogExporter};
8-
use std::sync::Arc;
8+
use std::sync::{Arc, Mutex};
99
use std::time;
10-
use tokio::sync::Mutex;
1110
use tonic::{codegen::CompressionEncoding, service::Interceptor, transport::Channel, Request};
1211

1312
use opentelemetry_proto::transform::logs::tonic::group_logs_by_resource_and_scope;
@@ -85,24 +84,26 @@ impl LogExporter for TonicLogsClient {
8584
let batch_clone = Arc::clone(&batch);
8685

8786
// Execute the export operation
88-
let (mut client, metadata, extensions) = match self.inner.lock().await.as_mut() {
89-
Some(inner) => {
90-
let (m, e, _) = inner
91-
.interceptor
92-
.call(Request::new(()))
93-
.map_err(|e| {
94-
// Convert interceptor errors to tonic::Status for retry classification
95-
tonic::Status::internal(format!("interceptor error: {e:?}"))
96-
})?
97-
.into_parts();
98-
(inner.client.clone(), m, e)
99-
}
100-
None => {
101-
return Err(tonic::Status::failed_precondition(
102-
"exporter already shutdown",
103-
))
104-
}
105-
};
87+
let (mut client, metadata, extensions) = self
88+
.inner
89+
.lock()
90+
.map_err(|e| tonic::Status::internal(format!("Failed to acquire lock: {e:?}")))
91+
.and_then(|mut inner| match &mut *inner {
92+
Some(inner) => {
93+
let (m, e, _) = inner
94+
.interceptor
95+
.call(Request::new(()))
96+
.map_err(|e| {
97+
// Convert interceptor errors to tonic::Status for retry classification
98+
tonic::Status::internal(format!("interceptor error: {e:?}"))
99+
})?
100+
.into_parts();
101+
Ok((inner.client.clone(), m, e))
102+
}
103+
None => Err(tonic::Status::failed_precondition(
104+
"log exporter is already shut down",
105+
)),
106+
})?;
106107

107108
let resource_logs = group_logs_by_resource_and_scope(&batch_clone, &self.resource);
108109

@@ -143,13 +144,11 @@ impl LogExporter for TonicLogsClient {
143144
}
144145

145146
fn shutdown_with_timeout(&self, _timeout: time::Duration) -> OTelSdkResult {
146-
// TODO: Implement actual shutdown
147-
// Due to the use of tokio::sync::Mutex to guard
148-
// the inner client, we need to await the call to lock the mutex
149-
// and that requires async runtime.
150-
// It is possible to fix this by using
151-
// a dedicated thread just to handle shutdown.
152-
// But for now, we just return Ok.
147+
self.inner
148+
.lock()
149+
.map_err(|e| OTelSdkError::InternalFailure(format!("Failed to acquire lock: {e}")))?
150+
.take();
151+
153152
Ok(())
154153
}
155154

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ impl MetricsClient for TonicMetricsClient {
9393
Ok((inner.client.clone(), m, e))
9494
}
9595
None => Err(tonic::Status::failed_precondition(
96-
"exporter is already shut down",
96+
"metrics exporter is already shut down",
9797
)),
9898
})?;
9999

0 commit comments

Comments
 (0)