Rule formatting tool.

[juliusv]

Like "gofmt" for Go, we ought to have a "promfmt" for Prometheus since we have a syntax tree. The idea being that the system produces uniform style that minimizes deviation and learning curve.

Update after we have totally moved to YAML rule files: In addition to formatting the PromQL expressions, we also want to format the YAML files to have a fixed structure, while preserving comments for both PromQL expressions and the YAML file.

[grobie]

:+1: * :100:

[fabxc]

Yep, that'd be great. Preserving comments is the biggest (and basically only) problem. And agreement on how to split expressions over multiple lines.

[brian-brazil]

Aren't we in the middle of adding this to promtool?

[mattbostock]

Related: https://github.com/prometheus/prometheus/pull/1779

[beorn7]

Some observations:

• With the 2.x rule format (YAML embedding PromQL), we want this to act on YAML files, too.
• It should preserve both comments in YAML and in PromQL.
• Indentation is crucial (not just line breaks).
• It has to be seen if certain line breaks in the input should be preserved in the output (cf. how gofmt does it).
• Wherever a rule is rendered, the same formatting should apply, in particular on the Alerts and Rules tabs of the Prometheus web UI. This implies that the AST needs to keep comments (and possibly certain line breaks, see previous point).

[brian-brazil]

It should preserve both comments in YAML

Our current YAML library doesn't do this, but is hoping to "soon".

[geekodour]

I'd like to include this in my gsoc proposal. @brian-brazil, can you link to the YAML library you're referring to?

[brian-brazil]

https://github.com/go-yaml/yaml, but I'd scope this to just the PromQL as this stands.

[juliusv]

@brian-brazil But since the PromQL from rules is always embedded in YAML files, it at least has to preserve YAML comments, right? Otherwise the tool wouldn't be very useful as nobody wants to lose their comments the benefit of formatting.

[brian-brazil]

That'd be nice, but we can get benefits without that. I'd rather also not depend on something with an unclear timeline for a gsoc project.

[beorn7]

Let's start with the PromQL part. The YAML part should be almost trivial once the library preserves comments and things like the |2 indentation hint for multiline strings.

[juliusv]

Ok!

[brian-brazil]

v3 of the YAML library is out, which should allow for all of this.

[sylr]

FYI perfectly readable rule in configuration

(
    kube_job_status_failed > 0
    UNLESS kube_job_status_succeeded > 0
)
* on (namespace, job_name) group_left(maintainer) label_replace(kube_job_labels, "maintainer", "$1", "label_maintainer", "(.*)")
* on (namespace, job_name) group_left(pager) label_replace(kube_job_labels, "pager", "$1", "label_pager", "(.*)")
* on (namespace, job_name) group_left(paging) label_replace(kube_job_labels, "paging", "$1", "label_paging", "(.*)")

VS its garbage representation in the Web UI

(kube_deployment_status_replicas_available
  / kube_deployment_spec_replicas * 100) * on(namespace, deployment) group_left(maintainer)
  label_replace(kube_deployment_labels, "maintainer", "$1", "label_maintainer",
  "(.*)") * on(namespace, deployment) group_left(pager) label_replace(kube_deployment_labels,
  "pager", "$1", "label_pager", "(.*)") * on(namespace,
  deployment) group_left(paging) label_replace(kube_deployment_labels, "paging",
  "$1", "label_paging", "(.*)") < 100

[geekodour]

@sylr I think you posted some other rule from the web UI. also @beorn7 I am starting to work on this issue.

here's what it shows for me for your rule:

(kube_job_status_failed
  > 0 unless kube_job_status_succeeded > 0) * on(namespace, job_name) group_left(maintainer)
  label_replace(kube_job_labels, "maintainer", "$1", "label_maintainer",
  "(.*)") * on(namespace, job_name) group_left(pager) label_replace(kube_job_labels,
  "pager", "$1", "label_pager", "(.*)") * on(namespace,
  job_name) group_left(paging) label_replace(kube_job_labels, "paging", "$1",
  "label_paging", "(.*)")

