Skip to content

Commit c9ce64b

Browse files
authored
[exporter/awsxrayexporter] fix messaging system logic (#42633)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Set `namespace="remote"` for producer spans so that X-ray will infer a downstream service node. Changed consumer spans to be segments so that they can correlate to their own node while being downstream from the producer. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #40995 <!--Describe what testing was performed and which tests were added.--> #### Testing Added unit tests for producer span and updated unit tests for consumer spans. <!--Please delete paragraphs that you did not use before submitting.-->
1 parent af3ad5f commit c9ce64b

File tree

3 files changed

+118
-8
lines changed

3 files changed

+118
-8
lines changed
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: awsxrayexporter
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: infer downstream service for producer spans
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [40995]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

exporter/awsxrayexporter/internal/translator/segment.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,7 @@ func MakeSegment(span ptrace.Span, resource pcommon.Resource, indexedAttrs []str
318318

319319
storeResource := true
320320
if span.Kind() != ptrace.SpanKindServer &&
321+
span.Kind() != ptrace.SpanKindConsumer &&
321322
!span.ParentSpanID().IsEmpty() {
322323
segmentType = "subsegment"
323324
// We only store the resource information for segments, the local root.
@@ -447,8 +448,7 @@ func MakeSegment(span ptrace.Span, resource pcommon.Resource, indexedAttrs []str
447448
if name == "" {
448449
name = fixSegmentName(span.Name())
449450
}
450-
451-
if namespace == "" && span.Kind() == ptrace.SpanKindClient {
451+
if namespace == "" && (span.Kind() == ptrace.SpanKindClient || span.Kind() == ptrace.SpanKindProducer) {
452452
namespace = "remote"
453453
}
454454

exporter/awsxrayexporter/internal/translator/segment_test.go

Lines changed: 89 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,6 +1165,7 @@ func TestProducerSpanWithAwsRemoteServiceName(t *testing.T) {
11651165
segment, _ := MakeSegment(span, resource, nil, false, nil, false)
11661166
assert.Equal(t, "ProducerService", *segment.Name)
11671167
assert.Equal(t, "subsegment", *segment.Type)
1168+
assert.Equal(t, "remote", *segment.Namespace)
11681169

11691170
jsonStr, err := MakeSegmentDocumentString(span, resource, nil, false, nil, false)
11701171

@@ -1175,6 +1176,22 @@ func TestProducerSpanWithAwsRemoteServiceName(t *testing.T) {
11751176
assert.NotContains(t, jsonStr, "user")
11761177
}
11771178

1179+
func TestProducerSpanNonAwsRemoteServiceName(t *testing.T) {
1180+
spanName := "my-topic send"
1181+
parentSpanID := newSegmentID()
1182+
attributes := make(map[string]any)
1183+
attributes[string(conventionsv112.PeerServiceKey)] = "ProducerService"
1184+
resource := constructDefaultResource()
1185+
span := constructProducerSpan(parentSpanID, spanName, ptrace.StatusCodeOk, "OK", attributes)
1186+
1187+
segment, _ := MakeSegment(span, resource, nil, false, nil, false)
1188+
1189+
assert.NotNil(t, segment)
1190+
assert.Equal(t, "ProducerService", *segment.Name)
1191+
assert.Equal(t, "subsegment", *segment.Type)
1192+
assert.Equal(t, "remote", *segment.Namespace)
1193+
}
1194+
11781195
func TestConsumerSpanWithAwsRemoteServiceName(t *testing.T) {
11791196
spanName := "ABC.payment"
11801197
parentSpanID := newSegmentID()
@@ -1288,6 +1305,72 @@ func validateLocalRootServiceSegment(t *testing.T, segment *awsxray.Segment, spa
12881305
assert.Nil(t, segment.Namespace)
12891306
}
12901307

1308+
func validateLocalRootSegmentTypeDependencySubsegment(t *testing.T, segment *awsxray.Segment, span ptrace.Span, parentID string) {
1309+
tempTraceID := span.TraceID()
1310+
expectedTraceID := "1-" + fmt.Sprintf("%x", tempTraceID[0:4]) + "-" + fmt.Sprintf("%x", tempTraceID[4:16])
1311+
1312+
assert.Equal(t, "subsegment", *segment.Type)
1313+
assert.Equal(t, "myRemoteService", *segment.Name)
1314+
assert.Equal(t, span.SpanID().String(), *segment.ID)
1315+
assert.Equal(t, parentID, *segment.ParentID)
1316+
assert.Equal(t, expectedTraceID, *segment.TraceID)
1317+
assert.NotNil(t, segment.HTTP)
1318+
assert.Equal(t, "POST", *segment.HTTP.Request.Method)
1319+
assert.Len(t, segment.Annotations, 2)
1320+
assert.Nil(t, segment.Annotations[awsRemoteService])
1321+
assert.Nil(t, segment.Annotations[remoteTarget])
1322+
assert.Equal(t, "myAnnotationValue", segment.Annotations["myAnnotationKey"])
1323+
1324+
assert.Len(t, segment.Metadata["default"], 30)
1325+
assert.Equal(t, "receive", segment.Metadata["default"][string(conventionsv112.MessagingOperationKey)])
1326+
assert.Equal(t, "LOCAL_ROOT", segment.Metadata["default"][awsSpanKind])
1327+
assert.Equal(t, "myRemoteOperation", segment.Metadata["default"][awsRemoteOperation])
1328+
assert.Equal(t, "myTarget", segment.Metadata["default"][remoteTarget])
1329+
assert.Equal(t, "k8sRemoteNamespace", segment.Metadata["default"][k8sRemoteNamespace])
1330+
assert.Equal(t, "myLocalService", segment.Metadata["default"][awsLocalService])
1331+
assert.Equal(t, "awsLocalOperation", segment.Metadata["default"][awsLocalOperation])
1332+
assert.Equal(t, "service.name=myTest", segment.Metadata["default"]["otel.resource.attributes"])
1333+
1334+
assert.Equal(t, "MySDK", *segment.AWS.XRay.SDK)
1335+
assert.Equal(t, "1.20.0", *segment.AWS.XRay.SDKVersion)
1336+
assert.True(t, *segment.AWS.XRay.AutoInstrumentation)
1337+
1338+
assert.Equal(t, "UpdateItem", *segment.AWS.Operation)
1339+
assert.Equal(t, "AWSAccountAttribute", *segment.AWS.AccountID)
1340+
assert.Equal(t, "AWSRegionAttribute", *segment.AWS.RemoteRegion)
1341+
assert.Equal(t, "AWSRequestIDAttribute", *segment.AWS.RequestID)
1342+
assert.Equal(t, "AWSQueueURLAttribute", *segment.AWS.QueueURL)
1343+
assert.Equal(t, "TableName", *segment.AWS.TableName)
1344+
1345+
assert.Equal(t, "remote", *segment.Namespace)
1346+
}
1347+
1348+
func validateLocalRootSegmentTypeServiceSegment(t *testing.T, segment *awsxray.Segment, span ptrace.Span) {
1349+
tempTraceID := span.TraceID()
1350+
expectedTraceID := "1-" + fmt.Sprintf("%x", tempTraceID[0:4]) + "-" + fmt.Sprintf("%x", tempTraceID[4:16])
1351+
1352+
assert.Nil(t, segment.Type)
1353+
assert.Equal(t, "myLocalService", *segment.Name)
1354+
assert.Equal(t, expectedTraceID, *segment.TraceID)
1355+
assert.Nil(t, segment.HTTP)
1356+
assert.Len(t, segment.Annotations, 1)
1357+
assert.Equal(t, "myAnnotationValue", segment.Annotations["myAnnotationKey"])
1358+
assert.Len(t, segment.Metadata["default"], 23)
1359+
assert.Equal(t, "service.name=myTest", segment.Metadata["default"]["otel.resource.attributes"])
1360+
assert.Equal(t, "MySDK", *segment.AWS.XRay.SDK)
1361+
assert.Equal(t, "1.20.0", *segment.AWS.XRay.SDKVersion)
1362+
assert.True(t, *segment.AWS.XRay.AutoInstrumentation)
1363+
assert.Nil(t, segment.AWS.Operation)
1364+
assert.Nil(t, segment.AWS.AccountID)
1365+
assert.Nil(t, segment.AWS.RemoteRegion)
1366+
assert.Nil(t, segment.AWS.RequestID)
1367+
assert.Nil(t, segment.AWS.QueueURL)
1368+
assert.Nil(t, segment.AWS.TableName)
1369+
assert.Nil(t, segment.Namespace)
1370+
1371+
assert.Nil(t, segment.Namespace)
1372+
}
1373+
12911374
func getBasicAttributes() map[string]any {
12921375
attributes := make(map[string]any)
12931376

@@ -1352,10 +1435,10 @@ func TestLocalRootConsumer(t *testing.T) {
13521435
assert.Len(t, segments, 2)
13531436
assert.NoError(t, err)
13541437

1355-
validateLocalRootDependencySubsegment(t, segments[0], span, *segments[1].ID)
1438+
validateLocalRootSegmentTypeDependencySubsegment(t, segments[0], span, *segments[1].ID)
13561439
assert.Nil(t, segments[0].Links)
13571440

1358-
validateLocalRootServiceSegment(t, segments[1], span)
1441+
validateLocalRootSegmentTypeServiceSegment(t, segments[1], span)
13591442
assert.Len(t, segments[1].Links, 1)
13601443

13611444
// Checks these values are the same for both
@@ -1387,7 +1470,7 @@ func TestNonLocalRootConsumerProcess(t *testing.T) {
13871470
expectedTraceID := "1-" + fmt.Sprintf("%x", tempTraceID[0:4]) + "-" + fmt.Sprintf("%x", tempTraceID[4:16])
13881471

13891472
// Validate segment 1 (dependency subsegment)
1390-
assert.Equal(t, "subsegment", *segments[0].Type)
1473+
assert.Nil(t, segments[0].Type)
13911474
assert.Equal(t, "destination operation", *segments[0].Name)
13921475
assert.NotEqual(t, parentSpanID.String(), *segments[0].ID)
13931476
assert.Equal(t, span.SpanID().String(), *segments[0].ID)
@@ -1397,7 +1480,7 @@ func TestNonLocalRootConsumerProcess(t *testing.T) {
13971480
assert.Equal(t, http.MethodPost, *segments[0].HTTP.Request.Method)
13981481
assert.Len(t, segments[0].Annotations, 1)
13991482
assert.Equal(t, "myAnnotationValue", segments[0].Annotations["myAnnotationKey"])
1400-
assert.Len(t, segments[0].Metadata["default"], 7)
1483+
assert.Len(t, segments[0].Metadata["default"], 29)
14011484
assert.Equal(t, "Consumer", segments[0].Metadata["default"][awsSpanKind])
14021485
assert.Equal(t, "myLocalService", segments[0].Metadata["default"][awsLocalService])
14031486
assert.Equal(t, "receive", segments[0].Metadata["default"][string(conventionsv112.MessagingOperationKey)])
@@ -1662,8 +1745,8 @@ func TestNotLocalRootConsumer(t *testing.T) {
16621745
assert.NoError(t, err)
16631746

16641747
// Validate segment
1665-
assert.Equal(t, "subsegment", *segments[0].Type)
1666-
assert.Equal(t, "remote", *segments[0].Namespace)
1748+
assert.Nil(t, segments[0].Type)
1749+
assert.Nil(t, segments[0].Namespace)
16671750
assert.Equal(t, "myRemoteService", *segments[0].Name)
16681751
}
16691752

0 commit comments

Comments
 (0)