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

Reconsider required property for process.* semantic conventions #997

Closed
arminru opened this issue Sep 24, 2020 · 3 comments · Fixed by #1137
Closed

Reconsider required property for process.* semantic conventions #997

arminru opened this issue Sep 24, 2020 · 3 comments · Fixed by #1137
Labels
area:semantic-conventions Related to semantic conventions priority:p3 Lowest priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:miscellaneous For issues that don't match any other spec label

Comments

@arminru
Copy link
Member

arminru commented Sep 24, 2020

Currently we define process.pid and "at least one of process.executable.name, process.executable.path, process.command, or process.command_line" as required, if any resource attribute in the process namespace is present. I don't think this is a sensible requirement, since there are multiple concerns (or multiple features from a backend's perspective) mixed together in the process namespace. I could well imagine that one might want to report the runtime (added in #882) but has no need for reporting the PID and/or the entire commmand line. This would currently not be "legal", however.
I think we should drop the entire required column for the process namespace.

This is related to #653. I do, however, think of the trace semantic conventions more as distinct features, where defining some set of attributes as essential to provide some reasonable output makes more sense than for the process resource attributes.

@arminru arminru added area:semantic-conventions Related to semantic conventions spec:miscellaneous For issues that don't match any other spec label labels Sep 24, 2020
@Oberon00
Copy link
Member

I think we should just put process.runtime into it's own resouce type (without changing the names)

@andrewhsu andrewhsu added the release:after-ga Not required before GA release, and not going to work on before GA label Sep 25, 2020
@Oberon00
Copy link
Member

Oberon00 commented Oct 23, 2020

@open-telemetry/technical-committee I think this issue should be allowed-for-ga, maybe even P2. EDIT: I made a PR #1137 that would fix this.

@Oberon00 Oberon00 removed the release:after-ga Not required before GA release, and not going to work on before GA label Oct 23, 2020
@andrewhsu andrewhsu added release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs priority:p3 Lowest priority level labels Oct 27, 2020
@andrewhsu
Copy link
Member

from the spec sig mtg today, re-triaged for allowed-for-ga

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:semantic-conventions Related to semantic conventions priority:p3 Lowest priority level release:allowed-for-ga Editorial changes that can still be added before GA since they don't require action by SIGs spec:miscellaneous For issues that don't match any other spec label
Projects
None yet
3 participants