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

update the semconv to v1.7.0 and the build tools to 0.7.0 #3686

Merged
merged 1 commit into from
Oct 8, 2021

Conversation

jkwatson
Copy link
Contributor

No description provided.

* implementations.
* </ul>
*/
public static final AttributeKey<Long> MESSAGE_ID = longKey("message.id");
Copy link
Member

@Oberon00 Oberon00 Sep 30, 2021

Choose a reason for hiding this comment

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

Interesting that this was not present previously. The attribute was there since a long time in the YAML.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like it was added here: open-telemetry/opentelemetry-specification#1843. By you. :)

Copy link
Member

Choose a reason for hiding this comment

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

Oh, this is for RPC! Didn't realize... Yeah we should definitely change that name 😄

open-telemetry/opentelemetry-specification#1914 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, the id of the group is rpc.message, not just message. I thought id instead of prefix would be used for the name. But I guess that would lead to ugly identifiers in some cases where we split client/server into different groups. On the other hand, these identifiers may even be helpful.

Please consider that (well) before declaring the semconv artifact stable.

Copy link
Member

@Oberon00 Oberon00 Oct 1, 2021

Choose a reason for hiding this comment

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

For now, it may make sense to at least add the group ID to the generated javadoc, otherwise this documentation here is misleading, since the RPC context is completely missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the group information even available to the templates at the moment? I poked around and tried various things but couldn't sort it out. It's very likely I missed it somewhere, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

bump @Oberon00

@anuraaga
Copy link
Contributor

anuraaga commented Oct 8, 2021

As far as I can tell the names we generate here match what's in the .md in the spec, so looks ready to merge

@anuraaga anuraaga merged commit c7c352b into open-telemetry:main Oct 8, 2021
This was referenced Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants