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

Add filter support to prometheus for /api/v1/labels /api/v1/{label}/values #270

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

Hedius
Copy link
Contributor

@Hedius Hedius commented Apr 1, 2024

Not sure if my implementation is really ideal. Atm the querier.go of prometheus does not supprt filter support for metric and label searches. please review if what I do is really ok :)

This pull request implements the following changes:

  1. Ensure that the matchers are matched against the Tags Array/Column.
  2. For LabelNames() the matchers are performed against the Tags array column (by using index.TaggedTermWhereN)
  3. For LabelValues() the same is done + LIKE name=% is added as a where condition for Tag1.

If no matchers are provided graphite-clickhouse should work exactly like before.

If a matcher is provided it is matched against the Tags column.

In our case we use the stack with multi tenant support. So a proxy before graphite-clickhouse performs authentication and injects a label into all requests. {x=y}.

This change ensures that LabelNames() and LabelValues() only returns results which match {x=y}

@msaf1980
Copy link
Collaborator

I can't have meny expririece with Prometheus, but looks good.
Branch confilct against current muster need be resolved.
And no integrations tests for prometheus (and other API at now, obly for graphite). It's bad, but I haven't free time for that.
Do you have free time and desire for add prometheus integration tests ?

@Hedius
Copy link
Contributor Author

Hedius commented Apr 24, 2024

Hi, will fix the conflict + will look into writing tests when I have time. maybe this or next week.

@msaf1980
Copy link
Collaborator

Hi, will fix the conflict + will look into writing tests when I have time. maybe this or next week.

Hi, it's good news. Also merge with current muster for work test (one of clickhouse version is broken and integrations tests failed).

@Hedius
Copy link
Contributor Author

Hedius commented May 24, 2024

So, but now. Will look into this in the coming 2 weeks.

Also will merge master into it again.

@Hedius
Copy link
Contributor Author

Hedius commented May 24, 2024

Will have to check how the feature flags added in #266 interact with prometheus.

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

Successfully merging this pull request may close these issues.

2 participants