Skip to content

Commit 2c24988

Browse files
committed
chore: modify LogExporter and SpanExporter interfaces to support returning failure
Address review comments
1 parent acf16ed commit 2c24988

File tree

16 files changed

+71
-37
lines changed

16 files changed

+71
-37
lines changed

opentelemetry-otlp/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
# Changelog
22

33
## vNext
4-
54
- Bump msrv to 1.75.0.
6-
5+
- `OtlpHttpClient.shutdown` `TonicLogsClient.shutdown`, and `TonicTracesClient.shutdown` now explicitly return a result. The
6+
semantics of the method have not changed, but you will have a new lint encouraging you to consume these results.
77

88
## 0.27.0
99

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,9 @@ impl LogExporter for OtlpHttpClient {
4949
Ok(())
5050
}
5151

52-
fn shutdown(&mut self) {
53-
let _ = self.client.lock().map(|mut c| c.take());
52+
fn shutdown(&mut self) -> LogResult<()> {
53+
let _ = self.client.lock()?.take();
54+
Ok(())
5455
}
5556

5657
fn set_resource(&mut self, resource: &opentelemetry_sdk::Resource) {

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::sync::Arc;
22

33
use futures_core::future::BoxFuture;
44
use http::{header::CONTENT_TYPE, Method};
5-
use opentelemetry::{otel_debug, trace::TraceError};
5+
use opentelemetry::trace::{TraceError, TraceResult};
66
use opentelemetry_sdk::export::trace::{ExportResult, SpanData, SpanExporter};
77

88
use super::OtlpHttpClient;
@@ -64,8 +64,9 @@ impl SpanExporter for OtlpHttpClient {
6464
})
6565
}
6666

