Skip to content

Conversation

@SylvainJuge
Copy link
Contributor

@SylvainJuge SylvainJuge commented Oct 22, 2025

Fixes #14753

  • update feature description and link to semconv
  • change unit to 1 where it was previously "" (empty string)

In the existing semantic conventions, there are at least two examples of "state metrics" that already use 1 as unit, so it is probably simpler to align to semconv than try to change semconv to use an empty metric.

Only the state metric unit changes, so while it's technically a breaking change I think the impact should be quite limited:

  • 1 as unit is used mostly for fractions of something, usually with values between 0.0 and 1.0.
  • Because current empty unit for state metrics is used to indicate "lack of unit", the value is unlikely to be used for meaningful purpose. However any tool or mapping relying on the current unit might need some (but minor) adjustment.

So, in the current state I don't think this requires to have a dedicated configurable toggle to leave this breaking change for 3.0, however I don't mind taking the "extra safe" way and add a config option just to make the transition smoother.

In addition, due to how the feature is currently built, the configuration options are not automatically propagated to usages of the library like in the jmx-scraper so adding a new configuration option would require to also implement it in every usage of the library, then deprecate and revert the change. This inconvenience might be removed once #14674 is completed, but I'm not 100% sure about that, so in the mean time it's probably simpler to deal with what appears to be a minor breaking change.

Migration notes

Prior to this change, the state metrics were generated with an empty "" unit, they are now using 1 to align with semconv recommendation. Aligning with semconv on metric name and metric attributes names is left to the end-user.

@SylvainJuge SylvainJuge self-assigned this Oct 22, 2025
@SylvainJuge SylvainJuge marked this pull request as ready for review October 22, 2025 15:09
@SylvainJuge SylvainJuge requested a review from a team as a code owner October 22, 2025 15:09
Copy link
Contributor

@breedx-splk breedx-splk left a comment

Choose a reason for hiding this comment

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

This is great. I definitely favor correct over minor breaking. Thanks!

@github-actions
Copy link
Contributor

⚠️ Breaking Change Documentation Required

This PR has been labeled as a breaking change. Please ensure you provide the following information:

Migration Notes Required

Please add detailed migration notes to help users understand:

  • What is changing and why
  • How to update their code/configuration
  • Any alternative approaches if applicable
  • Code examples showing before/after usage

Checklist

  • Migration notes added to the PR description
  • Breaking change is documented in the changelog entry for the next release
  • Consider if this change requires a major version bump

Your migration notes will be included in the release notes to help users upgrade smoothly. The more detailed and helpful they are, the better the user experience will be.


This comment was automatically generated because the breaking change label was applied to this PR.

@trask trask merged commit 25471f9 into open-telemetry:main Oct 30, 2025
92 checks passed
@SylvainJuge SylvainJuge deleted the state-metrics-semconv branch October 30, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Consider state metric conventions

4 participants