-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[batchprocessor] Update metric units #10658
[batchprocessor] Update metric units #10658
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #10658 +/- ##
=======================================
Coverage 91.64% 91.64%
=======================================
Files 406 406
Lines 19001 19001
=======================================
Hits 17414 17414
Misses 1227 1227
Partials 360 360 ☔ View full report in Codecov by Sentry. |
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.
Is there any way we can ensure consistency in the units used for all components? Maybe having a common list or guidelines about this?
I suspect we'll have better guidelines in the near future with open-telemetry/oteps#259, but maybe we can start a doc in this repo until the otep lands? |
Opened #10695 to provide guidelines around units |
#### Description Update telemetry units to be more clear. Related to #10658, we may be blocked here waiting for official units to use. Note: I've kept [one metric](https://github.com/open-telemetry/opentelemetry-collector/blob/7d5b1ba387e4e171e25bfeea9b70d6fdfbe08fab/cmd/mdatagen/internal/samplereceiver/metadata.yaml#L110) with the unit "1", as it's not meant to be meaningful, just to show a metric that's disabled by default. Since this code is internal and only used for tests, from what I can tell, I don't believe we need a changelog entry. #### Link to tracking issue Fixes #10653
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Description
1
isn't an informative metric unit. I've made a best-guess attempt at updating metric units, feedback is very necessary here, the units are not very clear here. I attempted to match the unit with metric descriptions.Semantic conventions for reference.
Link to tracking issue
Fixes #10652