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

Clarify stabilization plan does not just apply to attributes #278

Merged

Conversation

alanwest
Copy link
Member

Clarification inspired by the following discussion open-telemetry/opentelemetry-dotnet#4766 (comment).

This text shows up in a lot of places. I can update them all, but wanted to get an initial review of the text here before I copy it everywhere. I assume this will not need to be updated in the specification repo once open-telemetry/opentelemetry-specification#3667 is merged.

@trask

@alanwest alanwest requested review from a team August 22, 2023 17:47
Copy link
Member

@trask trask left a comment

Choose a reason for hiding this comment

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

❤️

joaopgrassi
joaopgrassi previously approved these changes Aug 23, 2023
@Oberon00
Copy link
Member

Oberon00 commented Aug 23, 2023

I think this is inconsistent with other wording we have elsewhere that says that semantic conventions only encompass the attribute keys and nothing else.

EDIT: Found it, it's this here: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/versioning-and-stability.md#semantic-conventions-stability

@bryannaegele
Copy link
Contributor

bryannaegele commented Aug 23, 2023

@Oberon00 I don't think that's accurate. That doc says Semantic Conventions defines the set of fields in the OTLP data model: and notes properties such as The span name under tracing. I don't see where it says it only encompasses attribute keys. Conventions for values are noted there.

@alanwest
Copy link
Member Author

alanwest commented Aug 23, 2023

I don't see where it says it only encompasses attribute keys.

Me either... it seems this section in particular is pretty exhaustive.

@Oberon00 is there specific wording you're referring to that could be improved?

@Oberon00
Copy link
Member

Oberon00 commented Aug 24, 2023

You are right it encompasses quite a bit. Even though it does not encompass attribute values or types, we can have stricter requirements for this special case anyway.

@alanwest
Copy link
Member Author

Given the good number of approvals, shall I update the rest of the files in this PR with this updated text?

@trask
Copy link
Member

trask commented Aug 24, 2023

Given the good number of approvals, shall I update the rest of the files in this PR with this updated text?

👍

@joaopgrassi joaopgrassi dismissed their stale review August 28, 2023 08:14

Will review again, since now it's touching all other files

@joaopgrassi joaopgrassi merged commit 4fcf4df into open-telemetry:main Aug 30, 2023
9 checks passed
@alanwest alanwest deleted the alanwest/clarify-stabilization-plan branch August 31, 2023 17:26
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.

10 participants