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: expose disabling staleness markers #34

Conversation

thampiotr
Copy link

@thampiotr thampiotr commented May 3, 2024

Addresses the prometheus#14049 by exposing a method on scrape manager to disable staleness markers for specified targets.

@thampiotr thampiotr changed the title Expose disabling staleness markers scrape: expose disabling staleness markers May 3, 2024
@thampiotr thampiotr force-pushed the thampiotr/expose-disabling-staleness-markers branch from a71d93a to 92cb90d Compare May 3, 2024 11:40
Signed-off-by: Piotr Gwizdala <17101802+thampiotr@users.noreply.github.com>
Signed-off-by: Piotr Gwizdala <17101802+thampiotr@users.noreply.github.com>
Signed-off-by: Piotr Gwizdala <17101802+thampiotr@users.noreply.github.com>
@thampiotr thampiotr force-pushed the thampiotr/expose-disabling-staleness-markers branch from 92cb90d to 53f3689 Compare May 3, 2024 11:44
@thampiotr thampiotr marked this pull request as ready for review May 7, 2024 11:27
@thampiotr
Copy link
Author

I can create a new branch with different name for this, but this PR is against the cmp_header_order as it's the one currently used in Alloy.

@mattdurham
Copy link

What is the functional change to the front end with this? Also and this might be suitable for a separate PR but there is likely some (alot) memory we can not allocate if staleness markers are disabled.

@mattdurham
Copy link

My gut is that it may trigger some of the issues from this very old blog post https://promcon.io/2017-munich/slides/staleness-in-prometheus-2-0.pdf

@thampiotr
Copy link
Author

What is the functional change to the front end with this?

No changes to frontend - this is to be used by library users.

Also and this might be suitable for a separate PR but there is likely some (alot) memory we can not allocate if staleness markers are disabled.

This is end-of-run staleness only, so I don't think there is much saving in Prometheus, as this is the same code path that executes when targets currently go away. Library users such as Alloy will also not just disable the staleness markers for all targets all the time. Only disable them for targets that move from one instance to another.

@thampiotr
Copy link
Author

thampiotr commented May 8, 2024

My gut is that it may trigger some of the issues from this very old blog post https://promcon.io/2017-munich/slides/staleness-in-prometheus-2-0.pdf

Can you give a specific example? For clarity: this does not disable staleness markers permanently. It gives library users such as Alloy an option to disable them when it is needed. And in Alloy's case we only disable them when series migrates from one instance to another, so the staleness is still correctly written to TSDB at the end of the target lifecycle by its new owning instance. Let me know what issues you think this can cause.

@thampiotr thampiotr changed the base branch from cmp_header_order to cmp_header_order_and_staleness_disabling May 13, 2024 09:39
@thampiotr
Copy link
Author

I'm going ahead and merge this into its own branch as grafana/alloy#703 is approved and I'd like to use this. If there's still some specific feedback, I'll be more than happy to address in a follow-up 👍

@thampiotr thampiotr merged commit 793c8c9 into cmp_header_order_and_staleness_disabling May 13, 2024
36 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.

2 participants