Skip to content

Commit c03abe2

Browse files
committed
fix: address review findings in span processor API refactor
- Fix end_with_timestamp bug: use explicit end_time_set flag instead of sentinel comparison (start_time == end_time), which silently overwrote user-provided timestamps equal to start time - Restore mem::replace for span_context in end_and_export to avoid unnecessary clone of TraceState (span is dead after data.take()) - Rewrite end_and_export to take data before shutdown check, preventing potential data leak on re-entrant Drop - Add FinishedSpan::is_consumed() method and document post-consume behavior - Add instrumentation_scope() to ReadableSpan trait (OnceLock for MSRV 1.75) - Fix FinishedSpan::Debug to always show consumed=true when span data is gone - Add ReadableSpan trait doc explaining post-end behavior on Span - Use InMemorySpanExporter in test to verify end_time preservation - Normalize formatting in ReadableSpan impl for FinishedSpan - Remove redundant imports, fix doc typo, remove double blank line - Update benchmark results for Apple M4 Max
1 parent bd2eb3a commit c03abe2

6 files changed

Lines changed: 253 additions & 134 deletions

File tree

opentelemetry-sdk/CHANGELOG.md

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,16 @@
2828
- **Breaking** `SpanProcessor::on_end` now takes `&mut FinishedSpan` instead of `SpanData`.
2929
Processors that only need to read span data can use the `ReadableSpan` trait without cloning.
3030
Processors that need ownership call `FinishedSpan::consume()` — the last processor receives
31-
ownership via move (zero-copy), earlier processors receive a clone.
31+
ownership via move (zero-copy), earlier processors receive a clone. Both `Span` (live) and
32+
`FinishedSpan` (ended) implement `ReadableSpan`, enabling processors to inspect span data
33+
in both `on_start` and `on_end` hooks. New `FinishedSpan::is_consumed()` method allows
34+
checking whether span data has been consumed.
3235
Supersedes [#2962](https://github.com/open-telemetry/opentelemetry-rust/pull/2962).
3336
Relates to [#2940](https://github.com/open-telemetry/opentelemetry-rust/issues/2940),
3437
[#2726](https://github.com/open-telemetry/opentelemetry-rust/issues/2726),
3538
[#2939](https://github.com/open-telemetry/opentelemetry-rust/issues/2939).
39+
- Fix `Span::end_with_timestamp` preserving explicit end times even when equal to start time,
40+
instead of silently overwriting with the current time.
3641

3742
[3227]: https://github.com/open-telemetry/opentelemetry-rust/pull/3227
3843
[3277]: https://github.com/open-telemetry/opentelemetry-rust/pull/3277

opentelemetry-sdk/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ name = "span_processor_api"
130130
harness = false
131131
required-features = ["testing"]
132132

133-
134133
[lib]
135134
bench = false
136135

opentelemetry-sdk/benches/span_processor_api.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,17 +12,17 @@ use pprof::criterion::{Output, PProfProfiler};
1212

1313
/*
1414
Adding results in comments for a quick reference.
15-
Chip: Apple M1 Max
16-
Total Number of Cores: 10 (8 performance and 2 efficiency)
15+
Chip: Apple M4 Max
16+
Total Number of Cores: 16 (12 performance and 4 efficiency)
1717
1818
SpanProcessorApi/0_processors
19-
time: [385.15 ns 386.14 ns 387.25 ns]
19+
time: [190.51 ns 190.82 ns 191.13 ns]
2020
SpanProcessorApi/1_processors
21-
time: [385.73 ns 387.17 ns 388.85 ns]
21+
time: [191.60 ns 192.53 ns 193.42 ns]
2222
SpanProcessorApi/2_processors
23-
time: [384.84 ns 385.66 ns 386.50 ns]
23+
time: [191.24 ns 191.67 ns 192.15 ns]
2424
SpanProcessorApi/4_processors
25-
time: [386.78 ns 388.17 ns 389.58 ns]
25+
time: [192.94 ns 193.48 ns 194.12 ns]
2626
*/
2727

2828
#[derive(Debug)]

0 commit comments

Comments
 (0)