Skip to content

Streaming labels values PR 4 - #15349

Merged
tcp13equals2 merged 30 commits into
mainfrom
streaming-labels-values-PR-4
May 28, 2026
Merged

Streaming labels values PR 4#15349
tcp13equals2 merged 30 commits into
mainfrom
streaming-labels-values-PR-4

Conversation

@tcp13equals2

@tcp13equals2 tcp13equals2 commented May 15, 2026

Copy link
Copy Markdown
Contributor

What this PR does

This is the 4th PR in the streaming labels/values search API stack.

See prometheus/proposals#74.

This PR builds upon #15233, #15301 and #15347

This PR adds three experimental NDJSON-streaming HTTP endpoints on the querier;

  • /api/v1/search/metric_names — searches values of the name label
  • /api/v1/search/label_names — searches for label names
  • /api/v1/search/label_values — searches for values of a single label

The HTTP endpoints have been placed behind a feature flag ( -querier.experimental-search-api-enabled ) and this defaults to false.

Note - in a subsequent PR will be support for the metadata query param to decorate metric names, additional documentation, bechmarks and updates to dashboard mixins.

Which issue(s) this PR fixes or relates to

Fixes #

Checklist

  • Tests updated.
  • Documentation added.
  • CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]. If changelog entry is not needed, please add the changelog-not-needed label to the PR.
  • about-versioning.md updated with experimental features.

Note

Medium Risk
New query surface can drive expensive label scans across ingesters/store-gateways; mitigated by default-off flag, limits, and streaming, but multi-tenant federation fan-out increases blast radius when enabled.

Overview
Adds experimental Prometheus-style streaming label/value search on the querier: NDJSON endpoints at /api/v1/search/{metric_names,label_names,label_values}, gated by -querier.experimental-search-api-enabled (default off). When disabled, callers get 404 feature_not_enabled.

The new search_handler layer parses query params (fuzz, sort, limits, match[] OR-union), opens a mimirSearcher, streams batched NDJSON with success/error trailers, and sets has_more using a limit+1 probe plus typed MaxLimitError clamp warnings. MaxLimitError is refactored so handlers can read which limit was enforced without parsing messages.

Routes are wired in pkg/api and handlers.go with usage stats. Tenant federation merge_queryable fans out SearchLabelNames / SearchLabelValues, merges result sets, and special-cases synthetic __tenant_id__ (and retain-prefix) label values like existing LabelValues.

Changelog and config docs flag the feature as experimental.

Reviewed by Cursor Bugbot for commit bc74dda. Bugbot is set up for automated code reviews on this repo. Configure here.

@tcp13equals2
tcp13equals2 changed the base branch from main to streaming-labels-values-PR-3 May 15, 2026 07:31
@github-actions

github-actions Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor

💻 Deploy preview deleted (Mimir).

@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from 7c60812 to 5cf875c Compare May 18, 2026 03:05
tcp13equals2 added a commit that referenced this pull request May 18, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from c4595bd to 1c54be1 Compare May 18, 2026 03:05
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from 5cf875c to 74ffeca Compare May 18, 2026 05:06
tcp13equals2 added a commit that referenced this pull request May 18, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from 1c54be1 to 04a5e0a Compare May 18, 2026 05:06
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from 74ffeca to 5c1e105 Compare May 18, 2026 05:48
tcp13equals2 added a commit that referenced this pull request May 18, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from 04a5e0a to 061fb7f Compare May 18, 2026 05:48
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from a50005a to 06b3a65 Compare May 18, 2026 06:28
tcp13equals2 added a commit that referenced this pull request May 18, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from 061fb7f to d94c245 Compare May 18, 2026 06:28
batchSize = parsed
}

includeScore := false

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers - the include_metadata option will be added in a subsequent PR

@@ -0,0 +1,493 @@
// SPDX-License-Identifier: AGPL-3.0-only
// Provenance-includes-location: https://github.com/prometheus/prometheus/blob/e8e25eb09e41bf295e0c9e847cd27cf9016a553a/web/api/v1/search.go

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note - once the Prometheus side PRs are all merged these links will be updated

