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

.*: /api/v1/label/<name>/values matchers not working #6959

Closed
Jakob3xD opened this issue Dec 4, 2023 · 8 comments
Closed

.*: /api/v1/label/<name>/values matchers not working #6959

Jakob3xD opened this issue Dec 4, 2023 · 8 comments

Comments

@Jakob3xD
Copy link
Contributor

Jakob3xD commented Dec 4, 2023

Thanos, Prometheus and Golang version used:
Thanos: quay.io/thanos/thanos:v0.32.5
Prometheus: quay.io/prometheus/prometheus:v2.47.2

Object Storage Provider:
Ceph S3

What happened:
Similar to #6878 I get unexpected amount of returned fields when accessing /api/v1/label/<name>/values.

curl api.com/api/v1/label/region/values?match[]=opensearch_cluster_status{monitor="opensearch", cluster="fsn1-dc14-opensearch1"}&start=1701065544&end=1701076344
{
  "status":"success",
  "data":["ash","fsn1","hel1","hil","nbg1"]
}

What you expected to happen:
I expect to only get the label value "fsn1" but instead I get all regions.

How to reproduce it (as minimally and precisely as possible):

Anything else we need to know:

Query Range with the same query and time range. I reduced the json output to keep it simple. There are some more values returned and some more nodes that all have similar labels like in the example below. Only the instance and exported_instance changes.

Query

curl api.com/api/v1/query_range?query=opensearch_cluster_status{monitor%3D"opensearch"%2C cluster%3D"fsn1-dc14-opensearch1"}&start=1701065544&end=1701076344&step=30s
{
  "status": "success",
  "data": {
    "resultType": "matrix",
    "result": [
      {
        "metric": {
          "__name__": "opensearch_cluster_status",
          "cluster": "fsn1-dc14-opensearch1",
          "exported_instance": "172.27.9.1:9200",
          "exported_job": "opensearch",
          "instance": "fsn1-dc14-opensearch1-x1.<someurl>",
          "job": "internal",
          "monitor": "opensearch",
          "region": "fsn1"
        },
        "values": [
          [
            1701065520,
            "0"
          ]
        ]
      },
      {
        "metric": {
          "__name__": "opensearch_cluster_status",
          "cluster": "fsn1-dc14-opensearch1",
          "exported_instance": "172.27.9.2:9200",
          "exported_job": "opensearch",
          "instance": "fsn1-dc14-opensearch1-x1.<someurl>",
          "job": "internal",
          "monitor": "opensearch",
          "region": "fsn1"
        },
        "values": [
          [
            1701065520,
            "0"
          ]
        ]
      },
      {
        "metric": {
          "__name__": "opensearch_cluster_status",
          "cluster": "fsn1-dc14-opensearch1",
          "exported_instance": "172.27.9.3:9200",
          "exported_job": "opensearch",
          "instance": "fsn1-dc14-opensearch1-x2.<someurl>",
          "job": "internal",
          "monitor": "opensearch",
          "region": "fsn1"
        },
        "values": [
          [
            1701065520,
            "0"
          ]
        ]
      },
      {
        "metric": {
          "__name__": "opensearch_cluster_status",
          "cluster": "fsn1-dc14-opensearch1",
          "exported_instance": "172.27.9.4:9200",
          "exported_job": "opensearch",
          "instance": "fsn1-dc14-opensearch1-x2.<someurl>",
          "job": "internal",
          "monitor": "opensearch",
          "region": "fsn1"
        },
        "values": [
          [
            1701065520,
            "0"
          ]
        ]
      }
    ]
  }
}

@MichaHoffmann
Copy link
Contributor

Fairly sure that this is solved by #6879 if its about external labels

@yeya24
Copy link
Contributor

yeya24 commented Dec 6, 2023

Thanks @MichaHoffmann. Can we close this issue now or we need more verification?

@GiedriusS
Copy link
Member

Closing because we believe it was fixed by #6879. Let us know if this persists.

@sepich
Copy link
Contributor

sepich commented Jan 20, 2024

I've tested thanosio/thanos:v0.34.0-rc.0 and it is still broken. But now instead of returning all external labels it returns nothing
{"status":"success","data":[]}
Steps to reproduce:
2 prometheuses write data to thanos-receive to separate tenants. Both of them have external_label prometheus=A and B
Only one of them has job="C".
Series request is correct:

$ curl -i 'localhost:10902/api/v1/series?match[]=up{job="C"}'
HTTP/1.1 200 OK
{"status":"success","data":[{"__name__":"up","instance":"node-exporter:9100","job":"C","prometheus":"A"}]}

But Labels request is broken:
v0.32.5

$ curl -i 'localhost:10902/api/v1/label/prometheus/values?match[]=up{prometheus="A"}'
HTTP/1.1 200 OK
{"status":"success","data":["A"]}

$ curl -i 'localhost:10902/api/v1/label/prometheus/values?match[]=up{job="C"}'
HTTP/1.1 200 OK
{"status":"success","data":["A","B"]}

v0.34.0-rc.0

$ curl -i 'localhost:10902/api/v1/label/prometheus/values?match[]=up{prometheus="A"}'
HTTP/1.1 200 OK
{"status":"success","data":[]}

$ curl -i 'localhost:10902/api/v1/label/prometheus/values?match[]=up{job="C"}'
HTTP/1.1 200 OK
{"status":"success","data":[]}

Correct response in both cases should be: {"status":"success","data":["A"]}
This does not work forever: grafana/grafana#76743

@MichaHoffmann
Copy link
Contributor

MichaHoffmann commented Jan 20, 2024

Thanks for the reproducer, let me take a look !

Edit: I cannot reproduce quickly with this test, the test passes 🤔

		{
			desc: "issue",
			appendFn: func(app storage.Appender) {
				_, err := app.Append(0, labels.FromStrings("__name__", "up", "job", "C", "prometheus", "A"), 0, 0)
				testutil.Ok(t, err)
				_, err = app.Append(0, labels.FromStrings("__name__", "up", "job", "C", "prometheus", "B"), 0, 0)
				testutil.Ok(t, err)
				testutil.Ok(t, app.Commit())
			},
			labelValuesCalls: []labelValuesCallCase{
				{
					start: timestamp.FromTime(minTime),
					end:   timestamp.FromTime(maxTime),
					label: "prometheus",
					matchers: []storepb.LabelMatcher{
						{Type: storepb.LabelMatcher_EQ, Name: "job", Value: "C"},
					},
					expectedValues: []string{"A", "B"},
				},
			},
		},

Edit2: Ok i think i found the issue!

@MichaHoffmann
Copy link
Contributor

I think #7082 should fix this; at least the new test showed that if we have series matchers that dont match the external label we would return empty result which is buggy.

@MichaHoffmann
Copy link
Contributor

@sepich I included the bugfix into 0.34.0-rc.1; can you test it out by any chance?

@sepich
Copy link
Contributor

sepich commented Jan 24, 2024

Thank you for working on this!
I've checked thanosio/thanos:v0.34.0-rc.1 and the issue above for matching on not external_label is fixed. Will try to deploy this, and test all other cases

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

5 participants