-
Notifications
You must be signed in to change notification settings - Fork 888
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
added cloud.infrastructure_service attribute to resource spec #1112
Conversation
Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com>
@@ -37,3 +37,55 @@ groups: | |||
note: > | |||
In AWS, this is called availability-zone. | |||
examples: ['us-central1-a'] | |||
- id: infrastructure.service |
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.
Sorry my PR suggestion was in the wrong place - should have renamed here and then regenerated the md
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.
Oh duh, will fix
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@tigrannajaryan @open-telemetry/specs-approvers do you mind taking a look at this? Thanks! |
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.
Not a fan of enumerating all proprietary names of services like that. What about names used in IBM Cloud, Oracle Cloud, Alibaba Cloud, etc.... ? Where does it end?
@yurishkuro Ends when there isn't interest in certain service names and resumes when it returns? :) It seems to companion |
I think this is the call we need to make. Do we record provider-specific data as values of provider-independent attribute names (e.g. I think this warrants a separate discussion since each approach has its cons and pros. @willarmiros @anuraaga would you mind creating an issue to discuss this first and then this PR will follow whatever decision we make. |
@tigrannajaryan @anuraaga @yurishkuro I've made #1151 to continue discussion. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@tigrannajaryan @yurishkuro @anuraaga any chance you and any other stakeholders could weigh in on #1151 to make a decision here? |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
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 think it's a sound approach. Stakeholders are free to add their services to the enum as they see fit. Instrumentations providing this attribute only need to know about subset of values they apply to and back ends only have limited platform support already (often with generic fallbacks for unknown values) and thus only need to handle the platforms they claim to support. Maintaining the enum centrally here can be a bit cumbersome but this shouldn't be a big deal IMHO and ensures different instrumentations in different frameworks and languages stay aligned.
@yurishkuro could you please take another look or otherwise comment on the alternative proposed in #1151? Thanks!
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
@@ -37,3 +37,55 @@ groups: | |||
note: > | |||
In AWS, this is called availability-zone. | |||
examples: ['us-central1-a'] | |||
- id: infrastructure_service |
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 agree. However, given that "infrastructure" could be interpreted in a broader sense, it should be fine, and the enumeration of well-known values will also make it clearer.
Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
@arminru any chance this could be merged now? :) |
@willarmiros Sorry for the long delay on your PR. |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Stale bot clearly doesn't respect the holiday break :) |
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
@arminru As the time has passed think we can go ahead and merge this? |
…elemetry#1112) Co-authored-by: Anuraag Agrawal <anuraaga@gmail.com> Co-authored-by: Armin Ruech <armin.ruech@dynatrace.com>
Resolves #1151. Resolves #907.
Changes
This PR adds a new attribute to
cloud.md
:cloud.infrastructure_service
. The new attribute is a single-value descriptor of the type of compute infrastructure, with several suggested values for the most popular compute platforms from the currently supported cloud providers. This takes out the guess work for users who may be interested in seeing what platform their telemetry is originating from. So, for example, instead of having to noticecloud.provider
==aws
andfaas.id
is defined in order to know this trace is coming from AWS Lambda, it will just be defined concretely here.I'd appreciate someone from GCP and Azure giving signoff on the names and descriptions for the services from their platforms :)
Split off from #1099 on the recommendation of @anuraaga.