jacksontj / promxy

An aggregating proxy to enable HA prometheus
MIT License
1.12k stars 127 forks source link

Supporting VictoriaMetric's Extended PromQL #253

Open AeroNotix opened 4 years ago

AeroNotix commented 4 years ago

There are some nice convenience functions here: https://github.com/VictoriaMetrics/VictoriaMetrics/wiki/ExtendedPromQL which make writing queries, alerts etc much simpler.

When I try to use them with promxy, I get errors such as:

Error executing query: invalid parameter 'query': parse error at char 3: unknown function with name "ru" for a query such as: ru(kubelet_volume_stats_available_bytes, kubelet_volume_stats_capacity_bytes).

I understand you are using the underlying prometheus libraries for a lot of the query layer and it's likely that layer which is bubbling this error - but would promxy consider adding support for VM's extended PromQL?

jacksontj commented 4 years ago

I'd definitely consider it, I actually have an issue upstream (https://github.com/VictoriaMetrics/VictoriaMetrics/issues/56) to move the extended promql stuff into a more re-usable library. At this point I don't have the driving use case to spend my time rearchitecting that library to work with promql's types, but I'm definitely for it being in there eventually, PRs are definitely welcome!

AeroNotix commented 4 years ago

Cool, worthwhile changing the labels to feature or similar?

jacksontj commented 4 years ago

Done :)

valyala commented 4 years ago

FYI, we already have a PR that moves extended PromQL code into a separate library - https://github.com/VictoriaMetrics/VictoriaMetrics/pull/256

valyala commented 4 years ago

FYI, PromQL parser from VictoriaMetrics has been extracted into a standalone library - see docs.

jacksontj commented 4 years ago

From a quick look it'll require some more work to be supported in promxy, as promxy actually uses a forked version of promql with support for NodeReplacer (a mechanism to rewrite the query tree post-parse pre-execution). I have patches to add that functionality on prometheus that I maintain, presumably we could add something similar here.

valyala commented 4 years ago

This sounds good!

valyala commented 4 years ago

FYI, the MetricsQL parser has been moved to a separate repository with its own release schedule - https://github.com/VictoriaMetrics/metricsql . This should significantly simplify its' integration into third-party projects such as Promxy.

jeromegn commented 3 years ago

I'd be very interested in this.

@jacksontj if you can point me at the right bits of code, I'm sure i could come up with a PR.

My company would be happy to sponsor this issue if you'd prefer that.

jacksontj commented 3 years ago

The place that I think we'd want to implement this is to implement the Engine interface (https://github.com/jacksontj/promxy/blob/master/cmd/promxy/main.go#L188) and then add an option to swap between those.

TBH I haven't spent a lot of time looking into it so I'm not sure exactly how difficult it'll be. I'd be more than happy to review a PR and if you are interested in other options we can discuss those as well :)

sdryga commented 3 years ago

I use Promxy in conjunction with VictoriaMetrics. I really lack the support of MetricsQL in Promxy. I'll be glad to see this in the next release. Hope it'll be implemented soon! )

davidsome commented 3 years ago

@jacksontj I use Promxy in conjunction with VictoriaMetrics too. I really lack the support of MetricsQL in Promxy. I'll be glad to see this in the next release. Hope it'll be implemented soon! )

dnull88 commented 3 years ago

+1 for supporting of MetricsQL in Promxy

IrekFasikhov commented 3 years ago

+1 for supporting of MetricsQL in promxy

IrekFasikhov commented 3 years ago

@jacksontj Hi, Thomas. Need some help for the implementation?

ihard commented 2 years ago

+1 very demanded functionality

jc16180 commented 2 years ago

+1 Would love to see this implemented. Only thing stopping me from switching to promxy. 🙂

anvpetrov commented 2 years ago

+1 for supporting of MetricsQL in promxy

autokilla47 commented 2 years ago

+1 for supporting of MetricsQL in promxy

valdis932 commented 2 years ago

+1

AeroNotix commented 2 years ago

yikes, lads, +1 city in here.

Pull out the editor and get to work. Sheesh.

AeroNotix commented 2 years ago

https://github.com/jacksontj/promxy/issues/253#issuecomment-569083961 @jacksontj where are these patches?

G-Asura commented 2 years ago

+1 for supporting of MetricsQL in promxy

sc7565 commented 2 years ago

+1 for supporting of MetricsQL in promxy

tiagosanto737 commented 2 years ago

+1 for supporting MetricsQL in promxy. This is extremely important for us to be able to use promxy in our production environment that has multiple victoriametrics clusters. We have 1000s of dashboards built using promQL and metricsQL and can not change them.

hagen1778 commented 2 years ago

Does anyone work on this? If no, I'd like to take it)

V1pr commented 1 year ago

Does anyone work on this? If no, I'd like to take it)

@valyala were you able to make any progress? :)

psalaberria002 commented 1 year ago

Related, but not the same. With the 0.79.0 release we are getting the following error when querying absent_over_time against a Victoria Metrics backend.

expanding series: remote server http://vmselect.victoriametrics:8481/select/1/prometheus/api/v1/read returned HTTP status 400 Bad Request: remoteAddr: "10.24.11.33:51258"; requestURI: /select/1/prometheus/api/v1/read; unsupported path requested: "/select/1/prometheus/api/v1/read"

I understand VM doesn't support remote read. Any workaround?

oOHenry commented 1 year ago

I understand VM doesn't support remote read. Any workaround?

set remote_read: false in promxy config ;)

mohammadkhavari commented 1 year ago

Hey, Is it still in progress? @valyala @jacksontj

sergeyshevch commented 9 months ago

Hi! We're very interested in this feature and I will try to make PR within the next few days. @jacksontj Can you please guide me to the right entry point for such modifications?

Is this still relevant? (3 years after) https://github.com/jacksontj/promxy/issues/253#issuecomment-662597713

sergeyshevch commented 9 months ago

After codebase research, I've found the following points.

It will be great if VictoriaMetrics has some QueryEngine implementation on their own like the Thanos project. https://github.com/thanos-io/promql-engine

isodude commented 7 months ago

Does it not make sense to just support MetricsQL out of the box, but only if downstream targets support it? I.e. adding a QueryLanguage variable to the ServerGroup model. If the query is to be sent to that one, MetricsQL is used to parse.

jacksontj commented 6 months ago

@isodude to be clear here; it would be nice to add support, but as @sergeyshevch mentioned there are a few issues with doing so.

In addition to the list @sergeyshevch mentioned (specifically adding support for a different engine in Options). We'd also need to add support for a NodeReplacer type interface -- as this is the main "secret sauce" that makes promxy performance what it is. This is what requires me to maintain a fork of prometheus (as it doesn't support this either).