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

feat: scrape timeout #558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

johanneswuerbach
Copy link

Allow to configure a scrape timeout to prevent queries from hanging forever.

@roidelapluie
Copy link
Contributor

Prometheus sends the scrape timeout in HTTP headers, we should reuse it.

Signed-off-by: Johannes Würbach <johannes.wuerbach@googlemail.com>
@johanneswuerbach
Copy link
Author

johanneswuerbach commented Jul 13, 2021

While I've seen that, I wasn't sure how well this plays with caching of slow queries 🤔 Could there be queries, where the cache was previously populated after 2-3 fetches, which would always fail with a small timeout?

@johanneswuerbach
Copy link
Author

johanneswuerbach commented Jul 13, 2021

Looking at https://github.com/prometheus/client_golang support for the X-Prometheus-Scrape-Timeout-Seconds header doesn't seem to be provided yet, any pointers how that works @roidelapluie , @SuperQ ?

This exporter is using the handler provided by the golang client at the moment

http.Handle(*metricPath, promhttp.Handler())

@sysadmind
Copy link
Contributor

I do not think that the context is propagated to any of the collectors at this time. While the http.Request may be cancelled by the header timeout, the downstream functions in the collector have no way to access it. I think this implementation is reasonable at this time based on the interfaces that we have to work with.

@johanneswuerbach
Copy link
Author

Any further steps I could do to drive this forward @sysadmind @roidelapluie @SuperQ ?

@johanneswuerbach
Copy link
Author

Ping :-)

@SuperQ
Copy link
Contributor

SuperQ commented May 31, 2022

Yes, in order to do this the "usual way", you need to implement http.HandlerFunc().

The mysqld_exporter does this.

I think we should focus on getting #618 finished so we have the new multi-target handler functionality.

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

4 participants