perf(sdk): store InstrumentationScope in Arc for cheaper tracer clones#3378
perf(sdk): store InstrumentationScope in Arc for cheaper tracer clones#3378bryantbiggs wants to merge 6 commits intoopen-telemetry:mainfrom
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3378 +/- ##
=====================================
Coverage 83.1% 83.2%
=====================================
Files 128 128
Lines 24877 24905 +28
=====================================
+ Hits 20688 20724 +36
+ Misses 4189 4181 -8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ssor SdkTracer now wraps InstrumentationScope in Arc internally, which makes tracer cloning significantly cheaper (pointer bump instead of deep clone with attribute Vec allocation). The multi-processor span export path is also optimized: instead of calling build_export_data() per processor (which clones the scope from the tracer each time), SpanData is now built once and cloned for each additional processor. The last processor receives the original without a clone. This is a non-breaking change — SpanData's public instrumentation_scope field remains InstrumentationScope (not Arc). Supersedes open-telemetry#3267, thanks to @taisho6339 for the original approach.
Verifies that when multiple span processors are configured, all processors receive the span data with correct name and attributes.
Use split_last() instead of index tracking for the multi-processor on_end loop. Removes unnecessary break statement and makes the clone-for-rest, move-for-last intent explicit. Fix changelog entry to describe what actually changed without overstating the scope clone savings.
bryantbiggs
force-pushed
the
perf/avoid-scope-clone-per-processor
branch
from
f584bc2 to
8eea322
Compare
…rtions Verify that instrumentation_scope is correctly propagated to all processors in both the clone (rest) and move (last) paths. Also clarify changelog wording.
|
@bantonsson this seems like something you'd have good insight on! |
bantonsson
left a comment
bantonsson
left a comment
There was a problem hiding this comment.
I think the Arc<...> change is good to merge, and the other optimization as well depending on how far out #2962 is
opentelemetry-sdk/src/trace/span.rs
Outdated
| for processor in rest { | ||
| processor.on_end(export_data.clone()); | ||
| } | ||
| last.on_end(export_data); |
There was a problem hiding this comment.
This optimization will be redundant after #2962 is revived and merged, but I don't know the state of that /cc @paullegranddc
It would be great if you could take a look at #2962 and come with input @bryantbiggs
There was a problem hiding this comment.
Thanks for the pointer @bantonsson. I took a look at #2962 — it was approved by @scottgerring and @TommyCpp back in May/June 2025 but stalled waiting for the next breaking-change release window and has since gone stale (merge conflicts, ~10 months behind main).
I've rebased it onto current main and opened #3410 to supersede it, preserving @paullegranddc's authorship on the core commit. The remaining review feedback (CHANGELOG entry) has been addressed.
For this PR (#3378), I've removed the multi-processor on_end build-once optimization since it's redundant with the FinishedSpan approach in #3410. What remains here is just the Arc<InstrumentationScope> change in SdkTracer, which is independent — it avoids deep cloning on span creation, not on_end.
bryantbiggs
changed the title
3 participants
Summary
SdkTracernow stores itsInstrumentationScopeinArc. Since tracers are cloned on every span creation (build_recording_spancallsself.clone()), this turns the scope copy from a deep clone (allocates a newVecfor attributes) into an Arc refcount increment.This is a non-breaking change.
SpanData.instrumentation_scoperemainsInstrumentationScope, notArc<InstrumentationScope>. Theinstrumentation_scope()accessor still returns&InstrumentationScopeviaDeref.Supersedes #3267 — thanks to @taisho6339 for the original approach and discussion.
Relates to #3368.
Note: The multi-processor
on_endbuild-once optimization that was previously in this PR has been removed — it is superseded by theFinishedSpanapproach in #3410.Changes
SdkTracer.scope:InstrumentationScope→Arc<InstrumentationScope>(internal field, not public API)instrumentation_scopepropagation assertions