type searchResultRecord struct {
Name string `json:"name"`
Score *float64 `json:"score,omitempty"`
Type string `json:"type,omitempty"`

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that support for setting this metadata will come in a subsequent PR

@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from a1d1b8a to faf7f73 Compare May 19, 2026 02:25
tcp13equals2 added a commit that referenced this pull request May 19, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from 84a40d0 to b48a56a Compare May 19, 2026 02:36
@tcp13equals2 tcp13equals2 changed the title Streaming labels values pr 4 Streaming labels values PR 4 May 19, 2026
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from faf7f73 to 4f60313 Compare May 20, 2026 08:12
tcp13equals2 added a commit that referenced this pull request May 20, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from 47d3b45 to fa75e68 Compare May 20, 2026 08:30
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-3 branch from 4f60313 to 8b402b3 Compare May 21, 2026 01:47
tcp13equals2 added a commit that referenced this pull request May 21, 2026
CHANGELOG.md: single [FEATURE] entry covering PRs #1-#4 of the
streaming search stack (#15349). The four-PR sequence builds up the
underlying infrastructure (PR #1 leaf RPCs + filters, PR #2
cross-server fan-out, PR #3 querier wiring) culminating in the
user-visible HTTP endpoints landed in this PR.

about-versioning.md: tag the endpoints under "Experimental features →
Querier" and cross-link to Prometheus PR #18573 (the upstream API
contract Mimir mirrors).

Detailed per-endpoint API reference pages under
docs/sources/mimir/references/http-api/ are deferred to PR #5, where
the integration tests can validate the documented request/response
examples end-to-end.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from fa75e68 to d428fbf Compare May 21, 2026 01:53
@tcp13equals2
tcp13equals2 marked this pull request as ready for review May 21, 2026 02:29
@tcp13equals2
tcp13equals2 requested review from a team as code owners May 21, 2026 02:29
tcp13equals2 and others added 15 commits May 22, 2026 14:33
Four changes to bring the handler's wire surface closer to upstream:

* Rename the label-values URL param from "name" to "label" and update
  the missing-param error to `missing required parameter "label"`,
  matching Prometheus PR #18573 verbatim. A client written against
  upstream now works against Mimir without modification.

* Reject sort_by=score when no search[] term is provided. The score
  ordering is only meaningful relative to scores produced by the search
  terms; upstream rejects this with `sort_by=score requires search[] to
  be set` and so does Mimir now.

* Cap the number of search[] terms per request at 32
  (maxSearchTermsPerRequest), bounding per-term filter construction
  cost. Mirrors the same constant upstream and uses the upstream error
  wording `too many search[] terms: got %d, maximum is %d`.

* Align batch_size validation with upstream: 0 means "server-determined"
  and falls back to searchDefaultBatchSize; only negative values are
  rejected, with the upstream wording `invalid batch_size %q: must be
  non-negative integer`. Drops the previous Mimir-only behaviour of
  rejecting batch_size=0.

The audit also flagged an "empty sort_by required" claim that turned
out to be a non-bug — Mimir already defaults sort_by to "alpha" before
parseSortOrder runs, matching upstream's "natural order" default. No
change there.

Tests cover the new precondition (sort_by=score requires search[]),
the cap (n+1 terms rejected), and the new batch_size semantics
(negative rejected, 0 accepted via the round-trip path).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Five comment / dead-code touch-ups from the self-review pass:

* parseSearchRequest godoc said "name parameter is mandatory" — the
  rename to "label" landed in the body but not the doc. Fixed.
* searchBatchEnvelope carried a Warnings field that streamSearchNDJSON
  never sets — all warnings land on the trailer. Removed the dead
  field and trimmed the doc to match what the code actually does.
* maxSearchTermsPerRequest comment dropped an algorithmic justification
  ("O(terms) for each Filter.Accept call") that belongs in the filter
  package, not at the call site.
* labelName URL-param comment trimmed; the file's provenance header
  already credits Prometheus PR #18573 so the inline reference adds no
  signal.
* Replaced the local metricNameLabel = "__name__" const with
  labels.MetricName from the already-imported model/labels package;
  removes drift risk against the upstream constant.

Four new tests filling real coverage gaps:

* TestParseSearchRequest_BatchSizeZeroKeepsDefault pins the post-rename
  contract that batch_size=0 means "server-determined" and falls back
  to searchDefaultBatchSize.
* TestSearchLabelNamesHandler_AcceptsPOSTForm covers the spec parity
  case: routes are registered for POST and r.ParseForm() must reach
  parameters carried on the form body.
* Three searcherForRequest error-branch tests:
  TestSearchLabelNamesHandler_MissingTenantReturns400 (no tenant
  injection); TestSearchLabelNamesHandler_QueryableOpenError (queryable
  surfaces an error from Querier()); and
  TestSearchLabelNamesHandler_QuerierNotMimirSearcherReturns400
  (queryable returns a Querier that doesn't satisfy mimirSearcher).
  Each was an uncovered branch.
* TestSearchLabelNamesHandler_EmptyResultsEmitsTrailerOnly pins that
  the handler does not flush a {"results":[]} batch line when the
  iterator is empty — only the success trailer reaches the wire.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two Codex-flagged issues addressed together because both touch the same
parse + dispatch path.

* Cap user-supplied batch_size at maxSearchBatchSize (10000). batch_size
  is used as the capacity argument to make([]searchResultRecord, 0, n)
  in streamSearchNDJSON, so an unchecked value let a single request
  force a multi-GB allocation before any data flowed. The cap mirrors
  the upstream --web.search.max-limit ceiling (10000) so a batch can't
  meaningfully exceed the largest reasonable result count. Negative and
  zero are still handled as before (zero means "server-determined",
  negative is rejected). New BadParams subtest pins the cap.

* Tenant-federated search support. With -tenant-federation.enabled=true,
  the queryable wrapping the search handler is tenantfederation's
  mergeQuerier, which did not implement the cross-source Mimir search
  shape — so every search request returned "queryable does not support
  search" as HTTP 400, and multi-tenant contexts were rejected by
  tenant.TenantID before the merge layer could resolve them. Fix:

  - MergeQuerierUpstream gains SearchLabelNames / SearchLabelValues with
    a per-tenant federation ID, matching the existing LabelNames /
    LabelValues shape. The concrete tenantQuerier wrapper type-asserts
    its upstream storage.Querier to a local search interface (declared
    next to the type to avoid the import cycle on pkg/querier) and
    forwards each call with user.InjectOrgID applied.

  - mergeQuerier implements the public SearchLabelNames /
    SearchLabelValues with the same tenant-resolve / bypass-with-single
    / fan-out shape as the existing label aggregation. The fan-out
    produces a []storage.SearchResultSet (one per tenant) and merges
    them via PairwiseMergeSearchSetsWithHints — streaming, ordered,
    deduplicated. Unlike LabelNames, the result does not synthesise the
    idLabelName ("__tenant_id__"); callers searching label values
    against that label get an empty per-tenant response.

  - searcherForRequest in the search handler now calls tenant.TenantIDs
    (plural) so multi-tenant contexts reach the federation layer
    instead of being rejected upfront.

  Two new federation tests cover multi-tenant fan-out (dedup across
  tenants, value-asc ordering) and the single-tenant bypass path.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
parseSearchMatchers flattened every match[] entry into a single
[]*labels.Matcher slice, turning repeated selectors into a conjunction
(AND). Prometheus PR #18573 keeps them separate and runs one search
per selector, merging the SearchResultSets — so the same request shape
that returns results from either selector upstream was returning the
intersection (often empty) in Mimir.

Changes:

* parseSearchMatchers now returns [][]*labels.Matcher — one inner slice
  per match[] entry — so the dispatcher can see each selector
  independently.

* searchRequest.matchers gains the nested shape with a comment calling
  out the OR semantics.

* New dispatchSearchOverMatcherSets helper handles all three cardinality
  cases: 0 sets → one call with no matchers; 1 set → one call with that
  set; N sets → N calls, results merged via the existing pairwise k-way
  merger (dedup + ordering inherited from the merge primitive). Closure
  lets it work for both SearchLabelNames and SearchLabelValues.

* All three handlers (label_names, label_values, metric_names) route
  their searcher call through the dispatcher.

A new test pins the OR-semantics contract: a request with two distinct
match[] selectors triggers two SearchLabelNames calls with the per-
selector matchers, and the response unions the per-selector results
with duplicates collapsed. The existing TestParseSearchRequest_-
ParamRoundTrip is updated to index the nested matcher slice.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
labels.MetricName is deprecated per the upstream staticcheck SA1019
warning; the direct replacement is model.MetricNameLabel from
prometheus/common/model, which the handler already imports for time
defaults. Lint job 76484945533 failed on the previous tip with this
hint; swapping the constant fixes the lint cleanly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
faillint disallows sync/atomic in pkg/... outside of explicit
//lint:ignore exceptions. The bounded-concurrency tests added in the
previous commit imported sync/atomic for Int32; switching to
go.uber.org/atomic clears CI lint and keeps the API call sites
identical (Add / Load / CompareAndSwap).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…opes

streamSearchNDJSON previously only set the NDJSON Content-Type and
internal streaming header from inside flushBatch. When the result set
was empty, no batch was flushed and the trailer encode triggered an
implicit WriteHeader(200) with no Content-Type — Go's response
sniffing then surfaced text/plain. Set the headers before writing the
trailer when nothing was flushed.

writeSearchFeatureDisabled and writeSearchBadRequest emit a single
JSON object, not an NDJSON stream, so application/json is the correct
Content-Type. This also matches the pre-flush error path
(writePreFlushSearchError).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The three search handlers previously mapped every searcherForRequest
failure to HTTP 400, conflating client errors (missing/invalid tenant
on the request context) with server errors (queryable.Querier()
failure, mimirSearcher type-assertion failure). Wrap tenant resolution
failures in a *searcherClientError so the handler can classify them
explicitly: client errors continue to use writeSearchBadRequest (400),
and everything else routes through writePreFlushSearchError, which
already picks 499/503/500 by error class and emits an
application/json envelope consistent with the rest of the search API.

Also rename the warnings-loop variable from w to warn so it no longer
shadows the outer http.ResponseWriter.

Tests updated to assert HTTP 500 + application/json + the standard
error envelope for the open-error and type-assertion-failure branches,
and to lock down the bad-request envelope for the missing-tenant case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The "sort_dir with sort_by=score rejected" bad-params case used
"sort_by=score&sort_dir=asc" without a search[] term, so the request
was rejected earlier by parseSearchRequest's "sort_by=score requires
search[] to be set" check and never reached parseSortOrder. The
sort_dir+score branch (search_handler.go parseSortOrder, line 276-278)
was therefore uncovered, and the existing "sort_by=score without
search[]" case already exercised the path this test was accidentally
hitting.

Add search[]=foo to the query so the no-search[] check passes and the
intended parseSortOrder validation runs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tcp13equals2
tcp13equals2 force-pushed the streaming-labels-values-PR-4 branch from b67a07f to 4cad376 Compare May 22, 2026 06:34

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

There are 3 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Bugbot Autofix prepared fixes for both issues found in the latest run.

  • ✅ Fixed: Label values wrong JSON key
    • Label values now emit the JSON key "value" (names endpoints still emit "name") by adding a Value field and populating it only for the label-values endpoint.
  • ✅ Fixed: Has-more on limit zero clamp
    • has_more is no longer set from clamp warnings when limit=0 by requiring a finite user limit before treating a clamp as truncation.

Create PR

Or push these changes by commenting:

@cursor push bad3b9dea0
Preview (bad3b9dea0)
diff --git a/pkg/querier/search_handler.go b/pkg/querier/search_handler.go
--- a/pkg/querier/search_handler.go
+++ b/pkg/querier/search_handler.go
@@ -50,7 +50,8 @@
 // wire. Score is optional (controlled by include_score); Type/Help/Unit
 // are optional (controlled by include_metadata).
 type searchResultRecord struct {
-	Name  string   `json:"name"`
+	Name  string   `json:"name,omitempty"`
+	Value string   `json:"value,omitempty"`
 	Score *float64 `json:"score,omitempty"`
 	Type  string   `json:"type,omitempty"`
 	Help  string   `json:"help,omitempty"`
@@ -513,7 +514,12 @@
 			break
 		}
 		r := rs.At()
-		rec := searchResultRecord{Name: r.Value}
+		var rec searchResultRecord
+		if req.labelName != "" {
+			rec = searchResultRecord{Value: r.Value}
+		} else {
+			rec = searchResultRecord{Name: r.Value}
+		}
 		if req.includeScore {
 			s := r.Score
 			rec.Score = &s
@@ -576,7 +582,7 @@
 			clampFired = true
 		}
 	}
-	if clampFired || (req.limit > 0 && emitted > req.limit) {
+	if (req.limit > 0 && clampFired) || (req.limit > 0 && emitted > req.limit) {
 		trailer.HasMore = true
 	}
 	_ = enc.Encode(trailer)

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit 4cad376. Configure here.

Comment thread pkg/querier/search_handler.go Outdated
Comment thread pkg/querier/search_handler.go
@tcp13equals2 tcp13equals2 mentioned this pull request May 25, 2026
4 tasks
Comment thread pkg/querier/search_handler.go Outdated
// vs writeStreamSearchError). Once any batch has been flushed, headers are
// on the wire and a later iterator error rides on a status="error" NDJSON
// trailer instead of an HTTP error code.
func streamSearchNDJSON(w http.ResponseWriter, rs storage.SearchResultSet, req *searchRequest) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function seems a bit more complex than it should.

Why do we need to distinguish if it's an error on/before the first batch or not? It feels like: "we still didn't send the HTTP status, so we might as well", but I'm not sure that's a good idea.

For instance, if rs already had an error (on creation), than we aren't streaming yet it probably makes sense to return an HTTP error, the same way we do for invalid params. Arguably, it should even be the caller that would do it.

When we enter this function, we are effectively streaming the response. I think it would be more consistent and reduce coupling to simply handle all the error cases the same way. We could:

  1. Set the header from the beginning.
  2. Iterate and flush without tracking if we already flushed.
  3. Send errors if any without distinguishing
  4. ...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also thinking from the client perspective and don't see any big benefit of treating those errors differently.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason for doing this extra complexity is to behave in a similar manner to Prometheus. It has this same pre-stream vs in-stream split (respondPreStreamSearchError vs writeStreamSearchError), with its own equivalent of flushedAny (the "did we open the NDJSON writer yet?" phase boundary).

Do you have a strong feeling for whether we should try and match Prometheus in this error handling?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't, though, I checked and it didn't seem that Prometheus went this way explicitly. I guess it does make sense to match Prometheus behavior though.

Comment thread pkg/querier/search_handler.go Outdated
Comment thread pkg/querier/search_handler.go
Comment thread pkg/querier/search_handler.go Outdated
Comment thread pkg/querier/search_handler.go Outdated
@tcp13equals2
tcp13equals2 merged commit 41171ad into main May 28, 2026
89 checks passed
@tcp13equals2
tcp13equals2 deleted the streaming-labels-values-PR-4 branch May 28, 2026 06:28
tcp13equals2 added a commit that referenced this pull request May 28, 2026
#### What this PR does

This is the fifth PR in the streaming label/value search API
implementation.

See also #15233,
#15301,
#15347,
#15349.

This PR focuses on adding support for the `include_metadata` enrichment
on results from ` /api/v1/search/metric_names`.

This feature allows for metric metadata to be included in the metric
name results. This allows for a client to avoid needing to make a
separate API call to retrieve metadata records.

It should be noted that in Mimir, only the ingesters maintain metric
metadata information - and only if it has been included in the remote
write. This metadata information is only maintained in the ingesters for
a short period of time (10 minutes).

To maintain simplicity, this implementation only decorates in the most
recent metadata record for a metric. This is regardless of any time
range submitted in the search. In the future if Mimir was persist or
maintain metadata for longer this implementation choice could be
reviewed.

No metadata is included if the search time range only requires
store-gateways to full-fill the request. This is intentional since "old"
metrics which have not been seen (metadata record pushed) by an ingester
in the last 10 minutes will not have a metadata in Mimir.

#### Which issue(s) this PR fixes or relates to

Fixes #<issue number>

#### Checklist

- [x] Tests updated.
- [ ] Documentation added.
- [x] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> Additive, flag-gated experimental API behavior with ingester-only
enrichment; no auth or ingestion path changes.
> 
> **Overview**
> Adds optional **`include_metadata`** to the experimental streaming
search API so **`/api/v1/search/metric_names`** can return **type**,
**help**, and **unit** on each metric without a separate metadata call.
> 
> The flag is parsed on the querier HTTP handler, carried on
**`streaminglabelvalues.Params`**, and sent to ingesters as
**`include_metadata`** on **`SearchLabelValues`** when searching
**`__name__`**. Ingesters attach **`MetricMetadata`** per batch (most
recent in-memory record per metric on that replica); the distributor
maps wire metadata into **`storage.SearchResult`**, and the metric-names
NDJSON builder emits the extra fields. **Store-gateway–only** searches
stay un-enriched by design.
> 
> Also tightens search plumbing: shared **`parseBoolParam`**,
**`buildSearchHints`** rejects negative limits and clamps huge values,
and dev **`mimir.yaml`** turns on **`experimental_search_api_enabled`**.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
1b2c64f. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tcp13equals2 added a commit that referenced this pull request Jun 4, 2026
#### What this PR does

This is the 6th PR in the streaming label/value search API
implementation.

See also #15233,
#15301,
#15347,
#15349 and
#15364.

This PR adds a README.md to assist in documenting this implementation,
benchmark test files and some enhancements found whilst running the
newly added benchmarks.

No changelog has been recorded since these are internal implementation,
testing and documentation changes with no user facing modifications.

#### Benchmarks: legacy `LabelValues` vs new `SearchLabelValues`
Apple M4 Pro, `go test -benchmem -benchtime=5s -count=3`, medians of 3
runs reported.
##### Ingester (`pkg/ingester`)
`BenchmarkIngester_LegacyVsSearchLabelValues` — in-process, no RPC
framing.
| Cardinality | Path | ns/op | B/op | allocs/op | Δ time | Δ B/op | Δ
allocs |
| ----------- | ------ | ----------: | ----------: | ---------: |
-------- | -------- | -------- |
| 4 199 | legacy | 211 794 | 90 418 | 4 209 | | | |
| 4 199 | new | 216 027 | 223 578 | 21 | +2% | +147% | **−99.5%** |
| 1 000 003 | legacy | 149 987 546 | 23 942 547 | 1 000 161 | | | |
| 1 000 003 | new | 130 728 720 | 48 140 526 | 158 | **−13%** | +101% |
**−99.98%** |
**Observations**
- At 4 199 values: parity on wall-clock, but allocations drop from 4 209
→ 21 (≈200×).
- At 1 000 003 values: new path is 13% faster, uses 2× the memory, but
allocates **6 330× fewer objects** (158 vs 1 000 161).

The allocation reduction is due to the batch buffer reuse and
synchronous-send which make skipping the clone safe.

```
  q, err := db.Querier(...)                                                                                                                                                                                                              
  defer q.Close()                                                                                                                                                                                                                                    
  ...                                                                                                                                                                                                                                                        
  rs := searcher.SearchLabelValues(ctx, ...)            // result set holds yolo strings                                                                                                                                                              
  defer rs.Close()                                      //                                                                                                                                                                                              
  return streamSearchResults(ctx, rs, stream.Send, ...) // synchronous; blocks until all batches sent
```

The increase in B/op is due to the wrapping of each record in a
`SearchResult{Value, Score}`.

##### Store-gateway (`pkg/storegateway`)
`BenchmarkBucketStoreSearchLabelValuesVsLabelValues` — bucket store,
cold index cache (`worstCaseFetchedDataStrategy`).
| Cardinality | Path | ns/op | B/op | allocs/op | Δ time | Δ B/op | Δ
allocs |
| ----------- | ------ | -------------: | ----------: | ----------: |
-------- | ------ | -------- |
| 1 000 | legacy | 21 346 956 | 13 931 796 | 61 668 | | | |
| 1 000 | new | 21 368 149 | 14 004 859 | 61 681 | +0.1% | +0.5% |
+0.02% |
| 1 000 000 | legacy | 32 330 190 178 | 3 482 e6 | 51 001 391 | | | |
| 1 000 000 | new | 29 567 861 292 | 3 569 e6 | 51 009 234 | **−9%** |
+2.5% | +0.02% |
**Observations**
- At 1 000 values: parity.
- At 1 000 000 values: new path is ~9% faster on time. Memory and
allocation count are essentially unchanged because the bulk of the work
at the store-gateway layer is in the shared TSDB postings/chunk walk.

Although the same SearchResult wrapper is present it is lost in the
other SG B/op noise.

#### Which issue(s) this PR fixes or relates to

Fixes #<issue number>

#### Checklist

- [x] Tests updated.
- [x] Documentation added.
- [ ] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Low Risk**
> No user-facing API or behavior changes beyond optional
micro-optimizations on an experimental, flag-gated search path; risk is
mainly benchmark/doc churn and regression in edge fast paths (single
tenant/block, cancel lifecycle).
> 
> **Overview**
> Adds **`docs/internal/streaming-label-value-search/README.md`**, an
internal guide for the experimental streaming label/value search stack
(NDJSON HTTP endpoints, data flow, merge layers, wire batching,
benchmarks).
> 
> **Benchmark coverage** is added across ingester, distributor,
store-gateway, tenant federation, and HTTP handler tests (including
legacy vs new parity benches and distributor merge/dedup scenarios).
> 
> **Performance tweaks** found while benchmarking: HTTP
**`search_handler`** pools NDJSON batch envelopes, splits score vs
no-score record types to avoid `*float64` allocs, and writes a prebuilt
success trailer; **distributor** stores stream cancel funcs on
`ingesterSearchResultSet` instead of per-ingester closures; **tenant
federation** uses `tenantJobsForSearch` (skip map work when no id-label
matcher) and a single-tenant job bypass; **store-gateway** skips
`MergeSearchResultSets` when only one block matches.
Ingester/store-gateway sources link to the new README.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
3990db9. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@ldufr ldufr mentioned this pull request Jul 22, 2026
4 tasks
ldufr added a commit that referenced this pull request Jul 22, 2026
#### What this PR does

While preparing the release 3.2, I notice that two changelog entries
where not at the right place and one had a typo.

PR #15349 introduced "experimental
streaming label/value search HTTP endpoints" after 3.1 and we also don't
find it in the [CHANGELOG.md of
3.1](https://github.com/grafana/mimir/blob/release-3.1/CHANGELOG.md).
Same thing for "Use series selected for one side to reduce" that was
introduced in PR #15178.

#### Checklist

- [ ] Tests updated.
- [ ] Documentation added.
- [ ] `CHANGELOG.md` updated - the order of entries should be
`[CHANGE]`, `[FEATURE]`, `[ENHANCEMENT]`, `[BUGFIX]`. If changelog entry
is not needed, please add the `changelog-not-needed` label to the PR.
- [ ]
[`about-versioning.md`](https://github.com/grafana/mimir/blob/main/docs/sources/mimir/configure/about-versioning.md)
updated with experimental features.
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

Successfully merging this pull request may close these issues.

3 participants