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

[release 2.54] [BUGFIX] Scraping: allow multiple samples on same series #14685

Merged

Conversation

bboreham
Copy link
Member

So long as they specify timestamps. We don't check that the timestamps are different.

Fixes #14503

Extend test, and use client_golang/prometheus/testutil to simplify metric check.

So long as they specify timestamps. We don't check that the timestamps
are different.

Extend test, and use client_golang/prometheus/testutil to simplify metric check.

Signed-off-by: Bryan Boreham <[email protected]>
Copy link
Collaborator

@machine424 machine424 left a comment

Choose a reason for hiding this comment

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

lgtm!

@machine424
Copy link
Collaborator

If we could add a test stating that we assume with this fix test_metric 1\ntest_metric 1 1001 not to get flagged as well, it'd be great.

@machine424
Copy link
Collaborator

Actually, if they arrive in this order test_metric 1 1001\ntest_metric 1, they'll still be flagged.
I'm not sure if this is a valid use case though.

@bboreham
Copy link
Member Author

This came up on the issue - I don’t think we should make guarantees about mixing timestamp and no-timestamp.

At low probability, the specified timestamp would end up equal to the unspecified one.

@bboreham bboreham merged commit 144470c into prometheus:release-2.54 Aug 19, 2024
30 checks passed
@bboreham bboreham deleted the scrape-multiple-timestamps2 branch August 19, 2024 09:58
bboreham added a commit that referenced this pull request Aug 27, 2024
So long as they specify timestamps. We don't check that the timestamps
are different.

Extend test, and use client_golang/prometheus/testutil to simplify metric check.

Signed-off-by: Bryan Boreham <[email protected]>
bboreham added a commit that referenced this pull request Aug 27, 2024
[release 2.53] Backport #14685 Scraping: allow multiple samples on same series
@Masmiiadm
Copy link

Masmiiadm commented Oct 11, 2024

Hello , will this change be merged to 2.53.0 LTS version

zuozp8 added a commit to zuozp8/prometheus that referenced this pull request Oct 11, 2024
Change introduced in prometheus#12933 and partially reversed in prometheus#14685

Signed-off-by: Konrad <[email protected]>
zuozp8 added a commit to zuozp8/prometheus that referenced this pull request Oct 11, 2024
zuozp8 added a commit to zuozp8/prometheus that referenced this pull request Oct 11, 2024
Change introduced in prometheus#12933; old behavior partially recoved in prometheus#14685

Signed-off-by: Konrad <[email protected]>
@bboreham
Copy link
Member Author

No we will not change 2.53.0. There should be a patch release with #14740.

@Masmiiadm
Copy link

Masmiiadm commented Oct 12, 2024

@bboreham
Following the bug fix [[release 2.54] [BUGFIX] , I updated Prometheus from 2.53.0 to version 2.54.1 (kube-prometheus-stack Helm chart v63.0.0), but the issue of 'Error on Ingesting Samples with Different Values but Same Timestamp' still persists.
Is there a flag to enable or any configuration to add in order to allow multiple samples on the same series?
ts=2024-10-11T13:29:45.818Z caller=scrape.go:1754 level=warn component="scrape manager" scrape_pool=consul-appli target=http://x.x.x.x:8080/metrics msg="Error on ingesting samples with different value but same timestamp" num_dropped=1320
ts=2024-10-11T13:29:48.830Z caller=scrape.go:1744 level=warn component="scrape manager" scrape_pool=serviceMonitor/monitoring/monitoring-k8s-kube-state-metrics/0 target=http://10.139.28.168:8080/metrics msg="Error on ingesting samples with different value but same timestamp" num_dropped=25

I downgraded to v2.47 and there is no scrapping errors .

@bboreham
Copy link
Member Author

Please open an issue and include the metrics being scraped.

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.

4 participants