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 metric attributes should be namespaced #786

Merged

Conversation

joaopgrassi
Copy link
Member

@joaopgrassi joaopgrassi commented Feb 29, 2024

Fixes #394

Changes

Adds a section on the general metric guidelines about attributes. Specifically, calling out the TC decision about metric attributes being required to be namespaced.

Merge requirement checklist

@joaopgrassi joaopgrassi requested review from a team February 29, 2024 12:19
@joaopgrassi joaopgrassi changed the title [core] Clarify metric attribute must be namespaced [chore] Clarify metric attribute must be namespaced Feb 29, 2024
@joaopgrassi joaopgrassi added the Skip Changelog Label to skip the changelog check label Feb 29, 2024
Copy link
Member

@arminru arminru left a comment

Choose a reason for hiding this comment

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

LGTM!

I think this would still be worth an entry in the changelog to make others aware.

docs/general/metrics.md Outdated Show resolved Hide resolved
docs/general/metrics.md Outdated Show resolved Hide resolved
docs/general/metrics.md Outdated Show resolved Hide resolved
docs/general/metrics.md Outdated Show resolved Hide resolved
@joaopgrassi joaopgrassi changed the title [chore] Clarify metric attribute must be namespaced Clarify metric attribute must be namespaced Mar 1, 2024
@joaopgrassi joaopgrassi removed the Skip Changelog Label to skip the changelog check label Mar 1, 2024
@joaopgrassi joaopgrassi changed the title Clarify metric attribute must be namespaced Clarify metric attributes must be namespaced Mar 1, 2024
@joaopgrassi joaopgrassi changed the title Clarify metric attributes must be namespaced Clarify metric attributes should be namespaced Mar 1, 2024
docs/general/metrics.md Outdated Show resolved Hide resolved
@joaopgrassi
Copy link
Member Author

@arminru @ChrsMark @trask I reset your approvals since I changed the wording according to our discussion in our last sig meeting. Can you please take another look? Thank you!

Copy link
Contributor

@lmolkova lmolkova left a comment

Choose a reason for hiding this comment

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

Left a comment on must vs should, but looks great otherwise

docs/general/metrics.md Outdated Show resolved Hide resolved
docs/general/metrics.md Outdated Show resolved Hide resolved
docs/general/metrics.md Outdated Show resolved Hide resolved
joaopgrassi and others added 2 commits March 7, 2024 13:04
@arminru arminru merged commit a3252cb into open-telemetry:main Mar 8, 2024
10 checks passed
@arminru arminru deleted the clarify-metric-namespace branch March 8, 2024 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

"metric attributes must have namespaces" decision is not documented in the general metrics guidelines
8 participants