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

Grafana Label values variable does not work if filtered for a promxy-added label #665

Closed
SipSeb opened this issue Jul 30, 2024 · 8 comments · Fixed by #679 or #684
Closed

Grafana Label values variable does not work if filtered for a promxy-added label #665

SipSeb opened this issue Jul 30, 2024 · 8 comments · Fixed by #679 or #684
Labels

Comments

@SipSeb
Copy link

SipSeb commented Jul 30, 2024

Hi,

we have Grafana running collecting data through promxy from multiple prometheus servers. Our setup looks like this:

Grafana
  -> promxy
         -> dev
                 -> dc1
                 -> euc1
                 -> beta
         -> test
                 -> dc1
                 -> euc1
                 -> beta
         -> prod
                 -> dc1
                 -> euc1
                 -> beta

Behind each dc1/euc1/beta target there are multiple prometheus servers. To distinguish the metrics, promxy adds two labels to each metric: cluster and promsrc. The first is filled with dev/test/prod and the latter with dc1/euc1/beta.

We updated Grafana recently, to version 11.1.0. And with the update, a lot of dashboards stopped working. We first thought this is a Grafana bug (and reported it there), but apparently it is something with the labels added in promxy.

See these two examples.
variable_filter_job
variable_filter_cluster

From the logs, it looks like Grafana uses the /api/v1/label/values endpoint to gather the variable data. So to make it work, promxy would have to intercept the request, clean the added labels and then pass on the request, but only to those servers where the filter criteria matches.

Is this something that is possible? Or is it impossible by design?

jacksontj added a commit that referenced this issue Sep 10, 2024
…s to downstream

If a label was only added by promxy *and* a matcher we'd still pass that
matcher downstream. This is a problem as promxy is the one who added it
-- so it doesn't matter if downstream knows about it (usually it
doesn't).

Fixes #665
@jacksontj jacksontj added the bug label Sep 10, 2024
@jacksontj
Copy link
Owner

First off; welcome and thanks for the report! This does indeed look like a bug!

IIRC grafana used to not implement the match[] params for the label values -- so we didn't run into this before. But from review it seems that specifically LabelValues has this problem -- where promxy is leaving the matcher in the request to the downstream (we remove it in all the other query* calls).

So I have created #679 with a fix (and test!) for this case. If you want you can give that PR build a whirl; either way I'll likely merge this tomorrow (after I get some time to sit on the idea -- which seems fine, but just to be sure :) ).

@SipSeb
Copy link
Author

SipSeb commented Sep 11, 2024

@jacksontj Thanks for fixing this. Unfortunately I could not test it, but will when a new release (and thus a new docker image) will be available.

@jacksontj
Copy link
Owner

@SipSeb FYI I cut a new release recently which includes this fix: https://github.com/jacksontj/promxy/releases/tag/v0.0.88

@SipSeb
Copy link
Author

SipSeb commented Sep 17, 2024

@jacksontj I'm afraid I have to reopen it. (I can't, but I have to ask you to reopen it.)

Looks better now, but only if I filter for just one promxy-added label, nothing else. In my setup, promxy adds two labels: cluster and promsrc. All other labels come from the downstream prometheus metrics.

Just one label

Works.
Bildschirmfoto 2024-09-17 um 11 35 27

Query string sent to promxy:

match%5B%5D=up%7Bcluster%3D%22dev%22%7D&start=1726543909&end=1726565509
URLdecoded: match[]=up{cluster="dev"}&start=1726543909&end=1726565509

Query string passed on to prometheus servers:

end=1726565509&match%5B%5D=%7B__name__%3D%22up%22%7D&start=1726543909
URLdecoded: end=1726565509&match[]={__name__="up"}&start=1726543909

Try adding another label

I only get the other promxy-added label in the dropdown, if I selected the cluster label before. I would expect to see all labels.
Bildschirmfoto 2024-09-17 um 11 35 55

But when I select the promsrc label as second filter, it breaks.
Bildschirmfoto 2024-09-17 um 11 45 19

Query string sent to promxy:

end=1726565764&match%5B%5D=up%7Bcluster%3D%22dev%22%2C+promsrc%3D%22beta%22%7D&start=1726544164
URLdecoded: end=1726565764&match[]=up{cluster="dev",+promsrc="beta"}&start=1726544164

Query string passed on:

end=1726565764&match%5B%5D=%7Bpromsrc%3D%22beta%22%2C__name__%3D%22up%22%7D&start=1726544164
URLdecoded: end=1726565764&match[]={promsrc="beta",__name__="up"}&start=1726544164

The promsrc label should not be passed downstream.

Using one promxy and one prometheus label as filter

For this, I had to first select the native prometheus label in Grafana and then the promxy-added label. But it does not return any results.
Bildschirmfoto 2024-09-17 um 11 55 14

Query string sent to promxy:

match%5B%5D=up%7Bjob%3D%22promtail%22%2C+cluster%3D%22dev%22%7D&start=1726545395&end=1726566995
URLdecoded: match[]=up{job="promtail",+cluster="dev"}&start=1726545395&end=1726566995

Query string sent downstream:

end=1726566995&match%5B%5D=%7Bjob%3D%22promtail%22%2Ccluster%3D%22dev%22%2C__name__%3D%22up%22%7D&start=1726545395
URLdecoded: end=1726566995&match[]={job="promtail",cluster="dev",__name__="up"}&start=1726545395

Can you take a look at this again, please?

@jacksontj jacksontj reopened this Sep 17, 2024
jacksontj added a commit that referenced this issue Sep 17, 2024
As reported in #665 the initial implementation fixed *some* cases but
not all. This fixes the typo and edge-case handling missed in the
previous PR.

Fixes #665
@jacksontj
Copy link
Owner

Thanks for the report (and I'm a bit surprised you can't re-open since you are the reporter; must be a setting I have to change).

That aside; I was able to find and fix the issue (#684) if you want to give that PR build a spin.

@SipSeb
Copy link
Author

SipSeb commented Sep 18, 2024

Thanks for the fix again. Much better now. But found another request, where it doesn't get filtered correctly. When adding more labels to a filter where I started with a promxy-added filter first, the dropdown is still empty (except for vars and promxy-added labels) - see my second screenshot in the last message. I can enter the desired label by hand and it will work afterwards, but it's probably not how it's supposed to be.
Grafana sends a /api/v1/labels POST request for the dropdown.

Request string sent to promxy:

start=1726628280&end=1726649940&match%5B%5D=%7Bcluster%3D%22dev%22%2C+__name__%3D%22up%22%7D
URLdecoded: start=1726628280&end=1726649940&match[]={cluster="dev",+__name__="up"}

Request string passed on downstream:

end=1726649940&match%5B%5D=%7Bcluster%3D%22dev%22%2C__name__%3D%22up%22%7D&start=1726628280
URLdecoded: end=1726649940&match[]={cluster="dev",__name__="up"}&start=1726628280

Thanks for the efforts put in my report, I really appreciate it.

jacksontj added a commit that referenced this issue Sep 18, 2024
As reported in #665 the initial implementation fixed *some* cases but
not all. This fixes the typo and edge-case handling missed in the
previous PR.

Fixes #665
@jacksontj
Copy link
Owner

Thanks for the quick response! I was able to find that issue and adjusted the patch (and added more test cases). So that should cover it.

While double checking I also added support for LabelNames (/labels) filtering as well (tests as well).

@SipSeb
Copy link
Author

SipSeb commented Sep 19, 2024

Thanks! Looks good now from my first test. All issues reported above are resolved.

Thank you very much.

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