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

Implement filters for Prometheus legend labels #6627

Closed
wants to merge 2 commits into from

Conversation

FauxFaux
Copy link

@FauxFaux FauxFaux commented Nov 17, 2016

Fix #3545.

This allows replacements in labels in legends for the Prometheus data source. e.g.

{{ foo | replace("bar", "baz") }}

I also implemented the toPercent() function as an example. Other functions should be easy to add. I omitted the hostname parsing functions as they are pretty complex, but would be great to add as another PR. Personally, I care most about the replace() functionality.

There are various things I consider bugs in the old parser, which I have maintained, e.g. {{ this could never work }} expands to this could never work.

I stuck with a regex-based parser as:

  • the existing code was broken specifically so it could use a simple regex based solution
  • Angular's $parse can't be used, as it's not safe for arbitrary inputs (like the previous code).
  • I couldn't find any reasonable JS libraries for this.

@torkelo
Copy link
Member

torkelo commented Nov 17, 2016

looks interesting. Will have to see if this is something others are interested in :)

A little hesitant to add so much code and complexity on a whim so will have to wait to see what demand there is. Would prefer if this kind of logic was in the Prometheus query language instead.

@FauxFaux
Copy link
Author

I count at least 28 unhappy people on the bug report (excluding me!). The bug report also suggests that the Prometheus people think it is a dashboarding concern, not a query language concern (in that they implemented it in dashboarding, before deprecating their dashboarding because Grafana was doing it).

We're actually still running 3.0, and using {{ foo.replace(//,'') }}, because this is considered such a serious problem.

@bergquist
Copy link
Contributor

I think this sounds like a good idea. I wonder if we could make this more generic for other datasources as well.

Btw. Please be patient with this pull request. We are currently super swapped with work. We will get back to it.

@RichiH
Copy link
Member

RichiH commented Nov 18, 2016

Sounds useful to me.

@coryfklein
Copy link

This would be extremely useful for us. Our legend labels are extremely verbose and often take up way too much space on our dashboards. Being able to display only the uniquely identifying info from the label would have a significant impact for us.

@torkelo
Copy link
Member

torkelo commented Nov 22, 2016

this sounds very useful, but think it either needs to be a feature of the prometheus query language or a general feature built into core Grafana templating

@raiviskrumins
Copy link

+1
In our use case "substring" function would help a lot:

const functions = {
  ...
  substring: function(val, start, end) {
  	return val.substring(start, end)
  }
};

@FauxFaux
Copy link
Author

For @raiviskrumins' substring, it might be useful to allow numerals; currently foo | substring("5", "7") would be required. As such, I don't plan to update the pull request.

@torkelo torkelo added the type/discussion Issue to start a discussion label Dec 6, 2016
@rdtr
Copy link

rdtr commented Dec 8, 2016

super useful! looking forward to seeing this merged! 🙏

@phantium
Copy link

phantium commented Jan 9, 2017

Would love to see this merged.

@fredsig
Copy link

fredsig commented Jan 11, 2017

+1

@adamlamers
Copy link

+1, definitely missing this feature after upgrading.

@jstsch
Copy link

jstsch commented Jan 13, 2017

+1, labels in combination with Prometheus and many instances really need some sort of transformation.

@torkelo
Copy link
Member

torkelo commented Jan 31, 2017

The template functions and syntax needs to implemented in the backend code as well, otherwise Prometheus queries that use this new legend format filters will not work when used in Alerting conditions.

https://github.com/grafana/grafana/blob/master/pkg/tsdb/prometheus/prometheus.go

@torkelo torkelo added pr/changes-needed and removed type/discussion Issue to start a discussion labels Jan 31, 2017
@FauxFaux
Copy link
Author

Does the pr_action: changes needed label imply that you would like me to do the work? I am happy to do so, but I don't understand what changes I could make that are likely to be accepted.

I can work out how to raise the code up to a level where it applies to all legend formatting, but that sounds like a very abusive change; presumably other data sources already have some syntax in their legend formatting which we would probably like to be bug-compatible with, maybe this is impossible? Is it worth investigating this?

I still believe there is significant value in merging this change as-is. Is there something you object to in the change itself that I could fix? I realise the code is.. hairy, and you might not want to support it long-term, but it's very, very isolated, and I have attempted to document its behaviour with tests. It's string processing in JS so doesn't feel risky from a security point of view.

@torkelo
Copy link
Member

torkelo commented Feb 11, 2017

Sorry for not be clear. The changes needed is to the backend prometheus code, as these new filter will not work if you use it for a query that is used in alerting.

@torkelo
Copy link
Member

torkelo commented May 17, 2017

Would like to merge this but cannot in it's current state as the legend format filter syntax & interpolation needs to be implemented in backend code as well.

https://github.com/grafana/grafana/blob/master/pkg/tsdb/prometheus/prometheus.go#L115

If not done soon we will have to close this PR.

