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

Prometheus Datasource: Improve Prom query variable editor #58292

Merged
merged 20 commits into from
Feb 9, 2023

Conversation

bohandley
Copy link
Contributor

@bohandley bohandley commented Nov 5, 2022

Fixes #59559 (allow newlines in Prom query result variable query editor)
Fixes https://github.com/grafana/observability-metrics-squad/issues/48

What is this feature?
This changes the prometheus query variable editor to a group of selects and inputs to better explain the functions, arguments, and endpoint calls used in the prometheus variable queries.

Previous single input:
Screen Shot 2022-11-05 at 12 16 57 PM

New function select and variable inputs:
Screen Shot 2022-11-05 at 12 17 30 PM

Full proposal here.

Why do we need this feature?
This is in line with the Grafana 10 goal of "Getting Started." More clarity around creating variables in Prometheus will help onboard new customers.

Who is this feature for?
This feature is for anyone using the Prometheus datasource plugin who wants to create template variables.
This is for all users of prometheus datasource plugin, from expert to beginner.

Special notes for your reviewer:

  1. Please create a prometheus variable in the main branch, for example, label_names()
  2. Switch to the new branch and open the variable editor to see that the function has been correctly passed to the inputs.
  3. Check that the variables are returned.
  4. Look for confusing language and readability. Are the tooltips helpful?

@grafanabot
Copy link
Contributor

@bohandley bohandley added the pr/early-feedback WIP state but looking for early feedback label Nov 7, 2022
Copy link
Contributor

@itsmylife itsmylife left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After I check this locally I will approve it.

Comment on lines +3 to +6
const labelNamesRegex = /^label_names\(\)\s*$/;
const labelValuesRegex = /^label_values\((?:(.+),\s*)?([a-zA-Z_][a-zA-Z0-9_]*)\)\s*$/;
const metricNamesRegex = /^metrics\((.+)\)\s*$/;
const queryResultRegex = /^query_result\((.+)\)\s*$/;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are those all the options that we can use? I remember we can use scalar() too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While scalar() is a function provided by Prometheus, these are function created by Grafana to list variables. The scalar() function can be used in the query_result() function like so, query_result(scalar(<some prom query>))

@grafanabot
Copy link
Contributor

@bohandley bohandley removed the pr/early-feedback WIP state but looking for early feedback label Nov 11, 2022
@grafanabot
Copy link
Contributor

@leandro-deveikis leandro-deveikis modified the milestones: 9.3.0, 9.3.0-beta1 Nov 14, 2022
@grafanabot
Copy link
Contributor

This pull request was removed from the 9.3.0-beta1 milestone because 9.3.0-beta1 is currently being released.

@grafanabot grafanabot removed this from the 9.3.0-beta1 milestone Nov 15, 2022
@bohandley bohandley added this to the 9.4.0 milestone Nov 28, 2022
@bohandley bohandley added no-backport Skip backport of PR and removed no-backport Skip backport of PR labels Nov 28, 2022
@itsmylife
Copy link
Contributor

For instance for the "label values" we had this before:
image

After this change we have this:
image

There is no way to get explicitly label values of one metric. Or am I missing something?

@itsmylife
Copy link
Contributor

This is from one of my existing dashboards. It doesn't seem to handle existing variables properly.
image

@bohandley
Copy link
Contributor Author

For instance for the "label values" we had this before: image

After this change we have this: image

There is no way to get explicitly label values of one metric. Or am I missing something?

Oh! This could cause a problem. I brought Galen's check for the label values endpoint into this work, so if a prometheus datasource doesn't have flavor and version defined in the configuration page, and the flavor and version specified don't have this end point, a user cannot query with this. I think I might have to remove this because it assumes people have configured their datasource page, but they don't have to do that.
Screen Shot 2022-11-29 at 6 15 53 PM
Screen Shot 2022-11-29 at 6 17 32 PM

@bohandley
Copy link
Contributor Author

@itsmylife I think I will have to remove the flavor and version check for that function label_values!

@grafanabot
Copy link
Contributor

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 2 weeks if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!

@bohandley bohandley force-pushed the bohandley/improve-prom-query-var-editor branch from da6da15 to f37e340 Compare February 9, 2023 20:22
@bohandley bohandley merged commit eedcd7d into main Feb 9, 2023
@bohandley bohandley deleted the bohandley/improve-prom-query-var-editor branch February 9, 2023 20:35
bohandley added a commit that referenced this pull request Feb 10, 2023
bohandley added a commit that referenced this pull request Feb 10, 2023
…63278)

Revert "Prometheus Datasource: Improve Prom query variable editor (#58292)"

This reverts commit eedcd7d.
ryantxu pushed a commit that referenced this pull request Feb 10, 2023
…63278)

Revert "Prometheus Datasource: Improve Prom query variable editor (#58292)"

This reverts commit eedcd7d.
ryantxu pushed a commit that referenced this pull request Mar 2, 2023
* add prom query var editor with tests and migrations

* fix migration, now query not expr

* fix label_values migration

* remove comments

* fix label_values() variables order

* update UI and use more clear language

* fix tests

* use null coalescing operators

* allow users to query label values with label and metric if they have not set there flavor and version

* use enums instead of numbers for readability

* fix label&metrics switch

* update type in qv editor

* reuse datasource function to get all label names, getLabelNames(), prev named getTagKeys()

* use getLabelNames in the query var editor

* make label_values() variables label and metric more readable in the migration

* fix tooltip for label_values to remove API reference

* clean up tooltips and allow newlines in query_result function

* change function wording and exprType to query type/qryType for readability

* update prometheus query variable docs

* Update public/app/plugins/datasource/prometheus/components/VariableQueryEditor.tsx

Co-authored-by: Galen Kistler <109082771+gtk-grafana@users.noreply.github.com>

---------

Co-authored-by: Galen Kistler <109082771+gtk-grafana@users.noreply.github.com>
ryantxu pushed a commit that referenced this pull request Mar 2, 2023
…63278)

Revert "Prometheus Datasource: Improve Prom query variable editor (#58292)"

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

Successfully merging this pull request may close these issues.

Template Variables: Query-based. Newlines in query cause silent failure
5 participants