Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

contrib/aws/aws-sdk-go-v2: add aws_service, region, and resourcename #1888

Merged
merged 38 commits into from
May 12, 2023

Conversation

zARODz11z
Copy link
Contributor

@zARODz11z zARODz11z commented Apr 11, 2023

What does this PR do?

Adds

aws_service
region
<resourcename> such as queuename

Motivation

Serverless inferred related resources, pivot from traces to aws integration metrics ootb

Describe how to test/QA your changes

Reviewer's Checklist

  • Changed code has unit tests for its functionality.
  • If this interacts with the agent in a new way, a system test has been added.

Sorry, something went wrong.

@pr-commenter
Copy link

pr-commenter bot commented Apr 28, 2023

Benchmarks

Comparing candidate commit 1cdb93d in PR branch ar/ConsistentApmSpanTagsForAwsRequests with baseline commit d1c023c in branch main.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 18 metrics, 0 unstable metrics.

@zARODz11z zARODz11z marked this pull request as ready for review April 28, 2023 23:08
@zARODz11z zARODz11z requested a review from a team April 28, 2023 23:09
@zARODz11z zARODz11z requested a review from a team as a code owner April 28, 2023 23:09
Andrew Rodriguez added 2 commits May 1, 2023 10:26
Copy link
Contributor

@katiehockman katiehockman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR! I didn't review the tests yet, but here are some starting comments for the main aws.go file changes.

Could you please update the PR description to meet the criteria described here: https://github.com/DataDog/dd-trace-go/blob/main/CONTRIBUTING.md#contributing

We also try to avoid linking to internal documentation in the PR descriptions. That can be sent via Slack instead to provide any necessary context for our review. Can you remove those links from the PR description?

jonbodner
jonbodner previously approved these changes May 8, 2023
Copy link

@jonbodner jonbodner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@katiehockman katiehockman changed the title initial implementation of adding resourcename i.e. queuename, region,… contrib/aws/aws-sdk-go-v2: add aws_service, region, and resourcename May 9, 2023
katiehockman
katiehockman previously approved these changes May 9, 2023
Copy link
Contributor

@katiehockman katiehockman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few nits, but otherwise looks good. Thanks!

@zARODz11z zARODz11z dismissed stale reviews from katiehockman and jonbodner via 13d450e May 9, 2023 23:05
@katiehockman katiehockman force-pushed the ar/ConsistentApmSpanTagsForAwsRequests branch from 7513ed5 to 78aa78f Compare May 12, 2023 15:13
@katiehockman katiehockman merged commit 8b09491 into main May 12, 2023
@katiehockman katiehockman deleted the ar/ConsistentApmSpanTagsForAwsRequests branch May 12, 2023 18:21
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.

None yet

4 participants