@gdhgdhgdh
Copy link

FYI it's possible to work around this by making the PromQL queries more ugly, e.g. based on the default buckets where 5 is the largest quantified bucket, and +Inf above it, I'm successfully doing this:

label_replace(probe_mqtt_duration_seconds_bucket, "le", "5+", "le", ".Inf")

Now I have +Inf replaced by 5+ and my pseudo-histogram in mode: series is displaying correctly:

image

@eloo
Copy link
Contributor

eloo commented Jul 13, 2017

Hi,
is there any progress for this PR?
Would really like to use this feature :)

@tboerger
Copy link

So who is gonna do the backend integration? Just found this PR because of @bergquist at PromCon. This is really a feature I need to keep the labels more simple like dropping the hostname and port from scraped hosts.

@mmoleksf
Copy link

mmoleksf commented Dec 6, 2017

Any news on this? It would be really handy to have RegEx in this field.

@pkcinna01
Copy link

Can it just be documented that this may break alerts and update the alert panel to show a warning if it sees replacement filters in legend of prometheus queries? It would be great to have this functionality back and it sounds like waiting for the solution that addresses back end concerns might take a long time (if ever).

@bergquist
Copy link
Contributor

@pkcinna01 having two different implementations is not an option. That would create a bad experience for those who use Grafana alerting. If anyone is up for contributing the backend part of this it would be very appreciated.

@Astro03
Copy link

Astro03 commented Aug 1, 2018

@FauxFaux what happened to this PR? is it no longer going in?

@nicktimko
Copy link

1040 clicks to this issue from this post: https://community.grafana.com/t/regexp-in-legend-format/995/4

Is the solution to use 3.0.x?

@Armadill0
Copy link

I would really appreciate this feature!

@vvp83
Copy link

vvp83 commented Sep 4, 2018

Hi! We need this feature! May be merge? From 6 Feb 2017 PR. This is JOKE???

@tboerger
Copy link

tboerger commented Sep 4, 2018

@vvp83 provide this and it will be merged I guess => #6627 (comment)

@pengisgood
Copy link

any progress on this extremely useful feature?

@sherif84
Copy link

+1

1 similar comment
@chrisandrews7
Copy link

+1

@ntavares
Copy link

ntavares commented Feb 1, 2019

+1

@bagofmice
Copy link

Absolutely would love to do some string manipulation on legends. Would help clear screen of redundant data, like the shared part of a FQDN.

@davkal
Copy link
Contributor

davkal commented Oct 18, 2019

Would love this feature too, but closing it for inactivity. This does not mean we won't consider this again at some point. But it was highlighted that a solution needs to be implemented in both the backend and the frontend. I'm happy to guide someone through the process: david@grafana.com

@davkal davkal closed this Oct 18, 2019
@lrstanley
Copy link
Contributor

lrstanley commented Oct 18, 2019

I'm.. very confused. @torkelo mentions that this needs backend changes. Why would it need backend changes? It's only for visual effect (from my understanding?) and has nothing to do with the backend (prometheus)?

EDIT: to clarify, visual effect probably isn't the best terminology. I guess I don't understand why this isn't just a parser/injection item, and why backends have to do anything special. When using {{ xxx }}, this doesn't get passed to the backend (prometheus in this case), so not sure why it would with this change.

I.e. do the replacement/substring/etc at injection time. The variable is already known, and it gets injected before the backend even knows about it.

Also don't see this having anything to do with prometheus specifically.. this should be something that's possible on any backend because it has no ties to the backend type?

@SanderMertens
Copy link

I second @lrstanley's comment. I'm not sure what would be preventing support for a regex in the "legend format" field similar to the "Regex" field in the variable editor.

@tboerger
Copy link

With backend it doesn't mean the resource like prometheus, this simply means the grafana backend/api.

@benitogf
Copy link

The label_replace function worked for me on this one, removing a prefix on a metric name with:

label_replace({__name__="prefix_metric_x"}, "__name__", "$2", "__name__", "(prefix_)(.*)")

@m-kratochvil
Copy link

@benitogf the label_replace function is not supported on range-vector queries, so at best it's a partial workaround.. but better than nothing for now I gues..

@jdx-john
Copy link

Simply being able to treat the Legend text as a regex seems like it would be really useful and rather cosmetic. e.g. we have label values which are taken automatically from an enumeration so all values are like MY_ENUM__FOO, MY_ENUM_BAR, etc. Being able to trivially replace these as FOO, BAR would be great as I don't want to manually alias each possible value.

@ying-jeanne ying-jeanne added the pr/external This PR is from external contributor label Apr 29, 2021
@ishangupta-ds
Copy link

We also have a similar requirement where we want to replace a label's value on rendering it in the legend table and annotation by a regex match. This feature would be helpful if implemented. eg. - {{ chaos_injection_time | replace("", "N/A") }}

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.

Add filters to legend formatter with Prometheus datasource