Skip to content

Fix units in metrics, stop using cached time #133456

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Aug 25, 2025
Merged

Fix units in metrics, stop using cached time #133456

merged 3 commits into from
Aug 25, 2025

Conversation

@nicktindall
Copy link
Contributor

A couple of the snapshot metrics had incorrect units specified

@nicktindall nicktindall added >non-issue :Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs labels Aug 25, 2025
@elasticsearchmachine elasticsearchmachine added Team:Distributed Coordination Meta label for Distributed Coordination team v9.2.0 labels Aug 25, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-distributed-coordination (Team:Distributed Coordination)

@nicktindall nicktindall requested a review from ywangd August 25, 2025 06:38
ywangd
ywangd approved these changes Aug 25, 2025
View reviewed changes
Copy link
Member

@ywangd ywangd 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 don't remember whether there is any implication in changing either the description or the unit of a metric. Probably not. It would be great if you could confirm. Thanks!

@nicktindall
Copy link
Contributor Author

nicktindall commented Aug 25, 2025 •
edited
Loading

LGTM

I don't remember whether there is any implication in changing either the description or the unit of a metric. Probably not. It would be great if you could confirm. Thanks!

The opentelemetry docs say

The data point type, unit, and intrinsic properties are considered identifying, whereas the description field is explicitly not identifying in nature.

so it'd be considered a new time-series according to the otel data model, but these metrics are brand new and they're not consumed by anything yet, other than a prototype dashboard I put together, which doesn't care about the time-series.

@nicktindall nicktindall changed the title Fix units in snapshot metric registrations Fix units in metrics, stop using cached time Aug 25, 2025
@nicktindall nicktindall merged commit 31088f5 into elastic:main Aug 25, 2025
33 checks passed
@nicktindall nicktindall deleted the fix_metrics_snapshot_registrations branch August 25, 2025 23:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ywangd ywangd ywangd approved these changes

Assignees

No one assigned

Labels

:Distributed Coordination/Snapshot/Restore Anything directly related to the `_snapshot/*` APIs >non-issue Team:Distributed Coordination Meta label for Distributed Coordination team v9.2.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@nicktindall @elasticsearchmachine @ywangd