search

prometheus-community / promql-langserver

PromQL language server
Apache License 2.0
176 stars 18 forks source link

Linting support #48

Open GiedriusS opened 4 years ago

GiedriusS commented 4 years ago

Hello, thanks for this project! I hope that one day it will get merged into Prometheus. I had this one idea for a similar thing that you are trying to build here: it could return some kind of "hints" to the user about how to fix the query that they are writing. For example, imagine a case where the user writes irate(foo[15s]) with a scraping interval of 10s. This means that only one sample is in those 15 seconds hence no results would be returned to the user. The user could get some kind of "hint" that they must increase the time between the brackets to at least 20s.

The most promiment open question AFAICT in this case is the format of those hints: how they should look like? Also, as far as I can tell, you haven't considered this in your features list. What do you think?

slrtbtfs commented 4 years ago

Thanks for your request!

The most promiment open question AFAICT in this case is the format of those hints: how they should look like? Also, as far as I can tell, you haven't considered this in your features list. What do you think?

The LSP protocol allows publishing diagnostics, like errors or warnings, that are then displayed by the client, so that's actually the easiest part of your request.

There have been numerous request to provide some sort of linting and showing warnings about common errors to the user, so this is actually already on the radar, e.g. in #6 .

There will probably be some generic way to add diagnostics and documentation providers in the future. Once these exist, the feature you described could simply be one of them.

It is also planned to be able to connect the language server to a prometheus instance (#33, #5). For implementing this specific feature one could then simply request the scrape interval from the config api and use that to emit warnings, if appropriate.

I hope that one day it will get merged into Prometheus.

There are plans for that, see https://github.com/prometheus/prometheus/issues/6160

GiedriusS commented 4 years ago

Awesome to hear all of that! I have read through your documentation more closely and I haven't even realized before that a protocol exists for this kind of stuff. Either way, I hope that we will be able to leverage your work in the future on the https://github.com/thanos-io/thanos side - it would be very useful to tell our users why their queries might be returning no results or peculiar results! (:

slrtbtfs commented 4 years ago

We're getting kind of closer to be able to implement such a thing.

Another example found by @LiliC is the case of <expr> <bin_op> on (<labels>) topk(<expr>).

In this case the topk throws away data that should have been passed to the binary operator.

As an general approach to this it would make sense to have some kind of subtree matching, combined with a cache of the new metrics metadata API of Prometheus.

slrtbtfs commented 4 years ago

Another thing that a linter could warn against: https://github.com/prometheus/prometheus/issues/4543