Skip to content

Convert throughput SLO metrics from histogram to counter #4748

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

Merged
merged 1 commit into from
Feb 28, 2025

Conversation

carles-grafana
Copy link
Contributor

For performance

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

  • Tests updated
  • Documentation added
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

@carles-grafana carles-grafana marked this pull request as ready for review February 26, 2025 14:33
@carles-grafana carles-grafana marked this pull request as draft February 26, 2025 14:49
@carles-grafana carles-grafana marked this pull request as ready for review February 26, 2025 14:54
Copy link
Contributor

@javiermolinar javiermolinar 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 a breaking change, what's the reason to convert it to a counter? We are losing the ability to calculate throughput percentiles

@carles-grafana
Copy link
Contributor Author

This is a breaking change, what's the reason to convert it to a counter? We are losing the ability to calculate throughput percentiles

it's expected to perform better, I have discussed this with Joe, and I don't think we or anyone else is actually using the buckets, but I'll add the changelog line

@carles-grafana carles-grafana force-pushed the counter_throughput branch 2 times, most recently from 857326e to 3dd2538 Compare February 26, 2025 15:23
@joe-elliott
Copy link
Collaborator

I don't believe the histogram gives us any value. The goal here is just to count the bytes processed per tenant.

In the case that we do need quantiles on this metric it can be calculated from logs.

@carles-grafana carles-grafana marked this pull request as draft February 27, 2025 14:39
@carles-grafana carles-grafana marked this pull request as ready for review February 27, 2025 14:39
@carles-grafana carles-grafana merged commit 5d98dcd into grafana:main Feb 28, 2025
15 checks passed
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.

3 participants