Skip to content

Fix prometheus connector to work with prometheuses on sub-path#11578

Closed
Muxa1L wants to merge 4 commits intotrinodb:masterfrom
Muxa1L:master
Closed

Fix prometheus connector to work with prometheuses on sub-path#11578
Muxa1L wants to merge 4 commits intotrinodb:masterfrom
Muxa1L:master

Conversation

@Muxa1L
Copy link
Copy Markdown
Member

@Muxa1L Muxa1L commented Mar 18, 2022

Description

This fix is for prometheus connector, aimed to allow trino work with prometheus'es working with api exposed in non-root url (behind reverse proxy or victoriametrics in cluster mode, needing to use urls like /select/<tenantid>/prometheus/api/.... )
Trino can get table (metric) names correctly but cant get any values for them.
And if you set uri with sub-path in prometheus.uri - this subpath is overwrited by this code.

Is this change a fix, improvement, new feature, refactoring, or other?
Fix
Is this a change to the core query engine, a connector, client library, or the SPI interfaces? (be specific)
Prometheus Connector
How would you describe this change to a non-technical end user or system administrator?
aimed to allow trino work with prometheus'es working in non-root url (or to work with victoriametrics in cluster mode, needing to use urls like /select//prometheus/api/.... )

Related issues, pull requests, and links

Documentation

( ) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
( ) Release notes entries required with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 18, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@cla-bot
Copy link
Copy Markdown

cla-bot bot commented Mar 21, 2022

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please submit the signed CLA to cla@trino.io. For more information, see https://github.com/trinodb/cla.

@Muxa1L Muxa1L changed the title Fix prometheus connector to work with prometheus on sub-path Fix prometheus connector to work with prometheuses on sub-path Mar 28, 2022
@cla-bot cla-bot bot added the cla-signed label Mar 28, 2022
@Muxa1L Muxa1L requested a review from bentito April 6, 2022 19:08
@bentito
Copy link
Copy Markdown
Member

bentito commented Apr 7, 2022

LGTM, but one thing, you said no docs needed, but maybe we should let users know this is happening with the URI and why? It may help them debug if it's not working.

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jun 15, 2022

@Muxa1L #12391 should add support for sub path in prometheus.uri config property. Could you try if it works in your environment?

@ebyhr
Copy link
Copy Markdown
Member

ebyhr commented Jul 13, 2022

Superseded by #12391

@ebyhr ebyhr closed this Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants