Streaming labels values PR 5 - #15364
Merged
Merged
Conversation
tcp13equals2
changed the title
Contributor
|
💻 Deploy preview available (Mimir): |
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 18, 2026 03:05
67cdc46 to
520e520
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
from
May 18, 2026 05:06
1c54be1 to
04a5e0a
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 18, 2026 05:06
325f690 to
fe8c5b7
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
from
May 18, 2026 05:48
04a5e0a to
061fb7f
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 18, 2026 05:48
fe8c5b7 to
e68f233
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
from
May 18, 2026 06:28
061fb7f to
d94c245
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 18, 2026 06:28
e68f233 to
1792c19
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
2 times, most recently
from
May 20, 2026 08:30
47d3b45 to
fa75e68
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 20, 2026 23:00
1792c19 to
fda8816
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
from
May 21, 2026 01:53
fa75e68 to
d428fbf
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
3 times, most recently
from
May 21, 2026 05:31
84da6c5 to
3e1734c
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
from
May 22, 2026 02:20
3c6bda7 to
cc943d7
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 22, 2026 03:51
3e1734c to
0e2217c
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-4
branch
2 times, most recently
from
May 22, 2026 06:34
b67a07f to
4cad376
Compare
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 22, 2026 07:10
163468a to
8853ab6
Compare
tcp13equals2
marked this pull request as ready for review
May 22, 2026 07:20
4 tasks
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 28, 2026 04:21
8853ab6 to
e6982ad
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON. A cloud agent has been kicked off to fix the reported issue. You can view the agent here.
Reviewed by Cursor Bugbot for commit e6982ad. Configure here.
When the caller passes metadata=true and a MetadataSupplier is wired in, SearchMetricNamesHandler fetches the tenant's metric metadata once at request start and enriches each emitted record with Type/Help/Unit at the batch boundary. The Type/Help/Unit JSON tags on searchResultRecord already exist from PR #4 with omitempty, so the wire shape is unchanged — fields appear only when populated. Implementation notes: - Lookup pattern: one MetricsMetadata RPC per request (not per batch). The spec's per-batch suggestion would refetch all-tenant metadata on every batch when called with Metric=""; per-request caches that fetch across batches with the same memory footprint (bounded by tenant cardinality). Tradeoff is one extra round-trip up front; the metadata map is small (~100B per metric) and the fetch is fast. - Failure mode: supplier errors degrade to a trailer warning ("metadata enrichment failed: ...") — never an HTTP error. The search results themselves remain useful without enrichment. - String safety: metadata strings are deep-copied with strings.Clone before being stored in the lookup map. The supplier's response may be backed by a pooled gRPC buffer (CLAUDE.md § "Unsafe memory tricks") and the enrichment data must survive into the response- write goroutine. - Nil-safe supplier: a nil MetadataSupplier silently disables enrichment regardless of the URL param — useful for tests and for deployments where the supplier isn't wired in. - Label-names / label-values: silently ignore metadata=true (the param is metric-specific; accepting it preserves forward- compatibility if future PRs plumb it into label paths). The pkg/api/handlers.go change forwards the existing metadataSupplier parameter (currently only consumed by NewMetadataHandler) into SearchMetricNamesHandler. No new module wiring in pkg/mimir/modules.go. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Eight new test cases plus a mockMetadataSupplier fixture (closure- based fn field with a called counter so tests can both assert on the supplier's response shape and verify call vs no-call behaviour): - TestSearchMetricNamesHandler_MetadataEnrichesRecords: metadata=true populates Type/Help/Unit, supplier called exactly once per request. - TestSearchMetricNamesHandler_MetadataDefaultFalseOmitsFields: default (metadata=false) leaves the fields absent in JSON output and the supplier is never called. - TestSearchMetricNamesHandler_SupplierErrorDegradesToTrailerWarning: supplier error → HTTP 200 + warning on trailer (never HTTP 5xx). - TestSearchMetricNamesHandler_EmptyMetadataEmitsRecordsAnyway: empty metadata leaves enrichment fields absent on the record but emits no spurious warning. - TestSearchMetricNamesHandler_IncludeScoreAndMetadataCompose: both Score and Type/Help/Unit appear on the same record. - TestSearchMetricNamesHandler_NilSupplierDisablesEnrichmentSilently: metadata=true with nil supplier returns 200 (no panic). - TestSearchLabelNamesHandler_MetadataParamSilentlyIgnored: the param is accepted on label-names (no 400) and never produces enrichment. - TestSearchMetricNamesHandler_InvalidMetadataParamReturns400: an unparseable metadata bool returns 400. Test fixtures use scrape.MetricMetadata (the upstream Prometheus type the existing MetricsMetadata supplier returns), and model.MetricType constants for the Type field — model.MetricTypeCounter, model.MetricTypeGauge — so the assertion strings match what the handler writes via string(m.Type). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…heus Spec invariant 10 (User-facing HTTP contract matches Prometheus PR PR-5 implementation parsed `metadata=true`, but upstream actually uses `include_metadata=true` — verified at prometheus/prometheus@e8e25eb09e:web/api/v1/search.go:758. The include_ prefix is symmetric with `include_score`, which is why callers naturally reach for `include_metadata` first. This commit changes the URL key (still bool, still default false) and updates the eight metadata-related tests. The internal `metadata` field on searchRequest keeps its name — that's a Go field, not part of the wire contract. Spec invariant 10 was the explicit driver; we want Mimir to be a drop-in replacement for clients hitting Prometheus's search API. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
tcp13equals2
force-pushed
the
streaming-labels-values-PR-5
branch
from
May 28, 2026 06:34
e6982ad to
1b2c64f
Compare
ldufr
approved these changes
May 28, 2026
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 -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_metadataenrichment 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 #
Checklist
CHANGELOG.mdupdated - the order of entries should be[CHANGE],[FEATURE],[ENHANCEMENT],[BUGFIX]. If changelog entry is not needed, please add thechangelog-not-neededlabel to the PR.about-versioning.mdupdated with experimental features.Note
Low Risk
Additive, flag-gated experimental API behavior with ingester-only enrichment; no auth or ingestion path changes.
Overview
Adds optional
include_metadatato the experimental streaming search API so/api/v1/search/metric_namescan 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 asinclude_metadataonSearchLabelValueswhen searching__name__. Ingesters attachMetricMetadataper batch (most recent in-memory record per metric on that replica); the distributor maps wire metadata intostorage.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,buildSearchHintsrejects negative limits and clamps huge values, and devmimir.yamlturns onexperimental_search_api_enabled.Reviewed by Cursor Bugbot for commit 1b2c64f. Bugbot is set up for automated code reviews on this repo. Configure here.