[sylr]

@geekodour Yes, I mixed up two rules.

[haibeey]

since this is still open and added in project ideas for upcoming GSOC (https://github.com/cncf/soc#rule-formatting-tool) i would like to work on the completion of this during GSOC. Any recent update and additional info regarding this would be appreciated @codesome @brian-brazil @juliusv @geekodour @simonpasquier @sylr . From reading the comment and other reference link it seems the next action point is building a formatter for promQL expression with the promQL AST?

[slrtbtfs]

@haibeey There already has been some progress in that direction.

When building such a formatter it probably would be best to start with what is already implemented here and add proper white space and comment handling.

It would be also nice, when such features could be integrated with the upcoming PromQL language server. Inside the language server code a lot of the stuff that is needed to format YAML files is already implemented, and it might make sense to reuse some of it.

[haibeey]

alright! Thanks @slrtbtfs . i will start perusing the links shared ASAP .

[Harkishen-Singh]

Can I take up this issue? Would love to implement this.

[roidelapluie]

It looks like @haibeey is already somehow planning working on this.

[Harkishen-Singh]

@roidelapluie I have already implemented a part of it way before. It's just that I had not mentioned here.

[haibeey]

FYI perfectly readable rule in configuration

(
    kube_job_status_failed > 0
    UNLESS kube_job_status_succeeded > 0
)
* on (namespace, job_name) group_left(maintainer) label_replace(kube_job_labels, "maintainer", "$1", "label_maintainer", "(.*)")
* on (namespace, job_name) group_left(pager) label_replace(kube_job_labels, "pager", "$1", "label_pager", "(.*)")
* on (namespace, job_name) group_left(paging) label_replace(kube_job_labels, "paging", "$1", "label_paging", "(.*)")

VS its garbage representation in the Web UI

(kube_deployment_status_replicas_available
  / kube_deployment_spec_replicas * 100) * on(namespace, deployment) group_left(maintainer)
  label_replace(kube_deployment_labels, "maintainer", "$1", "label_maintainer",
  "(.*)") * on(namespace, deployment) group_left(pager) label_replace(kube_deployment_labels,
  "pager", "$1", "label_pager", "(.*)") * on(namespace,
  deployment) group_left(paging) label_replace(kube_deployment_labels, "paging",
  "$1", "label_paging", "(.*)") < 100

Is this formatting style acceptable? @codesome @juliusv @brian-brazil @beorn7 I'm writing my proposal based on this style.

[codesome]

The final representation after formatting can be discussed and decided during the GSoC period (maybe even now if any maintainer wants to chime in, but I don't have the bandwidth to review it at the moment). I would suggest (to all GSoC aspirants) to focus on how would you achieve this.

[brian-brazil]

https://prometheus.io/docs/practices/rules/ uses the best practices.

[haibeey]

i wrote a rough prototype to preserve comments by augmenting the goyacc grammar rules a little. commit here https://github.com/haibeey/prometheus/commit/885566786ac20bfd400b2e7db470c92545919690

Rules file recording rule in web ui

[brian-brazil]

That doesn't look right, the operators aren't on their own line

[haibeey]

oh yes. that commit doesn't do formmating. it is just to preserve comments in promql expr after evaluation for printing. previous versions ignores all comments after evaluation.

[slrtbtfs]

i wrote a rough prototype to preserve comments by augmenting the goyacc grammar rules a little. commit here [haibeey@8855667]

Putting the comments in the AST seems like a reasonable idea.

I've left some comments about the implementation there.

[sylr]

https://prometheus.io/docs/practices/rules/ uses the best practices.

Please don't enforce:

sum without (instance)(instance_path:requests:rate5m{job="myjob"})

over

sum(instance_path:requests:rate5m{job="myjob"}) without (instance)