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

template: Add reReplaceAll function #639

Merged
merged 1 commit into from
Mar 4, 2017
Merged

template: Add reReplaceAll function #639

merged 1 commit into from
Mar 4, 2017

Conversation

jamesog
Copy link
Contributor

@jamesog jamesog commented Feb 28, 2017

This allows for using string replace inside templates.

e.g.:

{{ replace .GroupLabels.alertname "_" " " -1 }}

@jamesog
Copy link
Contributor Author

jamesog commented Feb 28, 2017

I made this change from the v0.4.2 tag so submitted the PR against the release-0.4 branch, but not sure if this would be wanted against another (newer) branch.

@brian-brazil
Copy link
Contributor

This should go against master.

This should follow the naming and semantics of Prometheus templates: https://prometheus.io/docs/visualization/template_reference/#strings

@stuartnelson3
Copy link
Contributor

@jamesog can you branch from master and add this change? v0.4.x isn't being supported.

@jamesog
Copy link
Contributor Author

jamesog commented Mar 3, 2017

Yep, will do. I've been sick the last couple of days but I'll get to it ASAP.

Can you clarify what you mean about the naming? I was following the example of the join function.

@brian-brazil
Copy link
Contributor

reReplaceAll is the function name in Prometheus.

@jamesog
Copy link
Contributor Author

jamesog commented Mar 3, 2017

Would stringReplace be acceptable?

@brian-brazil
Copy link
Contributor

The naming should be consistent across both.

@jamesog
Copy link
Contributor Author

jamesog commented Mar 3, 2017

Right, but I assume the re in reReplaceAll refers to the fact that it's using regexp.ReplaceAll. In this case I'm using strings.Replace, so I'm just trying to clarify how you would want to name it in this case, or whether you'd prefer it to use regexp instead of strings.

@brian-brazil
Copy link
Contributor

Stick with the regex, if someone has a use case not covered by that we can consider it then.

This allows for using regexp replacement in templates.
@jamesog jamesog changed the base branch from release-0.4 to master March 4, 2017 11:46
@jamesog jamesog changed the title template: Add replace function template: Add reReplaceAll function Mar 4, 2017
@jamesog
Copy link
Contributor Author

jamesog commented Mar 4, 2017

Thanks very much, appreciate the clarification. I've made the change to use reReplaceAll as in Prometheus. I haven't been able to test this yet as we're still using 0.4.2 in production, but I'll see if I can do that now.

@jamesog
Copy link
Contributor Author

jamesog commented Mar 4, 2017

That's tested now and working as expected.

@brian-brazil brian-brazil merged commit 00e3b98 into prometheus:master Mar 4, 2017
@brian-brazil
Copy link
Contributor

Thanks!

@gaelreyrol
Copy link

This feature is not included in the latest available release which is obvious but kind of confusing for newbies like me !

@jamesog jamesog deleted the template-replace branch April 1, 2017 19:20
hh pushed a commit to ii/alertmanager that referenced this pull request Aug 17, 2017
* Switch to kingpin flags

* Fix logrus vendoring

* Fix flags in main tests

* Fix vendoring versions
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.

4 participants