Skip to content

Add cortex_ingester_active_native_histogram_series metric to track ac… #6695

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

Conversation

SungJin1212
Copy link
Member

@SungJin1212 SungJin1212 commented Apr 14, 2025

Currently, the metric cortex_ingester_active_series tracks the number of active series, including native histogram(NH) and float series. So, we cannot track the number of NH series only.
This PR adds a cortex_ingester_active_native_histogram_series metric to track only the number of active NH series.

Which issue(s) this PR fixes:
Fixes #

Checklist

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

Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

This seems a very nice improvement. @harry671003 Can you help take a look?

Copy link
Contributor

@harry671003 harry671003 left a comment

Choose a reason for hiding this comment

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

LGTM!

Should we also track native histogram samples separately in distributor?

Copy link
Contributor

@justinjung04 justinjung04 left a comment

Choose a reason for hiding this comment

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

Nice, thank you!

@SungJin1212 SungJin1212 force-pushed the Add-cortex_ingester_active_native_histogram_series branch from 7aca81c to 4b42a00 Compare April 16, 2025 02:12
@SungJin1212
Copy link
Member Author

SungJin1212 commented Apr 16, 2025

@harry671003
Do you mean the cortex_distributor_received_samples_total? It has a type label to separate float and native histogram.

We could change cortex_distributor_received_samples_total to contain NH and float samples and create a new one for NH.
Would it be more nicer to add a new metric for NH?
cc. @yeya24

@yeya24
Copy link
Contributor

yeya24 commented Apr 16, 2025

We could change cortex_distributor_received_samples_total to contain NH and float samples and create a new one for NH.
Would it be more nicer to add a new metric for NH?

Do you mean we add a dedicated metric for NH? Similar to cortex_distributor_received_samples_total? I am not sure if it is necessary given that cortex_distributor_received_samples_total already has a type label

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 16, 2025
@yeya24 yeya24 requested a review from alanprot April 17, 2025 04:03
@yeya24 yeya24 merged commit 22cd00c into cortexproject:master Apr 17, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ingester lgtm This PR has been approved by a maintainer size/L
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants