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

Add possibility to change text case #1548

Open
grobie opened this Issue Apr 12, 2016 · 14 comments

Comments

Projects
None yet
5 participants
@grobie
Copy link
Member

grobie commented Apr 12, 2016

We have unfortunately two systems which expose labels in both upper case and lower case. While we're about to consolidate these, this takes time. It'd be helpful if prometheus would allow to transform such labels, so that the issue can be mitigated by creating an evaluation rule.

I'm not sure if lowercase() and uppercase() make sense, or if we could come up with a more generic variant to replace any kind of characters (like tr). Or is there some trick to achieve that with label_replace already?

@grobie grobie changed the title [Feature request] Add possibility to change text case Add possibility to change text case Apr 12, 2016

@fabxc

This comment has been minimized.

Copy link
Member

fabxc commented Apr 12, 2016

To be clear, you are taking about PromQL here, not relabel configs?

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Apr 12, 2016

Yes. Maybe in the form of replace(<vector>, <label>, <regex>, <replacement>), example replace(foo, service, "[A-Z]", "[a-z]").

The problem with "use configuration mangement" is that a static list of all label values would need to be known upfront.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Apr 13, 2016

Any other comments or concerns? I've been thinking about the function name for a bit now, maybe label_translate(<vector>, <label>, <regex>, <replacement>) in the style of the tr command? Other options value_replace or also value_translate.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Apr 13, 2016

I think offering a tr variant would get too complicated, as we can't sanely do all the options in one function. Keeping it simple with upper/lowercase would be best.

value_replace and value_translate would operate on values not labels, and the value_ would be redundant as all functions operate on values unless they contain label.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 13, 2016

I'll add lower(str) and upper(str) functions if people are ok with that.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Apr 13, 2016

I'm not sure how that would work, can you give an example? How can I apply a function with such signature to a specific label value? For example to allow label matching with other metrics?

The reason I suggested a more generic translate function is that it'd cover another issue we have with a system which removes all dashes from a name. Though, the cardinality of values is way lower, so we might be able to solve that with rules.

I agree that value_ is misleading and ambiguous. I meant label_value_ here to make clear it's not replacing full labels but only certain characters of a single label value.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 13, 2016

Whoops, you're right, that wouldn't actually help for your use case. The question then is whether we want upper/lower methods at all or something more complex, or nothing.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Apr 14, 2016

How about adding something to the syntax of the replacement parameter in the label_replace() function? We just have $<number> so far as special syntax. We could implement a subset of bash-style parameter expansion, starting just with the upper-/lower-case feature, which could easily be expanded as needed.

So

  • ${1^} uppercases the 1st character of the 1st match
  • ${1^^} uppercases the all characters of the 1st match
  • ${1,} lowercases the 1st character of the 1st match
  • ${1,,} leoercases the all characters of the 1st match

If needed one day, we could implement the full Bash-style ${parameter^pattern} etc. or any other parameter expansions as described in man bash.

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Apr 14, 2016

We just have $ so far as special syntax.

We're using Go's standard ExpandString function for regexes. Let's not badly reinvent perl just for one user's broken setup.

If we were to allow something this powerful, the sanest way would be to give direct access to console templates from promql.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Apr 14, 2016

If we were to allow something this powerful, the sanest way would be to give direct access to console templates from promql.

That would be way more consistent. Do you have an idea how to do this concretely?

@brian-brazil

This comment has been minimized.

Copy link
Member

brian-brazil commented Apr 14, 2016

Something like label_template(vector, dst_label, template)

It's already possible to access console templates via ALERTS, so we may push users that way rather than making it too easy to get them as there will be performance issues.

Console templates don't currently have case functions, but they're easy to add.

@beorn7

This comment has been minimized.

Copy link
Member

beorn7 commented Apr 14, 2016

That makes sense to me.

@juliusv

This comment has been minimized.

Copy link
Member

juliusv commented Apr 14, 2016

Currently that'd introduce a circular dependency between the template and promql packages, because of the query template function. So introducing this would require refactoring some things into a third package. Just as an additional thing to consider when thinking about whether it's worth it.

@grobie

This comment has been minimized.

Copy link
Member Author

grobie commented Apr 15, 2016

I like the proposal, it's basically a more flexible label_replace.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.