Add args to serve application behind a route prefix/external URL#1335
Add args to serve application behind a route prefix/external URL#1335kwilt wants to merge 3 commits intoprometheus:mainfrom
Conversation
|
Supporting exporter-toolkit pull request: prometheus/exporter-toolkit#283 |
|
we need a new version from the https://github.com/prometheus/exporter-toolkit before we can get this merged |
|
I have released an update to the exporter-toolkit. (#1360) |
|
Needs some |
|
pprof profile - endpoint |
|
The pprof endpoints are not configurable easily. We would have to custom register our own handlers. We basically need to completely re-work how we handle the web endpoint for exporters to handle this better. Really, I don't think we should be doing this at all. This is too much extra boilerplate and makes our exporters inconsistent. The fact that this has been hand-implemented by the blackbox exporter, and this PR is a mess of additional code, shows that we should not be doing this. |
SuperQ
left a comment
SuperQ
left a comment
There was a problem hiding this comment.
This is far too much boilerplate as-is. I will not merge this until it is simpler for exporter maintainers to add universally across the ecosystem.
I have attempted to do this in this pull request prometheus/exporter-toolkit#293 |
kwilt
force-pushed
the
web-external-url
branch
from
f3db7b9 to
0d9c3b4
Compare
main.go
Outdated
| concurrency = kingpin.Flag("snmp.module-concurrency", "The number of modules to fetch concurrently per scrape").Default("1").Int() | ||
| debugSNMP = kingpin.Flag("snmp.debug-packets", "Include a full debug trace of SNMP packet traffics.").Default("false").Bool() | ||
| expandEnvVars = kingpin.Flag("config.expand-environment-variables", "Expand environment variables to source secrets").Default("false").Bool() | ||
| externalURL = kingpin.Flag("web.external-url", "The URL under which snmp exporter is externally reachable (for example, if snmp exporter is served via a reverse proxy). Used for generating relative and absolute links back to snmp exporter itself. If the URL has a path portion, it will be used to prefix all HTTP endpoints served by snmp exporter. If omitted, relevant URL components will be derived automatically.").PlaceHolder("<url>").String() |
There was a problem hiding this comment.
Shouldn't these flags be part of github.com/prometheus/exporter-toolkit/web/kingpinflag?
There was a problem hiding this comment.
I plan to start diving into this again soon. I've been quite busy lately with my day job and other responsibilities 😄
Signed-off-by: kwilt <kwilt@pm.me>
|
Hello everyone - is there still someone working on this particular enhancement? This would be super useful. |
I worked on this several months ago (nearly a year at this point) and have been using it in a production environment ever since without issue. This feature depends on this 8 month old PR to get merged, which hasn't yet been acknowledged by maintainers. Until that's merged, the tests on this will continue to fail. I'm not a Go developer, so I would appreciate anyone taking initiative to improve/fix my code on either PR to get this merged. |
4 participants
This is a duplicate of #1328
Sorry for the duplicate. Didn't realize deleting my branch would auto-close the pull request. Anyways... 🤦♂️
Add feature to serve application behind a route prefix or external URL by supplying args --web.external-url/--web.route-prefix, in the same way as blackbox_exporter or other exporters.
I am also submitting a pull request for exporter-toolkit so that this will actually work. Currently you cannot serve the landing page on anything other than root ("/"), so the tests are going to fail initially.
I would consider these additional args to be "must-have" features to bring snmp_exporter closer to feature parity with the other exporters. I needed to serve this exporter behind a prefix like I'm doing with the other exporters, so I figured I would make an attempt at this update myself, as it does appear to be something other people have been wanting for quite some time. See: #648