-
Notifications
You must be signed in to change notification settings - Fork 579
fix(opentelemetry-instrumentation-aws-sdk)!: rename aws.region to cloud.region
#2842
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
fix(opentelemetry-instrumentation-aws-sdk)!: rename aws.region to cloud.region
#2842
Conversation
6ccf4ed
to
dc9663c
Compare
cloud.region
cloud.region
cloud.region
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I updated the PR title (following conventional commits) to indicate that this is a breaking change. That's necessary to ensure the next version is 0.54.0 rather than 0.53.1.
Code owners: any strong opinions here?
@@ -15,7 +15,7 @@ | |||
*/ | |||
export enum AttributeNames { | |||
AWS_OPERATION = 'aws.operation', | |||
AWS_REGION = 'aws.region', | |||
CLOUD_REGION = 'cloud.region', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cloud.region
does have a constant in @opentelemetry/semantic-conventions
. However, we can defer using that constant (and the others in this instrumentation) to #2377
The current constant using the old, deprecated naming (SEMRESATTRS_CLOUD_REGION
) is/was intended for use as a Resource attribute. That was before open-telemetry/semantic-conventions#2238 expanded the use of cloud.region
beyond resource attributes. That's why I'm not suggesting this PR use SEMRESATTRS_CLOUD_REGION
.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2842 +/- ##
==========================================
+ Coverage 89.65% 89.66% +0.01%
==========================================
Files 185 185
Lines 9034 9034
Branches 1852 1852
==========================================
+ Hits 8099 8100 +1
+ Misses 935 934 -1
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, no strong opinions from me.
Fixes #2841
Which problem is this PR solving?
cloud.region
instead of vendor specificaws.region
.Short description of the changes
aws.region
withcloud.region
.Notes
aws.region
attribute.