Skip to content

Commit

Permalink
[exporter/awsxray] Add aws sdk http error events to x-ray subsegment …
Browse files Browse the repository at this point in the history
…and strip prefix `AWS.SDK.` from aws remote service name (#27232)

**Description:** <Describe what has changed.>
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->

- Convert individual HTTP error events into exceptions within
subsegments for AWS SDK spans.
- Normalize the service name from `awsxray.AWSServiceAttribute`
attribute by removing the `AWS.SDK.` prefix (in some aws sdk
instrumentation, we have added the prefix to produce metrics with the
prefix to clearly indicate the resource). This change ensures that X-Ray
backend recognizes standard service names like "DynamoDb", "S3", etc.,
enabling correct identification of AWS service types.


**Link to tracking Issue:** 
NA

**Testing:** 
Unit tests are added.

**Documentation:**
NA

---------

Co-authored-by: John Knollmeyer <jknollm@amazon.com>
Co-authored-by: John Knollmeyer <jaknollmeyer@gmail.com>
  • Loading branch information
3 people authored Nov 3, 2023
1 parent 4abb732 commit 9dd2258
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 6 deletions.
27 changes: 27 additions & 0 deletions .chloggen/add-aws-http-error-event.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: awsxrayexporter

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Convert individual HTTP error events into exceptions within subsegments for AWS SDK spans and strip AWS.SDK prefix from remote aws service name

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [27232]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
35 changes: 33 additions & 2 deletions exporter/awsxrayexporter/internal/translator/cause.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ import (
// ExceptionEventName the name of the exception event.
// TODO: Remove this when collector defines this semantic convention.
const ExceptionEventName = "exception"
const AwsIndividualHTTPEventName = "HTTP request failure"
const AwsIndividualHTTPErrorEventType = "aws.http.error.event"
const AwsIndividualHTTPErrorCodeAttr = "http.response.status_code"
const AwsIndividualHTTPErrorMsgAttr = "aws.http.error_message"

func makeCause(span ptrace.Span, attributes map[string]pcommon.Value, resource pcommon.Resource) (isError, isFault, isThrottle bool,
filtered map[string]pcommon.Value, cause *awsxray.CauseData) {
Expand All @@ -34,14 +38,21 @@ func makeCause(span ptrace.Span, attributes map[string]pcommon.Value, resource p
errorKind string
)

hasExceptions := false
isAwsSdkSpan := isAwsSdkSpan(span)
hasExceptionEvents := false
hasAwsIndividualHTTPError := false
for i := 0; i < span.Events().Len(); i++ {
event := span.Events().At(i)
if event.Name() == ExceptionEventName {
hasExceptions = true
hasExceptionEvents = true
break
}
if isAwsSdkSpan && event.Name() == AwsIndividualHTTPEventName {
hasAwsIndividualHTTPError = true
break
}
}
hasExceptions := hasExceptionEvents || hasAwsIndividualHTTPError

switch {
case hasExceptions:
Expand Down Expand Up @@ -76,6 +87,26 @@ func makeCause(span ptrace.Span, attributes map[string]pcommon.Value, resource p

parsed := parseException(exceptionType, message, stacktrace, isRemote, language)
exceptions = append(exceptions, parsed...)
} else if isAwsSdkSpan && event.Name() == AwsIndividualHTTPEventName {
errorCode, ok1 := event.Attributes().Get(AwsIndividualHTTPErrorCodeAttr)
errorMessage, ok2 := event.Attributes().Get(AwsIndividualHTTPErrorMsgAttr)
if ok1 && ok2 {
eventEpochTime := event.Timestamp().AsTime().UnixMicro()
strs := []string{
errorCode.AsString(),
strconv.FormatFloat(float64(eventEpochTime)/1_000_000, 'f', 6, 64),
errorMessage.Str(),
}
message = strings.Join(strs, "@")
segmentID := newSegmentID()
exception := awsxray.Exception{
ID: aws.String(hex.EncodeToString(segmentID[:])),
Type: aws.String(AwsIndividualHTTPErrorEventType),
Remote: aws.Bool(true),
Message: aws.String(message),
}
exceptions = append(exceptions, exception)
}
}
}
cause = &awsxray.CauseData{
Expand Down
33 changes: 33 additions & 0 deletions exporter/awsxrayexporter/internal/translator/cause_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,39 @@ Caused by: java.lang.IllegalArgumentException: bad argument`)
assert.Empty(t, cause.Exceptions[2].Message)
}

func TestMakeCauseAwsSdkSpan(t *testing.T) {
errorMsg := "this is a test"
attributeMap := make(map[string]interface{})
attributeMap[conventions.AttributeRPCSystem] = "aws-api"
span := constructExceptionServerSpan(attributeMap, ptrace.StatusCodeError)
span.Status().SetMessage(errorMsg)

event1 := span.Events().AppendEmpty()
event1.SetName(AwsIndividualHTTPEventName)
event1.Attributes().PutStr(AwsIndividualHTTPErrorCodeAttr, "503")
event1.Attributes().PutStr(AwsIndividualHTTPErrorMsgAttr, "service is temporarily unavailable")
timestamp := pcommon.NewTimestampFromTime(time.UnixMicro(1696954761000001))
event1.SetTimestamp(timestamp)

res := pcommon.NewResource()
isError, isFault, isThrottle, _, cause := makeCause(span, nil, res)

assert.False(t, isError)
assert.True(t, isFault)
assert.False(t, isThrottle)
assert.NotNil(t, cause)

assert.Equal(t, 1, len(cause.CauseObject.Exceptions))
exception := cause.CauseObject.Exceptions[0]
assert.Equal(t, AwsIndividualHTTPErrorEventType, *exception.Type)
assert.True(t, *exception.Remote)

messageParts := strings.SplitN(*exception.Message, "@", 3)
assert.Equal(t, "503", messageParts[0])
assert.Equal(t, "1696954761.000001", messageParts[1])
assert.Equal(t, "service is temporarily unavailable", messageParts[2])
}

func TestCauseExceptionWithoutError(t *testing.T) {
var nonErrorStatusCodes = []ptrace.StatusCode{ptrace.StatusCodeUnset, ptrace.StatusCodeOk}

Expand Down
20 changes: 16 additions & 4 deletions exporter/awsxrayexporter/internal/translator/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ const (
defaultSegmentName = "span"
// maxSegmentNameLength the maximum length of a Segment name
maxSegmentNameLength = 200
// rpc.system value for AWS service remotes
awsAPIRPCSystem = "aws-api"
)

const (
Expand All @@ -79,6 +81,14 @@ func MakeSegmentDocumentString(span ptrace.Span, resource pcommon.Resource, inde
return jsonStr, nil
}

func isAwsSdkSpan(span ptrace.Span) bool {
attributes := span.Attributes()
if rpcSystem, ok := attributes.Get(conventions.AttributeRPCSystem); ok {
return rpcSystem.Str() == awsAPIRPCSystem
}
return false
}

// MakeSegment converts an OpenTelemetry Span to an X-Ray Segment
func MakeSegment(span ptrace.Span, resource pcommon.Resource, indexedAttrs []string, indexAllAttrs bool, logGroupNames []string, skipTimestampValidation bool) (*awsxray.Segment, error) {
var segmentType string
Expand Down Expand Up @@ -130,6 +140,10 @@ func MakeSegment(span ptrace.Span, resource pcommon.Resource, indexedAttrs []str
if span.Kind() == ptrace.SpanKindClient || span.Kind() == ptrace.SpanKindProducer {
if remoteServiceName, ok := attributes.Get(awsRemoteService); ok {
name = remoteServiceName.Str()
// only strip the prefix for AWS spans
if isAwsSdkSpan(span) && strings.HasPrefix(name, "AWS.SDK.") {
name = strings.TrimPrefix(name, "AWS.SDK.")
}
}
}

Expand All @@ -142,10 +156,8 @@ func MakeSegment(span ptrace.Span, resource pcommon.Resource, indexedAttrs []str
}

if namespace == "" {
if rpcSystem, ok := attributes.Get(conventions.AttributeRPCSystem); ok {
if rpcSystem.Str() == "aws-api" {
namespace = conventions.AttributeCloudProviderAWS
}
if isAwsSdkSpan(span) {
namespace = conventions.AttributeCloudProviderAWS
}
}

Expand Down
28 changes: 28 additions & 0 deletions exporter/awsxrayexporter/internal/translator/segment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,34 @@ func TestClientSpanWithAwsRemoteServiceName(t *testing.T) {
assert.False(t, strings.Contains(jsonStr, "user"))
}

func TestAwsSdkSpanWithAwsRemoteServiceName(t *testing.T) {
spanName := "DynamoDB.PutItem"
parentSpanID := newSegmentID()
user := "testingT"
attributes := make(map[string]interface{})
attributes[conventions.AttributeRPCSystem] = "aws-api"
attributes[conventions.AttributeHTTPMethod] = "POST"
attributes[conventions.AttributeHTTPScheme] = "https"
attributes[conventions.AttributeRPCService] = "DynamoDb"
attributes[awsRemoteService] = "AWS.SDK.DynamoDb"

resource := constructDefaultResource()
span := constructClientSpan(parentSpanID, spanName, 0, "OK", attributes)

segment, _ := MakeSegment(span, resource, nil, false, nil, false)
assert.Equal(t, "DynamoDb", *segment.Name)
assert.Equal(t, "subsegment", *segment.Type)

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

assert.NotNil(t, jsonStr)
assert.Nil(t, err)
assert.True(t, strings.Contains(jsonStr, "DynamoDb"))
assert.False(t, strings.Contains(jsonStr, "DynamoDb.PutItem"))
assert.False(t, strings.Contains(jsonStr, user))
assert.False(t, strings.Contains(jsonStr, "user"))
}

func TestProducerSpanWithAwsRemoteServiceName(t *testing.T) {
spanName := "ABC.payment"
parentSpanID := newSegmentID()
Expand Down

0 comments on commit 9dd2258

Please sign in to comment.