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

scrape: Prometheus server support exemplar in native histogram #13449

Merged

Conversation

fatsheep9146
Copy link
Contributor

@fatsheep9146 fatsheep9146 commented Jan 24, 2024

After prometheus/client_model#80 is merged
we should mirror the change and support parsing those exemplars and ingesting them, as @beorn7 suggested

prometheus/client_model#80 (comment)

Signed-off-by: Ziqi Zhao <zhaoziqi9146@gmail.com>
@beorn7 beorn7 changed the title [WIP] prometheus server support exemplar in native histogram scrape: Prometheus server support exemplar in native histogram Jan 25, 2024
Copy link
Member

@beorn7 beorn7 left a comment

Choose a reason for hiding this comment

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

Thank you.

The next step would be to actually use the new protobuf field.

Currently, we parse the exemplars from the classic buckets, even if we are ingesting native histograms. I guess the behavior should be that we don't do that if there is at least one exemplar in the new dedicated field. @fatsheep9146 would you be interested in implementing that, too?

@beorn7 beorn7 merged commit 0fe34f8 into prometheus:main Jan 25, 2024
25 checks passed
@fatsheep9146
Copy link
Contributor Author

Thank you.

The next step would be to actually use the new protobuf field.

Currently, we parse the exemplars from the classic buckets, even if we are ingesting native histograms. I guess the behavior should be that we don't do that if there is at least one exemplar in the new dedicated field. @fatsheep9146 would you be interested in implementing that, too?

Yes, of course. I will also implement that too! @beorn7

@Nexucis
Copy link
Member

Nexucis commented Jan 26, 2024

Is it safe to include this PR in the next release (v2.50) @beorn7 @fatsheep9146 ? Just asking if I can simply rebase the release branch based on main or I will need to cherry pick a bug fixed that has been merged this morning on main to avoid to include this one.

@beorn7
Copy link
Member

beorn7 commented Jan 26, 2024

I think it is safe to include this PR. The code added here is simply not exercised anywhere.

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.

None yet

3 participants