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

Support metrics with special characters in query builder #131

Open
rul-hydro opened this issue Jan 11, 2024 · 14 comments
Open

Support metrics with special characters in query builder #131

rul-hydro opened this issue Jan 11, 2024 · 14 comments
Assignees
Labels
enhancement New feature or request

Comments

@rul-hydro
Copy link

As mentioned in #128, the query builder generates invalid code when the metric name contains special characters, such as slash (/) or dash (-), and quite possibly others.

When picking a metric with special characters (e.g. foo/bar) from the drop-down, the builder should write the query as {__name__="foo/bar"} instead of just foo/bar. When switching back from Code to Builder, the label filter __name__= should also be translated to the Metric drop-down.

I suppose the distinction for the metric names could be done by a regex. Prometheus docs state that a valid metric name should match [a-zA-Z_:][a-zA-Z0-9_:]*. The expression for compatible metric names could be a bit broader than that, like [a-z-A-Z0-9_:.] or something like that. Anything not matching this expression would be written as a label filter.

(Beware of quotes in metric names - if anyone should be crazy enough to have them.)

@dmitryk-dk dmitryk-dk added the enhancement New feature or request label Jan 11, 2024
@matejsp
Copy link

matejsp commented Jan 11, 2024

Sorry guys ... I got it all wrong in #128 :D

Actually how it should be is to just escape the names from query builder:

Like for our case:

bts\-erp.sys_metrics_heap\-used

vs

{__name__="bts-erp.sys_metrics_heap-used"}

So just escape the names and take them into account when parsing or selecting the metrics from dropdown.

(based on idea from: https://stackoverflow.com/questions/52537029/escape-special-characters-in-prometheus-metric-names)

@matejsp
Copy link

matejsp commented Jan 11, 2024

I tried this in grafana too.

Entering the metrics without builder:
image

But when trying to switch to builder:
image

After accepting the risk and continue:
image

@Loori-R Loori-R self-assigned this Jan 12, 2024
dmitryk-dk pushed a commit that referenced this issue Jan 31, 2024
* add support metrics with special symbols #131

* refactor: change CompletionType from type to enum
@Loori-R
Copy link
Contributor

Loori-R commented Jan 31, 2024

Support metrics with special characters has been added at datasource starting from v0.6.0. Closing the feature request as done.

@Loori-R Loori-R closed this as completed Jan 31, 2024
@rul-hydro
Copy link
Author

Looks great, thank you!

@matejsp
Copy link

matejsp commented Jan 31, 2024

@Loori-R I tried and it's actually a lot better. However for such metrics we are unable to select label filters in dropdown.

image

Raw query works (and we have 4 labels (color, host, ...):

bts\-erp.heartbeat_metrics_value{color="blue"}

When changing from raw query to builder it populates the fields correctly. Just dropdown does not resolve all the possible labels.
image

Like when filtering labels or fetching them it does not take \ into account ?

@Loori-R
Copy link
Contributor

Loori-R commented Jan 31, 2024

Hey @matejsp, thanks for pointing this out! You're right about the dropdown issue with the special characters. We've just opened Issue #140 to tackle this.

@chenlujjj
Copy link

hi @Loori-R , I found that the query builder doesn't support label names containing dot. In the following example, k8s.cluster.name is converted to k8s in builder

image image image

@Loori-R
Copy link
Contributor

Loori-R commented May 11, 2024

Hello, @chenlujjj! Thanks for your message. It looks like we missed handling special characters in labels. I've reopened it.

@chenlujjj
Copy link

Hello @Loori-R , @hagen1778 , Thanks for your work, seems you have fixed it, cool!

May I know when the fix will be released so that we can install it into our Grafana ?

Does it also fix the auto-completion and syntax highlighting when the label names contain dots?

@dmitryk-dk
Copy link
Contributor

Hi @chenlujjj ! Can you try to install the latest release? It contains the bug fix which should solve your issue.

@chenlujjj
Copy link

Hello @dmitryk-dk , @Loori-R , the new release do fix the issue of query builder, thanks.

However, there are still problems with auto-completion and syntax highlighting:

  1. auto-completion
    After I typed the metric name, the braces and the prefix of label name, it cannot auto complete the remaining part of the label name.

image
NOTE: for the first time it can auto complete, but when I deleted the labels in braces and tried to input label names, it cannot auto complete.

  1. syntax highlighting
    image
    I think the k8s.cluster.name label should be of green color, but now only the name part is green.

@dmitryk-dk
Copy link
Contributor

@chenlujjj thank you! We will take a look into this issue

@chenlujjj
Copy link

hi @dmitryk-dk wondering what's the progress of this issue ?

@dmitryk-dk
Copy link
Contributor

hi @dmitryk-dk wondering what's the progress of this issue ?

Hi! Sorry for the delay. We will take a look as soon as possible, maybe by the end of next week.

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

No branches or pull requests

5 participants