Skip to content

Commit 2aedfa5

Browse files
authored
Merge pull request #8752 from Krishnachaitanyakc/implement/query-exemplar-proxy-strips-external-label-matcher
query: fix exemplar proxy stripping external label matchers in multi-tier topologies
2 parents 59a325d + 7b851b7 commit 2aedfa5

7 files changed

Lines changed: 167 additions & 5 deletions

File tree

.mdox.validate.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,10 @@ validators:
6161
type: 'ignore'
6262
- regex: 'codeburst\.io'
6363
type: 'ignore'
64+
# Promtail docs removed after EOL (March 2026).
65+
- regex: 'grafana\.com\/docs\/loki\/latest\/clients\/promtail'
66+
type: 'ignore'
67+
# Frequent DNS/timeout issues from CI.
68+
- regex: 'db\.cs\.cmu\.edu'
69+
type: 'ignore'
6470

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ It is recommend to upgrade the storage components first (Receive, Store, etc.) a
1414

1515
### Fixed
1616

17+
- [#8702](https://github.com/thanos-io/thanos/issues/8702): Query: Fix exemplar proxy stripping external label matchers in multi-tier query topologies. In Query A → Query B → Sidecar setups, external label matchers are now preserved when forwarding to downstream Query nodes so they can route to the correct stores.
1718
- [#8726](https://github.com/thanos-io/thanos/pull/8726): *: Bump `thanos-community/grpc-go` fork to fix CVE-2026-33186 (CVSS 9.1), an authorization bypass via malformed `:path` headers that could bypass path-based "deny" rules in `grpc/authz` interceptors.
1819
- [#8714](https://github.com/thanos-io/thanos/pull/8714): Tracing: Fix `tls_config` fields (`ca_file`, `cert_file`, `key_file`) being silently ignored when using the OTLP gRPC exporter. Previously, deployments using a private CA or mTLS client certificates had to work around this via `OTEL_EXPORTER_OTLP_CERTIFICATE` and related environment variables.
1920
- [#8128](https://github.com/thanos-io/thanos/issues/8128): Query-Frontend: Fix panic in `AnalyzesMerge` caused by indexing the wrong slice variable, leading to an out-of-range access when merging more than two query analyses.

docs/components/receive.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,7 +372,8 @@ Please see the metric `thanos_receive_forward_delay_seconds` to see if you need
372372

373373
The following formula is used for calculating quorum:
374374

375-
```go mdox-exec="sed -n '1068,1078p' pkg/receive/handler.go"
375+
```go mdox-exec="sed -n '1067,1078p' pkg/receive/handler.go"
376+
// writeQuorum returns minimum number of replicas that has to confirm write success before claiming replication success.
376377
func (h *Handler) writeQuorum() int {
377378
// NOTE(GiedriusS): this is here because otherwise RF=2 doesn't make sense as all writes
378379
// would need to succeed all the time. Another way to think about it is when migrating

pkg/exemplars/exemplarspb/custom.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ import (
1717
// ExemplarStore wraps the ExemplarsClient and contains the info of external labels.
1818
type ExemplarStore struct {
1919
ExemplarsClient
20-
LabelSets []labels.Labels
20+
LabelSets []labels.Labels
21+
SupportsExternalLabels bool
2122
}
2223

2324
// UnmarshalJSON implements json.Unmarshaler.

pkg/exemplars/proxy.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,10 +95,13 @@ func (s *Proxy) Exemplars(req *exemplarspb.ExemplarsRequest, srv exemplarspb.Exe
9595

9696
labelMatchers = labelMatchers[:0]
9797
for _, m := range matcherSet {
98-
if containsLabelName(m.Name, extLbls) {
98+
if !st.SupportsExternalLabels && containsLabelName(m.Name, extLbls) {
9999
// If the current matcher matches one external label,
100100
// we don't add it to the current metric selector
101101
// as Prometheus' Exemplars API cannot handle external labels.
102+
// However, if the downstream store supports external labels
103+
// (e.g., a Query node), we preserve the matchers so it can
104+
// use them for its own routing.
102105
continue
103106
}
104107
labelMatchers = append(labelMatchers, m.String())

pkg/exemplars/proxy_test.go

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,155 @@ func TestProxy(t *testing.T) {
379379
}
380380
}
381381

382+
// testExemplarClientWithQueryCapture extends testExemplarClient to capture the forwarded query.
383+
type testExemplarClientWithQueryCapture struct {
384+
testExemplarClient
385+
capturedQuery atomic.String
386+
}
387+
388+
func (t *testExemplarClientWithQueryCapture) Exemplars(ctx context.Context, in *exemplarspb.ExemplarsRequest, opts ...grpc.CallOption) (exemplarspb.Exemplars_ExemplarsClient, error) {
389+
t.capturedQuery.Store(in.Query)
390+
return t.testExemplarClient.Exemplars(ctx, in, opts...)
391+
}
392+
393+
var _ exemplarspb.ExemplarsClient = &testExemplarClientWithQueryCapture{}
394+
395+
func TestProxyExternalLabelsPreservedForQueryStores(t *testing.T) {
396+
logger := log.NewLogfmtLogger(os.Stderr)
397+
398+
// Simulate a multi-tier topology: Query A → [Query B (cluster=A), Sidecar (cluster=B)]
399+
// When querying with {cluster="A"}, Query B should receive cluster="A" in its query
400+
// (SupportsExternalLabels=true), while a Sidecar would have it stripped.
401+
402+
queryClient := &testExemplarClientWithQueryCapture{
403+
testExemplarClient: testExemplarClient{
404+
response: exemplarspb.NewExemplarsResponse(&exemplarspb.ExemplarData{
405+
SeriesLabels: labelpb.ZLabelSet{Labels: labelpb.ZLabelsFromPromLabels(labels.FromMap(map[string]string{"__name__": "http_request_duration_bucket"}))},
406+
Exemplars: []*exemplarspb.Exemplar{{Value: 1}},
407+
}),
408+
},
409+
}
410+
411+
sidecarClient := &testExemplarClientWithQueryCapture{
412+
testExemplarClient: testExemplarClient{
413+
response: exemplarspb.NewExemplarsResponse(&exemplarspb.ExemplarData{
414+
SeriesLabels: labelpb.ZLabelSet{Labels: labelpb.ZLabelsFromPromLabels(labels.FromMap(map[string]string{"__name__": "http_request_duration_bucket"}))},
415+
Exemplars: []*exemplarspb.Exemplar{{Value: 2}},
416+
}),
417+
},
418+
}
419+
420+
clients := []*exemplarspb.ExemplarStore{
421+
{
422+
ExemplarsClient: queryClient,
423+
LabelSets: []labels.Labels{labels.FromMap(map[string]string{"cluster": "A"})},
424+
SupportsExternalLabels: true, // This is a Query node.
425+
},
426+
{
427+
ExemplarsClient: sidecarClient,
428+
LabelSets: []labels.Labels{labels.FromMap(map[string]string{"cluster": "B"})},
429+
SupportsExternalLabels: false, // This is a Sidecar.
430+
},
431+
}
432+
433+
p := NewProxy(logger, func() []*exemplarspb.ExemplarStore {
434+
return clients
435+
}, labels.EmptyLabels())
436+
437+
server := &testExemplarServer{}
438+
439+
// Query with cluster="A" and namespace="foo".
440+
err := p.Exemplars(&exemplarspb.ExemplarsRequest{
441+
Query: `http_request_duration_bucket{cluster="A", namespace="foo"}`,
442+
PartialResponseStrategy: storepb.PartialResponseStrategy_WARN,
443+
}, server)
444+
testutil.Ok(t, err)
445+
446+
// Only the Query store (cluster=A) should have been queried.
447+
testutil.Equals(t, 1, len(server.responses))
448+
449+
// Verify the Query node received the full matchers including external label.
450+
queryForwarded := queryClient.capturedQuery.Load()
451+
testutil.Assert(t, queryForwarded != "", "query store should have been called")
452+
453+
// The forwarded query to the Query node must contain cluster="A"
454+
// because it needs it for its own downstream routing.
455+
expr, err := extpromql.ParseExpr(queryForwarded)
456+
testutil.Ok(t, err)
457+
selectors := parser.ExtractSelectors(expr)
458+
testutil.Assert(t, len(selectors) > 0, "expected at least one selector")
459+
460+
foundCluster := false
461+
foundNamespace := false
462+
for _, matcherSet := range selectors {
463+
for _, m := range matcherSet {
464+
if m.Name == "cluster" && m.Value == "A" {
465+
foundCluster = true
466+
}
467+
if m.Name == "namespace" && m.Value == "foo" {
468+
foundNamespace = true
469+
}
470+
}
471+
}
472+
testutil.Assert(t, foundCluster, "query store should receive cluster matcher for downstream routing")
473+
testutil.Assert(t, foundNamespace, "query store should receive namespace matcher")
474+
475+
// Verify the Sidecar was NOT queried (cluster=B doesn't match cluster="A").
476+
sidecarForwarded := sidecarClient.capturedQuery.Load()
477+
testutil.Assert(t, sidecarForwarded == "", "sidecar with cluster=B should not be queried for cluster=A request")
478+
}
479+
480+
func TestProxyExternalLabelsStrippedForSidecarStores(t *testing.T) {
481+
logger := log.NewLogfmtLogger(os.Stderr)
482+
483+
// Verify that the existing behavior of stripping external labels for Sidecars is preserved.
484+
sidecarClient := &testExemplarClientWithQueryCapture{
485+
testExemplarClient: testExemplarClient{
486+
response: exemplarspb.NewExemplarsResponse(&exemplarspb.ExemplarData{
487+
SeriesLabels: labelpb.ZLabelSet{Labels: labelpb.ZLabelsFromPromLabels(labels.FromMap(map[string]string{"__name__": "http_request_duration_bucket"}))},
488+
Exemplars: []*exemplarspb.Exemplar{{Value: 1}},
489+
}),
490+
},
491+
}
492+
493+
clients := []*exemplarspb.ExemplarStore{
494+
{
495+
ExemplarsClient: sidecarClient,
496+
LabelSets: []labels.Labels{labels.FromMap(map[string]string{"cluster": "A"})},
497+
SupportsExternalLabels: false, // Sidecar.
498+
},
499+
}
500+
501+
p := NewProxy(logger, func() []*exemplarspb.ExemplarStore {
502+
return clients
503+
}, labels.EmptyLabels())
504+
505+
server := &testExemplarServer{}
506+
507+
err := p.Exemplars(&exemplarspb.ExemplarsRequest{
508+
Query: `http_request_duration_bucket{cluster="A", namespace="foo"}`,
509+
PartialResponseStrategy: storepb.PartialResponseStrategy_WARN,
510+
}, server)
511+
testutil.Ok(t, err)
512+
testutil.Equals(t, 1, len(server.responses))
513+
514+
// Verify the Sidecar received the query WITHOUT the external label matcher.
515+
sidecarForwarded := sidecarClient.capturedQuery.Load()
516+
testutil.Assert(t, sidecarForwarded != "", "sidecar should have been called")
517+
518+
expr, err := extpromql.ParseExpr(sidecarForwarded)
519+
testutil.Ok(t, err)
520+
selectors := parser.ExtractSelectors(expr)
521+
testutil.Assert(t, len(selectors) > 0, "expected at least one selector")
522+
523+
for _, matcherSet := range selectors {
524+
for _, m := range matcherSet {
525+
testutil.Assert(t, m.Name != "cluster",
526+
"sidecar should NOT receive external label matcher cluster, but got: %s", m.String())
527+
}
528+
}
529+
}
530+
382531
// TestProxyDataRace find the concurrent data race bug ( go test -race -run TestProxyDataRace -v ).
383532
func TestProxyDataRace(t *testing.T) {
384533
logger := log.NewLogfmtLogger(os.Stderr)

pkg/query/endpointset.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -600,8 +600,9 @@ func (e *EndpointSet) GetExemplarsStores() []*exemplarspb.ExemplarStore {
600600
for _, er := range endpoints {
601601
if er.HasExemplarsAPI() {
602602
exemplarStores = append(exemplarStores, &exemplarspb.ExemplarStore{
603-
ExemplarsClient: exemplarspb.NewExemplarsClient(er.cc),
604-
LabelSets: labelpb.ZLabelSetsToPromLabelSets(er.metadata.LabelSets...),
603+
ExemplarsClient: exemplarspb.NewExemplarsClient(er.cc),
604+
LabelSets: labelpb.ZLabelSetsToPromLabelSets(er.metadata.LabelSets...),
605+
SupportsExternalLabels: er.ComponentType() == component.Query,
605606
})
606607
}
607608
}

0 commit comments

Comments
 (0)