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

Add event name field, fix leading dot in FQN of attribute w/o prefix. #67

Merged

Conversation

Oberon00
Copy link
Member

@Oberon00 Oberon00 commented Sep 15, 2021

All event semantic conventions now have a name: str attribute. This defaults to the prefix, but if no prefix is given or a different name is desired, can be manually set with the name property in the YAML.

The markdown generator adds a single line before the table for event semantic conventions, see semantic-conventions/src/tests/data/markdown/event/expected.md

The event name MUST be myname.

Events with dynamic names are not supported, as there is currently no use case in the spec.

CC @bogdandrutu

This needs a spec follow-up, I created open-telemetry/opentelemetry-specification#1922

@Oberon00 Oberon00 added enhancement New feature or request semconv Related to the semantic convention generator. semconv/md Related specifically to the markdown output of the semantic convention generator semconv/model Related to the data model or YAML format of the semantic convention generator labels Sep 15, 2021
@Oberon00 Oberon00 requested a review from thisthat as a code owner September 15, 2021 15:26
@Oberon00 Oberon00 requested a review from a team September 15, 2021 15:26
@@ -117,7 +117,10 @@ def parse(
if attr_id is not None:
validate_id(attr_id, position_data["id"])
attr_type, brief, examples = SemanticAttribute.parse_id(attribute)
fqn = "{}.{}".format(prefix, attr_id)
if prefix:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not add this bugfix to the changelog because IMHO the prefix being optional is a misfeature, and we should require the prefix and/or default it to the semantic convention ID.

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The semantics LGTM, not at all a Python expert :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request semconv/md Related specifically to the markdown output of the semantic convention generator semconv/model Related to the data model or YAML format of the semantic convention generator semconv Related to the semantic convention generator.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants