perf(sdk,otlp): add SpanBatch for borrowed span export, eliminate exporter clones#3379
Open
bryantbiggs wants to merge 4 commits intoopen-telemetry:mainfrom
Open
perf(sdk,otlp): add SpanBatch for borrowed span export, eliminate exporter clones#3379bryantbiggs wants to merge 4 commits intoopen-telemetry:mainfrom
bryantbiggs wants to merge 4 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3379 +/- ##
=======================================
- Coverage 83.2% 83.2% -0.1%
=======================================
Files 128 128
Lines 25045 25112 +67
=======================================
+ Hits 20858 20905 +47
- Misses 4187 4207 +20 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
eb202cb to
d7b14e1
Compare
d7b14e1 to
cb3e7ec
Compare
Change SpanExporter::export to take SpanBatch<'_> instead of Vec<SpanData>, matching the LogBatch pattern already used in the logs pipeline. The key win is in BatchSpanProcessor: instead of calling batch.split_off(0) on every export (which allocates a new Vec and moves all elements), the processor now wraps the existing buffer in a SpanBatch reference and clears it after export returns. This eliminates one Vec allocation per export cycle. Changes across 6 crates: - opentelemetry-sdk: new SpanBatch<'a> type, updated processors - opentelemetry-proto: added From<&SpanData> and From<&Link>, group_spans_by_resource_and_scope now takes &[SpanData] - opentelemetry-otlp: updated tonic and http trace exporters - opentelemetry-stdout: updated trace exporter - opentelemetry-zipkin: updated trace exporter - benchmarks: updated no-op exporters Breaking change: SpanExporter::export signature changed. Implementors need to update their export method to accept SpanBatch<'_> and use batch.iter() to access spans.
The test module imported SpanData but only used SpanBatch and SpanExporter. CI caught this via #[deny(warnings)].
…xporters The tonic trace exporter deep-cloned every SpanData into a Vec on each export call, even though SpanBatch already provides borrowed access to the underlying slice. Replace Arc<Vec<SpanData>> with Arc<SpanBatch> to wrap the borrowed data directly (~free), matching the existing pattern used by the tonic logs exporter with Arc<LogBatch>. The HTTP trace exporter had the same unnecessary clone in build_trace_export_body — removed by using SpanBatch::as_slice() directly. Adds SpanBatch::as_slice() for callers needing &[SpanData].
The batchspanprocessor_sync_ignores_max_concurrent_exports test was added on main after this branch diverged and used the old Vec<SpanData> signature. Updated to use SpanBatch<'_> to match the new trait.
cb3e7ec to
817cf54
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
SpanExporter::exportfromVec<SpanData>toSpanBatch<'_>— a thin borrowed wrapper around&[SpanData]. This matches theLogBatch<'_>pattern already used in the logs pipeline. Additionally eliminates unnecessarySpanDatadeep clones in the tonic and HTTP trace exporters.This is a breaking change to the
SpanExportertrait. Implementors need to update theirexportsignature and usebatch.iter()orbatch.as_slice()to access spans.Supersedes #3066. Related to #3368.
Motivation
The span export path currently requires ownership transfer of a
Vec<SpanData>, which forces unnecessary allocations and clones:batch.split_off(0)inBatchSpanProcessor— allocates a newVecevery export cycle just to hand ownership to the exporter, even though the exporter only needs to read the spansvec![span]inSimpleSpanProcessor— heap-allocates aVecfor a single spanspan_data.clone().into()ingroup_spans_by_resource_and_scope— deep-clones everySpanDataduring proto conversion because the function consumed theVecbatch.iter().cloned().collect()in tonic trace exporter — deep-clones everySpanDatainto anArc<Vec<_>>for the retry closure, even though retries rarely happenspans.iter().cloned().collect()in HTTP trace exporter — deep-clones spans into an intermediateVecbefore proto conversion, serving no purposeThe logs pipeline already solved this with
LogBatch<'_>. There's even a TODO in the codebase (span_processor.rs):Allocation impact (512-span batch via tonic)
split_off)Ecosystem alignment
[]ReadOnlySpan(borrowed slice)Collection<SpanData>(immutable)LogBatch<'_>(borrowed)Vec<SpanData>(owned)SpanBatch<'_>(borrowed)What changed
opentelemetry-sdk (core change)
SpanBatch<'a>type wrapping&'a [SpanData]withnew(),iter(), andas_slice()methodsSpanExporter::exportsignature:Vec<SpanData>→SpanBatch<'_>BatchSpanProcessor: replacedsplit_off(0)withSpanBatch::new(&batch)+batch.clear()SimpleSpanProcessor: replacedvec![span]with stack array +SpanBatch::new(&spans)InMemorySpanExporter, test exporters, and benchmarksopentelemetry-proto
From<&SpanData> for SpanandFrom<&Link> for span::Link(borrowing conversions)group_spans_by_resource_and_scopenow takes&[SpanData]instead ofVec<SpanData>opentelemetry-otlp
Arc<Vec<SpanData>>(512 deep clones) withArc<SpanBatch<'_>>(~free), matching the existingArc<LogBatch>pattern in the tonic logs exporterVec<SpanData>clone, usingspans.as_slice()directlyopentelemetry-stdout, opentelemetry-zipkin
SpanBatch<'_>Migration guide
Verification