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

chore(aws-sdk): migrate SEMATTRS_* to ATTR_* #2550

Closed

Conversation

trivikr
Copy link
Contributor

@trivikr trivikr commented Nov 26, 2024

Which problem is this PR solving?

Short description of the changes

  • Migrates SEMATTRS_* to ATTR_* wherever a replacement is available. The ones without replacement are deprecated, and can be removed in future PRs.

Copy link

codecov bot commented Nov 26, 2024

Codecov Report

Attention: Patch coverage is 97.14286% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.81%. Comparing base (2784357) to head (6333ee3).
Report is 8 commits behind head on main.

Files with missing lines Patch % Lines
...try-instrumentation-aws-sdk/src/services/lambda.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2550      +/-   ##
==========================================
+ Coverage   90.75%   90.81%   +0.05%     
==========================================
  Files         169      169              
  Lines        8018     8021       +3     
  Branches     1632     1632              
==========================================
+ Hits         7277     7284       +7     
+ Misses        741      737       -4     
Files with missing lines Coverage Δ
...entelemetry-instrumentation-aws-sdk/src/aws-sdk.ts 95.06% <100.00%> (ø)
...y-instrumentation-aws-sdk/src/services/dynamodb.ts 100.00% <100.00%> (ø)
...emetry-instrumentation-aws-sdk/src/services/sns.ts 94.28% <100.00%> (+0.16%) ⬆️
...emetry-instrumentation-aws-sdk/src/services/sqs.ts 100.00% <100.00%> (ø)
...opentelemetry-instrumentation-aws-sdk/src/utils.ts 97.29% <ø> (ø)
...try-instrumentation-aws-sdk/src/services/lambda.ts 97.87% <66.66%> (+0.09%) ⬆️

... and 1 file with indirect coverage changes

@trivikr trivikr changed the title chore(aws-sdk): migrate 'SEMATTRS_*' to 'ATTR_*' chore(aws-sdk): migrate SEMATTRS_* to ATTR_* Nov 26, 2024
Copy link
Contributor

@maryliag maryliag left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution!
The majority of replacements were good, but there are a few that you didn't just update the semantic convention, but you change the actual key being used, this needs to be clarified.

@trivikr trivikr marked this pull request as draft November 26, 2024 16:03
@david-luna david-luna self-requested a review November 26, 2024 16:17
@trivikr trivikr force-pushed the aws-sdk-semattrs-migration branch from faae679 to 3626c52 Compare November 26, 2024 21:28
@trivikr trivikr marked this pull request as ready for review November 26, 2024 21:57
@trivikr trivikr force-pushed the aws-sdk-semattrs-migration branch from ad845bd to 6333ee3 Compare November 26, 2024 21:57
@trivikr
Copy link
Contributor Author

trivikr commented Dec 2, 2024

Closing this as of now. This can be resposted after existing open PRs are merged/closed.

@trivikr trivikr closed this Dec 2, 2024
@trivikr trivikr deleted the aws-sdk-semattrs-migration branch December 2, 2024 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants