fix(exporter/awsxray): use local service name for Consumer local root segments#47568
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75033f54fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| // support x-ray specific service name attributes as segment name if it exists | ||
| if span.Kind() == ptrace.SpanKindServer { | ||
| if span.Kind() == ptrace.SpanKindServer || (span.Kind() == ptrace.SpanKindConsumer && isLocalRoot(span)) { |
There was a problem hiding this comment.
For local-root Consumer spans, this new condition also runs while building the dependency subsegment via MakeDependencySubsegmentForLocalRootDependencySpan. When aws.remote.service is absent, the later name override (if myAwsRemoteService, ok := ...) is skipped, so the subsegment now keeps a local-service name (or resource.service.name from the later fallback) instead of a dependency-oriented name. This regression is specific to local-root Consumer spans missing aws.remote.service, and it can make dependency subsegments collapse onto the same name as the local service segment in X-Ray.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid catch. Moved the fix out of MakeSegment (which also runs for the dependency subsegment) and into MakeServiceSegmentForLocalRootDependencySpan directly, so only the service segment is affected, regardless of whether aws.remote.service is present
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
… segments Before this change, an X-Ray segment produced from a Consumer span that is a local root used the remote service name (aws.remote.service) or, if that attribute was absent, fell back to the span (operation) name. Server local root spans already used aws.local.service / resource service.name. Consumer local root spans should behave the same way now that PR open-telemetry#42633 promotes them to Segments. Changes in MakeSegment: 1. Extend the aws.local.service attribute check to also cover Consumer spans that are local roots (mirrors the existing Server logic). 2. Extend the resource.service.name fallback to also cover Consumer local root spans when neither aws.local.service nor aws.remote.service is set. Fixes open-telemetry#43432 Assisted-by: Claude Sonnet 4.6
… segments After PR open-telemetry#42633 promoted Consumer local root spans to X-Ray Segments, the service segment naming still fell through to the span (operation) name when aws.local.service was absent, while Server segments already fell back to resource.service.name. Fix is applied directly in MakeServiceSegmentForLocalRootDependencySpan rather than in MakeSegment, so the dependency subsegment produced alongside is unaffected regardless of whether aws.remote.service is present. Naming priority for the service segment (unchanged for all other paths): 1. aws.local.service attribute (existing behavior) 2. resource.service.name (new fallback, mirrors Server span behavior) 3. span name / other existing fallbacks Fixes open-telemetry#43432 Assisted-by: Claude Sonnet 4.6
ogulcanaydogan
force-pushed
the
fix/awsxray-consumer-local-root-segment-name
branch
from
4ccd610 to
53a1295
Compare
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
3 participants
Description
Fixes #43432.
After PR #42633, Consumer spans that are local roots are promoted to X-Ray Segments (rather than just subsegments). However, the segment naming logic in
MakeSegmentstill treated Consumer spans like dependency spans (usingaws.remote.service/ operation name), while Server segments already usedaws.local.service/resource.service.name.Root cause
Two gaps in
MakeSegment(both insegment.go):aws.local.servicecheck (line 361) — only guarded bySpanKindServer, so Consumer local root spans never used this attribute for naming.resource.service.namefallback (line 425) — also only forSpanKindServer, so Consumer local root spans fell through to the span (operation) name.Fix
Extend both checks to also cover
SpanKindConsumer && isLocalRoot(span):The dependency subsegment produced alongside the service segment is unaffected:
MakeDependencySubsegmentForLocalRootDependencySpanexplicitly overrides the name toaws.remote.serviceafter callingMakeSegment, so the subsegment name remains correct.Tests
TestLocalRootConsumerandTestLocalRootConsumerAWSNamespacecontinue to pass.TestLocalRootConsumerUsesResourceServiceName: Consumer local root span withoutaws.local.serviceattribute — service segment name must beresource.service.name("signup_aggregator"), not the operation name ("destination operation").Checklist
Fixes #43432linked