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: adding toTime function to TemplateExpander #10993

Merged
merged 1 commit into from
Jul 14, 2022

Conversation

jrkt
Copy link
Contributor

@jrkt jrkt commented Jul 6, 2022

This PR adds a formatTime function in the TemplateExpander. As it stands now, the way to get the current time of a PromQL execution is to do something like this: {{ query "time()" | first | value || humanizeTime }}. This works great but only allows one format of time to be expressed. I'm proposing a simple function that does virtually the exact same thing as humanizeTime except that it allows you to pass in the format you expect to get back like this: {{ query "time()" | first | value || formatTime "2006-01-02 15:04:05" }}

@jrkt jrkt force-pushed the jrkt-format-time branch 4 times, most recently from cd2dc99 to 1397984 Compare July 6, 2022 19:36
@dgl
Copy link
Member

dgl commented Jul 7, 2022

Thanks, I think it would be better to instead add a "toTime" function that takes a unix epoch time and can convert it to a time.Time, that way other methods are available. (Formatting would look something like {{ (query "time()" | first | value | toTime).Format("...") }})

(This is similar to what is possible in alertmanager, e.g. prometheus/alertmanager#1188 (comment))

@jrkt
Copy link
Contributor Author

jrkt commented Jul 7, 2022

I can do that.

@jrkt
Copy link
Contributor Author

jrkt commented Jul 7, 2022

@dgl done.

@jrkt jrkt changed the title template: adding formatTime function to TemplateExpander template: adding toTime function to TemplateExpander Jul 8, 2022
@roidelapluie
Copy link
Member

Thanks!

  • Can you please add tests?
  • Can you please document this?
  • We could extract this logic to a convertToTime function, used in humanizeTimestamp and toTime

@jrkt jrkt force-pushed the jrkt-format-time branch 5 times, most recently from 2140509 to 7f3b18f Compare July 9, 2022 14:06
@jrkt
Copy link
Contributor Author

jrkt commented Jul 9, 2022

@roidelapluie I have separated out the pieces of the functions that I could. I had to leave convertToFloat out of the shared function since the converted float is used in an error case in humanizeTimestamp. Everything else has been broken out though. I also added tests and updated the documentation. I added a new "Time" section to the documentation since there are now two functions directly related to time. Let me know if I missed anything.

@jrkt jrkt force-pushed the jrkt-format-time branch 3 times, most recently from c20c19c to 331d24e Compare July 11, 2022 20:02
@jrkt
Copy link
Contributor Author

jrkt commented Jul 14, 2022

@roidelapluie is there anything I missed here?

template/template.go Outdated Show resolved Hide resolved
@jrkt
Copy link
Contributor Author

jrkt commented Jul 14, 2022

@roidelapluie requests resolved.

@jrkt jrkt force-pushed the jrkt-format-time branch 2 times, most recently from 567f922 to 613de5d Compare July 14, 2022 16:50
Signed-off-by: Jonathan Stevens <jonathanstevens89@gmail.com>
Signed-off-by: Jonathan Stevens <jon.stevens@getweave.com>
@roidelapluie roidelapluie merged commit ce1bf8b into prometheus:main Jul 14, 2022
@roidelapluie
Copy link
Member

Thanks!

@jrkt jrkt deleted the jrkt-format-time branch July 14, 2022 23:16
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this pull request Aug 14, 2022
valyala added a commit to VictoriaMetrics/VictoriaMetrics that referenced this pull request Aug 14, 2022
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.

None yet

3 participants