Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Experimental Semantic Convention Attribute Registration #224
Experimental Semantic Convention Attribute Registration #224
Changes from all commits
2eec904
71fce3b
c988af0
aee2069
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Currently, besides the name and value definitions, semantic conventions define a description, notes, examples, and requirement levels for attributes.
Is it intended that any of those are part of the dictionary? Or is this some enrichment that takes place in the 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.
I think the attribute definition in the dictionary should cover the following:
name
(required)type
(required)description
(required)examples
(required)notes
(optional)stability
(depending on what we decide on the above discussion)requirement_level
,sampling relevance
andtag
must not be part of the definition but will be specified on the usage of the attributes. And usages should be abled to overwrite all of the above (except forname
andtype
)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.
Makes sense.
I think the OTEP should me more specific about this and give a definition of what the dictionary contains. It's important for understanding and working out how the attribute dictionary and semantic conventions interact, which I think is crucial.
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'll update the otep to add more specificity
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'm going to add this but I'm going to make it a "suggested" set of fields. If there is some concern during implementation that comes up I don't want the OTEP to disallow some flexibility to solve problems.
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.
How about we even forbid it technically (e.g. by adopting the yaml syntax for defining vs. using attributes in 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.
Isn't the opposite the goal here? The motivation states that with this proposal we want to make it easier to experiment. But, with forbidding breaking changes on attributes registered in the dictionary we wouldn't it have the opposite effect?
At the same time I see the need for users of the attributes (e.g. semconv authors, instrumentation authors, etc.) to know how much they can rely on the stability of attributes they would use.
How about having stability levels on attributes (similar to what we have with semantic conventions today)?
This is also how we handle it in ECS.
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 want instrumentation authors to easily answer the question "how do I capture this piece of data that isn't covered by semconv?" As JS maintainer it is one of the most common questions I get and we are constantly being asked to add attributes which are not in semconv. Those users need telemetry stability just as much as any other. I consider their entries to be experiments, but if they are failed experiments it doesn't mean the attribute should go away or be broken, but a new attribute created taking into account the learnings of experience.
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.
If you feel your issue is resolved can you please mark it resolved (or if you don't have permission just say "resolved" so I can).
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.
OK, I see, this is sort of the answer to my question above.