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

/metrics and probe routes does not respect web.route-prefix #1700

Closed
marcusramberg opened this issue Oct 31, 2019 · 10 comments
Closed

/metrics and probe routes does not respect web.route-prefix #1700

marcusramberg opened this issue Oct 31, 2019 · 10 comments
Labels

Comments

@marcusramberg
Copy link

marcusramberg commented Oct 31, 2019

Tested on Thanos 0.8.1, but still applies in master.

Object Storage Provider:

What happened:
Even if you run query as query --web-route-prefix="/thanos" - metrics and probes are served from root.

What you expected to happen:
Metrics should be served from /thanos/metrics

How to reproduce it (as minimally and precisely as possible):

thanos query --web.route-prefix="/thanos"
curl localhost:10902/metrics # gives metrics
curl localhost:10902/thanos/metrics # gives 404

Full logs to relevant components:

Anything else we need to know:

@kakkoyun
Copy link
Member

I have introduced this regression, sorry for the inconvenience.
I missed this prefix behaviour for querier and ruler, I'll fix it immediately.

And add necessary tests to prevent it from happening again.

@bwplotka
Copy link
Member

bwplotka commented Oct 31, 2019

BTW is it really regression?

@kakkoyun
Copy link
Member

@bwplotka This I want to discuss as well.

I guess intended usage --web.route-prefix="/thanos is only for API and UI. This flag is only exist in rule and query subcommand. For other subcommands, we don't have it and we don't use any prefix for /metrics and /debug/pprof/ paths without prefix.

And it flag description it says,"Prefix for API and UI endpoints. This allows Thanos UI to be served on a sub-path. This option is analogous to --web.route-prefix of Promethus."

I think it's not a regression, at least comparing to intended behaviour, I haven't checked what happens in older versions.

What do you think?

@bwplotka
Copy link
Member

It is really mixed on Prometheus side as well. Metric endpoint is local, rest (debug and readiness) are prefixed.

The question now is what is intended here (:

@kakkoyun
Copy link
Member

@marcusramberg Assuming this was the behaviour for pre v0.8.1, wasn't it?

@marcusramberg
Copy link
Author

I have no idea, I'm setting up a new cluster. But it is definitely the behavior I would expect.

@bwplotka
Copy link
Member

I would vote to stick debug and internal endpoints under the root, it is usually something you don't want to expose to outside, but happy to hear more opinions.

@FUSAKLA
Copy link
Member

FUSAKLA commented Nov 8, 2019

Even though I agree that the debug endpoints should stay at the root it feels like doing some decision for the user.

If he needs the prefix he probably already has some proxy in front of it where he can block those endpoints and on the other hand it can just complicate things for some uncommon use-cases 🤔

@marcusramberg
Copy link
Author

Fwiw Prometheus remaps health check according to web.external-url.

@stale
Copy link

stale bot commented Jan 11, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants