Skip to content

Commit

Permalink
Addressing PR
Browse files Browse the repository at this point in the history
  • Loading branch information
atshaw43 committed Oct 5, 2023
1 parent 4760b02 commit 2afa443
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 14 deletions.
36 changes: 22 additions & 14 deletions exporter/awsxrayexporter/internal/translator/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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)

Expand Down
56 changes: 56 additions & 0 deletions exporter/awsxrayexporter/internal/translator/segment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 2afa443

Please sign in to comment.