-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Switch from detecting faas.id to faas.instance in the gcp detector. #26486
Switch from detecting faas.id to faas.instance in the gcp detector. #26486
Conversation
8445ffe
to
615783f
Compare
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.
I believe since this is an end-user impacting breaking change it should be done via feature-gates https://github.com/open-telemetry/opentelemetry-collector/blob/main/CONTRIBUTING.md#end-user-impacting-changes
940b453
to
1b4abc1
Compare
@codeboten I added a feature gate, and changed the PR from |
1b4abc1
to
3c8dc69
Compare
419fff0
to
12417ee
Compare
@codeboten @dmitryax @Aneurysm9 PTAL if you have time. |
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.
Thanks @dashpole, just one suggestion
Please resolve the conflict |
fd7704a
to
84b5d76
Compare
@codeboten conflict resolved. |
Co-authored-by: Alex Boten <aboten@lightstep.com>
@codeboten sorry. Forgot to pull in origin when I fixed go.mod |
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.
np! thanks for following up
…pen-telemetry#26486) Context: GoogleCloudPlatform/opentelemetry-operations-go#679 faas.id was removed from the semantic conventions: open-telemetry/opentelemetry-specification#3188 We were previously using it improperly to store the instance id of the faas, which should be mapped to faas.instance instead. --------- Co-authored-by: Alex Boten <aboten@lightstep.com>
Description:
Context: GoogleCloudPlatform/opentelemetry-operations-go#679
faas.id was removed from the semantic conventions: open-telemetry/opentelemetry-specification#3188
We were previously using it improperly to store the instance id of the faas, which should be mapped to faas.instance instead.
Testing:
Unit tests