Skip to content

Conversation

@maxbrunet
Copy link
Contributor

What type of PR is this?

What this PR does / why we need it:

The usage is label_values(<metric>, <label>), so I do not know if only passing the label works anywhere, but it does not work with Google's managed Prometheus.

This changes the query to extract the envoy_cluster_name values from the envoy_cluster_warming_state metric, which I believe, should be available for all configured clusters at all time.

Which issue(s) this PR fixes:

Fixes #

Release Notes: Yes/No

@maxbrunet maxbrunet requested a review from a team as a code owner August 15, 2025 22:51
@maxbrunet maxbrunet force-pushed the fix/gateway-addons-helm/envoy-cluster-variable branch from 8d4a48d to b71e1e8 Compare August 15, 2025 22:52
@codecov
Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.07%. Comparing base (ae449b6) to head (aa8589a).
⚠️ Report is 9 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6798   +/-   ##
=======================================
  Coverage   71.07%   71.07%           
=======================================
  Files         225      225           
  Lines       39810    39810           
=======================================
+ Hits        28293    28295    +2     
+ Misses       9852     9850    -2     
  Partials     1665     1665           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@arkodg arkodg requested a review from zirain August 20, 2025 01:56
@arkodg arkodg added this to the v1.6.0 Milestone milestone Aug 20, 2025
@zirain zirain force-pushed the fix/gateway-addons-helm/envoy-cluster-variable branch from b71e1e8 to aa8589a Compare August 20, 2025 02:03
@zirain
Copy link
Member

zirain commented Aug 20, 2025

the grafana doc said that only label is requried.

Deprecated, classic version of variable query editor. Enter a string with the query type using a syntax like the following: label_values(, )

is that a deprecated way?

@maxbrunet
Copy link
Contributor Author

It could be a deprecated way, but even in that way it seems the metric is optional:

https://github.com/grafana/grafonnet/blob/5a8f3d6aa89b7e7513528371d2d1265aedc844bc/gen/grafonnet-v11.4.0/custom/dashboard/variable.libsonnet#L258-L261

I am not able to tell what the non-deprecated syntax would be as the JSON schema is not well documented.

Anyway, that does work for GMP, Google restricts some queries that could retrieve too much data like by requiring the metric name everywhere (another example is omitting the metric name in PromQL and using a regex on the __name__ meta label as selector, that's not possible with GMP). What I propose should work for GMP, vanilla Prometheus, and every other PromAPI-compatible backends

It is also consistent with other dashboard variables of the Envoy Gateway project:

"query": "label_values(envoy_cluster_upstream_rq_time_bucket,namespace)",

@zirain
Copy link
Member

zirain commented Aug 20, 2025

To be clear, if GMP support label_values(label, metric), prefer to change all existing things to that way if possible

@maxbrunet
Copy link
Contributor Author

Yes, this is the only outlier

@zirain
Copy link
Member

zirain commented Aug 21, 2025

Yes, this is the only outlier

Perhaps I misunderstood you, what I mean is that can you use label_values(label, metric) everywhere instead of label_values(metric, label)? will it work for GMP?

@maxbrunet
Copy link
Contributor Author

label_values(label, metric) will not work anywhere, it is label_values(metric, label) or label_values(label)

@zirain zirain requested a review from shawnh2 August 21, 2025 01:52
@arkodg arkodg merged commit fc0be32 into envoyproxy:main Aug 22, 2025
31 checks passed
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.

3 participants