bootstrap: allow exporters to add routes - #430
Open
nicolastakashi wants to merge 3 commits into
Open
Conversation
The bootstrap package builds its own mux, so an exporter that serves more than /metrics and the landing page cannot adopt it. postgres_exporter, for example, serves the multi-target /probe endpoint and the net/http/pprof handlers, and node_exporter-style adoption would silently drop both. Give Bootstrap a Handle/HandleFunc pair. Routes registered from the metrics handler factory are applied to the mux next to the metrics endpoint, which keeps them constructible from state that only exists after flags are parsed, such as the logger and the loaded config. Exporters that shipped --web.telemetry-path with an environment variable also cannot keep that behavior once bootstrap owns the flag, so allow the envar name to be configured. Signed-off-by: Nicolas Takashi <nicolas.tcs@hotmail.com>
nicolastakashi
force-pushed
the
bootstrap-extra-routes
branch
from
August 18, 2026 20:00
e9cdc36 to
1518a47
Compare
nicolastakashi
marked this pull request as ready for review
August 18, 2026 20:01
ArthurSens
requested changes
Aug 20, 2026
ArthurSens
left a comment
ArthurSens
left a comment
Member
There was a problem hiding this comment.
I don't think we want to continue using env vars in exporters. It would be better to deprecate and remove this functionality from existing exporters rather than continue building on top of it
nicolastakashi
added a commit
to nicolastakashi/exporter-toolkit
that referenced
this pull request
Aug 21, 2026
Per review feedback on prometheus#430, exporters should not gain new env-var-backed flags; existing env var support should be deprecated and removed rather than extended. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QwjFhjwbakiAgUbfnmfJLc
nicolastakashi
changed the title
Contributor
Author
Remove the env var support in here. |
Per review feedback on prometheus#430, exporters should not gain new env-var-backed flags; existing env var support should be deprecated and removed rather than extended. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QwjFhjwbakiAgUbfnmfJLc Signed-off-by: Nicolas Takashi <nicolas.tcs@hotmail.com>
The metricsPathFlag helper only existed to branch on MetricsPathEnvar, which was dropped in a3566c1. With no branching left, the extraction added nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QwjFhjwbakiAgUbfnmfJLc Signed-off-by: Nicolas Takashi <nicolas.tcs@hotmail.com>
nicolastakashi
force-pushed
the
bootstrap-extra-routes
branch
from
August 21, 2026 19:50
8ed863e to
58bf611
Compare
ArthurSens
reviewed
Aug 22, 2026
ArthurSens
left a comment
ArthurSens
left a comment
Member
There was a problem hiding this comment.
What happens if someone adds a new route that conflicts with / or /metrics? Should we provide guardrails against that?
What ever we decide, it would also be nice to have a test that confirms the behavior we want
Member
|
Could update your node/postgres PRs with a replace in go mod, just so we can see how these changes look like downstream before we merge? |
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
2 participants
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.
Problem
bootstrapbuilds the exporter's HTTP mux itself:Run()registers the metrics handler at--web.telemetry-pathand the landing page at/, and nothing else can be added to it.That is enough for
node_exporter, but it blocks exporters that serve more than those two endpoints.postgres_exporteris the immediate case:/probeendpoint, which is the documented way to scrape remote instances viaauth_modulesnet/http/pprofhandlersAdopting
bootstrapas it stands would silently 404 both, so the exporter cannot move onto the shared startup path without a user-visible regression. Any exporter with a second endpoint hits the same wall, which pushes them back to hand-rolledmain()blocks — the duplicationbootstrapexists to remove.A second constraint is when those handlers can be built.
postgres_exporter's probe handler needs the logger and the loaded config file, neither of which exists until flags are parsed, so a static list of routes onConfigis not sufficient.Follow-up
Consumed by prometheus-community/postgres_exporter#1368, which moves
postgres_exporterontobootstrapwhile keeping/probeand pprof working. That PR needs a release of this one before it builds.Note: an earlier revision of this PR also let exporters back
--web.telemetry-pathwith a custom env var, to preservepostgres_exporter'sPG_EXPORTER_WEB_TELEMETRY_PATH. Per @ArthurSens's review, that's dropped — new env-var-backed flags shouldn't be added, existing ones should be deprecated/removed instead. The follow-up postgres_exporter PR will need to handle that separately.