Repository navigation
Return the stored span from a bridged context - #20931
Open
Ayush0612005 wants to merge 1 commit into
Open
Ayush0612005 wants to merge 1 commit into
Ayush0612005 wants to merge 1 commit into
Conversation
Span.fromContext() on a context the application stored a span in went through ContextKeyBridge.get(), which built a new ApplicationSpan on every read. The new wrapper had an empty SpanContext cache, so each call converted the span context again (open-telemetry#20729). The span bridge now keeps the application span next to the agent span when it is stored, and get() returns that same instance as long as the agent span is unchanged. A span stored or replaced by agent code is still converted as before.
Pull request dashboard statusWaiting on reviewers · refreshed 2026-10-24 06:40 UTC Review the latest changes. Status above doesn't look right?
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The narrowly scoped change preserves conversion fallback and has focused regression coverage, with no blocking issues identified in static review.
No open issues 🎉
What changed in this PR
This PR extends the API bridge’s span-context caching work from #20442 by returning the original application span from a bridged context, avoiding repeated conversion on that path.
Changes:
- Adds opt-in retention of the application value alongside its agent counterpart.
- Enables retention only for spans, preserving conversion when agent code replaces or supplies the span.
- Adds regression tests for span identity, unrelated context updates, and conversion fallback.
| File | Description |
|---|---|
| instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/unitTests/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/v1_0/context/SpanContextKeyBridgeTest.java | Tests retained span identity and agent-side fallback behavior. |
| instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/v1_0/context/ContextKeyBridge.java | Stores application values optionally and reuses them while agent identity matches. |
| instrumentation/opentelemetry-api/opentelemetry-api-1.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/opentelemetryapi/v1_0/context/AgentContextWrapper.java | Enables application-value retention for the span bridge only. |
Review effort: Balanced
This branch has not been deployed
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.
Fixes #20729
Follow-up to #20442. That PR caches the converted
SpanContextonApplicationSpan, butSpan.fromContext(context)reads the span throughContextKeyBridge.get(), which builds a newApplicationSpanon every call. The new wrapper starts with an empty cache, soSpan.fromContext(context).getSpanContext()still converts each time.Change
ContextKeyBridgegets an opt-in mode, enabled only for the span bridge. When the application stores a span,with()also keeps the application span next to the agent span (under a private agent context key), andget()returns that same instance as long as the agent span in the context is unchanged.get()converts it as before.Span.fromContext(context.with(span))returnsspanitself.The trade-off is one extra entry in the agent context each time the application stores a span. Baggage and the other bridges are unchanged.
Testing
SpanContextKeyBridgeTestin theunitTestssuite. It checks that the stored span comes back as the same instance, also after other values are added to the context, and that a span replaced or stored by agent code is still converted.@dpol1, could you run the reproducer from #20729 against this when you have time?
context.with(span)would be worth checking too, since it now adds one more context entry.