PromQL formatting/prettifying support in promtool

[Harkishen Singh]

Hello everyone!

As part of the GSoC 2020, I am working on designing a Promql expression formatting/prettifying tool whose support will be as an extension in the current promtool. A design document related to the same has been made and it would be great for some comments/views/suggestions, etc.

Here is the link to the document: PromQL prettier

[Tobias Schmidt]

Thanks a lot for your great work! Expression formatting will likely require dozens of detailed rules in order to get things consistent, and style discussions are the perfect case for bikeshedding. I really appreciate your efforts and can't wait for a `promtool fmt` on-save editor integration. The proverb of Go has arguably held true: Gofmt's style is no one's favorite, yet gofmt is everyone's favorite.

I hope we can get it right without having to make (large) changes in later releases. The most annoying thing with auto-formatters is changing rules with every release creating constant diff noise (looking at you rubocop).

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/0e1b1867-b818-4afe-a970-1bbc21046844o%40googlegroups.com.

[Ben Kochie]

One subject that I don't see covered in the doc is spacing. There are several places where whitespace is optional, and it would be good to have a consistent opinion on it.

* Between label selectors {foo="foo",bar="bar"} vs {foo="foo", bar="bar"}

* Between params: histogram_quantile(0.9, rate(...))

* Between aggregation operator modifiers: sum without(label)(metric_name) vs sum without(label) (metric_name) vs sum without (label) (metric_name)

My personal prefernce is

* No whitespace between label selectors.

* A single whitespace between params.

* Whitespace on both sides of operator modifiers. IE sum without (label) (metric_name)

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAChBsdDHBjphxKUc_%3D7bcKuPoorGPxiy5duYqzvXM%2B3jNoNC%3Dw%40mail.gmail.com.

[Julius Volz]

On Tue, Jun 23, 2020 at 9:47 AM Ben Kochie <sup...@gmail.com> wrote:

One subject that I don't see covered in the doc is spacing. There are several places where whitespace is optional, and it would be good to have a consistent opinion on it.

* Between label selectors {foo="foo",bar="bar"} vs {foo="foo", bar="bar"}

* Between params: histogram_quantile(0.9, rate(...))

* Between aggregation operator modifiers: sum without(label)(metric_name) vs sum without(label) (metric_name) vs sum without (label) (metric_name)

My personal prefernce is

* No whitespace between label selectors.

* A single whitespace between params.

Agreed with those.

 

* Whitespace on both sides of operator modifiers. IE sum without (label) (metric_name)

I prefer "sum without(label) (expr)" :)

Additionally, there is the question of commas between labels in a label list like "by(foo,bar)" or "by(foo, bar)". I guess I prefer the latter, but not sure :P

 

On Sat, Jun 20, 2020 at 1:22 PM Tobias Schmidt <tob...@gmail.com> wrote:

Thanks a lot for your great work! Expression formatting will likely require dozens of detailed rules in order to get things consistent, and style discussions are the perfect case for bikeshedding. I really appreciate your efforts and can't wait for a `promtool fmt` on-save editor integration. The proverb of Go has arguably held true: Gofmt's style is no one's favorite, yet gofmt is everyone's favorite.

I hope we can get it right without having to make (large) changes in later releases. The most annoying thing with auto-formatters is changing rules with every release creating constant diff noise (looking at you rubocop).

On Fri, Jun 19, 2020 at 3:19 PM Harkishen Singh <harkishe...@gmail.com> wrote:

Hello everyone!

As part of the GSoC 2020, I am working on designing a Promql expression formatting/prettifying tool whose support will be as an extension in the current promtool. A design document related to the same has been made and it would be great for some comments/views/suggestions, etc.

Here is the link to the document: PromQL prettier

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/0e1b1867-b818-4afe-a970-1bbc21046844o%40googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAChBsdDHBjphxKUc_%3D7bcKuPoorGPxiy5duYqzvXM%2B3jNoNC%3Dw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CABbyFmocCQV0z9DiQ%2BDODmp0tC-ZNBGWgcf8GzsqaXJdGzc0Zg%40mail.gmail.com.

[Brian Brazil]

On Tue, 23 Jun 2020 at 22:19, Julius Volz <juliu...@gmail.com> wrote:

On Tue, Jun 23, 2020 at 9:47 AM Ben Kochie <sup...@gmail.com> wrote:

One subject that I don't see covered in the doc is spacing. There are several places where whitespace is optional, and it would be good to have a consistent opinion on it.

* Between label selectors {foo="foo",bar="bar"} vs {foo="foo", bar="bar"}

* Between params: histogram_quantile(0.9, rate(...))

* Between aggregation operator modifiers: sum without(label)(metric_name) vs sum without(label) (metric_name) vs sum without (label) (metric_name)

My personal prefernce is

* No whitespace between label selectors.

* A single whitespace between params.

Agreed with those.

 

* Whitespace on both sides of operator modifiers. IE sum without (label) (metric_name)

I prefer "sum without(label) (expr)" :)

Additionally, there is the question of commas between labels in a label list like "by(foo,bar)" or "by(foo, bar)". I guess I prefer the latter, but not sure :P

The latter is consistent with programming languages generally, and more readable.

Brian

 

 

On Sat, Jun 20, 2020 at 1:22 PM Tobias Schmidt <tob...@gmail.com> wrote:

Thanks a lot for your great work! Expression formatting will likely require dozens of detailed rules in order to get things consistent, and style discussions are the perfect case for bikeshedding. I really appreciate your efforts and can't wait for a `promtool fmt` on-save editor integration. The proverb of Go has arguably held true: Gofmt's style is no one's favorite, yet gofmt is everyone's favorite.

I hope we can get it right without having to make (large) changes in later releases. The most annoying thing with auto-formatters is changing rules with every release creating constant diff noise (looking at you rubocop).

On Fri, Jun 19, 2020 at 3:19 PM Harkishen Singh <harkishe...@gmail.com> wrote:

Hello everyone!

As part of the GSoC 2020, I am working on designing a Promql expression formatting/prettifying tool whose support will be as an extension in the current promtool. A design document related to the same has been made and it would be great for some comments/views/suggestions, etc.

Here is the link to the document: PromQL prettier

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/0e1b1867-b818-4afe-a970-1bbc21046844o%40googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CAChBsdDHBjphxKUc_%3D7bcKuPoorGPxiy5duYqzvXM%2B3jNoNC%3Dw%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CABbyFmocCQV0z9DiQ%2BDODmp0tC-ZNBGWgcf8GzsqaXJdGzc0Zg%40mail.gmail.com.

--
You received this message because you are subscribed to the Google Groups "Prometheus Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to prometheus-devel...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/prometheus-developers/CA%2BT6YoxoHhiVzuRXwymayce6GzdfP3%3DBcTY4ULfNqqSCuZMgog%40mail.gmail.com.

--

Brian Brazil

www.robustperception.io

[Harkishen Singh]

After discussions, I have updated the document with most of the comments addressed (except the ones that are in conflict). The whitespace issue is covered and is consistent now. A note section has been added to give a prior knowledge on the formatting on which the document is based.

Please comment if you find any inconsistency in the whitespace or any other aspect.

Thank you

Harkishen Singh

[Harkishen Singh]

With some discussions in the design docs with brian, it stills feels like a confusion that, do we want to split labels_matchers in vector_selector that are immediate child nodes to any binary expression?