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

Store: Use Histograms for bucket metrics #6131

Merged
merged 1 commit into from Feb 17, 2023

Conversation

SuperQ
Copy link
Contributor

@SuperQ SuperQ commented Feb 16, 2023

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

Convert store bucket metrics from Summary to Histogram so that they can be aggregated over multiple instances.

Verification

Convert store bucket metrics from Summary to Histogram so that they can
be aggregated over multiple instances.

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ changed the title store: Use Histograms for bucket metrics Store: Use Histograms for bucket metrics Feb 16, 2023
@SuperQ
Copy link
Contributor Author

SuperQ commented Feb 16, 2023

I'm not sure what's up with that e2e test. Will poke at it tomorrow.

@SuperQ
Copy link
Contributor Author

SuperQ commented Feb 16, 2023

Oh, looks like it was just a flaky test.

Copy link
Member

@saswatamcode saswatamcode left a comment

Choose a reason for hiding this comment

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

Thanks!

@saswatamcode saswatamcode merged commit 0edbf2b into thanos-io:main Feb 17, 2023
@douglascamata
Copy link
Contributor

douglascamata commented Feb 17, 2023

@SuperQ @saswatamcode @yeya24: I think we rushed too much with this one.

This is kind of a big breaking change for everyone already using these metrics, which exist since a while.

I understand Thanos doesn't offer any breaking change guarantees, but why wasn't a deprecation applied?

What is the motivation to change these metrics from summaries to histograms right now? Is aggregation the only reason? Wouldn't it be better to wait and use native histograms at least?

Currently the dashboards mixins and examples that were using these metrics are broken because of the change.

@yeya24
Copy link
Contributor

yeya24 commented Feb 17, 2023

Sorry for the quick merge and thanks @douglascamata for raising the point. We should take actions to fix the mixins and examples for sure before we do next release.

I understand Thanos doesn't offer any breaking change guarantees, but why wasn't a deprecation applied?

Thanos is still at 0.X release so breaking changes across releases are allowed. I see we made breaking changes in the past as well so this should be fine. We just need to mention the breaking change clearly in our changelog to notify our user.

SuperQ added a commit to SuperQ/thanos that referenced this pull request Feb 20, 2023
Update store dashboard for summary to histogram changes.
* Followup to thanos-io#6131
* Update changelog to note breaking changes.

Signed-off-by: SuperQ <superq@gmail.com>
@SuperQ SuperQ mentioned this pull request Feb 20, 2023
2 tasks
SuperQ added a commit to SuperQ/thanos that referenced this pull request Feb 20, 2023
Update store dashboard for summary to histogram changes.
* Followup to thanos-io#6131
* Update changelog to note breaking changes.

Signed-off-by: SuperQ <superq@gmail.com>
SuperQ added a commit to SuperQ/thanos that referenced this pull request Feb 20, 2023
Update store dashboard for summary to histogram changes.
* Followup to thanos-io#6131
* Update changelog to note breaking changes.

Signed-off-by: SuperQ <superq@gmail.com>
saswatamcode pushed a commit that referenced this pull request Feb 22, 2023
Update store dashboard for summary to histogram changes.
* Followup to #6131
* Update changelog to note breaking changes.

Signed-off-by: SuperQ <superq@gmail.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request Apr 17, 2023
Convert store bucket metrics from Summary to Histogram so that they can
be aggregated over multiple instances.

Signed-off-by: SuperQ <superq@gmail.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request Apr 17, 2023
Update store dashboard for summary to histogram changes.
* Followup to thanos-io#6131
* Update changelog to note breaking changes.

Signed-off-by: SuperQ <superq@gmail.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request Apr 17, 2023
Convert store bucket metrics from Summary to Histogram so that they can
be aggregated over multiple instances.

Signed-off-by: SuperQ <superq@gmail.com>
ngraham20 pushed a commit to ngraham20/thanos that referenced this pull request Apr 17, 2023
Update store dashboard for summary to histogram changes.
* Followup to thanos-io#6131
* Update changelog to note breaking changes.

Signed-off-by: SuperQ <superq@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants