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

Document that OpenTelemetry instrumentation should follow semantic conventions #3650

Open
jack-berg opened this issue Aug 9, 2023 · 4 comments
Labels
spec:miscellaneous For issues that don't match any other spec label triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted

Comments

@jack-berg
Copy link
Member

I don't think we explicitly say this anywhere but instrumentation that is hosted by / maintained in the OpenTelemetry org should follow the semantic conventions. Perhaps this is implicit / obvious, but I think it would be good to add normative language to the spec which makes this clear.

Some other thoughts on the subject:

  • Long term there will be an increasing amount of instrumentation embedded directly in libraries. We can't force library authors to follow the conventions, but we should set an example in the instrumentation we do write.
  • This would relieve instrumentation authors of having to duplicate the decision making process of whether a particular change the semantic convention is good. If its in the convention, embody it in the instrumentation. If you disagree with the convention, go engage with the semantic convention group.
  • The semantic conventions are in varying states of maturity, and there is a lot of instrumentation out there. Its not realistic (or necessarily desirable) to keep all instrumentation up to date with a semantic convention domain experiencing high churn. So there will be some grey area on when its appropriate to reflect some change to the semantic conventions in instrumentation. But the goal should ultimately be to align instrumentation and semantic conventions, especially as the conventions approach / become stable.
@jack-berg jack-berg added the spec:miscellaneous For issues that don't match any other spec label label Aug 9, 2023
@dyladan
Copy link
Member

dyladan commented Aug 16, 2023

In principle I agree with this, but I also think instrumentation authors are an important feedback mechanism for the semconv, even after the semconv is written. I think we should distinguish between experimental and stable semconv in this area. Experimental semconv is just that: experimental, and should be treated as such during the implementation phase. Taking the recent case of the JS lambda instrumentation, the experimental spec was updated in such a way that it broke a subset of existing users. During implementation of the JS instrumentation for this change the break was discovered, and now the semconv is being updated to address that break. If the PR had been merged blindly, we may never have discovered this issue. Even if it was discovered, it would have resulted in even more thrash (potentially breaking) when the spec was fixed after the change was merged.

@austinlparker austinlparker added the triage:deciding:needs-info Not enough information. Left open to provide the author with time to add more details label May 7, 2024
@trask
Copy link
Member

trask commented May 7, 2024

(in particular)

Stable instrumentations authored by OpenTelemetry SHOULD NOT produce telemetry that is not described by OpenTelemetry semantic conventions.

@jack-berg
Copy link
Member Author

If I recall correctly, the context for this issue was about lambda instrumentation. There was disagreement over a PR that was merged to semantic conventions and the lambda maintainers declined PRs to reflect the changes. So the language about stable instrumentation is good, but not good enough to resolve this situation.

But its tricky because there are valid reasons why a maintainer might decline to accept PRs for a new experimental convention. For example, they might wish to avoid churn if more changes are anticipated.

@trask trask added triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted and removed triage:deciding:needs-info Not enough information. Left open to provide the author with time to add more details labels Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
spec:miscellaneous For issues that don't match any other spec label triage:deciding:community-feedback Open to community discussion. If the community can provide sufficient reasoning, it may be accepted
Projects
None yet
Development

No branches or pull requests

5 participants