Display rules as defined in configuration

[sylr]

Proposal

Use case. Why is this important?

Prometheus rules can be quite complex thus the use of comments, new lines, uppercased keywords ... etc can help make it easier to understand.

E.G:

(
    # Pod is down and rename label "kubernetes_pod_name" to "pod"
    label_replace(up{job="kubernetes-pods"}, "pod", "$1", "kubernetes_pod_name", "(.*)")
    *
    # Add node label to up{}
    on (pod) group_left(node) avg(kube_pod_info) by (pod, node)
) == 0
AND
# Node is ready
on (node) kube_node_status_condition{condition="Ready",status="true"} == 1

Unfortunately the Prometheus UI does not display the rules as they were written but like if a "sanitizer" have been applied, i.e.:

(label_replace(up{job="kubernetes-pods"},
  "pod", "$1", "kubernetes_pod_name", "(.*)") * on(pod)
  group_left(node) avg by(pod, node) (kube_pod_info)) == 0 and on(node) kube_node_status_condition{condition="Ready",status="true"}
  == 1
)

The latter is really quite indigest so it would be nice if you could keep the formatting as it.

[brian-brazil]

This is a limitation of the YAML parser we use, once it's better we can do better.

[sylr]

The YAML parser seems capable to handle comments and multiline strings quite all right:

package main

import (
	"fmt"

	yaml "gopkg.in/yaml.v2"
)

type T struct {
	A string `yaml:"a"`
}

var (
	conf = `a: |
  (
    # Pod is down and rename label "kubernetes_pod_name" to "pod"
    label_replace(up{job="kubernetes-pods"}, "pod", "$1", "kubernetes_pod_name", "(.*)")
    *
    # Add node label to up{}
    on (pod) group_left(node) avg(kube_pod_info) by (pod, node)
  ) == 0
  AND  # Node is ready
  on (node) kube_node_status_condition{condition="Ready",status="true"} == 1`
)

func main() {
	t := T{}
	yaml.UnmarshalStrict([]byte(conf), &t)
	fmt.Printf("%s\n-----------\n", conf)
	fmt.Printf("%s\n", t.A)
}

a: |
  (
    # Pod is down and rename label "kubernetes_pod_name" to "pod"
    label_replace(up{job="kubernetes-pods"}, "pod", "$1", "kubernetes_pod_name", "(.*)")
    *
    # Add node label to up{}
    on (pod) group_left(node) avg(kube_pod_info) by (pod, node)
  ) == 0
  AND  # Node is ready
  on (node) kube_node_status_condition{condition="Ready",status="true"} == 1
-----------
(
  # Pod is down and rename label "kubernetes_pod_name" to "pod"
  label_replace(up{job="kubernetes-pods"}, "pod", "$1", "kubernetes_pod_name", "(.*)")
  *
  # Add node label to up{}
  on (pod) group_left(node) avg(kube_pod_info) by (pod, node)
) == 0
AND  # Node is ready
on (node) kube_node_status_condition{condition="Ready",status="true"} == 1

[brian-brazil]

Ah, you're looking at the rules page. Those are what the PromQL printer produces. On a page like that it's best to have the actual AST, rather than what may be a confusingly written expression.

The YAML parser seems capable to handle comments and multiline strings quite all right:

Actually that's all multi-line strings. PromQL supports # comments.

[sylr]

I'm talking at the alerts page really but it's the same problem.

On a page like that it's best to have the actual AST, rather than what may be a confusingly written expression.

I beg to differ, when you write a rule with indentation, comments and such to make it understandable and easily editable, having it reduced to it's simplest form is what is confusing.

[brian-brazil]

That's presuming you get the indentation etc. right. When debugging knowing how it's actually parsed is important.

[sylr]

That's presuming you get the indentation etc. right.

That's not a valid concern, a simple <pre> block would be enough.

When debugging knowing how it's actually parsed is important.

Maybe a few prometheus developpers are actually debugging prometheus query parser, users are concerned to understand the sens of query that have been written by their colleagues.

[brian-brazil]

That's not a valid concern, a simple

 block would be enough.

I don't see how that helps. If someone's has misleading indentation, it'll still be misleading.

Maybe a few prometheus developpers are actually debugging prometheus query parser, users are concerned to understand the sens of query that have been written by their colleagues.

I meant debugging queries, not Prometheus.

[sylr]

If someone's has misleading indentation, it'll still be misleading.

And so much for those who get it right ? Your leveling down philosophy is disturbing.

I think you've been embedded in this project for a long time now and you can't comprehend anymore the complexity of PromQL and how it is struggling for occasional users.

I've been writing the alert rules for the system of my company and improving them along the way made them more and more complex. I recently showed the more complex ones to my on call colleagues (the ones those very rules will be awaking at night) and they looked back at me with disgust.

Indentation & comments are essential for my colleagues to understand what's underlying the alerts.

[brian-brazil]

And so much for those who get it right ?

This is from experience, it's not the ones that get it right that I have to spend time on.

The rules are on disk if you wish to view them.

[brian-brazil]

Which isn't to say the current printer couldn't be improved with some indentation and newlines.

[sylr]

[brian-brazil]

That's non-standard indentation, anything on this page should be in a canonical form.

[sylr]

No, it should be as written in configuration.

[roidelapluie]

I agree with @brian-brazil

We have lots of complex PromQL and it helps to see how Prometheus "compiles" that. I do not want to see my "garbage" in the console. I have the files for it.

[sylr]

That's the problem here, you only think of yourselves and don't think about how other people might need to understand your rules in the middle of the night.

[roidelapluie]

That is not true. I am making heavy usage of annotations for on call people. I even implemented the option to see the annotations of the alerts on that page to help on call.

[roidelapluie]

(And on call also has access to the rule files + ansible repos.. with the original queries)

[beorn7]

I think the general idea is to have some kind of promfmt, similar to gofmt. Prometheus would format the parsed rules with promfmt to show the canonical indentation and formatting. In turn, humans writing rules can also use promfmt (similar to writing Go code, for example upon saving a rules file), so that everywhere the same canonical formatting would be used and nobody is confused.

Note that gofmt preserves comments and (certain) line breaks. promfmt (and the parsing in general) should do the same, both for comments in YAML as well as within PromQL expressions.

[beorn7]

Aaand this idea is actually ancient – which I knew, but I forgot we have an issue for it: #21. It is, in fact, the second oldest still open issue…

[beorn7]

I amended #21 with some observations from the discussion here.

Would it be OK to close this issue in lieu of #21?

[sylr]

All right.