67-
fn shutdown(&mut self) {
68-
let _ = self.client.lock().map(|mut c| c.take());
67+
fn shutdown(&mut self) -> TraceResult<()> {
68+
let _ = self.client.lock()?.take();
69+
Ok(())
6970
}
7071

7172
fn set_resource(&mut self, resource: &opentelemetry_sdk::Resource) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,8 +89,9 @@ impl LogExporter for TonicLogsClient {
8989
Ok(())
9090
}
9191

92-
fn shutdown(&mut self) {
92+
fn shutdown(&mut self) -> LogResult<()> {
9393
let _ = self.inner.take();
94+
Ok(())
9495
}
9596

9697
fn set_resource(&mut self, resource: &opentelemetry_sdk::Resource) {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,9 @@ impl SpanExporter for TonicTracesClient {
9292
})
9393
}
9494

95-
fn shutdown(&mut self) {
95+
fn shutdown(&mut self) -> TraceResult<()> {
9696
let _ = self.inner.take();
97+
Ok(())
9798
}
9899

99100
fn set_resource(&mut self, resource: &opentelemetry_sdk::Resource) {

opentelemetry-sdk/CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,11 @@
22

33
## vNext
44

5+
- If you are an exporter author, the trait functions `LogExporter.shutdown` and `TraceExporter.shutdown` must now return a result. Note that implementing shutdown is optional as the trait provides a default implementation that returns Ok(()).
6+
7+
- The trait functions `LogExporter.shutdown` and `TraceExporter.shutdown` now explicitly return a result. The
8+
semantics of the method have not changed, but you will have a new lint encouraging you to consume these results.
9+
510
- *Breaking(Affects custom metric exporter authors only)* `start_time` and `time` is moved from DataPoints to aggregations (Sum, Gauge, Histogram, ExpoHistogram) see [#2377](https://github.com/open-telemetry/opentelemetry-rust/pull/2377) and [#2411](https://github.com/open-telemetry/opentelemetry-rust/pull/2411), to reduce memory.
611

712
- *Breaking* `start_time` is no longer optional for `Sum` aggregation, see [#2367](https://github.com/open-telemetry/opentelemetry-rust/pull/2367), but is still optional for `Gauge` aggregation see [#2389](https://github.com/open-telemetry/opentelemetry-rust/pull/2389).
@@ -14,6 +19,7 @@
1419
[#2338](https://github.com/open-telemetry/opentelemetry-rust/pull/2338)
1520
- `ResourceDetector.detect()` no longer supports timeout option.
1621
- `opentelemetry::global::shutdown_tracer_provider()` Removed from the API, should now use `tracer_provider.shutdown()` see [#2369](https://github.com/open-telemetry/opentelemetry-rust/pull/2369) for a migration example. "Tracer provider" is cheaply cloneable, so users are encouraged to set a clone of it as the global (ex: `global::set_tracer_provider(provider.clone()))`, so that instrumentations and other components can obtain tracers from `global::tracer()`. The tracer_provider must be kept around to call shutdown on it at the end of application (ex: `tracer_provider.shutdown()`)
22+
1723
- *Feature*: Add `ResourceBuilder` for an easy way to create new `Resource`s
1824
- *Breaking*: Remove `Resource::{new,empty,from_detectors,new_with_defaults,from_schema_url,merge,default}` from public api. To create Resources you should only use `Resource::builder()` or `Resource::builder_empty()`. See [#2322](https://github.com/open-telemetry/opentelemetry-rust/pull/2322) for a migration guide.
1925
Example Usage:
@@ -156,6 +162,7 @@ metadata, a feature introduced in version 0.1.40. [#2418](https://github.com/ope
156162
- Continue enabling one of the async runtime feature flags: `rt-tokio`,
157163
`rt-tokio-current-thread`, or `rt-async-std`.
158164

165+
159166
## 0.27.1
160167

161168
Released 2024-Nov-27

opentelemetry-sdk/src/export/logs/mod.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,10 @@ pub trait LogExporter: Send + Sync + Debug {
8383
///
8484
async fn export(&self, batch: LogBatch<'_>) -> LogResult<()>;
8585
/// Shuts down the exporter.
86-
fn shutdown(&mut self) {}
86+
fn shutdown(&mut self) -> LogResult<()> {
87+
Ok(())
88+
}
89+
8790
#[cfg(feature = "spec_unstable_logs_enabled")]
8891
/// Chek if logs are enabled.
8992
fn event_enabled(&self, _level: Severity, _target: &str, _name: &str) -> bool {

opentelemetry-sdk/src/export/trace.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,13 @@ use std::borrow::Cow;
77
use std::fmt::Debug;
88
use std::time::SystemTime;
99

10-
/// Describes the result of an export.
11-
pub type ExportResult = Result<(), TraceError>;
10+
/// Describes the results of other operations on the trace API.
11+
pub type TraceResult<T> = Result<T, TraceError>;
12+
13+
/// Describes the results of an export
14+
/// Note: This is an alias we will remove in the future, favouring
15+
/// using TraceResult directly.
16+
pub type ExportResult = TraceResult<()>;
1217

1318
/// `SpanExporter` defines the interface that protocol-specific exporters must
1419
/// implement so that they can be plugged into OpenTelemetry SDK and support
@@ -30,7 +35,7 @@ pub trait SpanExporter: Send + Sync + Debug {
3035
///
3136
/// Any retry logic that is required by the exporter is the responsibility
3237
/// of the exporter.
33-
fn export(&mut self, batch: Vec<SpanData>) -> BoxFuture<'static, ExportResult>;
38+
fn export(&mut self, batch: Vec<SpanData>) -> BoxFuture<'static, TraceResult<()>>;
3439

3540
/// Shuts down the exporter. Called when SDK is shut down. This is an
3641
/// opportunity for exporter to do any cleanup required.
@@ -43,7 +48,9 @@ pub trait SpanExporter: Send + Sync + Debug {
4348
/// flush the data and the destination is unavailable). SDK authors
4449
/// can decide if they want to make the shutdown timeout
4550
/// configurable.
46-
fn shutdown(&mut self) {}
51+
fn shutdown(&mut self) -> TraceResult<()> {
52+
Ok(())
53+
}
4754

4855
/// This is a hint to ensure that the export of any Spans the exporter
4956
/// has received prior to the call to this function SHOULD be completed

opentelemetry-sdk/src/logs/log_processor.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ impl<T: LogExporter> LogProcessor for SimpleLogProcessor<T> {
144144
self.is_shutdown
145145
.store(true, std::sync::atomic::Ordering::Relaxed);
146146
if let Ok(mut exporter) = self.exporter.lock() {
147-
exporter.shutdown();
147+
exporter.shutdown()?;
148148
Ok(())
149149
} else {
150150
Err(LogError::MutexPoisoned("SimpleLogProcessor".into()))
@@ -650,7 +650,13 @@ impl<R: RuntimeChannel> BatchLogProcessorWithAsyncRuntime<R> {
650650
)
651651
.await;
652652

653-
exporter.shutdown();
653+
if let Err(e) = exporter.shutdown() {
654+
otel_warn!(
655+
name: "BatchLogProcessor.Shutdown.Failed",
656+
message = "failed shutting down exporter cleanly",
657+
error = format!("{:?}", e)
658+
);
659+
};
654660

655661
if let Err(send_error) = ch.send(result) {
656662
otel_debug!(
@@ -934,8 +940,6 @@ mod tests {
934940
Ok(())
935941
}
936942

937-
fn shutdown(&mut self) {}
938-
939943
fn set_resource(&mut self, resource: &Resource) {
940944
self.resource
941945
.lock()

opentelemetry-sdk/src/testing/logs/in_memory_exporter.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,10 +195,11 @@ impl LogExporter for InMemoryLogExporter {
195195
Ok(())
196196
}
197197

198-
fn shutdown(&mut self) {
198+
fn shutdown(&mut self) -> LogResult<()> {
199199
if self.should_reset_on_shutdown {
200200
self.reset();
201201
}
202+
Ok(())
202203
}
203204

204205
fn set_resource(&mut self, resource: &Resource) {

0 commit comments

Comments
 (0)