prometheus-community / prom-label-proxy

A proxy that enforces a given label in a given PromQL query.
Apache License 2.0
249 stars 95 forks source link

POST /api/v1/series: Incorrect match[] param set in the URL query #134

Open jrrdev opened 1 year ago

jrrdev commented 1 year ago

Summary

I was testing the prom-label-proxy v0.6.0 + Grafana 9.1.8 and I noticed that when using HTTP POST method with the /api/v1/series endpoint, prom-label-proxy is passing two different value for the match[] parameter: one in the POST body and the other one in the URL query parameters. This leads to incorrect results. For instance the thanos querier seems to read only the match[] value in the query parameter in this situation which is pretty much garbage.

For instance if we are doing:

curl 127.0.0.1:39090/api/v1/series  -X POST -H 'X-Tenant-Id: foo' --data-urlencode "match[]=up"

Then the resulting HTTP query sent by prom-label-proxy is (in JSON format as recorded by gohrec and URL-decoded) will be:

{
  "Body": "match[]={__name__=\"up\",tenant_id=\"foo\"}",
  "URI": "/api/v1/series?match[]={tenant_id=\"foo\"}"
}

Depending on its implementation, the upstream may only read the match[] value set in the URI (which is the case for the Thanos querier). Even if it is true that the upstream should only read the match[] value in the request body with Content-Type: application/x-www-form-urlencoded, this is still an issue in the way prom-label-proxy is writing the resulting query.

Full response recorded by gohrec ```json { "Protocol": "HTTP/1.1", "Headers": [ "Accept-Encoding: gzip", "Accept: */*", "Content-Length: 61", "Content-Type: application/x-www-form-urlencoded", "User-Agent: curl/7.58.0", "X-Forwarded-For: 172.29.0.1", "X-Tenant-Id: foo" ], "ContentLength": 61, "Body": "match[]={__name__=\"up\",tenant_id=\"foo\"}", "Trailers": [], "TransferEncodings": null, "RemoteAddr": "172.29.0.2:39070", "Host": "127.0.0.1:39090", "Method": "POST", "Path": "/api/v1/series", "Query": [ "match[]: {tenant_id=\"foo\"}" ], "URI": "/api/v1/series?match[]={tenant_id=\"foo\"}" } ```

Steps to reproduce

version: '3'

services:
  http-recorder:
    image: frxyt/gohrec:latest
    command: ["record", "--listen=:8080"]
    volumes:
      - ./logs:/gohrec/log

  prom-label-proxy:
    image: quay.io/prometheuscommunity/prom-label-proxy:v0.6.0
    ports:
      - 127.0.0.1:39090:39090
    command:
      - "-header-name=X-Tenant-Id"
      - "-label=tenant_id"
      - "-upstream=http://http-recorder:8080"
      - "-insecure-listen-address=0.0.0.0:39090"
      - "-enable-label-apis"

Expected behavior

The HTTP request sent to the upstream should be:

POST /api/v1/series HTTP/1.1
X-Tenant-Id: foo
Content-Type: application/x-www-form-urlencoded

match[]={__name__="up",tenant_id="foo"}

instead of

POST /api/v1/series?match%5B%5D=%7Btenant_id%3D%22foo%22%7D HTTP/1.1
X-Tenant-Id: foo
Content-Type: application/x-www-form-urlencoded

match[]={__name__="up",tenant_id="foo"}
logamanig commented 1 year ago

encountering the same issue and unable to use prom label proxy due to the incorrect results in query variable and huge performance impact (10-15 seconds for 2 query variable) for showing the query variable in grafana as the results are INACCURATE with huge number of incorrect metric results.

logamanig commented 1 year ago

does anyone know any workaround resolve this? I'm relying on prom-label-proxy to control access per tenant, but due to performance issue, it affects my implementation as well. It would be much appreciated if anyone have a quick fix

jrrdev commented 1 year ago

If you are using Grafana, a working workaround is to configure the "HTTP method" field of the datasource to GET instead of POST. I tested this workaround successfully with prom-label-proxy v0.6.0 + Grafana 9.1.8

John-Lin commented 11 months ago

Same here. The /api/v1/series endpoint via proxy will return 500 after sending a POST method.

Grafana 9.5.5 prom-label-proxy 0.7.0

John-Lin commented 11 months ago

While reading the README I came across this endpoint support GET method:

https://github.com/prometheus-community/prom-label-proxy/blob/c1160c9da84071b60ae9057906ca31fef484ded8/README.md?plain=1#L51

however from the code it seems that it should support GET and POST methods

https://github.com/prometheus-community/prom-label-proxy/blob/c1160c9da84071b60ae9057906ca31fef484ded8/injectproxy/routes.go#L291

Does it relate to this PR? https://github.com/prometheus-community/prom-label-proxy/pull/111