-
Notifications
You must be signed in to change notification settings - Fork 16.8k
prometheus-adapter: allow to set custom prometheus URL path #13174
Conversation
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
Hi @xaurx. Thanks for your PR. I'm waiting for a helm member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
…appy Signed-off-by: Alexander Lyashko <aurx@mail.ru>
@@ -16,6 +16,7 @@ nodeSelector: {} | |||
prometheus: | |||
url: http://prometheus.default.svc | |||
port: 9090 | |||
path: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be /prometheus by default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davidkarlsen By default prometheus serves / URI (root). There is an option to add some prefix for Prometheus URIs and make it serve /prefix/. This is used in environments where you want to share same hostname for multiple services and route HTTP requests based on URI path via API gateway.
As a user of the chart, I think it's pretty silly that you have to specify components of a URL (port and path) in addition to the |
@steven-sheehy Yes, I found it silly as well, but quickly realised PR won't be accepted if breaks current users. Another option is to check if port is zero, then we can omit ":" before the port and substitute URL only. |
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
/assign @lachie83 |
/approve |
Please bump chart version |
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
@steven-sheehy done |
@steven-sheehy anything else to do? |
@xaurx It LGTM, but we need someone with commit rights to lgtm it. @davidkarlsen or @mattfarina Mind reviewing? |
Signed-off-by: Alexander Lyashko <aurx@mail.ru>
cc @MattiasGees |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: mattfarina, steven-sheehy, xaurx The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
* prometheus-adapter: allow to set custom prometheus path Signed-off-by: Alexander Lyashko <aurx@mail.ru> * prometheus-adapter: bump version Signed-off-by: Alexander Lyashko <aurx@mail.ru> * prometheus-adapter: Bump version again, to make semantic versioning happy Signed-off-by: Alexander Lyashko <aurx@mail.ru> * Rework: omit port info at all if port is 0 Signed-off-by: Alexander Lyashko <aurx@mail.ru> * README: Remove stale path description Signed-off-by: Alexander Lyashko <aurx@mail.ru> * Bump prometheus-adapter chart version Signed-off-by: Alexander Lyashko <aurx@mail.ru>
* prometheus-adapter: allow to set custom prometheus path Signed-off-by: Alexander Lyashko <aurx@mail.ru> * prometheus-adapter: bump version Signed-off-by: Alexander Lyashko <aurx@mail.ru> * prometheus-adapter: Bump version again, to make semantic versioning happy Signed-off-by: Alexander Lyashko <aurx@mail.ru> * Rework: omit port info at all if port is 0 Signed-off-by: Alexander Lyashko <aurx@mail.ru> * README: Remove stale path description Signed-off-by: Alexander Lyashko <aurx@mail.ru> * Bump prometheus-adapter chart version Signed-off-by: Alexander Lyashko <aurx@mail.ru>
There is no way to specify prometheus listening on non-root endpoint to prometheus-adapter.
e.g. http://prometheus-svc:9090/prometheus.
There are solutions possible:
Checklist
[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]