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

Inconsistent behavoir when label already exists #162

Open
jacobbaungard opened this issue Sep 22, 2023 · 4 comments
Open

Inconsistent behavoir when label already exists #162

jacobbaungard opened this issue Sep 22, 2023 · 4 comments

Comments

@jacobbaungard
Copy link

The way prom-label-proxy deals with labels already existing differs between the query apis (/api/v1/query, /api/v1/query_range) and the non-query apis (like: /api/v1/series, /api/v1/labels ...), which can cause a little confusion.

Given the following setup:

./prom-label-proxy \
   -label "tenant_id" \
   -label-value prom_label_proxy \
   -upstream http://localhost:32788 \
   -insecure-listen-address 127.0.0.1:8081

Query APIs
A query such as:

curl "http://localhost:8081/api/v1/query?query=\{a=\"1\",tenant_id=\"tenant-01\"\}"

Is re-written to:

{a="1",tenant_id="prom_label_proxy"}

The user provided tenant_id label is silently replaced, by prom-label-proxy.

With -error-on-replace enabled, a 400 is returned:

{"error":"label matcher value (tenant_id=\"prom_label_proxy\") conflicts with injected value (tenant_id=\"tenant-01\")","errorType":"prom-label-proxy","status":"error"}

Non-query APIs

For non-query APIs, the behavior is a bit different, instead of silently replacing the existing label, the matcher is added on. For example:

curl -g 'http://127.0.0.1:8081/api/v1/series?' --data-urlencode 'match[]={a="1", tenant_id="1"}'

Results in the following match[] arguments being sent to Prometheus:

[a="1" tenant_id="1" tenant_id=~"prom_label_proxy"]
[tenant_id=~"prom_label_proxy"]

-error-on-replace has no effect on the behavior of the non-query APIs.

What is the desired functionality here? It seems like it would be nice for the behavior to be consistent between all endpoints?

@simonpasquier
Copy link
Contributor

I agree that there's room for improvement and the lack of consistency between the different endpoints is puzzling :)
Do you want to take a stab at it?

@jacobbaungard
Copy link
Author

I'm not sure I have a full overview of all the different use-cases, but seems like we'd probably need at least the following options ?

  • Error if label already exists
  • Silently replace label if it already exists
  • Keep existing labels, and append

That could be configured as one option, that applies to all endpoints, or two options, one for query endpoints, one for non-query endpoints. Not sure what makes most sense.

@simonpasquier
Copy link
Contributor

I'd rather make it consistent across all endpoints.

At the very least if -error-on-replace is specified, the proxy should always return an error if there's a label mismatch in the client request.

In the other case, I'd lean towards replacing the label silently but no strong opinion.

Regarding your curl command:
curl -g 'http://127.0.0.1:8081/api/v1/series?' --data-urlencode 'match[]={a="1", tenant_id="1"}'

Didn't you mean curl -G ...? I suspect that we're hitting the issue that #111 wants to fix.

@jacobbaungard
Copy link
Author

Sorry, this was a while already..

I think I probably did lowercase -g which meant it sent the param as a POST variable. So, likely I got the following, which yes I think is related to what #111 is talking about

post: [a="1" tenant_id="1" tenant_id=~"prom_label_proxy"]
query param: [tenant_id=~"prom_label_proxy"]

Although that was not really my main concern here, but more that prom-label-proxy is inconsistent in terms of the behavior of the label replacement on query/non-query APIs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants