Skip to content

Conversation

@ezhang6811
Copy link
Contributor

@ezhang6811 ezhang6811 commented Nov 29, 2024

Description of changes:
Support instrumentation for the following attributes. See this Python PR for the source of feature parity.

  • For SNS, extract TopicArn from request body and add to span as aws.sns.topic.arn.
  • For Secrets Manager, extract either SecretId from request body or ARN from response body and add to span as aws.secretsmanager.secret.arn.
  • For Step Functions, extract activityArn or stateMachineArn from request body, and add to span as aws.stepfunctions.activity.arn or aws.stepfunctions.state_machine.arn respectively.
  • For Lambda, extract UUID from request body for event source mapping functions, and add to span as aws.lambda.resource_mapping.id.

In addition, the new resource attribute aws.remote.resource.cfn.primary.identifier is also added for each new resource.

E2E tests show that the new attributes and their associated CFN primary identifiers have been correctly populated for each service.

SNS:
image

Secrets Manager:
image

Step Functions:
image

image

Lambda:
image

Contract tests verify the expected span attribute values with example function calls that involve each resource. https://github.com/aws-observability/aws-otel-dotnet-instrumentation/actions/runs/12127063253/job/33813330563?pr=143

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@ezhang6811 ezhang6811 requested a review from a team as a code owner November 29, 2024 21:05
mxiamxia
mxiamxia previously approved these changes Dec 2, 2024
@vastin
Copy link
Contributor

vastin commented Dec 6, 2024

Both this PR and PR #142 add aws.remote.resource.cfn.primary.identifier. Is there any dependency between them? Should PR #142 be merged firstly?

@ezhang6811
Copy link
Contributor Author

Both this PR and PR #142 add aws.remote.resource.cfn.primary.identifier. Is there any dependency between them? Should PR #142 be merged firstly?

Yes, #142 should be merged first as it originally implements aws.remote.resource.cfn.primary.identifier, and this PR was rebased on #142 so that this CFN Identifier attribute could be applied for the new resources.

@ezhang6811 ezhang6811 merged commit 83f5e04 into aws-observability:main Dec 12, 2024
8 checks passed
@ezhang6811 ezhang6811 deleted the zhaez/aws-resources branch December 12, 2024 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants