Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
jrrdev opened this issue Jan 26, 2023 · 5 comments
Open

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

jrrdev opened this issue Jan 26, 2023 · 5 comments
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.

Comments

@jrrdev
Copy link

jrrdev commented Jan 26, 2023

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
{
 "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

  • docker-compose.yml:
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"
  • Run:
curl 'http://127.0.0.1:39090/api/v1/series'  -X POST -H 'X-Tenant-Id: foo' --data-urlencode "match[]=up" -o /dev/null -sS
  • See recorded HTTP query in ./logs/[date]/

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
Copy link

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
Copy link

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
Copy link
Author

jrrdev commented Jan 31, 2023

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
Copy link

John-Lin commented Jul 28, 2023

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

@simonpasquier simonpasquier added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug. labels Jul 28, 2023
@John-Lin
Copy link

John-Lin commented Jul 28, 2023

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

* `/api/v1/series` for GET method (Prometheus/Thanos)

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

mux.Handle("/api/v1/series", r.el.ExtractLabel(enforceMethods(r.matcher, "GET", "POST"))),

Does it relate to this PR? #111

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

No branches or pull requests

4 participants