-
Notifications
You must be signed in to change notification settings - Fork 183
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
Clarify that service.* conventions apply to all telemetry sources #630
Clarify that service.* conventions apply to all telemetry sources #630
Conversation
cc @open-telemetry/technical-committee since this is an important detail that extends beyond just semantic-conventions |
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 with a changelog entry.
Does not have to be part of this PR, but adding the same wording / a link to that definition to the glossary may resolve this long standing issue (or make it obsolete if it is not added to the glossary: open-telemetry/opentelemetry-specification#2050) I am wondering if there should be an explicit note stating that this is "set in stone", and even if people disagree with this wording and definition, it will not be changed due to code relying on this definition: if someone not familar with the matter/new to the otel community comes across this definition, and disagrees with it, they can (and will?) still go ahead and raise an issue/PR/OTEP asking for a change. |
Would you want to add this to the glossary, since you have the issue? Otherwise I'm glad to work on it. Let me know :) |
Sure, I can do that, after this issue got merged. |
This was merged too fast for such a significant change. What does it mean to treat hardware devices as services? Are we saying that instead of |
That's a good point. I understood the change as applying to a software that uses Otel SDK to emit telemetry, but not necessarily to everything at all that can have telemetry associated with it. |
Agree, given the disagreement about this in the past.
I believe it should apply to all telemetry sources. Limiting to those which use the otel SDK to emit is a strange distinction because between async instruments and the metric producer concept, its possible to imagine telemetry sources like the hostmetricsreceiver emitting data from the SDK instead of directly producing metric data points. I see no advantage to having separate sets of identifying attributes for different types of telemetry sources. |
Cf. Monarch paper https://www.vldb.org/pvldb/vol13/p3181-adams.pdf We have a similar design at Meta - different "sources" may need different identity schemas, it's not reasonable to think that a single string field |
The
Putting the |
Broadly perhaps, but not universally. What is the meaning of
Who are we optimizing for, for us to make it easier to maintain the spec or for the end user to make it easier to understand the telemetry? And it's not like we don't have precedents - we have various vendor specific conventions (aws, gcp) which are distinct because they describe unique business entities, even though they may have some overlap in conceptually similar dimensions. |
When I see the I admit its not the most intuitive. In hindsight we might have used something like This is the real crux of the issue: we've rejected proposals to use alternatives to |
I disagree with this. There are clearly entities which are not Services (e.g. a Host, a Process, a Kubernetes Node or a Pod, etc). Including the If this was the intent behind this PR I think it needs to be reversed since I personally completely misunderstood it and would have not approved it. |
Yes this was the intent of the PR. I was under the impression I was clarifying the stance that has been implied by our actions (see PR description). Agree that should probably revert to continue discussion.
I can't reconcile that we reject attempts to add new sets of identifying attributes for specific types of entities like app.name, but need type specific identifying attributes for things like a host. I can't think of a good heuristic for when a type or telemetry source needs special identifying attributes which aligns with the precedent of rejecting
They have attributes to describe themselves and those same attributes are attached to other telemetry sources running on the host, process, node as correlating metadata. This means its challenging / impossible to identity the difference between something with a I think rejecting that |
Here's a possible solution:
Basically just replacing s/service/resource/ seems to address the concerns. And more resource-specific attributes like |
@jack-berg OK, I am open to considering it, but haven't though through the implications of every source having |
I agree with reverting for now, definitely some good points raised worth discussing more |
…rces (open-telemetry#630)" This reverts commit 7f35c49.
I created the revert PR #638. Note that we will need a new PR when we are ready to do this change again. |
…en-telemetry#630) Co-authored-by: Joao Grassi <[email protected]>
The question of whether the
service.*
conventions are applicable only to web services is an important one that has come up several times. If the answer is yes, then everything that isn't a web service needs its own version ofservice.name
,service.instance.id
,service.namespace
,service.version
to uniquely define the thing producing telemetry. We've discussed this at length several times and while we don't have anything written down yet in the spec / semantic-conventions, the actions we've taken (i.e. rejecting alternatives liketelemetry.source
,app.name
, etc) confirm that theservice.*
attributes are applicable to all telemetry services, not some narrower subset that some people consider a web service.This PR aims to clarify this to avoid repeating the same discussion.
Some PRs, issues that are related: