feat: Support a list of new AWS Resources for Python SDK #261
Conversation
| There exists SpanAttributes.MESSAGING_DESTINATION_NAME in the upstream logic that we could re-purpose here. However, | ||
| we are not using it here to maintain consistent naming patterns with other AWS resources. |
There was a problem hiding this comment.
Synced with @mxiamxia about this. Mentioned we should not use the upstream logic to maintain consistent naming patterns for other AWS resources
| @@ -407,6 +409,9 @@ def _set_remote_type_and_identifier(span: ReadableSpan, attributes: BoundedAttri | |||
| elif is_key_present(span, GEN_AI_REQUEST_MODEL): | |||
| remote_resource_type = _NORMALIZED_BEDROCK_SERVICE_NAME + "::Model" | |||
| remote_resource_identifier = _escape_delimiters(span.attributes.get(GEN_AI_REQUEST_MODEL)) | |||
| elif is_key_present(span, AWS_SNS_TOPIC_ARN): | |||
| remote_resource_type = _NORMALIZED_SNS_SERVICE_NAME + "::Topic" | |||
| remote_resource_identifier = _escape_delimiters(span.attributes.get(AWS_SNS_TOPIC_ARN)) | |||
There was a problem hiding this comment.
Please add TODO here that we'll need to parse the actual resource name from the long ARN string by following the rules defined in https://quip-amazon.com/ShMUAP1okoxs/Support-AWS-Resources-with-Different-CFN-Identifier-Formats#temp:C:IHL175110d2baee4144bbe0f54cc
There was a problem hiding this comment.
we can create a new attribute named aws.cloudformation.primary.identifier to store the orignal CFN prmiray identifier value from OTel instrumentation.
There was a problem hiding this comment.
can we use aws.remote.resource.cfn.primary.identifier and the new attribute name in EMF will be RemoteResourceCfnPrimaryIdentifier?
yiyuan-he
changed the title
|
I reset the main branch of my forked repo so that I can split my future work into multiple branches. It was getting confusing having everything on just my main branch. I am hoping this will enable me to break up my future PRs in a more organized and atomic manner for a better reviewer experience. |
### *Description of changes:* These changes support AWS Resources with different CFN Identifier formats. This new attribute will have the key `aws.remote.resource.cfn.primary.identifier`. More context can be found in the comments of this closed PR: #261 In most cases, this new attribute should have the same value as the existing attribute `aws.remote.resource.identifier`. However, there are some edge cases we must handle as seen in this PR: - `AWS::SQS::Queue` - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-sqs-queue.html - The CFN Id should be the URL of the Queue. - `AWS::Bedrock::DataSource` - https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-bedrock-datasource.html - The CFN Id should be a compound value where the knowledge base Id and the data source Id are separated by `|` Also updated Bedrock Agent request parameters to also include Knowledge Base ID: https://boto3.amazonaws.com/v1/documentation/api/latest/reference/services/bedrock-agent/client/get_data_source.html ### *Test Plan:* Set up a client-server with auto-instrumentation to verify that the correct span data is being generated.   Unit tests for instrumentation   By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Description of changes:
Adding support for SNS AWS resource for Python SDK with auto-instrumentation.
Specifically, this change populates
aws.sns.topic.arnin the Span by extracting theTopicArnfield fromSnsRequest."primaryIdentifier": [ "/properties/TopicArn" ]Testing Plan:
Set up a simple client-server locally and verified that the new data is added to the span.
Ran Code Style Checks:
Added unit tests for:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.