Repository navigation
bootstrap: apply --web.max-requests, allow opting routes in - #447
Merged
ArthurSens merged 2 commits intoSep 21, 2026
Merged
ArthurSens merged 2 commits into
ArthurSens merged 2 commits into
Conversation
--web.max-requests was registered, parsed, validated and stored, and then nothing acted on it. Wrap the metrics handler in a concurrency limiter that answers overflow with 503 instead of queuing, so a scrape that can't be served fails fast instead of piling up behind the ones already running. Rejections are logged at a bounded rate to avoid flooding logs when the endpoint is saturated. A limit of 0 disables the bound, as the flag help already says. Signed-off-by: Nicolas Takashi <nicolas.takashi@dash0.com>
The metrics endpoint is bound by --web.max-requests, but routes an exporter registers through Bootstrap.Handle/HandleFunc are not — deliberately, since a health or readiness check should stay responsive while a bounded endpoint is saturated. Some exporters register routes that are themselves scrape endpoints (postgres_exporter's /probe, for example) and currently get no concurrency protection from the toolkit at all. Export the metrics endpoint's own limiter as Bootstrap.MaxRequestsHandler so a caller can wrap a route in it before registering it, opting that specific route into the same bound, same 503 behavior, and same rate-limited rejection log — without forcing it onto every route. Signed-off-by: Nicolas Takashi <nicolas.takashi@dash0.com>
nicolastakashi
force-pushed
the
nicolastakashi/max-web-request-opt-in
branch
from
September 18, 2026 19:01
1488fc7 to
aebd3f3
Compare
nicolastakashi
marked this pull request as ready for review
September 18, 2026 19:04
ArthurSens
approved these changes
Sep 21, 2026
Comment on lines
-67
to
+69
| // DisableExporterMetrics reports whether exporter self-metrics should be disabled. | ||
| // DisableExporterMetrics is the parsed value of --web.disable-exporter-metrics. |
Member
There was a problem hiding this comment.
I know this is out of scope for this PR, but I'd love to see exporter-toolkit owning a Registry and this boolean would handle registering VersionCollector, GoCollector and HTTP metrics in the future :)
It sounds a bit weird to have this boolean here and then have an exporter miss it by accident.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
--web.max-requests is parsed but never enforced, and routes like postgres_exporter's
/probeget no concurrency protection at all (see prometheus-community/postgres_exporter#1368).This binds the metrics endpoint to the flag, and adds
Bootstrap.MaxRequestsHandlerso other scrape-shaped routes can opt in too. Health checks stay unbounded by default.