Fallback to rpc.method for segment operation when aws.operation missing #6231
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
82f3d54 to
de74588
Compare
de74588 to
7d0b686
Compare
anuraaga
reviewed
Nov 11, 2021
| @@ -62,13 +62,15 @@ func TestClientSpanWithRpcAwsSdkClientAttributes(t *testing.T) { | |||
| segment, _ := MakeSegment(span, resource, nil, false) | |||
| assert.Equal(t, "DynamoDB", *segment.Name) | |||
| assert.Equal(t, conventions.AttributeCloudProviderAWS, *segment.Namespace) | |||
| assert.Equal(t, "GetItem", *segment.AWS.Operation) | |||
There was a problem hiding this comment.
Can we also add the case to aws_test.go?
There was a problem hiding this comment.
There was a problem hiding this comment.
I think that's an excellent suggestion! I added a test case like this one just to test that rpc.method is read:
func TestAwsWithRpcAttributes(t *testing.T) {
resource := pdata.NewResource()
attributes := make(map[string]pdata.AttributeValue)
attributes[conventions.AttributeRPCMethod] = pdata.NewAttributeValueString("ListBuckets")
_, awsData := makeAws(attributes, resource)
assert.NotNil(t, awsData)
assert.Equal(t, "ListBuckets", *awsData.Operation)
}and added this following above the line you linked to test the case where they are both defined:
attributes[conventions.AttributeRPCMethod] = pdata.NewAttributeValueString("IncorrectAWSSDKOperation")
anuraaga
approved these changes
Nov 11, 2021
|
@tigrannajaryan @kbrockhoff @Aneurysm9 - can you please provide a second review so that this PR can be merged and be available in the next Collector release. We have a downstream dependency on this. Many thx! |
tigrannajaryan
approved these changes
Nov 12, 2021
|
Please populate the commit message in future PRs. Don't just write one-line commit message. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
When creating an AWS segment, the
exporter/awsxrayexporter/internal/translator/segment.gofile will create the "AWS" component of the segment (a map under the "aws" key) by reading through the attributes of an OTel span.Span generated for the AWS SDK will either have
aws.operationaccording to legacy, orrpc.methodto describes the "operation" made with the AWS SDK as explained in the specifications.Because some languages have moved to the new spec-defined
rpc.method, we update the translator to consider this attribute.NOTE: We do not need to map
rpc.servicetoaws.servicesince it is not used by the AWS X-Ray backend.Link to tracking Issue: Fixes #6109
Testing:
Tests mostly make sure the operation (e.g.
GetItem) is present in the final segment structure regardless if we are using the legacyaws.operationor the newrpc.method.In the case where both are defined, the legacy
aws.operationwill take precedence as remarked by the updated legacy unit test which defines both but which takesaws.operation.Documentation:
Not a breaking change, so N/A.