Skip to content

Limit LocalRootSpan usage to http.route and span name#19209

Open
trask wants to merge 6 commits into
open-telemetry:mainfrom
trask:use-current-span-v3-preview
Open

Limit LocalRootSpan usage to http.route and span name#19209
trask wants to merge 6 commits into
open-telemetry:mainfrom
trask:use-current-span-v3-preview

Conversation

@trask

@trask trask commented Jul 13, 2026

Copy link
Copy Markdown
Member

Limit LocalRootSpan usage.

LocalRootSpan isn't blessed by the OpenTelemetry Specification, and there's no semantic conventions around it.

These usages don't seem to require it anyways.

@trask trask changed the title Use current span for v3 preview attributes Limit LocalRootSpan usage to http.route and span name Jul 14, 2026
}
Span serverSpan = LocalRootSpan.fromContextOrNull(context);
Span serverSpan =
v3Preview() ? Span.fromContext(context) : LocalRootSpan.fromContextOrNull(context);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 14, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-21 23:34:32 UTC.

  • Waiting on: Author
  • Next step: Address or respond to 1 review feedback item:
    • Top-level feedback: 1
    • For each item, link to the commit that addresses it, explain why no change is needed, or ask a follow-up question.

This automated status or its linked feedback items may be incorrect. If something looks wrong, report it with the result you expected.

@trask
trask force-pushed the use-current-span-v3-preview branch from 3b8f777 to 51425ba Compare July 14, 2026 00:49
@trask
trask marked this pull request as ready for review July 14, 2026 00:49
@trask
trask requested a review from a team as a code owner July 14, 2026 00:49
Copilot AI review requested due to automatic review settings July 14, 2026 00:49

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Limits non-route attribute enrichment of local-root spans in v3 preview mode.

Changes:

  • Uses the current span for gateway, servlet, and ShenYu attributes in v3 preview.
  • Preserves legacy local-root behavior otherwise.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
ServerRequestHelper.java Redirects gateway attributes.
BaseServletHelper.java Redirects servlet attributes.
MetaDataHelper.java Redirects ShenYu metadata.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Comment thread instrumentation/apache-shenyu-2.4/javaagent/build.gradle.kts

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

@laurit

laurit commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

I'm not sure about this. The intent here is to augment the server span with additional attributes as stated in https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/apache-shenyu-2.4/metadata.yaml While the span from context is probably also the server span, unless someone uses method instrumentation or something like that to create additional span, using LocalRootSpan feels more clear. An alternative would be to rethink this instrumentation. Perhaps instead of augmenting the server span it could create a controller span like other framework instrumentations?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants