From 2afa443f1ce67d0471cf8625452090e00a047cea Mon Sep 17 00:00:00 2001 From: Adam Shaw Date: Wed, 4 Oct 2023 17:00:25 -0700 Subject: [PATCH] Addressing PR --- .../internal/translator/segment.go | 36 +++++++----- .../internal/translator/segment_test.go | 56 +++++++++++++++++++ 2 files changed, 78 insertions(+), 14 deletions(-) diff --git a/exporter/awsxrayexporter/internal/translator/segment.go b/exporter/awsxrayexporter/internal/translator/segment.go index da8720f0dc46..0796be83ef44 100644 --- a/exporter/awsxrayexporter/internal/translator/segment.go +++ b/exporter/awsxrayexporter/internal/translator/segment.go @@ -65,6 +65,13 @@ const ( localRoot = "LOCAL_ROOT" ) +var removeAnnotationsFromServiceSegment = []string{ + "aws.remote.service", + "aws.remote.operation", + "remote.target", + "K8s.RemoteNamespace", +} + var ( writers = newWriterPool(2048) ) @@ -92,15 +99,17 @@ func MakeSegmentDocuments(span ptrace.Span, resource pcommon.Resource, indexedAt } func HasDependencySubsegment(span ptrace.Span) bool { - return (span.Kind() != ptrace.SpanKindServer) && (span.Kind() != ptrace.SpanKindInternal) + _, hasAwsRemoteService := span.Attributes().Get(awsRemoteService) + + return span.Kind() != ptrace.SpanKindServer && + span.Kind() != ptrace.SpanKindInternal && + hasAwsRemoteService } // IsLocalRoot We will move to using isRemote once the collector supports deserializing it. Until then, we will rely on aws.span.kind. func IsLocalRoot(span ptrace.Span) bool { if myAwsSpanKind, ok := span.Attributes().Get(awsSpanKind); ok { - if localRoot == myAwsSpanKind.Str() { - return true - } + return localRoot == myAwsSpanKind.Str() } return false @@ -146,14 +155,7 @@ func MakeServiceSegment(span ptrace.Span, resource pcommon.Resource, indexedAttr // Set the span id to the one internally generated serviceSpan.SetSpanID(serviceSegmentID) - removeAnnotations := []string{ - "aws.remote.service", - "aws.remote.operation", - "remote.target", - "K8s.RemoteNamespace", - } - - for _, v := range removeAnnotations { + for _, v := range removeAnnotationsFromServiceSegment { serviceSpan.Attributes().Remove(v) } @@ -211,7 +213,12 @@ func MakeServiceSegmentWithoutDependency(span ptrace.Span, resource pcommon.Reso } // Make internal spans a segment - segment.Type = nil + // And Ensure consumer process spans are not made a segment + _, hasAwsRemoteService := span.Attributes().Get(awsRemoteService) + + if hasAwsRemoteService { + segment.Type = nil + } return []*awsxray.Segment{segment}, err } @@ -261,7 +268,8 @@ func MakeSegmentsFromSpan(span ptrace.Span, resource pcommon.Resource, indexedAt return MakeServiceSegmentAndDependencySubsegment(span, resource, indexedAttrs, indexAllAttrs, logGroupNames, skipTimestampValidation) } -// MakeSegmentDocumentString converts an OpenTelemetry Span to an X-Ray Segment and then serialzies to JSON +// MakeSegmentDocumentString converts an OpenTelemetry Span to an X-Ray Segment and then serializes to JSON +// MakeSegmentDocumentString will be deprecated in the future func MakeSegmentDocumentString(span ptrace.Span, resource pcommon.Resource, indexedAttrs []string, indexAllAttrs bool, logGroupNames []string, skipTimestampValidation bool) (string, error) { segment, err := MakeSegment(span, resource, indexedAttrs, indexAllAttrs, logGroupNames, skipTimestampValidation) diff --git a/exporter/awsxrayexporter/internal/translator/segment_test.go b/exporter/awsxrayexporter/internal/translator/segment_test.go index 8cc746673897..0926fc98eaf1 100644 --- a/exporter/awsxrayexporter/internal/translator/segment_test.go +++ b/exporter/awsxrayexporter/internal/translator/segment_test.go @@ -1158,6 +1158,62 @@ func TestLocalRootConsumer(t *testing.T) { assert.Equal(t, segments[0].EndTime, segments[1].EndTime) } +func TestLocalRootConsumerProcess(t *testing.T) { + spanName := "destination receive" + resource := constructDefaultResource() + parentSpanID := newSegmentID() + + attributes := make(map[string]interface{}) + attributes[conventions.AttributeHTTPMethod] = "POST" + attributes[conventions.AttributeMessagingOperation] = "receive" + attributes["otel.resource.attributes"] = "service.name=myTest" + attributes["aws.span.kind"] = "LOCAL_ROOT" + attributes["aws.local.service"] = "myLocalService" + attributes["myAnnotationKey"] = "myAnnotationValue" + attributes[awsxray.AWSOperationAttribute] = "UpdateItem" + + resource.Attributes().PutStr(conventions.AttributeTelemetrySDKName, "MySDK") + resource.Attributes().PutStr(conventions.AttributeTelemetrySDKVersion, "1.20.0") + resource.Attributes().PutStr(conventions.AttributeTelemetryAutoVersion, "1.2.3") + + span := constructConsumerSpan(parentSpanID, spanName, 200, "OK", attributes) + + spanLink := span.Links().AppendEmpty() + spanLink.SetTraceID(newTraceID()) + spanLink.SetSpanID(newSegmentID()) + + segments, err := MakeSegmentsFromSpan(span, resource, []string{"aws.remote.service", "myAnnotationKey"}, false, nil, false) + + assert.NotNil(t, segments) + assert.Equal(t, 1, len(segments)) + assert.Nil(t, err) + + tempTraceID := span.TraceID() + expectedTraceID := "1-" + fmt.Sprintf("%x", tempTraceID[0:4]) + "-" + fmt.Sprintf("%x", tempTraceID[4:16]) + + // Validate segment 1 (dependency subsegment) + assert.Equal(t, "subsegment", *segments[0].Type) + assert.Equal(t, "destination receive", *segments[0].Name) + assert.NotEqual(t, parentSpanID.String(), *segments[0].ID) + assert.Equal(t, span.SpanID().String(), *segments[0].ID) + assert.Equal(t, 1, len(segments[0].Links)) + assert.Equal(t, expectedTraceID, *segments[0].TraceID) + assert.NotNil(t, segments[0].HTTP) + assert.Equal(t, "POST", *segments[0].HTTP.Request.Method) + assert.Equal(t, 1, len(segments[0].Annotations)) + assert.Equal(t, "myAnnotationValue", segments[0].Annotations["myAnnotationKey"]) + assert.Equal(t, 4, len(segments[0].Metadata["default"])) + assert.Equal(t, "LOCAL_ROOT", segments[0].Metadata["default"]["aws.span.kind"]) + assert.Equal(t, "myLocalService", segments[0].Metadata["default"]["aws.local.service"]) + assert.Equal(t, "receive", segments[0].Metadata["default"]["messaging.operation"]) + assert.Equal(t, "service.name=myTest", segments[0].Metadata["default"]["otel.resource.attributes"]) + assert.Equal(t, "MySDK", *segments[0].AWS.XRay.SDK) + assert.Equal(t, "1.20.0", *segments[0].AWS.XRay.SDKVersion) + assert.Equal(t, true, *segments[0].AWS.XRay.AutoInstrumentation) + assert.Equal(t, "UpdateItem", *segments[0].AWS.Operation) + assert.Nil(t, segments[0].Namespace) +} + func TestLocalRootConsumerAWSNamespace(t *testing.T) { spanName := "destination receive" resource := constructDefaultResource()