From 13e5c1d10dec61847f65e64ca6e5369a46fbb8c2 Mon Sep 17 00:00:00 2001 From: szibis Date: Tue, 22 Sep 2026 08:44:46 +0200 Subject: [PATCH 1/2] fix: answer metric queries over the series limit like Loki and keep them on VictoriaLogs buckets MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A grouped range metric whose grouping had more values than -max-stats-query-series was answered with a silent cut: the busiest 200 values on the short-range path and on the two-phase path used from two hours on, with no signal that the rest existed. Loki fails such a query with `maximum number of series (N) reached for a single query` (HTTP 400) and only gives Grafana Logs Drilldown a partial result, so the proxy now does the same, on range and instant queries alike. The message stays Loki's word for word — Grafana renders it — and the log line for the rejected request names the flag that set the limit. The limit is enforced while the VictoriaLogs response is read, so an oversized answer is not buffered in full, and it never turns into a raw log scan. The Drilldown paths lose their hardcoded 500: they read the same flag, ask VictoriaLogs for one series over it so a truncated answer is distinguishable from an exact fit, and report the cut as Loki's partial-result warning instead of dropping series silently. Where Loki keeps the first series it encounters, the proxy keeps the busiest by total count, which is what a Drilldown panel renders. Also in this change: - `offset` samples are stamped at Loki's evaluation time instead of the shifted time, on range and instant queries, and are no longer cached with the shifted timestamps. Only sample responses are rewritten, so a log query still streams. - An unknown backend version (failed or pending probe) keeps anchored stats buckets instead of falling back to raw rows: the offset arg is ignored by a backend that predates it, which leaves the epoch-aligned buckets it would have had anyway, while raw rows scale with the stored data. A version known to be older than v1.45 still uses epoch buckets only where they match the query's windows. - Logs Drilldown panels with a range shorter than the step use the window evaluator, so they no longer count the whole step bucket. - The window-sampled /hits rewrite is limited to Drilldown: a Grafana dashboard or Explore panel grouping by a high-cardinality field gets the exact series Loki returns. - The top-value `in()` filter is budgeted against VictoriaLogs' default 16 KiB query length and never ends up empty. - Single-field groupings over two hours read the range once instead of ranking and then scanning it again; values are ranked only once the result exceeds the series limit. - The ordered JSON metric path failed one series early: Loki rejects only above the limit. The e2e compat suite gains a semantics case that compares offset stamping and both series-limit behaviours against live Loki. The compose proxies carry the series limit their Loki is configured with so parity tests compare answers rather than limits, with one variant left at the default so the limit itself stays under test. The conformance registry gains the cases this proves, and the docs and chart values describe the Loki error instead of the removed 502/503 messages. --- CHANGELOG.md | 60 ++++ charts/loki-vl-proxy/values.yaml | 2 +- .../drilldown-partial-with-warning.yaml | 39 +++ .../cases/limits/series-limit-error.yaml | 40 +++ .../semantics/offset-sample-timestamps.yaml | 35 +++ conformance/registry/generated/wiring.json | 53 +++- .../series-limits-and-partial-results.yaml | 11 +- .../window-bounds-and-step-alignment.yaml | 10 +- conformance/reports/gaps.md | 5 +- conformance/reports/roadmap.md | 6 +- docs/KNOWN_ISSUES.md | 2 +- docs/api-reference.md | 5 +- docs/compatibility-drilldown.md | 5 +- docs/configuration.md | 8 +- docs/operations.md | 2 +- docs/real-window-compatibility-gaps.md | 3 +- internal/proxy/backend.go | 9 +- .../proxy/bare_parser_sliding_parity_test.go | 22 +- internal/proxy/drilldown_compat_test.go | 10 +- internal/proxy/drilldown_field_batcher.go | 7 +- .../proxy/drilldown_regression_lock_test.go | 17 +- internal/proxy/drilldown_strip_test.go | 26 +- .../proxy/grouped_rate_count_paths_test.go | 4 +- internal/proxy/http_utils.go | 11 +- internal/proxy/manual_metric_budget.go | 255 +++++++++++++++++ .../proxy/manual_metric_output_limits_test.go | 2 +- internal/proxy/metric_binary.go | 264 ++++++++++++----- internal/proxy/offset_integration_test.go | 13 +- internal/proxy/ordered_json_drilldown_test.go | 2 +- internal/proxy/ordered_json_metric.go | 9 +- internal/proxy/perf_regression_test.go | 47 +-- internal/proxy/proxy.go | 34 ++- internal/proxy/proxy_test.go | 11 +- internal/proxy/query_translation.go | 2 +- internal/proxy/range_metric_compat.go | 268 +++++++++++------- internal/proxy/range_metric_compat_test.go | 51 ++-- internal/proxy/review_findings_test.go | 17 +- internal/proxy/sliding_window_parity_test.go | 62 ++-- internal/proxy/tumbling_axis_parity_test.go | 161 +++++++++-- test/e2e-compat/docker-compose.yml | 51 ++++ .../range_metric_offset_series_limit_test.go | 256 +++++++++++++++++ test/e2e-compat/raw_metric_limits_test.go | 2 +- 42 files changed, 1515 insertions(+), 384 deletions(-) create mode 100644 conformance/registry/cases/limits/drilldown-partial-with-warning.yaml create mode 100644 conformance/registry/cases/limits/series-limit-error.yaml create mode 100644 conformance/registry/cases/semantics/offset-sample-timestamps.yaml create mode 100644 test/e2e-compat/range_metric_offset_series_limit_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 238ba9a89..fcfba497a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,66 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Fail a metric query over the series limit the way Loki does, instead of + returning part of the data as if it were all of it. A grouped range metric + whose grouping has more values than `-max-stats-query-series` (500 by + default) was answered with a silent cut — the busiest 200 values on the + short-range path, the busiest 200 on the two-phase path used from two hours + on — so a chart or an alert read a subset as the whole answer. Loki answers + `maximum number of series (N) reached for a single query` with HTTP 400 for + such a query and only gives Grafana Logs Drilldown a partial result. The + proxy now does the same: the error names the limit and the + `-max-stats-query-series` flag that sets it, and Drilldown requests + (`X-Query-Tags: Source=grafana-lokiexplore-app`) get the busiest series plus + Loki's `returning partial results` warning in the response. The limit is + applied while the VictoriaLogs response is read, so an oversized answer is + no longer buffered in full, and it is never turned into a raw log scan. The + message stays Loki's word for word, since Grafana renders it; the proxy's log + line for the rejected request names the flag instead. Instant queries + (`/loki/api/v1/query`) follow the same rule, as they do in Loki. The Grafana + Logs Drilldown paths no longer carry a hardcoded 500: they read the flag, ask + VictoriaLogs for one series over it so a truncated answer is distinguishable + from an exact fit, and report the cut as the warning instead of dropping + series silently. Where Loki keeps the first series it encounters, the proxy + keeps the busiest by total count — the ones a Drilldown panel renders. The + e2e compose proxies now run with the series limit their Loki is configured + with, so a parity test compares two answers rather than two limits; one + variant keeps the default so the limit itself stays under test. +- Stamp `offset` samples at the evaluation time. `count_over_time({app="x"}[5m] + offset 5m)` evaluates the window `(T-10m, T-5m]` but Loki reports it at `T`; + the proxy reported it at `T-5m`, shifting the whole series left by the + offset on range and instant queries alike (and caching it that way). +- Keep evaluating range metrics on VictoriaLogs buckets while the backend + version is unknown. A failed or pending startup version probe made the proxy + assume the `stats_query_range` `offset` arg (VictoriaLogs v1.45+) is + unavailable, so any range whose start was not aligned to the window fell back + to fetching raw log rows — the amount of data read then scaled with the logs + stored, not with the query. The arg is now sent while the version is unknown + (a backend that does not know it ignores it, leaving the epoch-aligned + buckets it would have had anyway) and the version probe keeps retrying in the + background. A release known to be older than v1.45 still uses epoch-aligned + buckets only where they match the query's windows. +- Count Logs Drilldown's short ranges over Loki's window. A Drilldown panel + with a range shorter than the step (`[1m]` at step 300) was answered from the + whole step bucket, so a panel showed 30 where Loki answers 6. Drilldown now + uses the same window evaluator as every other client for those panels. +- Return exact series for high-cardinality groupings outside Drilldown. A + Grafana dashboard or Explore panel grouping by a high-cardinality field + (`sum by (trace_id) (...)`) was rewritten to the window-sampled `/hits` path, + which returns a bounded sample of the values rather than the series Loki + returns. That rewrite is now limited to Logs Drilldown, whose panels are + built for it. +- Fit the top-value `in()` filter to VictoriaLogs' query length. The bounded + top-N paths cut the value list at a fixed count, which could still exceed + VictoriaLogs' default 16 KiB query limit (long pod or trace values) and be + rejected, or — after filtering — leave an empty `in()` that matches nothing. + The list is now budgeted against the actual query text and keeps at least the + busiest value. +- Read a low-cardinality grouping once. Single-field groupings over two hours + ran a ranking query and then a full bucket query over the same range, so an + ordinary `sum by (namespace) (count_over_time(...))` scanned the range twice. + The bucket query now runs alone, and values are ranked only when the result + exceeds the series limit. - **Release metadata pull requests pass the changelog gate again.** The gate listed a pull request's files with a two-dot diff against the current base, so a branch cut before the base moved appeared to change every file merged diff --git a/charts/loki-vl-proxy/values.yaml b/charts/loki-vl-proxy/values.yaml index 4fc4162d2..9ef2f6bf0 100644 --- a/charts/loki-vl-proxy/values.yaml +++ b/charts/loki-vl-proxy/values.yaml @@ -110,7 +110,7 @@ extraArgs: # query-range-background-warm: "true" # continue warming failed windows in background # query-range-background-warm-max-windows: "24" # cap background warm fanout # default-max-query-length: "0" # max query time range for all tenants (0 = unlimited; per-tenant limits override) - # max-stats-query-series: "500" # max series returned by stats metric queries (count_over_time, rate, bytes_rate); 0 = built-in default of 500; proxy-evaluated metric paths return HTTP 502/503 instead of truncating + # max-stats-query-series: "500" # max series a metric query may return (count_over_time, rate, bytes_rate); 0 = built-in default of 500, Loki's own max_query_series default; above it the query fails with Loki's HTTP 400 "maximum number of series (N) reached for a single query" (Logs Drilldown gets a partial result with a warning) # stats-query-range-concurrency: "4" # max concurrent stats_query_range calls to VL; higher = more VL CPU pressure; 0 = built-in default of 4 # drilldown-burst-window-ms: "50" # drilldown-burst-max-fields: "30" diff --git a/conformance/registry/cases/limits/drilldown-partial-with-warning.yaml b/conformance/registry/cases/limits/drilldown-partial-with-warning.yaml new file mode 100644 index 000000000..60dc38c90 --- /dev/null +++ b/conformance/registry/cases/limits/drilldown-partial-with-warning.yaml @@ -0,0 +1,39 @@ +id: limits/drilldown-partial-with-warning +title: Logs Drilldown gets a partial result and Loki's warning, not an error +proves: + - series-limits-and-partial-results + - loki_api_v1_query_range + - loki_api_v1_query +consumer: drilldown +why: | + Loki keeps Logs Drilldown rendering when a query exceeds the series limit: it + returns the series it has with the warning "maximum number of series (N) + reached for a single query; returning partial results" in the response body, + where the plugin surfaces it. A partial result with no warning is the failure + this case guards against — the panel then looks complete. +request: + method: GET + path: /loki/api/v1/query_range + params: + query: 'sum by (pod) (count_over_time({app="wide"}[1m]))' + start: "" + end: "" + step: "300" + headers: + X-Scope-OrgID: "0" + X-Query-Tags: Source=grafana-lokiexplore-app +compare: + mode: structural + fields: [status, warnings, data.resultType, data.result[].metric] + values: + mode: exact + field: data.result[].values[][1] + reason: | + Every returned series must equal Loki's for the same labels: the result is + partial in how many series it holds, never in what each one says. The proxy + keeps the busiest series by total count where Loki keeps the first series it + encounters, so the set may differ; each kept series is compared against + Loki's answer for the same label set. +applies_to: + loki: ">=3.0.0" + victorialogs: ">=v1.44.0" diff --git a/conformance/registry/cases/limits/series-limit-error.yaml b/conformance/registry/cases/limits/series-limit-error.yaml new file mode 100644 index 000000000..630af6024 --- /dev/null +++ b/conformance/registry/cases/limits/series-limit-error.yaml @@ -0,0 +1,40 @@ +id: limits/series-limit-error +title: A metric query above the series limit fails with Loki's message, not a silent cut +proves: + - series-limits-and-partial-results + - loki_api_v1_query_range + - loki_api_v1_query +consumer: api +why: | + Above `max_query_series` Loki rejects the query rather than answering with part + of the data: a chart or an alert that reads a truncated answer as the whole one + is wrong in a way nothing on screen reveals. The proxy's limit is + `-max-stats-query-series` (default 500, Loki's own default), and the message + stays Loki's word for word because Grafana renders it; the flag that set the + limit is named in the proxy's log line instead. +request: + method: GET + path: /loki/api/v1/query_range + params: + query: 'sum by (pod) (count_over_time({app="wide"}[1m]))' + start: "" + end: "" + step: "300" + headers: + X-Scope-OrgID: "0" +compare: + mode: structural + fields: [status, errorType, error] + values: + mode: exact + reason: | + Loki's text is the contract: "maximum number of series (N) reached for a + single query; consider reducing query cardinality by adding more specific + stream selectors, reducing the time range, or aggregating results with + functions like sum(), count() or topk()", HTTP 400, errorType bad_data. The + comparison runs with both sides configured to the same limit; the e2e case + reads the proxy's limit out of its own answer and compares against a Loki + configured above the fixture's cardinality. +applies_to: + loki: ">=3.0.0" + victorialogs: ">=v1.44.0" diff --git a/conformance/registry/cases/semantics/offset-sample-timestamps.yaml b/conformance/registry/cases/semantics/offset-sample-timestamps.yaml new file mode 100644 index 000000000..49f9d8acc --- /dev/null +++ b/conformance/registry/cases/semantics/offset-sample-timestamps.yaml @@ -0,0 +1,35 @@ +id: semantics/offset-sample-timestamps +title: An offset modifier moves the window, not the sample timestamps +proves: + - window-bounds-and-step-alignment + - loki_api_v1_query_range + - loki_api_v1_query +consumer: explore +why: | + `count_over_time({app="x"}[5m] offset 5m)` evaluates the window (T-10m, T-5m] + but Loki reports the sample at T. Stamping it at T-5m shifts the whole series + left by the offset, which on a dashboard reads as data that moved in time + rather than a query that looked further back. +request: + method: GET + path: /loki/api/v1/query_range + params: + query: 'sum by (pod) (count_over_time({app="tick"}[5m] offset 5m))' + start: "" + end: "" + step: "300" + headers: + X-Scope-OrgID: "0" +compare: + mode: structural + fields: [status, data.resultType, data.result[].metric, data.result[].values[][0]] + values: + mode: exact + field: data.result[].values[][1] + reason: | + Timestamps and values must both match Loki exactly: the sample at T holds + the count of the window one offset earlier. The instant form of the same + query is compared the same way. +applies_to: + loki: ">=3.0.0" + victorialogs: ">=v1.44.0" diff --git a/conformance/registry/generated/wiring.json b/conformance/registry/generated/wiring.json index f23d097db..5c7b47e83 100644 --- a/conformance/registry/generated/wiring.json +++ b/conformance/registry/generated/wiring.json @@ -16,6 +16,10 @@ "internal/proxy/sliding_window_stats_test.go", "test/e2e-compat/heavy_query_protection_test.go" ], + "limits/drilldown-partial-with-warning": [ + "internal/proxy/tumbling_axis_parity_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" + ], "limits/drilldown-stats-rejection-keeps-429": [ "internal/proxy/backend_heavy_test.go", "test/e2e-compat/heavy_query_protection_test.go" @@ -50,6 +54,10 @@ "limits/raw-metric-row-precheck": [ "internal/proxy/backend_heavy_test.go" ], + "limits/series-limit-error": [ + "internal/proxy/tumbling_axis_parity_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" + ], "limits/stats-row-limit-per-grid": [ "internal/proxy/sliding_window_stats_test.go" ], @@ -67,11 +75,17 @@ "loki_api_v1_labels": [ "internal/proxy/backend_heavy_test.go" ], + "loki_api_v1_query": [ + "internal/proxy/tumbling_axis_parity_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" + ], "loki_api_v1_query_range": [ "internal/proxy/backend_heavy_test.go", "internal/proxy/metadata_query_validation_test.go", + "internal/proxy/tumbling_axis_parity_test.go", "internal/translator/regexp_escaping_test.go", - "test/e2e-compat/heavy_query_protection_test.go" + "test/e2e-compat/heavy_query_protection_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" ], "operator-configurable-limits": [ "cmd/proxy/main_test.go", @@ -85,6 +99,10 @@ "semantics/client-cancellation-propagates": [ "internal/proxy/backend_heavy_test.go" ], + "semantics/offset-sample-timestamps": [ + "internal/proxy/tumbling_axis_parity_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" + ], "semantics/sliding-window-recurrence-exact": [ "internal/proxy/sliding_window_stats_test.go", "test/e2e-compat/heavy_query_protection_test.go" @@ -95,7 +113,9 @@ "series-limits-and-partial-results": [ "internal/proxy/backend_heavy_test.go", "internal/proxy/sliding_window_stats_test.go", - "test/e2e-compat/heavy_query_protection_test.go" + "internal/proxy/tumbling_axis_parity_test.go", + "test/e2e-compat/heavy_query_protection_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" ], "severity-detected-level-derivation": [ "internal/proxy/detected_level_test.go", @@ -115,10 +135,12 @@ "internal/proxy/volume_test.go" ], "window-bounds-and-step-alignment": [ - "internal/proxy/sliding_window_stats_test.go" + "internal/proxy/sliding_window_stats_test.go", + "internal/proxy/tumbling_axis_parity_test.go", + "test/e2e-compat/range_metric_offset_series_limit_test.go" ] }, - "registry_items": 159, + "registry_items": 162, "routes_without_registry_item": [], "test_to_items": { "cmd/proxy/main_test.go": [ @@ -171,6 +193,15 @@ "series-limits-and-partial-results", "window-bounds-and-step-alignment" ], + "internal/proxy/tumbling_axis_parity_test.go": [ + "limits/drilldown-partial-with-warning", + "limits/series-limit-error", + "loki_api_v1_query", + "loki_api_v1_query_range", + "semantics/offset-sample-timestamps", + "series-limits-and-partial-results", + "window-bounds-and-step-alignment" + ], "internal/proxy/volume_test.go": [ "drilldown/index_volume/single-sample-returns-vector", "loki_api_v1_index_volume", @@ -204,9 +235,18 @@ "loki_api_v1_query_range", "semantics/sliding-window-recurrence-exact", "series-limits-and-partial-results" + ], + "test/e2e-compat/range_metric_offset_series_limit_test.go": [ + "limits/drilldown-partial-with-warning", + "limits/series-limit-error", + "loki_api_v1_query", + "loki_api_v1_query_range", + "semantics/offset-sample-timestamps", + "series-limits-and-partial-results", + "window-bounds-and-step-alignment" ] }, - "tests_with_markers": 15, + "tests_with_markers": 17, "unknown_ids": {}, "unwired_items": [ "api_prom_label", @@ -234,7 +274,6 @@ "loki_api_v1_label", "loki_api_v1_patterns", "loki_api_v1_push", - "loki_api_v1_query", "loki_api_v1_rules", "loki_api_v1_rules_namespace", "loki_api_v1_rules_namespace_groupname", @@ -247,5 +286,5 @@ "prometheus_api_v1_rules", "service-name-derivation" ], - "wired_items": 31 + "wired_items": 35 } diff --git a/conformance/registry/state/behaviours/series-limits-and-partial-results.yaml b/conformance/registry/state/behaviours/series-limits-and-partial-results.yaml index 34005f271..77981da65 100644 --- a/conformance/registry/state/behaviours/series-limits-and-partial-results.yaml +++ b/conformance/registry/state/behaviours/series-limits-and-partial-results.yaml @@ -1,6 +1,11 @@ id: series-limits-and-partial-results state: partial reason: | + limits/series-limit-error and limits/drilldown-partial-with-warning are written + and wired: a metric query above -max-stats-query-series fails with Loki's + message and status on every metric path, range and instant, and a Logs + Drilldown request gets a partial result with Loki's partial-result warning, + compared against live Loki by unit tests and an e2e case. limits/top-n-ranking-not-silent is written and wired: topk and bottomk over grouped counts now rank every series per step with bounded per-step heaps instead of the -max-stats-query-series busiest, proven by unit tests against a @@ -8,7 +13,9 @@ reason: | test/e2e-compat/heavy_query_protection_test.go. limits/drilldown-stats-rejection-keeps-429 pins the boundary of the Drilldown partial-result carve-out: it covers the series limit, not backpressure. - limits/series-limit-error, limits/drilldown-partial-with-warning and - limits/resolution-limit-precedence are still named but not written. + Still open: limits/resolution-limit-precedence (the 11,000-point rejection + ahead of parsing) is named but not written, and the Drilldown partial keeps the + busiest series where Loki keeps the first it encounters, so the kept set can + differ from Loki's for equal-volume series. owner_decision: null waiver: null diff --git a/conformance/registry/state/behaviours/window-bounds-and-step-alignment.yaml b/conformance/registry/state/behaviours/window-bounds-and-step-alignment.yaml index 56ef407f3..91c8448aa 100644 --- a/conformance/registry/state/behaviours/window-bounds-and-step-alignment.yaml +++ b/conformance/registry/state/behaviours/window-bounds-and-step-alignment.yaml @@ -1,6 +1,12 @@ id: window-bounds-and-step-alignment -state: gap +state: partial reason: | - Seeded from behaviour we have hit in production use; cases are named but not yet written or wired. + semantics/offset-sample-timestamps is written and wired: an offset modifier + moves the window and leaves the sample timestamps on Loki's grid, on range and + instant queries, proven by unit tests and an e2e case against live Loki. The + remaining cases named by this behaviour — lower-bound-exclusive, step-alignment, + tumbling-relabel, range-smaller-than-step, grafana-24h-split and + resolution-limit-11000 — have no case files yet; step alignment itself is + carried by the -align-queries-with-step work, tracked separately. owner_decision: null waiver: null diff --git a/conformance/reports/gaps.md b/conformance/reports/gaps.md index 08b7293f8..e81c36771 100644 --- a/conformance/reports/gaps.md +++ b/conformance/reports/gaps.md @@ -9,7 +9,6 @@ compares the endpoint against Loki, plus 2 when the proxy computes the result it |---:|---|---|---|---:|---:|---| | 12 | `/loki/api/v1/label/{name}/values` | explore, drilldown, datasource, api | native_vl | 0 | 0 | no test compares it against Loki | | 9 | `/loki/api/v1/detected_fields` | explore, drilldown, api | proxy_side | 29 | 9 | no test wired to the registry; proxy-side: justify in the registry or push down to VictoriaLogs | -| 7 | `/loki/api/v1/query` | explore, drilldown, api | hybrid | 131 | 34 | no test wired to the registry | | 7 | `/loki/api/v1/detected_field/{name}/values` | drilldown, api | native_vl | 0 | 0 | no test compares it against Loki; no test wired to the registry | | 6 | `/loki/api/v1/series` | explore, datasource, api | hybrid | 29 | 8 | no test wired to the registry | | 6 | `/loki/api/v1/index/volume_range` | drilldown, api | proxy_side | 22 | 4 | no test wired to the registry; proxy-side: justify in the registry or push down to VictoriaLogs | @@ -72,11 +71,11 @@ Semantics, severity, identity and data-quality behaviour the proxy must reproduc | semantics | `numeric-and-response-formatting` — Timestamps, number formatting and empty shapes | 5 | 0 | gap | | limits | `operator-configurable-limits` — Every bound on work is an operator flag, documented from one source | 11 | 4 | proven | | semantics | `parser-error-and-label-collision` — Parser errors, __error__ and _extracted collisions | 7 | 0 | gap | -| limits | `series-limits-and-partial-results` — Series limits: error, or partial result with a warning | 7 | 3 | partial | +| limits | `series-limits-and-partial-results` — Series limits: error, or partial result with a warning | 7 | 5 | partial | | identity | `service-name-derivation` — service_name follows Loki's discovery order | 8 | 0 | partial | | severity | `severity-detected-level-derivation` — detected_level is derived on the read path | 8 | 4 | partial | | severity | `severity-exposure-surfaces` — Where detected_level and level must appear | 6 | 3 | partial | -| semantics | `window-bounds-and-step-alignment` — Range windows, bucket edges and step alignment | 7 | 1 | gap | +| semantics | `window-bounds-and-step-alignment` — Range windows, bucket edges and step alignment | 7 | 3 | partial | ## LogQL surface (v3.7.7) diff --git a/conformance/reports/roadmap.md b/conformance/reports/roadmap.md index d72c94f19..7f8fb39f0 100644 --- a/conformance/reports/roadmap.md +++ b/conformance/reports/roadmap.md @@ -5,7 +5,7 @@ Generated by `conformance/scripts/roadmap.py` from the registry's state. Do not ## Where we are - Endpoints with a test comparing the proxy against Loki: **51%** -- Behaviour and feature items: **3 proven**, 7 partial, 5 gaps, 0 waived +- Behaviour and feature items: **3 proven**, 8 partial, 4 gaps, 0 waived ## Next: unproven behaviour @@ -18,7 +18,6 @@ Ranked by which clients depend on it. | `data-probing-and-freshness` — Probes the proxy runs, and their cost and staleness | — | 0 | Seeded from behaviour we have hit in production use; cases are named but not yet written or wired. | | `numeric-and-response-formatting` — Timestamps, number formatting and empty shapes | — | 0 | Seeded from behaviour we have hit in production use; cases are named but not yet written or wired. | | `parser-error-and-label-collision` — Parser errors, __error__ and _extracted collisions | — | 0 | Seeded from behaviour we have hit in production use; cases are named but not yet written or wired. | -| `window-bounds-and-step-alignment` — Range windows, bucket edges and step alignment | — | 1 | Seeded from behaviour we have hit in production use; cases are named but not yet written or wired. | ## In progress @@ -29,10 +28,11 @@ Proven in part; the rest is named in the item's cases. | `drilldown-service-landing` — Logs Drilldown ranks services on the landing page | — | 0 | 1/3 deltas proven. | | `explore-logs-volume-json` — Grafana Explore renders the logs volume for a `| json` query | — | 0 | 2/5 deltas proven. | | `heavy-metric-fetch-bounds` — Metric evaluation reads bounded work from VictoriaLogs, or refuses early | — | 3 | Proven for grouped counts and byte sums: the two-grid recurrence, the tumbling single grid, the per-grid row limit and t | -| `series-limits-and-partial-results` — Series limits: error, or partial result with a warning | — | 3 | limits/top-n-ranking-not-silent is written and wired: topk and bottomk over grouped counts now rank every series per ste | +| `series-limits-and-partial-results` — Series limits: error, or partial result with a warning | — | 5 | limits/series-limit-error and limits/drilldown-partial-with-warning are written and wired: a metric query above -max-sta | | `service-name-derivation` — service_name follows Loki's discovery order | — | 0 | The derivation matches Loki's 13-entry discover_service_name order and is used by selectors, label filters, grouping, la | | `severity-detected-level-derivation` — detected_level is derived on the read path | — | 4 | detected_level is derived per returned row from what VictoriaLogs stored: a stored detected_level field, then the first | | `severity-exposure-surfaces` — Where detected_level and level must appear | — | 3 | Log responses, /tail, /patterns and /detected_fields carry the derived detected_level; categorize-labels puts it and a s | +| `window-bounds-and-step-alignment` — Range windows, bucket edges and step alignment | — | 3 | semantics/offset-sample-timestamps is written and wired: an offset modifier moves the window and leaves the sample times | ## Proven diff --git a/docs/KNOWN_ISSUES.md b/docs/KNOWN_ISSUES.md index 33fbdf626..ee469ae53 100644 --- a/docs/KNOWN_ISSUES.md +++ b/docs/KNOWN_ISSUES.md @@ -85,7 +85,7 @@ caps the worst case. | Large body fields | Very large body fields can still be dropped on the VictoriaLogs side. Track the upstream issue: [VictoriaLogs issue #91](https://github.com/VictoriaMetrics/victorialogs-datasource/issues/91). | | Optional tenant header | By default the proxy accepts requests without `X-Scope-OrgID` and routes them to the default tenant. Use `-require-tenant-header` (or `-auth.enabled`) to reject requests that omit the header with HTTP 401. | | Execution limits | Bounded work limits reject oversized queries instead of truncating them: raw metric scans beyond `-manual-range-metric-row-limit` (default 1,000,000 rows) return an explicit error (HTTP 502 on the manual range-metric path) instead of a partial result; binary expressions are limited to 64 nesting levels, 1,024 child evaluations and shared memory/sample budgets; `line_format` to 64 KiB per line and 16 MiB per response (HTTP 400). See [Security hardening migration](security-hardening-migration.md#execution-and-storage-limits). A rejected query does not mean the logs are absent. | -| Metric series caps | Exact raw-sample metric paths fail with an error once a query exceeds `-max-stats-query-series` series (default 500). Native `stats_query_range` paths keep the 500 busiest series by total count and drop the rest. | +| Metric series caps | A metric query above `-max-stats-query-series` (default 500, Loki's own default `max_query_series`) fails with Loki's HTTP 400 `maximum number of series (N) reached for a single query` on every path, range and instant. Grafana Logs Drilldown receives a partial result with Loki's `... returning partial results` warning; it holds the busiest series by total count, where Loki keeps the first series it encounters. | | Multi-tenant fanout concurrency | When `X-Scope-OrgID` contains multiple tenants, the proxy fans out sub-requests in parallel (goroutine per tenant). Latency equals the slowest tenant, not the sum. Very high fan-out (10+ tenants) may increase backend load proportionally. | diff --git a/docs/api-reference.md b/docs/api-reference.md index 0518bc51a..82b847b17 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -148,8 +148,9 @@ All error responses from the proxy use the standard Loki JSON error envelope: | 406 / 422 | `not_acceptable` / `execution` | Loki status mapping (for example a backend returning that status) | | 499 | `canceled` | client canceled the request | | 500 | `internal` | proxy evaluation errors, including implicit many-to-one / multiple-match vector joins; a multi-tenant request where a tenant's backend request failed | -| 502 | `unavailable` | backend request failures, `manual range metric row limit exceeded`, `maximum metric series exceeded` while collecting raw samples | -| 503 | `timeout` | circuit breaker open, `manual metric series limit exceeded` | +| 400 | `bad_data` | invalid LogQL, out-of-range parameters, `maximum number of series (N) reached for a single query` (as Loki answers above `max_query_series`) | +| 502 | `unavailable` | backend request failures, `manual range metric row limit exceeded` | +| 503 | `timeout` | circuit breaker open | | 504 | `timeout` | backend or window timeouts, including a timeout on one tenant of a multi-tenant request | See [Fixed Execution Limits](configuration.md#fixed-execution-limits) for the limit values. diff --git a/docs/compatibility-drilldown.md b/docs/compatibility-drilldown.md index 664565927..943bb532e 100644 --- a/docs/compatibility-drilldown.md +++ b/docs/compatibility-drilldown.md @@ -108,8 +108,9 @@ by one nanosecond, so a line on a window edge counts where Loki counts it. The bucket count has no budget because VictoriaLogs returns only non-empty buckets. The raw-sample evaluator answers with the same `(t-range, t]` boundaries when a bucket would be below 1 ms, when the stats response exceeds its byte limit, or -when an unaligned grid meets a backend older than v1.45 or one whose version -could not be detected. On such older backends an +when an unaligned grid meets a backend known to be older than v1.45 (an +undetected version keeps the bucket path: such a backend ignores the `offset` +argument at worst). On such older backends an epoch-aligned grid still uses buckets without the one-nanosecond shift, so a line exactly on a window edge counts in the neighbouring window. diff --git a/docs/configuration.md b/docs/configuration.md index 3bb36c5bf..0d88d4ba1 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -459,7 +459,7 @@ These flags control Loki-compatible `query_range` split/merge execution with per | Flag | Env | Default | Description | |---|---|---|---| | `-drilldown-scan-timeout` | — | `5s` | Per-request timeout for the `detected_fields` / `detected_field/{name}/values` log-scan path. `0` disables the cap | -| `-max-stats-query-series` | — | `0` (uses `500`) | Maximum series returned by stats metric queries (`count_over_time`, `rate`, `bytes_rate`, ...). `0` uses the built-in default of `500`, matching the Drilldown series cap. VictoriaLogs-native stats results keep the busiest series; proxy-evaluated (manual) metric paths reject the query instead of truncating (see [Fixed Execution Limits](#fixed-execution-limits)) | +| `-max-stats-query-series` | — | `0` (uses `500`) | Maximum series a metric query may return (`count_over_time`, `rate`, `bytes_rate`, ...), the proxy's equivalent of Loki's `max_query_series`; `0` uses the built-in default of `500`, which is also Loki's default. Above it the query is rejected with HTTP `400` and Loki's message `maximum number of series (N) reached for a single query`, on range and instant queries alike; the log line for the rejected request names this flag. Grafana Logs Drilldown requests receive a partial result (the busiest series) with Loki's `... returning partial results` warning instead, as Loki does for that client (see [Fixed Execution Limits](#fixed-execution-limits)) | | `-stats-query-range-concurrency` | — | `0` (uses `4`) | Maximum concurrent `stats_query_range` calls to VictoriaLogs. `0` uses the built-in default of `4` | | `-stats-query-range-inter-query-delay-ms` | — | `200` | Minimum pause in ms between consecutive individual `stats_query_range` calls; the concurrency slot is held for this long after each call. `0` disables | | `-drilldown-burst-window-ms` | — | `50` | Window in ms for coalescing concurrent Drilldown Fields per-field count queries into one fused VictoriaLogs call. `0` disables the coalescer | @@ -766,7 +766,7 @@ See [Performance — Go Runtime Tuning](performance.md#go-runtime-tuning) for gu | Flag | Env | Default | Description | |---|---|---|---| | `-max-lines` | — | `1000` | Default max lines per query | -| `-manual-range-metric-row-limit` | — | `1000000` | Maximum rows fetched per proxy-side range-metric evaluation (`rate`, `count_over_time`, etc.): raw log rows, or stats rows (one per non-empty step bucket and series) on the window stats path. Exceeding it rejects the query with HTTP `502` (`manual range metric row limit exceeded ... increase -manual-range-metric-row-limit`) instead of returning truncated results. On the window stats path the limit bounds each bucket grid separately (at most one row per step and series), not their sum. For pipe-free selectors over at least `-backend-heavy-query-min-range`, a one-row count runs first and rejects the query before any log line is fetched. `count_over_time`, `rate`, `bytes_over_time` and `bytes_rate` normally avoid raw rows by summing `stats_query_range` buckets of `gcd(step, range)` (one bucket per step when the range is shorter than the step). When that grid would need more than 11,000 buckets, and for every `topk`/`bottomk` input, they instead read one window-sized seed bucket and two step-sized bucket grids from a streamed VictoriaLogs stats pipe and derive each window exactly; `topk`/`bottomk` then rank every series at each step. Raw rows remain the fallback only when the bucket is below 1 ms or the evaluation grid is not epoch-aligned and VictoriaLogs is older than v1.45 or its version could not be detected. The encoded range-metric response is capped at 64 MiB (HTTP `503` above it), and Loki's 11,000-points-per-series limit bounds the evaluation steps. Proxy memory on the window stats path is about 100 bytes per stats row, so the default bounds one evaluation to roughly 100 MiB | +| `-manual-range-metric-row-limit` | — | `1000000` | Maximum rows fetched per proxy-side range-metric evaluation (`rate`, `count_over_time`, etc.): raw log rows, or stats rows (one per non-empty step bucket and series) on the window stats path. Exceeding it rejects the query with HTTP `502` (`manual range metric row limit exceeded ... increase -manual-range-metric-row-limit`) instead of returning truncated results. On the window stats path the limit bounds each bucket grid separately (at most one row per step and series), not their sum. For pipe-free selectors over at least `-backend-heavy-query-min-range`, a one-row count runs first and rejects the query before any log line is fetched. `count_over_time`, `rate`, `bytes_over_time` and `bytes_rate` normally avoid raw rows by summing `stats_query_range` buckets of `gcd(step, range)` (one bucket per step when the range is shorter than the step). When that grid would need more than 11,000 buckets, and for every `topk`/`bottomk` input, they instead read one window-sized seed bucket and two step-sized bucket grids from a streamed VictoriaLogs stats pipe and derive each window exactly; `topk`/`bottomk` then rank every series at each step. Raw rows remain the fallback only when the bucket is below 1 ms or the evaluation grid is not epoch-aligned and VictoriaLogs is known to be older than v1.45 (an undetected version keeps the bucket path: the `offset` argument is ignored at worst). The encoded range-metric response is capped at 64 MiB (HTTP `503` above it), and Loki's 11,000-points-per-series limit bounds the evaluation steps. Proxy memory on the window stats path is about 100 bytes per stats row, so the default bounds one evaluation to roughly 100 MiB | | `-ordered-json-metric-max-bytes` | — | `1073741824` | Safety cap, not the fix for slow queries: the most bytes the proxy-side ordered JSON metric evaluator reads from the VictoriaLogs raw rows response, and the largest response it builds. `0` uses the 1 GiB default; there is no upper bound. Exceeding it rejects the query with HTTP `502` (`ordered JSON metric response exceeds N bytes; narrow the query or increase -ordered-json-metric-max-bytes`) instead of returning partial results. The default admits about one million rows of 1 KiB, the `-manual-range-metric-row-limit` default; on the e2e generator (about 290k lines and 300 MiB of rows per hour) that is roughly 3.5 hours. Rows are streamed, not buffered, so raising it costs VictoriaLogs scan work and network transfer; proxy memory grows with the retained rows, which `-manual-range-metric-row-limit` bounds. Lower it to protect VictoriaLogs from long raw scans. Helm: `extraArgs.ordered-json-metric-max-bytes`. The evaluator answers only `| json` range metrics that need Loki's per-line semantics (label filters, surviving parser errors, grouping by labels with underscores). Summed `count_over_time`, `rate`, `bytes_over_time` and `bytes_rate` that drop parser errors and group by `detected_level` or underscore-free labels, including Grafana's logs volume queries for plain, `| json` and `| logfmt` selectors, are computed from VictoriaLogs `stats_query_range` buckets and do not read raw rows. When a line in the range has a body that VictoriaLogs and Loki parse differently (for example JSON with a syntax error after the key, or logfmt with tabs), the query keeps the previous route: this evaluator for `| json`, the native stats route for plain and `| logfmt`. Grafana Logs Drilldown requests without a JSON parser keep their dedicated routes. | | `-backend-timeout` | — | `120s` | Timeout for non-streaming VL backend requests. The remaining request budget (this timeout or an earlier request deadline) is passed to VictoriaLogs as its `timeout` query argument, so VictoriaLogs stops executing a query the proxy has given up on; VictoriaLogs caps it at its own `-search.maxQueryDuration`. A client disconnect cancels the in-flight VictoriaLogs request | | `-backend-min-version` | — | `v1.30.0` | Minimum VictoriaLogs version considered fully supported at startup compatibility gate | @@ -806,7 +806,7 @@ Backend version gate notes: - If detected version is below `-backend-min-version`, startup is blocked by default. - Set `-backend-allow-unsupported-version=true` to bypass the gate at your own risk. - If version cannot be detected from headers, proxy logs a warning and continues startup. -- Version-gated behaviour stays on its conservative path while the version is unknown. Sliding range metrics then use `stats_query_range` buckets only for epoch-aligned grids (VictoriaLogs before v1.45 ignores the `offset` argument). While the version remains unknown, for example when the startup probe ran before VictoriaLogs was ready or `/metrics` is not routed through a proxy in front of it, the proxy retries the `/metrics` version probe in the background at most once every 5 minutes, bounded by `-backend-version-check-timeout`. There is no version override flag; make `/metrics` or a version response header reachable to enable version-gated paths. +- Version-gated behaviour keeps VictoriaLogs-native evaluation while the version is unknown: range metrics still use `stats_query_range` buckets anchored with the `offset` argument, which a release predating it (before v1.45) ignores, leaving the epoch-aligned buckets such a backend has anyway. Only a version *known* to be older than v1.45 restricts buckets to epoch-aligned grids. While the version remains unknown, for example when the startup probe ran before VictoriaLogs was ready or `/metrics` is not routed through a proxy in front of it, the proxy retries the `/metrics` version probe in the background at most once every 5 minutes, bounded by `-backend-version-check-timeout`. There is no version override flag; make `/metrics` or a version response header reachable to enable version-gated paths. ## Built-In Protection Defaults @@ -864,7 +864,7 @@ These protective limits bound what one request may do. Rejections are errors, no | Manual range-metric rows | `-manual-range-metric-row-limit` (default 1,000,000 raw or stats rows); 64 MiB raw backend response; stats rows of at most 64 KiB each | `502` | | Heavy VictoriaLogs calls | `-backend-max-concurrent-heavy-queries` (default 2) running, queued for `-backend-heavy-query-queue-wait` (default 20s) | `429` (`too many outstanding requests`) | | Ordered JSON metric evaluator | `-ordered-json-metric-max-bytes` (default 1 GiB) on the raw rows response and on the built response; `-manual-range-metric-row-limit` rows | `502` | -| Manual range-metric series | `-max-stats-query-series` (default 500) | `502` when hit while collecting raw samples (`maximum metric series exceeded`); `503` when hit while building the result (`manual metric series limit exceeded`) | +| Metric query series | `-max-stats-query-series` (default 500) | `400` with Loki's `maximum number of series (N) reached for a single query` on every metric path, range and instant; Grafana Logs Drilldown gets the busiest series and the `... returning partial results` warning | | Request coalescer | 256 MiB per shared response body | error instead of silent truncation | | Hot/cold merge | 64 MiB buffered per hot or cold response | error | | Multi-tenant reads | `-multi-tenant-max-fanout` (default 64) tenants per request; `-multi-tenant-max-merged-response-bytes` (default 32 MiB) merged response | `400` / `413`, each naming its flag | diff --git a/docs/operations.md b/docs/operations.md index 729df5837..591b31a6f 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -382,7 +382,7 @@ For exact proxy-only overhead on translated paths, use structured request logs w Not every `502` or `503` means VictoriaLogs is down. Built-in execution limits reject oversized work instead of returning truncated data: - `502` with `manual range metric row limit exceeded` — raise `-manual-range-metric-row-limit` or narrow the query -- `502` with `maximum metric series exceeded` or `503` with `manual metric series limit exceeded` — narrow the query or raise `-max-stats-query-series` +- `400` with `maximum number of series (N) reached for a single query` — Loki's own answer above `max_query_series`; narrow the query or raise `-max-stats-query-series`. The log line for the rejected request names that flag. Grafana Logs Drilldown requests instead receive a partial result with the warning `... returning partial results` - `503` with `too many concurrent queries` — the `-max-concurrent` admission cap was reached - `429` with `too many outstanding requests: heavy VictoriaLogs queries are limited to -backend-max-concurrent-heavy-queries=N` — heavy long-range VictoriaLogs calls waited longer than `-backend-heavy-query-queue-wait` for a slot. Retry, narrow the time range, or raise `-backend-max-concurrent-heavy-queries` together with VictoriaLogs memory (see [Heavy VictoriaLogs Query Admission](configuration.md#heavy-victorialogs-query-admission)) diff --git a/docs/real-window-compatibility-gaps.md b/docs/real-window-compatibility-gaps.md index 915edc7d8..82de83a98 100644 --- a/docs/real-window-compatibility-gaps.md +++ b/docs/real-window-compatibility-gaps.md @@ -64,8 +64,7 @@ Raw metric collectors request one extra row and reject overflow rather than retu | Guard | Setting | Client-visible result | | --- | --- | --- | | Raw rows per manual metric call | `-manual-range-metric-row-limit` (default 1,000,000) | HTTP 502, `errorType` `unavailable`, `manual range metric row limit exceeded` | -| Raw metric series during collection | `-max-stats-query-series` (default 500) | HTTP 502, `maximum metric series exceeded` | -| Manual result matrix series | `-max-stats-query-series` | HTTP 503, `manual metric series limit exceeded` | +| Metric query series | `-max-stats-query-series` (default 500) | HTTP 400, Loki's `maximum number of series (N) reached for a single query`; Logs Drilldown gets a partial result with the `... returning partial results` warning | | Native `stats_query_range` series | `-max-stats-query-series` | Busiest series kept; remaining series dropped without an error | Raw and ordered-parser metric paths therefore fail at the series limit, while native stats paths return at most that many series. diff --git a/internal/proxy/backend.go b/internal/proxy/backend.go index 590afb917..55c8f1461 100644 --- a/internal/proxy/backend.go +++ b/internal/proxy/backend.go @@ -380,15 +380,18 @@ const backendVersionReprobeInterval = 5 * time.Minute // supportsStatsRangeOffset reports whether stats_query_range honours the offset // arg (VictoriaLogs v1.45+). Older releases silently ignore it and align buckets -// to the epoch, so an unknown version (failed or pending probe) is treated as -// unsupported: only epoch-aligned grids use buckets, others the raw evaluator. -// While the version is unknown, the metrics probe is retried in the background. +// to the epoch. An unknown version (failed or pending probe) is treated as +// supported: the arg is then ignored rather than rejected, which is no worse +// than the epoch-aligned buckets such a backend has anyway, while buffering raw +// log rows instead would scale the query with the stored data. While the version +// is unknown, the metrics probe is retried in the background. func (p *Proxy) supportsStatsRangeOffset() bool { p.backendVersionMu.RLock() known, supported := p.backendVersionSemver != "", p.backendSupportsStatsRangeOffset p.backendVersionMu.RUnlock() if !known { p.maybeReprobeBackendVersion(time.Now()) + return true } return supported } diff --git a/internal/proxy/bare_parser_sliding_parity_test.go b/internal/proxy/bare_parser_sliding_parity_test.go index be8b8ed31..6c9e10554 100644 --- a/internal/proxy/bare_parser_sliding_parity_test.go +++ b/internal/proxy/bare_parser_sliding_parity_test.go @@ -230,15 +230,25 @@ func TestBareParserRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { if streamFields != nil { countRoute = "hits" } + // An unknown version (failed or pending probe) keeps anchored + // buckets: a backend that ignores the arg has the epoch-aligned + // buckets it would have had anyway, while raw rows would scale + // the query with the stored data. + unalignedRoute, unalignedStep, unalignedOffset := "raw", "", "" + alignedOffset, gcdOffset := "none", "" + if version == "" { + unalignedRoute, unalignedStep, unalignedOffset = "stats", "60s", "-30000000001ns" + alignedOffset, gcdOffset = "-1ns", "-10000000001ns" + } runBareParserCases(t, lines, version, streamFields, []bareParserCase{ - {query: `count_over_time({app=~"bare-.*"} | logfmt [2m])`, fn: "count_over_time", window: 2 * time.Minute, start: aligned, end: alignedEnd, route: countRoute, wantStep: "60s", wantOffset: "none"}, - {query: `count_over_time({app=~"bare-.*"} | logfmt [2m])`, fn: "count_over_time", window: 2 * time.Minute, start: unaligned, end: unalignedEnd, route: "raw"}, - {query: `bytes_over_time({app=~"bare-.*"} | logfmt [1m])`, fn: "bytes_over_time", window: time.Minute, start: aligned, end: alignedEnd, route: "stats", wantStep: "60s", wantOffset: "none"}, - {query: `bytes_over_time({app=~"bare-.*"} | logfmt [90s])`, fn: "bytes_over_time", window: 90 * time.Second, start: s0.Add(5*time.Minute + 10*time.Second), end: alignedEnd, route: "raw"}, + {query: `count_over_time({app=~"bare-.*"} | logfmt [2m])`, fn: "count_over_time", window: 2 * time.Minute, start: aligned, end: alignedEnd, route: countRoute, wantStep: "60s", wantOffset: alignedOffset}, + {query: `count_over_time({app=~"bare-.*"} | logfmt [2m])`, fn: "count_over_time", window: 2 * time.Minute, start: unaligned, end: unalignedEnd, route: map[bool]string{true: countRoute, false: "raw"}[version == ""], wantStep: unalignedStep, wantOffset: unalignedOffset}, + {query: `bytes_over_time({app=~"bare-.*"} | logfmt [1m])`, fn: "bytes_over_time", window: time.Minute, start: aligned, end: alignedEnd, route: "stats", wantStep: "60s", wantOffset: alignedOffset}, + {query: `bytes_over_time({app=~"bare-.*"} | logfmt [90s])`, fn: "bytes_over_time", window: 90 * time.Second, start: s0.Add(5*time.Minute + 10*time.Second), end: alignedEnd, route: map[bool]string{true: "stats", false: "raw"}[version == ""], wantStep: map[bool]string{true: "30s", false: ""}[version == ""], wantOffset: gcdOffset}, }) runBareParserCases(t, numberLines(lines), version, streamFields, []bareParserCase{ - {query: `sum_over_time({app=~"bare-.*"} | logfmt | unwrap n [2m])`, fn: "sum_over_time", window: 2 * time.Minute, start: aligned, end: alignedEnd, route: "stats", wantStep: "60s", wantOffset: "none"}, - {query: `max_over_time({app=~"bare-.*"} | logfmt | unwrap n [2m])`, fn: "max_over_time", window: 2 * time.Minute, start: unaligned, end: unalignedEnd, route: "raw"}, + {query: `sum_over_time({app=~"bare-.*"} | logfmt | unwrap n [2m])`, fn: "sum_over_time", window: 2 * time.Minute, start: aligned, end: alignedEnd, route: "stats", wantStep: "60s", wantOffset: alignedOffset}, + {query: `max_over_time({app=~"bare-.*"} | logfmt | unwrap n [2m])`, fn: "max_over_time", window: 2 * time.Minute, start: unaligned, end: unalignedEnd, route: unalignedRoute, wantStep: unalignedStep, wantOffset: unalignedOffset}, }) }) } diff --git a/internal/proxy/drilldown_compat_test.go b/internal/proxy/drilldown_compat_test.go index d85fc6a05..566bef3f7 100644 --- a/internal/proxy/drilldown_compat_test.go +++ b/internal/proxy/drilldown_compat_test.go @@ -1706,12 +1706,13 @@ func TestDrilldown_LabelCardMetricQuery_ServiceNameNonEmptyFilterUsesSyntheticAn w := httptest.NewRecorder() q := url.Values{} q.Set("query", `sum(count_over_time({service_name="argocd",service_name != ""}[5s])) by (service_name)`) + // The range holds the backend bucket (17:14:55): with range < step it is the + // step bucket that ends one range after that label, 17:15:00. q.Set("start", "2026-04-04T17:00:00Z") q.Set("end", "2026-04-04T17:30:00Z") q.Set("step", "300") r := httptest.NewRequest("GET", "/loki/api/v1/query_range?"+q.Encode(), nil) - // The label card is a Logs Drilldown panel; its range < step keeps the - // native Drilldown routing. + // The label card is a Logs Drilldown panel. r.Header.Set("X-Query-Tags", "Source=grafana-lokiexplore-app") p.handleQueryRange(w, r) @@ -1722,9 +1723,12 @@ func TestDrilldown_LabelCardMetricQuery_ServiceNameNonEmptyFilterUsesSyntheticAn // every row the exact matcher already selected, and grouping by // service_name computes the derived value before the stats pipe (with // format pipes: the test backend reports no version with the coalesce pipe). + // The range is shorter than the step, so the window evaluator adds its + // window-phase filter between the derivation pipes and the stats pipe. if !strings.HasPrefix(statsQuery, `(service_name:="argocd" OR `) || !strings.Contains(statsQuery, `) * | format "" as service_name keep_original_fields`) || - !strings.Contains(statsQuery, `as service_name keep_original_fields | stats by (service_name) count()`) { + !strings.Contains(statsQuery, `as service_name keep_original_fields |`) || + !strings.Contains(statsQuery, `| stats by (service_name) count()`) { t.Fatalf("expected derived service_name matchers, got %q", statsQuery) } diff --git a/internal/proxy/drilldown_field_batcher.go b/internal/proxy/drilldown_field_batcher.go index f6ed52a81..703e65161 100644 --- a/internal/proxy/drilldown_field_batcher.go +++ b/internal/proxy/drilldown_field_batcher.go @@ -214,11 +214,13 @@ func (batch *fieldBatch) fire() { } defer func() { b.sem <- struct{}{} }() + // One over the limit, so the caller can tell a truncated answer from + // one that happens to end exactly on the limit. perFieldQuery := batch.cleanBase + " | " + quoteLogsQLIdent(e.primaryVLField) + ":*" + " | stats by (" + quoteLogsQLIdent(e.primaryVLField) + ") count() as _c" + " | sort by (_c desc)" + - " | limit " + strconv.Itoa(maxDrilldownSeries) + " | limit " + strconv.Itoa(b.proxy.drilldownSeriesLimit()+1) params := buildStatsQueryRangeParams(perFieldQuery, batch.startRaw, batch.endRaw, batch.stepRaw) resp, err := b.proxy.vlPost(ctx30s, "/select/logsql/stats_query_range", params) @@ -240,7 +242,8 @@ func (batch *fieldBatch) fire() { if e.primaryVLField != e.lokiField { rawBody = renameStatsBodyMetricKey(rawBody, e.primaryVLField, e.lokiField) } - rawBody = limitLokiMatrixSeries(rawBody, maxDrilldownSeries) + // The cut itself happens per request (capSeriesToLimit), so each + // caller's response carries its own partial-result warning. if doZerofill { rawBody = zerofillStatsMatrix(rawBody, batchStartSec, batchEndSec, batchStepSec) } diff --git a/internal/proxy/drilldown_regression_lock_test.go b/internal/proxy/drilldown_regression_lock_test.go index 93907f527..4512009ba 100644 --- a/internal/proxy/drilldown_regression_lock_test.go +++ b/internal/proxy/drilldown_regression_lock_test.go @@ -230,15 +230,16 @@ func TestLock_HitsRunsForAllRanges(t *testing.T) { startSec int64 endSec int64 stepRaw string + window string expectHit bool }{ - // step must be >= range vector window (2m) so handleStatsCompatRange - // falls through to the Drilldown router instead of the manual path. - {"5m", 1700000000, 1700000300, "120", true}, - {"1h", 1700000000, 1700003600, "120", true}, - {"6h", 1700000000, 1700021600, "120", true}, - {"24h", 1700000000, 1700086400, "120", true}, - {"7d", 1700000000, 1700604800, "600", true}, + // The range vector window equals the step so handleStatsCompatRange + // falls through to the Drilldown router instead of the window evaluator. + {"5m", 1700000000, 1700000300, "120", "2m", true}, + {"1h", 1700000000, 1700003600, "120", "2m", true}, + {"6h", 1700000000, 1700021600, "120", "2m", true}, + {"24h", 1700000000, 1700086400, "120", "2m", true}, + {"7d", 1700000000, 1700604800, "600", "10m", true}, } for _, tc := range ranges { t.Run(tc.name, func(t *testing.T) { @@ -253,7 +254,7 @@ func TestLock_HitsRunsForAllRanges(t *testing.T) { defer vl.Close() p := newTestProxy(t, vl.URL) r := drilldownRequest(t, - `sum by (pod) (count_over_time({namespace="prod"}|pod!=""`+` [2m]))`, + `sum by (pod) (count_over_time({namespace="prod"}|pod!=""`+` [`+tc.window+`]))`, tc.startSec, tc.endSec, tc.stepRaw, "drilldown") p.proxyStatsQueryRange(httptest.NewRecorder(), r, `namespace:="prod" | filter pod:!"" | stats by (pod) count()`) diff --git a/internal/proxy/drilldown_strip_test.go b/internal/proxy/drilldown_strip_test.go index bbb903f1f..82a9a5ef5 100644 --- a/internal/proxy/drilldown_strip_test.go +++ b/internal/proxy/drilldown_strip_test.go @@ -67,8 +67,8 @@ func TestProxyStatsQueryRange_ParserDirectPath(t *testing.T) { t.Errorf("query lost | filter trace_id existence check: %s", receivedQuery) } // Parser-direct path must add VL-side | limit to bound per-bucket cardinality. - if !strings.Contains(receivedQuery, fmt.Sprintf("| limit %d", maxDrilldownSeries)) { - t.Errorf("parser-direct path must add | limit %d: %s", maxDrilldownSeries, receivedQuery) + if !strings.Contains(receivedQuery, fmt.Sprintf("| limit %d", defaultStatsQuerySeries+1)) { + t.Errorf("parser-direct path must add | limit %d: %s", defaultStatsQuerySeries+1, receivedQuery) } } @@ -91,7 +91,7 @@ func TestLimitLokiMatrixSeries(t *testing.T) { t.Run("under limit unchanged", func(t *testing.T) { body := makeSeries(10) - got := limitLokiMatrixSeries(body, 100) + got := limitLokiResultSeries(body, 100) if string(got) != string(body) { t.Errorf("expected body unchanged, got different result") } @@ -99,7 +99,7 @@ func TestLimitLokiMatrixSeries(t *testing.T) { t.Run("at limit unchanged", func(t *testing.T) { body := makeSeries(100) - got := limitLokiMatrixSeries(body, 100) + got := limitLokiResultSeries(body, 100) if string(got) != string(body) { t.Errorf("expected body unchanged at exact limit") } @@ -123,7 +123,7 @@ func TestLimitLokiMatrixSeries(t *testing.T) { sb.WriteString(`]}}`) body := []byte(sb.String()) - got := limitLokiMatrixSeries(body, 100) + got := limitLokiResultSeries(body, 100) // High-count series at index 150 must be in the top-100 output if !strings.Contains(string(got), `"id-150"`) { t.Errorf("high-count series (id-150) should survive top-by-count cut") @@ -132,7 +132,7 @@ func TestLimitLokiMatrixSeries(t *testing.T) { t.Run("invalid json returned unchanged", func(t *testing.T) { body := []byte(`not json`) - got := limitLokiMatrixSeries(body, 5) + got := limitLokiResultSeries(body, 5) if string(got) != string(body) { t.Errorf("invalid JSON should be returned unchanged") } @@ -175,7 +175,7 @@ func TestProxyStatsQueryRange_DrilldownPathAlwaysOn(t *testing.T) { req.Header.Set("X-Query-Tags", header) } p.proxyStatsQueryRange(httptest.NewRecorder(), req, effectiveQuery) - if got := strings.Contains(receivedQuery, fmt.Sprintf("| limit %d", maxDrilldownSeries)); got != wantDrilldown { + if got := strings.Contains(receivedQuery, fmt.Sprintf("| limit %d", defaultStatsQuerySeries+1)); got != wantDrilldown { t.Errorf("Drilldown path used=%v, want %v (header=%q); VL received: %s", got, wantDrilldown, header, receivedQuery) } } @@ -234,8 +234,8 @@ func TestProxyStatsQueryRangeDrilldown_ReturnsPerValueSeries(t *testing.T) { if !strings.Contains(receivedQuery, "stats by (status_code) count()") { t.Errorf("Drilldown path changed the query: %s", receivedQuery) } - if !strings.Contains(receivedQuery, fmt.Sprintf("| limit %d", maxDrilldownSeries)) { - t.Errorf("Drilldown path must add VL-side | limit %d: %s", maxDrilldownSeries, receivedQuery) + if !strings.Contains(receivedQuery, fmt.Sprintf("| limit %d", defaultStatsQuerySeries+1)) { + t.Errorf("Drilldown path must add VL-side | limit %d: %s", defaultStatsQuerySeries+1, receivedQuery) } // Must return 3 separate per-value series — NOT a single aggregate seriesCount := strings.Count(body, `"status_code"`) @@ -641,8 +641,8 @@ func TestDrilldownTwoPhase_BasicFlow(t *testing.T) { t.Errorf("Phase 1 step should be entire range 3600s, got %q", phase1Step) } // Phase 1 must include | limit 500 - if !strings.Contains(phase1Query, fmt.Sprintf("| limit %d", maxDrilldownSeries)) { - t.Errorf("Phase 1 query must append | limit %d: %q", maxDrilldownSeries, phase1Query) + if !strings.Contains(phase1Query, fmt.Sprintf("| limit %d", defaultStatsQuerySeries+1)) { + t.Errorf("Phase 1 query must append | limit %d: %q", defaultStatsQuerySeries+1, phase1Query) } // Phase 2 must use field:in(...) filter with the values from Phase 1 @@ -917,7 +917,7 @@ func TestRelaxStepForLowCardinality(t *testing.T) { { name: "cardinality at limit (500) means truncated — does not relax", origStep: "300s", - cardinality: maxDrilldownSeries, + cardinality: defaultStatsQuerySeries, rangeSec: 2 * day, wantOK: false, }, @@ -945,7 +945,7 @@ func TestRelaxStepForLowCardinality(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, ok := relaxStepForLowCardinality(tt.origStep, tt.cardinality, tt.rangeSec*int64(time.Second)) + got, ok := relaxStepForLowCardinality(tt.origStep, tt.cardinality, tt.rangeSec*int64(time.Second), defaultStatsQuerySeries) if ok != tt.wantOK { t.Fatalf("relaxStepForLowCardinality(%q, %d, %ds) ok = %v, want %v", tt.origStep, tt.cardinality, tt.rangeSec, ok, tt.wantOK) diff --git a/internal/proxy/grouped_rate_count_paths_test.go b/internal/proxy/grouped_rate_count_paths_test.go index 1b9ead849..e69949f1b 100644 --- a/internal/proxy/grouped_rate_count_paths_test.go +++ b/internal/proxy/grouped_rate_count_paths_test.go @@ -114,7 +114,7 @@ func TestGroupedRangeMetricSingleFieldLongRangeMatchesLoki(t *testing.T) { grafana bool field string want map[string]float64 - twoPhase bool // count control: the bounded two-phase path must still serve it + twoPhase bool // Drilldown count: the bounded two-phase path serves it forbidHit bool }{ {name: "rate 5m step 300 over 6h", query: `sum by (app) (rate({namespace="prod"}[5m]))`, step: 5 * time.Minute, rangeDur: 6 * time.Hour, field: "app", want: map[string]float64{"api": 4, "web": 1}}, @@ -122,7 +122,7 @@ func TestGroupedRangeMetricSingleFieldLongRangeMatchesLoki(t *testing.T) { {name: "rate with existence filter over 6h", query: `sum by (app) (rate({namespace="prod", app!=""}[5m]))`, step: 5 * time.Minute, rangeDur: 6 * time.Hour, field: "app", want: map[string]float64{"api": 4, "web": 1}}, {name: "bytes_rate 5m step 300 over 6h", query: `sum by (app) (bytes_rate({namespace="prod"}[5m]))`, step: 5 * time.Minute, rangeDur: 6 * time.Hour, field: "app", want: map[string]float64{"api": 4 * groupedRateLineBytes, "web": groupedRateLineBytes}}, {name: "grafana high-card rate over 6h", query: `sum by (trace_id) (rate({namespace="prod"}[5m]))`, step: 5 * time.Minute, rangeDur: 6 * time.Hour, grafana: true, field: "trace_id", want: map[string]float64{"api": 4, "web": 1}, forbidHit: true}, - {name: "count_over_time control over 6h", query: `sum by (app) (count_over_time({namespace="prod"}[5m]))`, step: 5 * time.Minute, rangeDur: 6 * time.Hour, field: "app", want: map[string]float64{"api": 1200, "web": 300}, twoPhase: true}, + {name: "count_over_time over 6h", query: `sum by (app) (count_over_time({namespace="prod"}[5m]))`, step: 5 * time.Minute, rangeDur: 6 * time.Hour, field: "app", want: map[string]float64{"api": 1200, "web": 300}}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { diff --git a/internal/proxy/http_utils.go b/internal/proxy/http_utils.go index f3dda285b..baad25fad 100644 --- a/internal/proxy/http_utils.go +++ b/internal/proxy/http_utils.go @@ -252,7 +252,13 @@ func (p *Proxy) writeError(w http.ResponseWriter, code int, msg string) { level = slog.LevelWarn } if p.log != nil && p.log.Enabled(context.Background(), level) { - p.log.Log(context.Background(), level, "request error", "code", code, "error", msg) + attrs := []any{"code", code, "error", msg} + if strings.HasPrefix(msg, seriesLimitMessagePrefix) { + // Loki's message carries no operator hint; the log says which flag + // sets the limit that rejected this query. + attrs = append(attrs, "limit_flag", "-max-stats-query-series") + } + p.log.Log(context.Background(), level, "request error", attrs...) } w.Header().Set("Content-Type", "application/json") w.WriteHeader(code) @@ -510,6 +516,9 @@ func (e *upstreamStatusError) StatusCode() int { return e.status } // rejections and keep the backend-failure handling (fallbacks, stale reads, // partial results, per-tenant skipping, 5xx mapping). func isUpstreamQueryRejected(err error) bool { + if isSeriesLimitError(err) { + return true + } var statusErr *upstreamStatusError if errors.As(err, &statusErr) { return statusErr.class == vlErrorQueryRejected diff --git a/internal/proxy/manual_metric_budget.go b/internal/proxy/manual_metric_budget.go index db8c109ff..5be227126 100644 --- a/internal/proxy/manual_metric_budget.go +++ b/internal/proxy/manual_metric_budget.go @@ -4,14 +4,20 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "math" "net/http" "net/url" + "sort" "strconv" "strings" + "sync/atomic" "time" + + logqlpkg "github.com/ReliablyObserve/Loki-VL-proxy/internal/logql" + fj "github.com/valyala/fastjson" ) // precheckRawMetricRows counts the rows a long-range raw metric fetch would @@ -50,6 +56,255 @@ func (p *Proxy) precheckRawMetricRows(ctx context.Context, baseQuery string, fet return nil } +// seriesLimitMessagePrefix opens Loki's series limit message; writeError uses it +// to name the proxy flag in the log without changing what the client reads. +const seriesLimitMessagePrefix = "maximum number of series (" + +// seriesLimitError is Loki's answer to a metric query over its series limit +// (logqlmodel.NewSeriesLimitError, HTTP 400), word for word: Grafana shows the +// message to the user, so it stays Loki's. The proxy's limit is set by +// -max-stats-query-series, which the log line names. +type seriesLimitError struct{ limit int } + +func (e *seriesLimitError) Error() string { + return fmt.Sprintf("%s%d) reached for a single query; consider reducing query cardinality by adding more specific stream selectors, reducing the time range, or aggregating results with functions like sum(), count() or topk()", seriesLimitMessagePrefix, e.limit) +} + +func isSeriesLimitError(err error) bool { + var limitErr *seriesLimitError + return errors.As(err, &limitErr) +} + +// seriesLimitScope records how a request answers a metric result over the series +// limit. Loki gives Grafana Logs Drilldown the first series with a warning +// (pkg/logql/engine.go JoinSampleVector) and every other client an error. +type seriesLimitScope struct { + drilldown bool + truncated atomic.Int64 // the limit that cut a Drilldown result; 0 = none +} + +type seriesLimitScopeKey struct{} + +// withSeriesLimitScope attaches a series limit scope for r to its context. +func withSeriesLimitScope(r *http.Request) (*http.Request, *seriesLimitScope) { + scope := &seriesLimitScope{drilldown: isGrafanaDrilldownRequest(r)} + return r.WithContext(context.WithValue(r.Context(), seriesLimitScopeKey{}, scope)), scope +} + +// seriesLimitReached is called when a metric result holds more than limit +// series. It returns Loki's error, or nil when the request keeps a partial +// result (Drilldown); the caller then keeps limit series. +func seriesLimitReached(ctx context.Context, limit int) error { + if scope, _ := ctx.Value(seriesLimitScopeKey{}).(*seriesLimitScope); scope != nil && scope.drilldown { + scope.truncated.Store(int64(limit)) + return nil + } + return &seriesLimitError{limit: limit} +} + +// logQLProducesSamples reports whether logql evaluates to samples (a matrix or +// vector) rather than log lines. Only those responses carry sample timestamps to +// shift or series to limit; a log query must stream straight through. +func logQLProducesSamples(logql string) bool { + parsed, err := logqlpkg.Parse(logql) + if err != nil { + return false + } + switch parsed.(type) { + case *logqlpkg.RangeAggregation, *logqlpkg.VectorAggregation, *logqlpkg.BinOpExpr, *logqlpkg.OpaqueMetricExpr, *logqlpkg.LiteralExpr: + return true + } + return false +} + +// metricResponseRewriter buffers a metric response and, before sending it, moves +// every sample timestamp forward by shift (a LogQL offset evaluates the shifted +// range but stamps samples at the requested times) and adds a Drilldown series +// limit warning. +type metricResponseRewriter struct { + http.ResponseWriter + buf bytes.Buffer + status int + shift time.Duration + scope *seriesLimitScope +} + +func (m *metricResponseRewriter) WriteHeader(code int) { m.status = code } + +func (m *metricResponseRewriter) Write(b []byte) (int, error) { return m.buf.Write(b) } + +func (m *metricResponseRewriter) finish() { + body := m.buf.Bytes() + if m.status == 0 || m.status == http.StatusOK { + body = shiftLokiResultTimestamps(body, m.shift) + body = addSeriesLimitWarning(body, m.scope) + } + if m.status != 0 { + m.ResponseWriter.WriteHeader(m.status) + } + _, _ = m.ResponseWriter.Write(body) +} + +// shiftLokiResultTimestamps adds shift to the sample timestamps of a Loki +// matrix or vector response. +func shiftLokiResultTimestamps(body []byte, shift time.Duration) []byte { + if shift == 0 { + return body + } + var resp map[string]json.RawMessage + if json.Unmarshal(body, &resp) != nil { + return body + } + var data map[string]json.RawMessage + if json.Unmarshal(resp["data"], &data) != nil { + return body + } + var result []json.RawMessage + if _, ok := data["result"]; !ok || json.Unmarshal(data["result"], &result) != nil { + return body + } + moveTS := func(point []json.RawMessage) { + if len(point) == 0 { + return + } + if ts, err := strconv.ParseFloat(string(point[0]), 64); err == nil { + ns := int64(math.Round(ts*1e9)) + int64(shift) + point[0] = json.RawMessage(strconv.FormatFloat(float64(ns)/1e9, 'f', -1, 64)) + } + } + for i, raw := range result { + var series map[string]json.RawMessage + if json.Unmarshal(raw, &series) != nil { + return body + } + if values, ok := series["values"]; ok { + var points [][]json.RawMessage + if json.Unmarshal(values, &points) != nil { + return body + } + for _, point := range points { + moveTS(point) + } + series["values"], _ = json.Marshal(points) + } + if value, ok := series["value"]; ok { + var point []json.RawMessage + if json.Unmarshal(value, &point) != nil { + return body + } + moveTS(point) + series["value"], _ = json.Marshal(point) + } + result[i], _ = json.Marshal(series) + } + data["result"], _ = json.Marshal(result) + rewritten, err := marshalOrderedJSONObject(data, []string{"resultType", "result", "stats"}) + if err != nil { + return body + } + resp["data"] = rewritten + // Loki answers status, warnings, data in that order; a plain map marshal + // would sort the keys and move data before status. + out, err := marshalOrderedJSONObject(resp, []string{"status", "warnings", "data"}) + if err != nil { + return body + } + return out +} + +// marshalOrderedJSONObject marshals fields in the given key order first, then +// any remaining keys in sorted order, so a rewritten body keeps Loki's shape +// instead of Go's map ordering. +func marshalOrderedJSONObject(fields map[string]json.RawMessage, order []string) ([]byte, error) { + rest := make([]string, 0, len(fields)) + ordered := make(map[string]bool, len(order)) + for _, key := range order { + ordered[key] = true + } + for key := range fields { + if !ordered[key] { + rest = append(rest, key) + } + } + sort.Strings(rest) + var buf bytes.Buffer + buf.WriteByte('{') + write := func(key string) error { + value, ok := fields[key] + if !ok || len(value) == 0 { + return nil + } + if buf.Len() > 1 { + buf.WriteByte(',') + } + name, err := json.Marshal(key) + if err != nil { + return err + } + buf.Write(name) + buf.WriteByte(':') + buf.Write(value) + return nil + } + for _, key := range append(append([]string{}, order...), rest...) { + if err := write(key); err != nil { + return nil, err + } + } + buf.WriteByte('}') + return buf.Bytes(), nil +} + +// addSeriesLimitWarning adds Loki's partial-result warning to a JSON response +// when the request's Drilldown result was truncated. +func addSeriesLimitWarning(body []byte, scope *seriesLimitScope) []byte { + limit := int64(0) + if scope != nil { + limit = scope.truncated.Load() + } + if limit == 0 || len(body) == 0 || body[0] != '{' || hasTopLevelWarnings(body) { + return body + } + warning, _ := json.Marshal([]string{fmt.Sprintf("maximum number of series (%d) reached for a single query; returning partial results", limit)}) + out := make([]byte, 0, len(body)+len(warning)+14) + out = append(out, `{"warnings":`...) + out = append(out, warning...) + if len(body) > 2 { + out = append(out, ',') + } + return append(out, body[1:]...) +} + +// hasTopLevelWarnings reports whether the response object already carries a +// top-level "warnings" key. A log line inside data can hold the same bytes, so +// the check parses instead of scanning. +func hasTopLevelWarnings(body []byte) bool { + v, err := fj.ParseBytes(body) + if err != nil { + return false + } + obj, err := v.Object() + if err != nil { + return false + } + return obj.Get("warnings") != nil +} + +// capSeriesToLimit enforces the series limit on a finished Loki response: Loki's +// error for a plain client, the busiest limit series for Logs Drilldown (Loki +// keeps the first series it encounters; the proxy keeps the busiest, which is +// what a Drilldown panel renders). The returned body is unchanged when the +// result is within the limit. +func capSeriesToLimit(ctx context.Context, body []byte, limit int) ([]byte, error) { + if limit <= 0 || lokiResultSeriesCount(body) <= limit { + return body, nil + } + if err := seriesLimitReached(ctx, limit); err != nil { + return nil, err + } + return limitLokiResultSeries(body, limit), nil +} + func (p *Proxy) manualMetricRowBudget() (int, error) { limit := p.rangeMetricRowLimit if limit <= 0 { diff --git a/internal/proxy/manual_metric_output_limits_test.go b/internal/proxy/manual_metric_output_limits_test.go index 4728d7a3e..18f75fab0 100644 --- a/internal/proxy/manual_metric_output_limits_test.go +++ b/internal/proxy/manual_metric_output_limits_test.go @@ -54,7 +54,7 @@ func TestManualMetricOutputSeriesLimitFailsClosed(t *testing.T) { if values["first"] != "2" || values["second"] != "4" { t.Fatalf("samples changed: %s", body) } - if partial, err := build(t.Context(), 1); err == nil || !strings.Contains(err.Error(), "series limit exceeded") || partial != nil { + if partial, err := build(t.Context(), 1); err == nil || !strings.Contains(err.Error(), "maximum number of series (1) reached") || partial != nil { t.Fatalf("series were silently discarded: %s %v", partial, err) } ctx := binaryEvaluationContext(t.Context()) diff --git a/internal/proxy/metric_binary.go b/internal/proxy/metric_binary.go index 17e276723..232d63dab 100644 --- a/internal/proxy/metric_binary.go +++ b/internal/proxy/metric_binary.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "math" "net/http" @@ -71,11 +72,11 @@ func (p *Proxy) proxyStatsQueryRange(w http.ResponseWriter, r *http.Request, log // Range == step: Loki's sample at T covers (T-W, T] while VL's bucket // labelled T covers [T, T+W). Fetch from start-W on a grid whose buckets are // (edge, edge+W] and relabel every bucket to its Loki evaluation timestamp - // (relabelSnappedTumblingStatsQueryRange). A backend without the offset arg - // has only epoch-aligned buckets, which match no window of an unaligned start; - // statsRangeIsTumbling sends those requests to the exact evaluator, except - // for Drilldown, which keeps its earlier routing. - if origSpec, origStartNs, ok := statsRateRangeEqualsStepShift(originalLogql, r); ok && p.statsRangeIsTumbling(r, origSpec.Window, origSpec.Window) { + // (relabelSnappedTumblingStatsQueryRange). A backend known to be without the + // offset arg has only epoch-aligned buckets, which match no window of an + // unaligned start; tumblingBucketsAligned sends those requests to the window + // evaluator instead. + if origSpec, origStartNs, ok := statsRateRangeEqualsStepShift(originalLogql, r); ok && p.tumblingBucketsAligned(r, origSpec.Window) { buf := &bufferedResponseWriter{} shiftedR := r.Clone(r.Context()) _ = shiftedR.ParseForm() @@ -216,7 +217,17 @@ func (p *Proxy) proxyStatsQueryRangeDirectAnchored(w http.ResponseWriter, r *htt return false } - body, bodyErr := readBodyLimited(resp.Body, maxStatsQueryRangeBytes) + // Clients that get Loki's series limit error stop reading once the response + // holds more series than the limit allows. + counter := &seriesCountingReader{r: resp.Body} + if !isGrafanaDrilldownRequest(r) { + counter.limit = p.resolvedMaxStatsQuerySeries() + } + body, bodyErr := readBodyLimited(counter, maxStatsQueryRangeBytes) + if errors.Is(bodyErr, errSeriesCountExceeded) { + p.writeError(w, http.StatusBadRequest, (&seriesLimitError{limit: counter.limit}).Error()) + return false + } if bodyErr != nil { // Response exceeds the 16 MB per-request cap. Before giving up with an // empty matrix, try a global top-N two-phase for single-field count @@ -260,10 +271,17 @@ func (p *Proxy) proxyStatsQueryRangeDirectAnchored(w http.ResponseWriter, r *htt // here. Without the cap VL's per-pod stats response is ~146k single-point // series at 24h (each churning pod appears once), which squeaks under the // 16 MB body cap and floods Grafana with unrenderable scattered spikes. - // limitLokiMatrixSeries ranks by total count so the busiest pods survive, + // the series-limit cap ranks by total count so the busiest pods survive, // not VL's alphabetical first-N (the count==1 noise floor). // See memory [[drilldown-high-card-fields-known-limit]]. - out := limitLokiMatrixSeries(wrapAsLokiResponse(body, "matrix"), p.resolvedMaxStatsQuerySeries()) + out := wrapAsLokiResponse(body, "matrix") + if limit := p.resolvedMaxStatsQuerySeries(); lokiResultSeriesCount(out) > limit { + if err := seriesLimitReached(r.Context(), limit); err != nil { + p.writeError(w, http.StatusBadRequest, err.Error()) + return false + } + out = limitLokiResultSeries(out, limit) + } w.Header().Set("Content-Type", "application/json") _, _ = w.Write(out) return false @@ -284,19 +302,39 @@ func (p *Proxy) proxyStatsQueryRangeDirectAnchored(w http.ResponseWriter, r *htt // alignment expectations and causes "no data" for all fields. const maxDrilldownResponseBytes = 32 << 20 // 32 MB — triggers two-phase fallback if exceeded -// maxDrilldownSeries is the maximum series returned for Drilldown single-field -// count queries. Matches Loki's default max_query_series (500) for Drilldown -// requests — Loki returns partial results with a series-limit notice at this -// threshold rather than an error, allowing Grafana Drilldown to display real -// per-value histogram data. -const maxDrilldownSeries = 500 +// drilldownSeriesLimit is the maximum series returned for Drilldown single-field +// count queries: the operator's -max-stats-query-series (default 500, Loki's own +// default max_query_series). Loki returns partial results with a series-limit +// warning at this threshold for Drilldown rather than an error, so a Drilldown +// panel still shows real per-value histogram data. +func (p *Proxy) drilldownSeriesLimit() int { + return p.resolvedMaxStatsQuerySeries() +} -// maxDrilldownPhase2Values caps the number of field values passed to the Phase 2 -// in() filter. VL's default -search.maxQueryLen is 16384 bytes; 200 UUID-length -// values (≈38 bytes each) consume ~7.6 KB, leaving ample room for the base query -// and stats clause. Drilldown's field-values breakdown UI caps at 100 visible series, -// so 200 is already 2× what the UI can display. -const maxDrilldownPhase2Values = 200 +// lokiResultSeriesCount returns the number of series in a Loki JSON response. +func lokiResultSeriesCount(body []byte) int { + v, err := fj.ParseBytes(body) + if err != nil { + return 0 + } + return len(v.GetArray("data", "result")) +} + +// fitTopValueFilter returns the in() filter for the busiest values (ranked +// order) that keeps prefix+filter+suffix within VictoriaLogs' default query +// length, and the values it holds. It never returns an empty in(). +func fitTopValueFilter(field string, values []string, prefix, suffix string) (string, []string, error) { + budget := vlDefaultMaxQueryLen - len(prefix) - len(suffix) + filter := buildVLInFilter(field, values) + for len(filter) > budget && len(values) > 1 { + values = values[:len(values)*budget/len(filter)] + filter = buildVLInFilter(field, values) + } + if len(values) == 0 || len(filter) > budget { + return "", nil, fmt.Errorf("top-value filter does not fit VictoriaLogs' query length (%d bytes)", vlDefaultMaxQueryLen) + } + return filter, values, nil +} // isGrafanaDrilldownRequest reports whether r originates from Grafana Logs Drilldown. // Grafana Logs Drilldown sets supportingQueryType="grafana-lokiexplore-app" on every @@ -522,7 +560,7 @@ const drilldownStatsCacheTTL = 5 * time.Minute // // Both caps are aligned at 120 so the transition at drilldownHybridThreshold (12h) // is seamless — a 13h query produces ~120 bars at ~390s step instead of the previous -// ~30 bars at ~1560s step. With high-card groupings already capped at maxDrilldownSeries +// ~30 bars at ~1560s step. With high-card groupings already capped at the series limit, // (500 series), the worst-case stats matrix is 120 × 500 = 60k cells ≈ 3 MB on the wire, // well within maxDrilldownResponseBytes (32 MB). const ( @@ -934,18 +972,18 @@ func highCardStepFloor(effectiveStepRaw string, rangeNs int64) string { // // Returns (newStep, true) only when: // - cardinality is small (≤ drilldownLowCardThreshold) -// - cardinality is known to be the full distinct set (< maxDrilldownSeries, i.e. not truncated by the limit param) +// - cardinality is known to be the full distinct set, i.e. not truncated by the limit param // - the original step parses // // The returned step floor is rangeNs/drilldownLowCardStatsBuckets so VL still // stays bounded for very wide ranges. -func relaxStepForLowCardinality(originalStepRaw string, cardinality int, rangeNs int64) (string, bool) { +func relaxStepForLowCardinality(originalStepRaw string, cardinality int, rangeNs int64, seriesLimit int) (string, bool) { if cardinality <= 0 || cardinality > drilldownLowCardThreshold { return "", false } - if cardinality >= maxDrilldownSeries { - // field_values was truncated by limit=maxDrilldownSeries; we don't know - // the true cardinality so assume high-card and keep the coarsened step. + if seriesLimit > 0 && cardinality >= seriesLimit { + // field_values was truncated by the series limit; we don't know the true + // cardinality so assume high-card and keep the coarsened step. return "", false } origStep, ok := parsePositiveStepDuration(originalStepRaw) @@ -1154,8 +1192,8 @@ func expandDrilldownStep(body []byte, fineStepRaw, coarseStepRaw, endRaw string) // ticks, second tab) are served from cache with zero VL calls. // // Fast path (low-cardinality fields, short ranges): appends VL-side sort+limit -// so only the top-maxDrilldownSeries unique values are transmitted per time -// bucket, then proxy-side caps to maxDrilldownSeries series. +// so only the busiest values are transmitted per time bucket, then the proxy +// caps the result to the series limit. // // Eager two-phase path: detected via drilldownShouldEagerTwoPhase (HC field // name, range > 60 buckets, or cached overflow). Bypasses the direct VL attempt @@ -1236,7 +1274,12 @@ func (p *Proxy) proxyStatsQueryRangeDrilldown(w http.ResponseWriter, r *http.Req } orgID := r.Header.Get("X-Scope-OrgID") if body := batcher.submit(r.Context(), orgID, cleanBase, lokiField, field, vlFields, startRaw, endRaw, effectiveStepRaw); body != nil { - body = expandDrilldownStep(body, stepRaw, effectiveStepRaw, endRaw) + capped, capErr := capSeriesToLimit(r.Context(), body, p.drilldownSeriesLimit()) + if capErr != nil { + p.writeError(w, http.StatusBadRequest, capErr.Error()) + return + } + body = expandDrilldownStep(capped, stepRaw, effectiveStepRaw, endRaw) p.setLocalReadCacheWithTTL(drillCacheKey, append([]byte(nil), body...), drilldownStatsCacheTTL) w.Header().Set("Content-Type", "application/json") _, _ = w.Write(body) @@ -1264,8 +1307,9 @@ func (p *Proxy) proxyStatsQueryRangeDrilldown(w http.ResponseWriter, r *http.Req if !p.drilldownShouldEagerTwoPhase(r, cleanBase, field) { // Direct path: VL-side | limit 500 per bucket bounds the per-bucket result; - // proxy-side limitLokiMatrixSeries caps the global matrix to maxDrilldownSeries. - limitedQuery := appendDrilldownSeriesLimit(logsqlQuery, maxDrilldownSeries) + // the proxy caps the global matrix to the series limit. One over the + // limit is fetched so truncation is distinguishable from an exact fit. + limitedQuery := appendDrilldownSeriesLimit(logsqlQuery, p.drilldownSeriesLimit()+1) params := buildStatsQueryRangeParams(limitedQuery, startRaw, endRaw, effectiveStepRaw) resp, err := p.vlPost(r.Context(), "/select/logsql/stats_query_range", params) if err != nil { @@ -1289,7 +1333,12 @@ func (p *Proxy) proxyStatsQueryRangeDrilldown(w http.ResponseWriter, r *http.Req keepFn = func(tsNs int64) bool { return tsNs <= endNs } } body = p.trimAndTranslateStatsQRFJ(r.Context(), body, keepFn, r.FormValue("query")) - body = limitLokiMatrixSeries(body, maxDrilldownSeries) + capped, capErr := capSeriesToLimit(r.Context(), body, p.drilldownSeriesLimit()) + if capErr != nil { + p.writeError(w, http.StatusBadRequest, capErr.Error()) + return + } + body = capped final := wrapAsLokiResponse(body, "matrix") final = expandDrilldownStep(final, stepRaw, effectiveStepRaw, endRaw) p.setLocalReadCacheWithTTL(drillCacheKey, append([]byte(nil), final...), drilldownStatsCacheTTL) @@ -1404,7 +1453,7 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownParserDirect( } } - limitedQuery := appendDrilldownSeriesLimit(logsqlQuery, maxDrilldownSeries) + limitedQuery := appendDrilldownSeriesLimit(logsqlQuery, p.drilldownSeriesLimit()+1) params := buildStatsQueryRangeParams(limitedQuery, startRaw, endRaw, effectiveStepRaw) resp, err := p.vlPost(r.Context(), "/select/logsql/stats_query_range", params) if err != nil { @@ -1442,7 +1491,12 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownParserDirect( keepFn = func(tsNs int64) bool { return tsNs <= endNs } } body = p.trimAndTranslateStatsQRFJ(r.Context(), body, keepFn, r.FormValue("query")) - body = limitLokiMatrixSeries(body, maxDrilldownSeries) + capped, capErr := capSeriesToLimit(r.Context(), body, p.drilldownSeriesLimit()) + if capErr != nil { + p.writeError(w, http.StatusBadRequest, capErr.Error()) + return + } + body = capped // Zero-fill missing time buckets so the Drilldown field histogram renders a // continuous chart rather than disconnected spikes. This is a Drilldown // rendering choice for this call site: Loki itself omits steps without @@ -1925,7 +1979,7 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( fvParams.Set("field", field) fvParams.Set("start", nanosToVLTimestamp(startNs)) fvParams.Set("end", nanosToVLTimestamp(endNs)) - fvParams.Set("limit", strconv.Itoa(maxDrilldownSeries)) + fvParams.Set("limit", strconv.Itoa(p.drilldownSeriesLimit())) var fvEntries []drilldownFVEntry if resp, err := p.vlGet(ctx, "/select/logsql/field_values", fvParams); err == nil { @@ -1966,7 +2020,7 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( // the result set is small. Restore the original requested step (capped to // drilldownLowCardStatsBuckets buckets as a safety floor) so the proxy // matches Loki's per-bucket resolution at 2d/7d. - if relaxed, ok := relaxStepForLowCardinality(r.FormValue("step"), len(fvEntries), rangeNs); ok { + if relaxed, ok := relaxStepForLowCardinality(r.FormValue("step"), len(fvEntries), rangeNs, p.drilldownSeriesLimit()); ok { effectiveStepRaw = relaxed } @@ -1980,7 +2034,7 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( // stubStepNs drives stub placement in mergeDrilldownWithFieldValues for values // that appear in field_values but not in the top-N stats result (values beyond - // the maxDrilldownSeries limit). Use effectiveStepRaw resolution so stubs + // the series limit). Use effectiveStepRaw resolution so stubs // align with the stats bars. var stubStepNs int64 if d, ok := parsePositiveStepDuration(effectiveStepRaw); ok { @@ -1991,7 +2045,7 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( // underscore-fallback variants (e.g. detected_level) added by addUnderscorefallbackByLabels, // which would produce duplicate series (level + detected_level) confusing Grafana. hybridStatsQuery := cleanBase + " | filter " + field + `:!"" | stats by (` + field + `) count()` - limitedQuery := appendDrilldownSeriesLimit(hybridStatsQuery, maxDrilldownSeries) + limitedQuery := appendDrilldownSeriesLimit(hybridStatsQuery, p.drilldownSeriesLimit()+1) statsParams := buildStatsQueryRangeParams(limitedQuery, histStartRaw, endRaw, effectiveStepRaw) var statsBody []byte @@ -2036,7 +2090,12 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( if field != lokiField { rawBody = renameStatsBodyMetricKey(rawBody, field, lokiField) } - rawBody = limitLokiMatrixSeries(rawBody, maxDrilldownSeries) + capped, capErr := capSeriesToLimit(r.Context(), rawBody, p.drilldownSeriesLimit()) + if capErr != nil { + p.writeError(w, http.StatusBadRequest, capErr.Error()) + return + } + rawBody = capped // Zero-fill missing time steps so the Drilldown chart draws a continuous // line rather than disconnected spikes. This is a Drilldown rendering // choice: Loki omits steps without samples, and generic range metrics @@ -2056,26 +2115,26 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( // Merge: stats histogram + field_values stubs for values missing from recent window. // // Critical ordering: when stats returned real per-bucket data, those series MUST - // survive the maxDrilldownSeries cap. The stub-fill emits drilldownSynthesizeBuckets + // survive the series-limit cap. The stub-fill emits drilldownSynthesizeBuckets // (or coarse-step) cells per series, all of value=1 (floored from hits/N when hits // is small). For unique-per-request fields like pod, real stats series have 1 // nonzero bucket with count ~20-30 (sum~30), while stub-only series have 96+ - // cells of value=1 (sum~96+). A naive post-merge limitLokiMatrixSeries ranks by + // cells of value=1 (sum~96+). A naive post-merge the series-limit cap ranks by // sum and DROPS the real stats series in favour of the synthetic stubs, producing // the "every bar is uniform value=1" symptom users see in Drilldown. // // Skip the post-merge limit entirely. statsBody is already capped to - // maxDrilldownSeries by the earlier limitLokiMatrixSeries call, and the merge + // the series limit by the earlier cap, and the merge // only adds fvEntries that are NOT already in stats. We cap the fvEntries used - // for stub-fill so the combined output stays within maxDrilldownSeries × ~2 at + // for stub-fill so the combined output stays within the series limit × ~2 at // worst — Grafana handles that fine and the real data is preserved. var final []byte switch { case statsBody != nil && len(fvEntries) > 0: final = mergeDrilldownWithFieldValues(statsBody, lokiField, fvEntries, endSec, rangeNs, stubStepNs) - // No post-merge limitLokiMatrixSeries — see comment above. The merge function + // No post-merge the series-limit cap — see comment above. The merge function // internally bounds output by stats_count + fv_count, both already capped at - // maxDrilldownSeries. + // the series limit. case statsBody != nil: final = statsBody pathLabel += "/no_fv" @@ -2101,11 +2160,11 @@ func (p *Proxy) proxyStatsQueryRangeDrilldownHybrid( // direct VL response exceeds maxDrilldownResponseBytes. It runs two VL calls: // // 1. Phase 1 — a single-bucket stats_query_range (step = entire range) to get -// the global top-maxDrilldownSeries field values. One bucket means VL's +// the busiest field values within the series limit. One bucket means VL's // per-bucket | limit N is equivalent to a true global limit. // // 2. Phase 2 — a range query filtered to the Phase 1 values via field:in(...), -// returning ≤maxDrilldownSeries unique series with the effective step. +// returning unique series within the series limit with the effective step. // // effectiveStep is the coarsened step (from coarsenDrilldownStep); Phase 2 uses // it instead of the raw Grafana step to cap bucket count and reduce VL CPU. @@ -2139,7 +2198,7 @@ func stripUnpackStagesForTopN(base string) string { // Two phases: // - Phase 1 (selection, fast): a single-bucket global top-N with `| unpack_*` // stripped so VL uses its column index — ~0.4s — returns the busiest -// maxDrilldownSeries field values. +// field values within the series limit. // - Phase 2 (values, correct): the ORIGINAL query (parser preserved for exact // detected_level semantics) restricted to those values via `field:in(...)`, // so VL only aggregates ≤maxDrilldownPhase2Values series — fast and accurate. @@ -2175,7 +2234,10 @@ func (p *Proxy) tryHighCardCountByWindowedHits(w http.ResponseWriter, r *http.Re if !ok { return false } - if !isGrafanaDrilldownRequest(r) && (!isGrafanaSourcedRequest(r) || !isLikelyHighCardinalityField(spec.GroupBy[0])) { + // Sampled top-N series are a Drilldown visualization; every other client + // (Explore and dashboards included) gets Loki's exact windows or its series + // limit error. + if !isGrafanaDrilldownRequest(r) { return false } startRaw, endRaw, stepRaw := r.FormValue("start"), r.FormValue("end"), r.FormValue("step") @@ -2208,6 +2270,11 @@ func (p *Proxy) tryHighCardCountByWindowedHits(w http.ResponseWriter, r *http.Re // the anchored bucket grid of proxyStatsQueryRangeDirectAnchored, and lines // without the field stay in as the unlabelled series when Phase 1 ranks them. func (p *Proxy) tryHighCardCountByTwoPhase(r *http.Request, logsqlQuery string, tumblingWindow time.Duration) []byte { + // A partial top-N answer is Drilldown's; every other client reads the exact + // direct path, which returns Loki's series limit error when over the limit. + if !isGrafanaDrilldownRequest(r) { + return nil + } spec, ok := parseSingleFieldCountSpec(logsqlQuery) if !ok { return nil @@ -2233,14 +2300,15 @@ func (p *Proxy) tryHighCardCountByTwoPhase(r *http.Request, logsqlQuery string, // Phase 1: fast single-bucket global top-N (unpack stripped → column index). p1Base := stripUnpackStagesForTopN(spec.BaseQuery) - p1Query := p1Base + " | stats by (" + quoteLogsQLIdent(field) + ") count() as _c | sort by (_c desc) | limit " + strconv.Itoa(maxDrilldownSeries) + limit := p.resolvedMaxStatsQuerySeries() + p1Query := p1Base + " | stats by (" + quoteLogsQLIdent(field) + ") count() as _c | sort by (_c desc) | limit " + strconv.Itoa(limit+1) p1Params := url.Values{} p1Params.Set("query", p1Query) p1Params.Set("start", nanosToVLTimestamp(startNs)) p1Params.Set("end", nanosToVLTimestamp(endNs)) p1Params.Set("step", strconv.FormatInt(rangeSec, 10)+"s") - resp1, err := p.vlPost(ctx, "/select/logsql/stats_query_range", p1Params) - if err != nil { + resp1, p1Err := p.vlPost(ctx, "/select/logsql/stats_query_range", p1Params) + if p1Err != nil { return nil } defer resp1.Body.Close() @@ -2257,8 +2325,9 @@ func (p *Proxy) tryHighCardCountByTwoPhase(r *http.Request, logsqlQuery string, // fall through to the exact direct path. return nil } - if len(topValues) > maxDrilldownPhase2Values { - topValues = topValues[:maxDrilldownPhase2Values] + ranked := len(topValues) + if len(topValues) > limit { + topValues = topValues[:limit] } keepUnlabelled := tumblingWindow > 0 && drilldownTopValuesHaveEmpty(p1Body, field) @@ -2271,7 +2340,17 @@ func (p *Proxy) tryHighCardCountByTwoPhase(r *http.Request, logsqlQuery string, // fans out ~15 of these concurrently no longer overloads VL. Column-level counts // run ~2x Loki's detected_level (a data-density quirk affecting both VL variants) // but are actually CLOSER to Loki than the unpack variant — verified live. - inFilter := buildVLInFilter(field, topValues) + prefix, suffix := p1Base+" | filter ", " | stats by ("+quoteLogsQLIdent(field)+") count()" + if keepUnlabelled { + prefix, suffix = prefix+"(", " or "+quoteLogsQLIdent(field)+`:"")`+suffix + } + inFilter, topValues, err := fitTopValueFilter(field, topValues, prefix, suffix) + if err != nil { + return nil + } + if len(topValues) < ranked { + _ = seriesLimitReached(r.Context(), len(topValues)) // Drilldown: partial with a warning + } if keepUnlabelled { inFilter = "(" + inFilter + " or " + quoteLogsQLIdent(field) + `:"")` } @@ -2295,7 +2374,7 @@ func (p *Proxy) tryHighCardCountByTwoPhase(r *http.Request, logsqlQuery string, } keepFn := func(tsNs int64) bool { return tsNs <= endNs } p2Body = p.trimAndTranslateStatsQRFJ(ctx, p2Body, keepFn, r.FormValue("query")) - p2Body = limitLokiMatrixSeries(p2Body, p.resolvedMaxStatsQuerySeries()) + p2Body = limitLokiResultSeries(p2Body, limit) return wrapAsLokiResponse(p2Body, "matrix") } @@ -2318,7 +2397,7 @@ func (p *Proxy) drilldownTwoPhase(r *http.Request, effectiveQuery, cleanBase, fi } p1Params := url.Values{} - p1Params.Set("query", appendDrilldownSeriesLimit(effectiveQuery, maxDrilldownSeries)) + p1Params.Set("query", appendDrilldownSeriesLimit(effectiveQuery, p.drilldownSeriesLimit()+1)) p1Params.Set("start", nanosToVLTimestamp(startNs)) p1Params.Set("end", nanosToVLTimestamp(endNs)) p1Params.Set("step", strconv.FormatInt(rangeSec, 10)+"s") @@ -2332,7 +2411,7 @@ func (p *Proxy) drilldownTwoPhase(r *http.Request, effectiveQuery, cleanBase, fi return nil } - // 1 MB is generous for ≤maxDrilldownSeries × ~256 bytes per UUID entry. + // 1 MB is generous for a series-limit-sized page of ~256-byte UUID entries. p1Body, p1Err := readBodyLimited(resp1.Body, 1<<20) if p1Err != nil { return nil @@ -2342,12 +2421,13 @@ func (p *Proxy) drilldownTwoPhase(r *http.Request, effectiveQuery, cleanBase, fi if len(topValues) == 0 { return emptyLokiMatrix } - if len(topValues) > maxDrilldownPhase2Values { - topValues = topValues[:maxDrilldownPhase2Values] - } - // Phase 2: range query restricted to the global top-N values. - inFilter := buildVLInFilter(field, topValues) + // Phase 2: range query restricted to the global top-N values that fit + // VictoriaLogs' query length. + inFilter, _, fitErr := fitTopValueFilter(field, topValues, cleanBase+" | filter ", " | stats by ("+quoteLogsQLIdent(field)+") count()") + if fitErr != nil { + return nil + } p2Query := cleanBase + " | filter " + inFilter + " | stats by (" + quoteLogsQLIdent(field) + ") count()" origGroupBy := parseOriginalByLabels(r.FormValue("query")) p2Query = p.addUnderscorefallbackByLabels(p2Query, origGroupBy) @@ -2372,7 +2452,14 @@ func (p *Proxy) drilldownTwoPhase(r *http.Request, effectiveQuery, cleanBase, fi keepFn = func(tsNs int64) bool { return tsNs <= endNs } } p2Body = p.trimAndTranslateStatsQRFJ(ctx, p2Body, keepFn, r.FormValue("query")) - p2Body = limitLokiMatrixSeries(p2Body, maxDrilldownSeries) + // Drilldown-only path: capSeriesToLimit records the partial-result warning + // and never errors here. A plain client that somehow reaches it gets no body, + // so the caller falls through to a path that applies the limit itself. + capped, capErr := capSeriesToLimit(r.Context(), p2Body, p.drilldownSeriesLimit()) + if capErr != nil { + return nil + } + p2Body = capped return wrapAsLokiResponse(p2Body, "matrix") } @@ -2433,13 +2520,13 @@ func buildVLInFilter(field string, values []string) string { return sb.String() } -// limitLokiMatrixSeries truncates the result array in a Loki matrix response to -// the top maxSeries entries by total count (sum of all sample values), matching -// Loki's max_series_per_query behaviour. Grafana Drilldown then picks the top 100 -// from those series for display — sorting here ensures the most active field values -// survive the cut when VL returns tens of thousands of unique series. +// limitLokiResultSeries truncates the result array of a Loki matrix or vector +// response to the top maxSeries entries by total sample value. Loki keeps the +// first series it encounters; keeping the busiest ones is what a Drilldown panel +// renders, and it stops an alphabetical VictoriaLogs ordering from returning the +// noise floor when tens of thousands of values exist. // Returns body unchanged if parsing fails or len(result) <= maxSeries. -func limitLokiMatrixSeries(body []byte, maxSeries int) []byte { +func limitLokiResultSeries(body []byte, maxSeries int) []byte { if maxSeries <= 0 { return body } @@ -2451,6 +2538,10 @@ func limitLokiMatrixSeries(body []byte, maxSeries int) []byte { if len(result) <= maxSeries { return body } + resultType := string(v.GetStringBytes("data", "resultType")) + if resultType == "" { + resultType = "matrix" + } // Compute total count per series and sort descending so the most active // field values survive the maxSeries cut, not an arbitrary VL ordering. @@ -2458,23 +2549,34 @@ func limitLokiMatrixSeries(body []byte, maxSeries int) []byte { idx int total float64 } + sampleValue := func(pair *fj.Value) float64 { + arr := pair.GetArray() + if len(arr) < 2 { + return 0 + } + f, e := strconv.ParseFloat(string(arr[1].GetStringBytes()), 64) + if e != nil { + return 0 + } + return f + } ranks := make([]ranked, len(result)) for i, entry := range result { var total float64 for _, pair := range entry.GetArray("values") { - arr := pair.GetArray() - if len(arr) >= 2 { - if f, e := strconv.ParseFloat(string(arr[1].GetStringBytes()), 64); e == nil { - total += f - } - } + total += sampleValue(pair) + } + if value := entry.Get("value"); value != nil { // instant vector sample + total += sampleValue(value) } ranks[i] = ranked{idx: i, total: total} } sort.Slice(ranks, func(i, j int) bool { return ranks[i].total > ranks[j].total }) buf := make([]byte, 0, maxSeries*256) - buf = append(buf, `{"status":"success","data":{"resultType":"matrix","result":[`...) + buf = append(buf, `{"status":"success","data":{"resultType":"`...) + buf = append(buf, resultType...) + buf = append(buf, `","result":[`...) for i := 0; i < maxSeries; i++ { if i > 0 { buf = append(buf, ',') @@ -3473,6 +3575,14 @@ func (p *Proxy) proxyStatsQuery(w http.ResponseWriter, r *http.Request, logsqlQu if topK, topKDesc, hasTopK := parseTopKWrapper(r.FormValue("query")); hasTopK { body = applyTopKToVector(body, topK, topKDesc) } + // Loki applies max_query_series to instant queries as well: an error for a + // plain client, a partial vector with a warning for Logs Drilldown. + capped, capErr := capSeriesToLimit(r.Context(), body, p.resolvedMaxStatsQuerySeries()) + if capErr != nil { + p.writeError(w, badRequestStatusOr(capErr, http.StatusBadRequest), capErr.Error()) + return + } + body = capped w.Header().Set("Content-Type", "application/json") _, _ = w.Write(body) } diff --git a/internal/proxy/offset_integration_test.go b/internal/proxy/offset_integration_test.go index 7c98498f2..8a9c44577 100644 --- a/internal/proxy/offset_integration_test.go +++ b/internal/proxy/offset_integration_test.go @@ -10,6 +10,13 @@ import ( "time" ) +// backendSeconds reads a start/end param the proxy sent to VictoriaLogs in any +// of the spellings it accepts (Unix seconds or RFC3339). +func backendSeconds(raw string) int64 { + ns, _ := parseLokiTimeToUnixNano(raw) + return ns / int64(time.Second) +} + func TestQueryRange_OffsetShiftsTimeWindow(t *testing.T) { // rate({app="nginx"}[60s] offset 1h) with start=T end=T+30m step=60 // range==step (tumbling window) → routes to stats_query_range. @@ -58,13 +65,13 @@ func TestQueryRange_OffsetShiftsTimeWindow(t *testing.T) { // The proxy applies a start-shift of -1×step for tumbling rate windows // (statsRateRangeEqualsStepShift) to correct VL's first-bucket drift. // Account for that: allow start to be [wantStart-step, wantStart]. - gotStartNs, _ := strconv.ParseInt(gotStart, 10, 64) + gotStartNs := backendSeconds(gotStart) wantStartNs, _ := strconv.ParseInt(wantStart, 10, 64) if gotStartNs < wantStartNs-60 || gotStartNs > wantStartNs { t.Errorf("start: got %s want in [%d, %d]", gotStart, wantStartNs-60, wantStartNs) } // VL extends end by one step; allow up to +step tolerance. - gotEndNs, _ := strconv.ParseInt(gotEnd, 10, 64) + gotEndNs := backendSeconds(gotEnd) wantEndNs, _ := strconv.ParseInt(wantEnd, 10, 64) if gotEndNs < wantEndNs || gotEndNs > wantEndNs+60 { t.Errorf("end: got %s want ~%s", gotEnd, wantEnd) @@ -102,7 +109,7 @@ func TestQueryRange_NoOffsetUnchanged(t *testing.T) { // The proxy applies a start-shift of -1×step for tumbling rate windows // (statsRateRangeEqualsStepShift). Allow [wantStart-step, wantStart]. wantStart := strconv.FormatInt(base.Unix(), 10) - gotStartNs, _ := strconv.ParseInt(gotStart, 10, 64) + gotStartNs := backendSeconds(gotStart) wantStartNs, _ := strconv.ParseInt(wantStart, 10, 64) if gotStartNs < wantStartNs-60 || gotStartNs > wantStartNs { t.Errorf("start should be ~unmodified: got %s want in [%d, %d]", gotStart, wantStartNs-60, wantStartNs) diff --git a/internal/proxy/ordered_json_drilldown_test.go b/internal/proxy/ordered_json_drilldown_test.go index 49f9c1de7..65799b4bc 100644 --- a/internal/proxy/ordered_json_drilldown_test.go +++ b/internal/proxy/ordered_json_drilldown_test.go @@ -81,7 +81,7 @@ func TestOrderedJSONExactHistogramLimitStillFailsClosed(t *testing.T) { return w } w := request() - if w.Code < 400 || !strings.Contains(w.Body.String(), "maximum metric series") || strings.Contains(w.Body.String(), `"result"`) { + if w.Code < 400 || !strings.Contains(w.Body.String(), "maximum number of series (2) reached") || strings.Contains(w.Body.String(), `"result"`) { t.Fatalf("overflow returned partial result: %d %s", w.Code, w.Body) } rows = 2 diff --git a/internal/proxy/ordered_json_metric.go b/internal/proxy/ordered_json_metric.go index 8a4c17461..785f52b9c 100644 --- a/internal/proxy/ordered_json_metric.go +++ b/internal/proxy/ordered_json_metric.go @@ -567,9 +567,10 @@ func (p *Proxy) orderedJSONStatsBucketsWithReason(ctx context.Context, plan *ord if err != nil { return nil, false, false, err } - if maxSeries := p.resolvedMaxStatsQuerySeries(); len(series) >= maxSeries { - // The bucket collector keeps the busiest series; Loki fails instead. - return nil, false, false, fmt.Errorf("maximum metric series exceeded (%d)", maxSeries) + if maxSeries := p.resolvedMaxStatsQuerySeries(); len(series) > maxSeries { + // The bucket collector keeps the busiest series; Loki fails instead, and + // only above the limit — a query with exactly maxSeries series passes. + return nil, false, false, &seriesLimitError{limit: maxSeries} } merged := make(map[string]manualSeriesSamples, len(series)) for _, entry := range series { @@ -1259,7 +1260,7 @@ func (p *Proxy) collectOrderedJSONMetric(ctx context.Context, plan *orderedJSONM entry, exists := series[key] if !exists { if len(series) >= p.resolvedMaxStatsQuerySeries() { - return nil, fmt.Errorf("maximum metric series exceeded (%d)", p.resolvedMaxStatsQuerySeries()) + return nil, &seriesLimitError{limit: p.resolvedMaxStatsQuerySeries()} } entry.Metric = labels } diff --git a/internal/proxy/perf_regression_test.go b/internal/proxy/perf_regression_test.go index 3ce6616ed..afd41a809 100644 --- a/internal/proxy/perf_regression_test.go +++ b/internal/proxy/perf_regression_test.go @@ -49,27 +49,16 @@ func TestCollectRangeMetric_CountOverTime_SingleStream(t *testing.T) { defer vlBackend.Close() p := newGapTestProxy(t, vlBackend.URL) - params := url.Values{} - // Use non-parser rate with range != step to force manual path (collectRangeMetricSamples). - params.Set("query", `rate({app="api",env="prod"}[2m])`) - params.Set("start", strconv.FormatInt(base.Unix(), 10)) - params.Set("end", strconv.FormatInt(base.Add(3*step).Unix(), 10)) - params.Set("step", "30") // step(30) != range(120) → manual path - req := httptest.NewRequest(http.MethodGet, "/loki/api/v1/query_range?"+params.Encode(), nil) - rec := httptest.NewRecorder() - p.handleQueryRange(rec, req) - + // The raw evaluator itself: bucketed stats serve these range metrics on the + // query path. + series, err := p.collectRangeMetricSamples(t.Context(), `app:="api" env:="prod"`, []string{"_stream"}, nil, false, "__count__", "", base, base.Add(3*step)) + if err != nil { + t.Fatalf("collect: %v", err) + } if !queryCalled { t.Fatal("expected /select/logsql/query to be called") } - if rec.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) - } - var resp lokiMatrixResponse - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v", err) - } - if len(resp.Data.Result) == 0 { + if len(series) == 0 { t.Fatalf("expected at least 1 series, got 0") } } @@ -99,24 +88,12 @@ func TestCollectRangeMetric_MultipleStreams_MultipleSeries(t *testing.T) { defer vlBackend.Close() p := newGapTestProxy(t, vlBackend.URL) - params := url.Values{} - params.Set("query", `rate({env="prod"}[2m])`) - params.Set("start", strconv.FormatInt(base.Unix(), 10)) - params.Set("end", strconv.FormatInt(base.Add(5*time.Minute).Unix(), 10)) - params.Set("step", "30") - req := httptest.NewRequest(http.MethodGet, "/loki/api/v1/query_range?"+params.Encode(), nil) - rec := httptest.NewRecorder() - p.handleQueryRange(rec, req) - - if rec.Code != http.StatusOK { - t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) - } - var resp lokiMatrixResponse - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("decode: %v", err) + series, err := p.collectRangeMetricSamples(t.Context(), `env:="prod"`, []string{"_stream"}, nil, false, "__count__", "", base, base.Add(5*time.Minute)) + if err != nil { + t.Fatalf("collect: %v", err) } - if len(resp.Data.Result) != len(streams) { - t.Fatalf("expected %d series (one per stream), got %d", len(streams), len(resp.Data.Result)) + if len(series) != len(streams) { + t.Fatalf("expected %d series (one per stream), got %d", len(streams), len(series)) } } diff --git a/internal/proxy/proxy.go b/internal/proxy/proxy.go index f3c0d74a2..72b6a602a 100644 --- a/internal/proxy/proxy.go +++ b/internal/proxy/proxy.go @@ -2142,6 +2142,7 @@ func (p *Proxy) handleQueryRange(w http.ResponseWriter, r *http.Request) { // so time-shifting is not applied — results may differ from Loki for offset // values but the proxy returns 200 rather than incorrectly rejecting the query. // Expressions with multiple *different* offsets still return 400 (same as Loki). + var rangeOffset time.Duration { offsetDur, strippedQuery, offsetErr := extractLogQLOffset(logqlQuery) if offsetErr != nil { @@ -2164,14 +2165,27 @@ func (p *Proxy) handleQueryRange(w http.ResponseWriter, r *http.Request) { // it does not alias the map captured by withOrgID's origRequestKey reference. _ = r.ParseForm() if startNs, ok := parseLokiTimeToUnixNano(r.FormValue("start")); ok { - r.Form.Set("start", nanosToVLTimestamp(startNs-offsetDur.Nanoseconds())) + r.Form.Set("start", strconv.FormatInt(startNs-offsetDur.Nanoseconds(), 10)) } if endNs, ok := parseLokiTimeToUnixNano(r.FormValue("end")); ok { - r.Form.Set("end", nanosToVLTimestamp(endNs-offsetDur.Nanoseconds())) + r.Form.Set("end", strconv.FormatInt(endNs-offsetDur.Nanoseconds(), 10)) } + // Samples are evaluated in the shifted range and stamped at Loki's + // evaluation times (metricResponseRewriter); the inner cache would + // keep the unshifted body. + rangeOffset = offsetDur + cacheable, cacheKey = false, "" } } } + r, seriesScope := withSeriesLimitScope(r) + // Only sample responses are rewritten: buffering a log query here would + // defeat -stream-response. + if logQLProducesSamples(logqlQuery) && (rangeOffset != 0 || seriesScope.drilldown) { + rewriter := &metricResponseRewriter{ResponseWriter: w, shift: rangeOffset, scope: seriesScope} + w = rewriter + defer rewriter.finish() + } // Enforce max query length AFTER offset is applied (start/end reflect shifted range). if startNs, okS := parseLokiTimeToUnixNano(r.FormValue("start")); okS { @@ -2303,12 +2317,12 @@ func (p *Proxy) handleQueryRange(w http.ResponseWriter, r *http.Request) { _, _ = w.Write(cacheOut) } if cacheable && sc.code == http.StatusOK { - p.setLocalReadCacheWithTTL(cacheKey, append([]byte(nil), cacheOut...), 5*time.Minute) + p.setLocalReadCacheWithTTL(cacheKey, addSeriesLimitWarning(append([]byte(nil), cacheOut...), seriesScope), 5*time.Minute) } } else if cacheTap != nil { if cacheable && sc.code == http.StatusOK { if body := cacheTap.CapturedBody(); len(body) > 0 { - p.setLocalReadCacheWithTTL(cacheKey, append([]byte(nil), body...), 5*time.Minute) + p.setLocalReadCacheWithTTL(cacheKey, addSeriesLimitWarning(append([]byte(nil), body...), seriesScope), 5*time.Minute) } } cacheTap.Release() @@ -2397,6 +2411,7 @@ func (p *Proxy) handleQuery(w http.ResponseWriter, r *http.Request) { // offset data actually lives. All downstream dispatch paths see the shifted time. // // Mixed-offset binary expressions are treated the same as in query_range above. + var instantOffset time.Duration { offsetDur, strippedQuery, offsetErr := extractLogQLOffset(logqlQuery) if offsetErr != nil { @@ -2421,11 +2436,20 @@ func (p *Proxy) handleQuery(w http.ResponseWriter, r *http.Request) { rawTime = strconv.FormatInt(time.Now().UnixNano(), 10) } if timeNs, ok := parseLokiTimeToUnixNano(rawTime); ok { - r.Form.Set("time", nanosToVLTimestamp(timeNs-offsetDur.Nanoseconds())) + r.Form.Set("time", strconv.FormatInt(timeNs-offsetDur.Nanoseconds(), 10)) + instantOffset = offsetDur // the sample is stamped at the requested time } } } } + // The series limit applies to instant queries too, so Drilldown gets its + // partial-result warning and every other client Loki's error. + r, instantScope := withSeriesLimitScope(r) + if logQLProducesSamples(logqlQuery) && (instantOffset != 0 || instantScope.drilldown) { + rewriter := &metricResponseRewriter{ResponseWriter: w, shift: instantOffset, scope: instantScope} + w = rewriter + defer rewriter.finish() + } if p.handleOrderedJSONMetric(w, r, start, logqlQuery, false) { return diff --git a/internal/proxy/proxy_test.go b/internal/proxy/proxy_test.go index 26901ac55..5119e69b4 100644 --- a/internal/proxy/proxy_test.go +++ b/internal/proxy/proxy_test.go @@ -596,12 +596,13 @@ func TestContract_QueryRange_MatrixFormat_TumblingRateRelabelsBucketsToLokiTimes resp := doGet(t, vlBackend.URL, "/loki/api/v1/query_range?query=rate(%7Bapp%3D%22nginx%22%7D%5B60s%5D)&start=1705312200&end=1705312260&step=60") assertLokiSuccess(t, resp) - if expectedStart := strconv.FormatInt(1705312200-60, 10); receivedStart != expectedStart { - t.Fatalf("expected backend start shifted back one window %q, got %q", expectedStart, receivedStart) + if got := backendSeconds(receivedStart); got != 1705312200-60 { + t.Fatalf("expected backend start shifted back one window %d, got %q", 1705312200-60, receivedStart) } - expectedEnd := strconv.FormatInt(1705312260+60, 10) - if receivedEnd != expectedEnd { - t.Fatalf("expected compensated backend end %q, got %q", expectedEnd, receivedEnd) + // The anchored grid fetches up to the request end itself, with the 1ns shift + // that makes each bucket right-closed like Loki's window. + if got := backendSeconds(receivedEnd); got != 1705312260 { + t.Fatalf("expected backend end %d, got %q", 1705312260, receivedEnd) } data, ok := resp["data"].(map[string]interface{}) diff --git a/internal/proxy/query_translation.go b/internal/proxy/query_translation.go index fa2a19283..dea9d128a 100644 --- a/internal/proxy/query_translation.go +++ b/internal/proxy/query_translation.go @@ -1127,7 +1127,7 @@ func (p *Proxy) fetchBareParserMetricSeries(ctx context.Context, originalQuery s if !ok { if len(seriesByKey) >= p.resolvedMaxStatsQuerySeries() { vlEntryPool.Put(entry) - return nil, fmt.Errorf("maximum metric series exceeded (%d)", p.resolvedMaxStatsQuerySeries()) + return nil, &seriesLimitError{limit: p.resolvedMaxStatsQuerySeries()} } series = &bareParserMetricSeries{ metric: metric, diff --git a/internal/proxy/range_metric_compat.go b/internal/proxy/range_metric_compat.go index 835bbb354..d6e0b6033 100644 --- a/internal/proxy/range_metric_compat.go +++ b/internal/proxy/range_metric_compat.go @@ -2,6 +2,7 @@ package proxy import ( "bufio" + "bytes" "context" "encoding/json" "errors" @@ -438,26 +439,17 @@ func (p *Proxy) handleStatsCompatRange(w http.ResponseWriter, r *http.Request, o // exactly one Loki window, and the backend can place bucket edges on the // request's windows (tumblingBucketsAligned). With range < step a bucket also // holds the lines between windows, which Loki never counts, and with range > -// step the windows overlap; both use the anchored window evaluator. -// -// Grafana Logs Drilldown requests keep their earlier routing, where range <= -// step is native: their hits and hybrid paths build, coarsen and zero-fill a -// bucket-start axis that every panel of the page shares. +// step the windows overlap; both use the anchored window evaluator, for Grafana +// Logs Drilldown requests too. func (p *Proxy) statsRangeIsTumbling(r *http.Request, window, step time.Duration) bool { - if step <= 0 || window <= 0 || window > step { - return false - } - if isGrafanaDrilldownRequest(r) { - return true - } - return window == step && p.tumblingBucketsAligned(r, window) + return step > 0 && window > 0 && window == step && p.tumblingBucketsAligned(r, window) } -// tumblingBucketsAligned reports whether stats_query_range buckets of window -// can cover the request's evaluation windows: always when the backend honours -// the offset arg (VictoriaLogs v1.45+), otherwise only when the start is -// epoch-aligned to window, like slidingStatsBucket. An unknown version (probe -// pending or failed) counts as no offset support. +// tumblingBucketsAligned reports whether stats_query_range buckets of window can +// cover the request's evaluation windows: whenever the backend honours the +// offset arg (VictoriaLogs v1.45+, and an unknown version, which ignores the arg +// at worst), otherwise only when the start is epoch-aligned to window, like +// slidingStatsBucket. func (p *Proxy) tumblingBucketsAligned(r *http.Request, window time.Duration) bool { if p.supportsStatsRangeOffset() { return true @@ -718,17 +710,26 @@ func (p *Proxy) proxyManualRangeMetricRange(w http.ResponseWriter, r *http.Reque // With parser stages the stats query keeps them: VictoriaLogs groups by the // parsed fields and counts every line, as the raw evaluator does. streamBuckets := origSpec.Window != step - if series, ok := p.collectStatsFastPathHits(r.Context(), spec, statsAggFunc, fetchStart, endTS, bucket, streamBuckets, false); ok { - p.writeHitsRangeMetricMatrix(w, manualFunc, shiftSeriesSamples(series, sampleShift), startTS, endTS, step, origSpec.Window) - return true - } - if series, ok := p.collectParserStageStatsFastPathHits(r.Context(), spec, statsAggFunc, fetchStart, endTS, bucket); ok { - p.writeHitsRangeMetricMatrix(w, manualFunc, shiftSeriesSamples(series, sampleShift), startTS, endTS, step, origSpec.Window) - return true - } - if series, ok := p.collectStatsFastPathHits(r.Context(), spec, statsAggFunc, fetchStart, endTS, bucket, streamBuckets, true); ok { - p.writeHitsRangeMetricMatrix(w, manualFunc, shiftSeriesSamples(series, sampleShift), startTS, endTS, step, origSpec.Window) - return true + for _, collect := range []func() (map[string]manualSeriesSamples, bool, error){ + func() (map[string]manualSeriesSamples, bool, error) { + return p.collectStatsFastPathHits(r.Context(), spec, statsAggFunc, fetchStart, endTS, bucket, streamBuckets, false) + }, + func() (map[string]manualSeriesSamples, bool, error) { + return p.collectParserStageStatsFastPathHits(r.Context(), spec, statsAggFunc, fetchStart, endTS, bucket) + }, + func() (map[string]manualSeriesSamples, bool, error) { + return p.collectStatsFastPathHits(r.Context(), spec, statsAggFunc, fetchStart, endTS, bucket, streamBuckets, true) + }, + } { + series, ok, err := collect() + if err != nil { + p.writeError(w, badRequestStatusOr(err, http.StatusBadGateway), err.Error()) + return true + } + if ok { + p.writeHitsRangeMetricMatrix(w, manualFunc, shiftSeriesSamples(series, sampleShift), startTS, endTS, step, origSpec.Window) + return true + } } fetchEnd := endTS @@ -744,7 +745,7 @@ func (p *Proxy) proxyManualRangeMetricRange(w http.ResponseWriter, r *http.Reque result, err := buildManualRangeMetricMatrixContext(r.Context(), manualFunc, quantile, series, startTS, endTS, step, origSpec.Window, p.resolvedMaxStatsQuerySeries()) if err != nil { - p.writeError(w, http.StatusServiceUnavailable, err.Error()) + p.writeError(w, badRequestStatusOr(err, http.StatusServiceUnavailable), err.Error()) return true } w.Header().Set("Content-Type", "application/json") @@ -792,7 +793,7 @@ func shiftSeriesSamples(series map[string]manualSeriesSamples, shift time.Durati func (p *Proxy) writeHitsRangeMetricMatrix(w http.ResponseWriter, manualFunc string, series map[string]manualSeriesSamples, start, end time.Time, step, window time.Duration) int { result, err := buildHitsRangeMetricMatrix(manualFunc, series, start, end, step, window, p.limits().BufferedBackendBodyBytes) if err != nil { - p.writeError(w, http.StatusServiceUnavailable, err.Error()) + p.writeError(w, badRequestStatusOr(err, http.StatusServiceUnavailable), err.Error()) return http.StatusServiceUnavailable } w.Header().Set("Content-Type", "application/json") @@ -857,7 +858,7 @@ func (p *Proxy) proxyManualRangeMetricInstant(w http.ResponseWriter, r *http.Req result, err := buildManualRangeMetricVectorContext(r.Context(), manualFunc, quantile, series, evalTS, origSpec.Window, p.resolvedMaxStatsQuerySeries()) if err != nil { - p.writeError(w, http.StatusServiceUnavailable, err.Error()) + p.writeError(w, badRequestStatusOr(err, http.StatusServiceUnavailable), err.Error()) return true } w.Header().Set("Content-Type", "application/json") @@ -871,53 +872,48 @@ func (p *Proxy) proxyManualRangeMetricInstant(w http.ResponseWriter, r *http.Req // in the raw evaluator) instead of queries without them. Per-stream grouping (the // _stream sentinel of bare range metrics) is served only without parser stages and // when streamBuckets is set. -// Returns nil, false when the fast path is not applicable or VL returns an error. -func (p *Proxy) collectStatsFastPathHits(ctx context.Context, spec statsCompatSpec, statsAggFunc string, windowStart, end time.Time, step time.Duration, streamBuckets, withParser bool) (map[string]manualSeriesSamples, bool) { +// Returns nil, false, nil when the fast path is not applicable or VL fails, so +// the caller may use the next path. A non-nil error is the request's answer: a +// series limit error, or a failed Drilldown top-value call. +func (p *Proxy) collectStatsFastPathHits(ctx context.Context, spec statsCompatSpec, statsAggFunc string, windowStart, end time.Time, step time.Duration, streamBuckets, withParser bool) (map[string]manualSeriesSamples, bool, error) { if statsAggFunc == "" || queryUsesParserStages(spec.BaseQuery) != withParser { - return nil, false + return nil, false, nil } for _, g := range spec.GroupBy { if g == "_stream" && (!streamBuckets || withParser) { - return nil, false + return nil, false, nil } } if len(spec.GroupBy) == 0 && !spec.ByExplicit { - return nil, false + return nil, false, nil } - singleField := len(spec.GroupBy) == 1 && spec.GroupBy[0] != "_stream" && !spec.ByExplicit - var ( - series map[string]manualSeriesSamples - err error - ) - if singleField && end.Sub(windowStart) >= 2*time.Hour { - // Over hours a field such as pod churns into thousands of values, and the - // full bucket response can take seconds only to overflow: rank first. + series, err := p.collectRangeMetricHits(ctx, spec.BaseQuery, spec.GroupBy, spec.OrigGroupBy, spec.ByExplicit, statsAggFunc, windowStart, end, step) + if errors.Is(err, errBodyTooLarge) && len(spec.GroupBy) == 1 && spec.GroupBy[0] != "_stream" && !spec.ByExplicit && requestKeepsPartialSeries(ctx) { + // Drilldown: a response too large for one read keeps the busiest values. series, err = p.collectTopValueRangeMetricHits(ctx, spec, statsAggFunc, windowStart, end, step) - } else { - series, err = p.collectRangeMetricHits(ctx, spec.BaseQuery, spec.GroupBy, spec.OrigGroupBy, spec.ByExplicit, statsAggFunc, windowStart, end, step) - if errors.Is(err, errBodyTooLarge) && singleField { - // Too many series for one bucket response: keep the busiest values. - series, err = p.collectTopValueRangeMetricHits(ctx, spec, statsAggFunc, windowStart, end, step) + if err != nil { + return nil, false, err } } + if isSeriesLimitError(err) { + return nil, false, err + } if err != nil { - return nil, false + return nil, false, nil } - return series, true + return series, true, nil } -// maxTopValueFilterBytes bounds the in() filter of collectTopValueRangeMetricHits, -// leaving room for the rest of the query under VictoriaLogs' default -// -search.maxQueryLen of 16384 bytes. -const maxTopValueFilterBytes = 12 << 10 +// vlDefaultMaxQueryLen is VictoriaLogs' default -search.maxQueryLen. +const vlDefaultMaxQueryLen = 16384 -// collectTopValueRangeMetricHits answers a single-field grouped bucket query in -// two phases: one stats_query ranks the field values by line count over the -// whole fetch window, up to the operator's -max-stats-query-series, then the -// bucket query runs, restricted to those values when the ranking was cut. The -// in() filter keeps the busiest values that fit maxTopValueFilterBytes. Lines -// without the field stay in when they rank. The series of the kept values are -// exact. +// collectTopValueRangeMetricHits answers a Drilldown single-field grouped bucket +// query whose response is too large to read, in two phases: one stats_query +// ranks the field values by line count over the fetch window, then the bucket +// query runs restricted to the busiest -max-stats-query-series values that fit +// VictoriaLogs' default query length. When more values exist the result is +// partial and the request carries Loki's warning. Lines without the field stay +// in when they rank. The series of the kept values are exact. func (p *Proxy) collectTopValueRangeMetricHits(ctx context.Context, spec statsCompatSpec, statsAggFunc string, windowStart, end time.Time, step time.Duration) (map[string]manualSeriesSamples, error) { field := quoteLogsQLIdent(spec.GroupBy[0]) limit := p.resolvedMaxStatsQuerySeries() @@ -940,53 +936,101 @@ func (p *Proxy) collectTopValueRangeMetricHits(ctx context.Context, spec statsCo return nil, err } values := drilldownTopValuesFromMatrix(body, spec.GroupBy[0]) + withEmpty := drilldownTopValuesHaveEmpty(body, spec.GroupBy[0]) if len(values) == 0 { - // No labelled value in the window: the unfiltered query is small. - return p.collectRangeMetricHits(ctx, spec.BaseQuery, spec.GroupBy, spec.OrigGroupBy, spec.ByExplicit, statsAggFunc, windowStart, end, step) + return nil, errBodyTooLarge } - if len(values) < limit { - // Every value ranked (the empty group may be one of them): no filter. - return p.collectRangeMetricHits(ctx, spec.BaseQuery, spec.GroupBy, spec.OrigGroupBy, spec.ByExplicit, statsAggFunc, windowStart, end, step) + // The whole bucket query must fit VictoriaLogs' query length. + prefix, suffix := spec.BaseQuery+" | filter ", " | stats by ("+field+") "+statsAggFunc + if withEmpty { + prefix, suffix = prefix+"(", " or "+field+`:"")`+suffix } - values = values[:limit] - filter := buildVLInFilter(spec.GroupBy[0], values) - for len(filter) > maxTopValueFilterBytes && len(values) > 1 { - values = values[:len(values)*maxTopValueFilterBytes/len(filter)] - filter = buildVLInFilter(spec.GroupBy[0], values) + ranked := len(values) + if len(values) > limit { + values = values[:limit] + } + filter, values, err := fitTopValueFilter(spec.GroupBy[0], values, prefix, suffix) + if err != nil { + return nil, err + } + if len(values) < ranked { + if err := seriesLimitReached(ctx, len(values)); err != nil { + return nil, err + } } - if drilldownTopValuesHaveEmpty(body, spec.GroupBy[0]) { + if withEmpty { filter = "(" + filter + " or " + field + `:"")` } return p.collectRangeMetricHits(ctx, spec.BaseQuery+" | filter "+filter, spec.GroupBy, spec.OrigGroupBy, spec.ByExplicit, statsAggFunc, windowStart, end, step) } +// requestKeepsPartialSeries reports whether the request answers a series limit +// with a partial result (Grafana Logs Drilldown) instead of an error. +func requestKeepsPartialSeries(ctx context.Context) bool { + scope, _ := ctx.Value(seriesLimitScopeKey{}).(*seriesLimitScope) + return scope != nil && scope.drilldown +} + +// seriesCountingReader fails a stats_query_range read once the response holds +// more than limit result entries, so an over-limit query stops without reading +// the rest of the response. +type seriesCountingReader struct { + r io.Reader + limit, seen int + tail, window []byte +} + +var statsResultMetricKey = []byte(`"metric":`) + +func (c *seriesCountingReader) Read(b []byte) (int, error) { + n, err := c.r.Read(b) + if n > 0 && c.limit > 0 { + c.window = append(append(c.window[:0], c.tail...), b[:n]...) + c.seen += bytes.Count(c.window, statsResultMetricKey) + keep := len(statsResultMetricKey) - 1 + if keep > len(c.window) { + keep = len(c.window) + } + c.tail = append(c.tail[:0], c.window[len(c.window)-keep:]...) + if c.seen > c.limit { + return n, errSeriesCountExceeded + } + } + return n, err +} + +var errSeriesCountExceeded = errors.New("stats response holds more series than the limit") + // collectParserStageStatsFastPathHits handles parser-stage GroupBy count queries // (e.g. Drilldown field histograms with | json | delete __error__) by stripping // the parser stages and retrying via stats_query_range against VL's column index. // Safe only when all remaining filters after stripping are field-existence checks. // Returns nil, false when inapplicable, on error, or when VL returns 0 series // (non-indexed JSON fields still need parser stages to evaluate correctly). -func (p *Proxy) collectParserStageStatsFastPathHits(ctx context.Context, spec statsCompatSpec, statsAggFunc string, windowStart, end time.Time, step time.Duration) (map[string]manualSeriesSamples, bool) { +func (p *Proxy) collectParserStageStatsFastPathHits(ctx context.Context, spec statsCompatSpec, statsAggFunc string, windowStart, end time.Time, step time.Duration) (map[string]manualSeriesSamples, bool, error) { if statsAggFunc == "" || !queryUsesParserStages(spec.BaseQuery) || len(spec.GroupBy) == 0 { - return nil, false + return nil, false, nil } if !strings.Contains(spec.BaseQuery, "| delete __error__") { - return nil, false + return nil, false, nil } for _, g := range spec.GroupBy { if g == "_stream" { - return nil, false + return nil, false, nil } } strippedBase := strings.TrimSpace(drilldownParserPipeRE.ReplaceAllString(spec.BaseQuery, "")) if strippedBase == spec.BaseQuery || !allFiltersAreExistenceChecks(strippedBase) { - return nil, false + return nil, false, nil } series, err := p.collectRangeMetricHits(ctx, strippedBase, spec.GroupBy, spec.OrigGroupBy, spec.ByExplicit, statsAggFunc, windowStart, end, step) + if isSeriesLimitError(err) { + return nil, false, err + } if err != nil || len(series) == 0 { - return nil, false + return nil, false, nil } - return series, true + return series, true, nil } func (p *Proxy) resolveManualMetricField(spec statsCompatSpec, origSpec originalRangeMetricSpec, manualFunc string) (string, float64, error) { @@ -1085,6 +1129,28 @@ func snapSlidingBucketNanos(ts int64, start time.Time, hitStep time.Duration) in return anchor + k*size } +// readStatsBucketBody reads a stats_query_range response, stopping with Loki's +// series limit error once it holds more result entries than the limit allows +// (two per series when byte presence counts ride along). A Drilldown request +// reads it all and keeps the busiest series instead. +func (p *Proxy) readStatsBucketBody(ctx context.Context, body io.Reader, limit int, withPresence bool) ([]byte, error) { + counter := &seriesCountingReader{r: body} + if !requestKeepsPartialSeries(ctx) { + counter.limit = limit + if withPresence { + counter.limit = 2 * limit + } + } + out, err := readBodyLimited(counter, maxStatsBucketResponseBytes) + if errors.Is(err, errSeriesCountExceeded) { + return nil, &seriesLimitError{limit: limit} + } + if err != nil { + return nil, err + } + return out, nil +} + // collectRangeMetricHits calls VL's /select/logsql/stats_query_range endpoint and // returns pre-bucketed samples (ts=bucket left edge ns on the start+k*hitStep // grid, value=bucket_value). This avoids reading all raw log entries for @@ -1144,7 +1210,9 @@ func (p *Proxy) collectRangeMetricHits( return nil, p.redactedBackendStatusError("stats_query_range backend", resp.StatusCode, body) } - body, err := readBodyLimited(resp.Body, maxStatsBucketResponseBytes) + limit := p.resolvedMaxStatsQuerySeries() + withPresence := strings.Contains(statsAggFunc, ", count() as __sample_count") + body, err := p.readStatsBucketBody(ctx, resp.Body, limit, withPresence) if err != nil { return nil, err } @@ -1170,22 +1238,6 @@ func (p *Proxy) collectRangeMetricHits( } results := v.GetArray("data", "result") - // Cap to maxStatsQuerySeries to bound response size for high-cardinality - // by() clauses (e.g. churn-heavy pod names, *_id fields where each value - // appears 1-2× in the window). Default 500 matches maxDrilldownSeries - // (the cap the Drilldown-specific paths use) and aligns with Loki's - // default max_query_series for the same purpose. The previous default - // of 5000 returned 10× more sparse series than Drilldown can render - // (the plugin's "Show all N" badge is literally the returned series - // count) and made charts look like scattered single-point spikes - // instead of meaningful top-N curves. See memory - // [[drilldown-high-card-fields-known-limit]] for the deep investigation. - // Keep the busiest maxSeries by total count (not the alphabetically-first - // maxSeries VL returns) so the chart shows signal, not the noise floor. - withPresence := strings.Contains(statsAggFunc, ", count() as __sample_count") - if !withPresence { - results = capStatsResultsByTotalCount(results, p.resolvedMaxStatsQuerySeries()) - } // Stream-grouped series carry the labels the raw evaluator derives from // _stream and level. streamGrouped := false @@ -1254,9 +1306,13 @@ func (p *Proxy) collectRangeMetricHits( seriesMap[seriesKey] = manualSeriesSamples{Metric: metric, Samples: samples} } } - if withPresence { - // Cap complete logical series, retaining both byte values and presence. - seriesMap = capSeriesByTotalCount(seriesMap, p.resolvedMaxStatsQuerySeries()) + // More series than -max-stats-query-series: Loki's error, or for Drilldown the + // busiest series (complete, with byte values and presence) and a warning. + if len(seriesMap) > limit { + if err := seriesLimitReached(ctx, limit); err != nil { + return nil, err + } + seriesMap = capSeriesByTotalCount(seriesMap, limit) } return seriesMap, nil } @@ -1424,7 +1480,7 @@ func (p *Proxy) collectRangeMetricSamples(ctx context.Context, baseQuery string, current := seriesMap[seriesEntry.key] if current.Metric == nil { if len(seriesMap) >= p.resolvedMaxStatsQuerySeries() { - return nil, fmt.Errorf("maximum metric series exceeded (%d)", p.resolvedMaxStatsQuerySeries()) + return nil, &seriesLimitError{limit: p.resolvedMaxStatsQuerySeries()} } current.Metric = seriesEntry.translated } @@ -1806,9 +1862,13 @@ func (p *Proxy) resolvedMaxStatsQuerySeries() int { if p != nil && p.maxStatsQuerySeries > 0 { return p.maxStatsQuerySeries } - return 500 + return defaultStatsQuerySeries } +// defaultStatsQuerySeries is Loki's stock max_query_series, the default of +// -max-stats-query-series. +const defaultStatsQuerySeries = 500 + // capStatsResultsByTotalCount keeps only the maxSeries VL stats results with the // highest summed bucket value, dropping the long tail. VL's stats_query_range // returns results in LABEL (alphabetical) order, so a plain results[:maxSeries] @@ -1906,7 +1966,7 @@ func buildManualRangeMetricMatrixContext(ctx context.Context, functionName strin return nil, err } if maxSeries > 0 && len(series) > maxSeries { - return nil, fmt.Errorf("manual metric series limit exceeded (%d); narrow the query", maxSeries) + return nil, &seriesLimitError{limit: maxSeries} } ctx = binaryEvaluationContext(ctx) if end.Before(start) { @@ -2068,7 +2128,7 @@ func buildManualRangeMetricVectorContext(ctx context.Context, functionName strin return nil, err } if len(maxSeries) > 0 && maxSeries[0] > 0 && len(series) > maxSeries[0] { - return nil, fmt.Errorf("manual metric series limit exceeded (%d); narrow the query", maxSeries[0]) + return nil, &seriesLimitError{limit: maxSeries[0]} } ctx = binaryEvaluationContext(ctx) keys := make([]string, 0, len(series)) diff --git a/internal/proxy/range_metric_compat_test.go b/internal/proxy/range_metric_compat_test.go index 55341bf69..76e9d4974 100644 --- a/internal/proxy/range_metric_compat_test.go +++ b/internal/proxy/range_metric_compat_test.go @@ -364,7 +364,7 @@ func TestQueryRange_RateParserStageSlidingProducesResult(t *testing.T) { } func TestQueryRange_JSONRatePreservesParsedSeriesAndSlidingBounds(t *testing.T) { - base := time.Unix(1700000000, 0).UTC() + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges var rawCalled, statsCalled bool backend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -505,7 +505,7 @@ func TestQueryRange_SummedBytesWithUnusedJSONScalesFromSumLen(t *testing.T) { // // 3 entries × 100 bytes each = 300 bytes total in the [T-120s, T+180s] window // → bytes_rate = 300 / 300s = 1 bytes/sec. - base := time.Unix(1700000000, 0).UTC() + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges statsCalled := false vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -621,7 +621,7 @@ func TestQueryRange_StdvarCompatSquaresStddev(t *testing.T) { // stats function, so first_over_time over unwrap is evaluated from raw rows and // never sent to stats_query_range. func TestQueryRange_FirstOverTimeUsesRawEvaluator(t *testing.T) { - base := time.Unix(1700000000, 0).UTC() + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/select/logsql/query" { t.Errorf("unexpected backend path %s (expected the raw query path)", r.URL.Path) @@ -677,7 +677,7 @@ func TestQueryRange_FirstOverTimeUsesRawEvaluator(t *testing.T) { // unwrap field routes to stats_query_range (not raw log fetch) and correctly // sums per-step bucket values across the sliding window. func TestQueryRange_SumOverTimeUsesStatsPath(t *testing.T) { - base := time.Unix(1700000000, 0).UTC() + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges statsCalled := false vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -687,7 +687,7 @@ func TestQueryRange_SumOverTimeUsesStatsPath(t *testing.T) { // Three 1-minute buckets with sum(duration) values: 100, 200, 300 _, _ = w.Write([]byte(`{"status":"success","data":{"resultType":"matrix","result":[` + `{"metric":{"_stream":"{app=\"api\"}"},` + - `"values":[[1700000000,"100"],[1700000060,"200"],[1700000120,"300"]]}]}}`)) + `"values":[[1699999980,"100"],[1700000040,"200"],[1700000100,"300"]]}]}}`)) default: t.Errorf("unexpected backend path %s — should use stats_query_range not raw log fetch", r.URL.Path) http.NotFound(w, r) @@ -726,9 +726,10 @@ func TestQueryRange_SumOverTimeUsesStatsPath(t *testing.T) { if len(resp.Data.Result) == 0 { t.Fatalf("expected at least one series, got empty result: %s", rec.Body.String()) } - // The bucket labelled L covers (L, L+60s]: the window (T-60s, T+60s] holds - // only the T+0 bucket, and (T, T+120s] the T+0 and T+60s buckets. - if got := fmt.Sprint(resp.Data.Result[0].Values); got != "[[1.70000006e+09 100] [1.70000012e+09 300]]" { + // The bucket labelled L covers (L, L+60s]: the 2m window ending T+60s holds + // the buckets labelled T-60s and T, and the one ending T+120s those labelled + // T and T+60s. + if got := fmt.Sprint(resp.Data.Result[0].Values); got != "[[1.7000001e+09 300] [1.70000016e+09 500]]" { t.Fatalf("unexpected sliding sums %s: %s", got, rec.Body.String()) } } @@ -816,7 +817,7 @@ func TestQueryRange_SumOverTimeRequiresUnwrap(t *testing.T) { } func TestQueryRange_QuantileManualFallback(t *testing.T) { - base := time.Unix(1700000000, 0).UTC() + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path != "/select/logsql/query" { t.Fatalf("unexpected backend path %s", r.URL.Path) @@ -980,10 +981,11 @@ func TestShouldUseManualRangeMetricCompat_WithoutSlidingWindowNowUsesManualPath( func TestQueryRange_SlidingWindowWithout_UsesManualPath(t *testing.T) { // Verify that sum without(level)(rate[10m]) with step=1m (sliding window) goes - // through the manual log-fetch path, not native VL tumbling stats. - manualPathHit := false + // through the manual window evaluator: anchored per-stream buckets, not native + // VL tumbling stats and not a raw log fetch. + manualPathHit, statsBuckets := false, "" - base := time.Unix(1700000000, 0).UTC() + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == "/select/logsql/query" { // Manual path: log-fetch endpoint @@ -997,9 +999,10 @@ func TestQueryRange_SlidingWindowWithout_UsesManualPath(t *testing.T) { return } if r.URL.Path == "/select/logsql/stats_query_range" { - t.Error("should NOT hit stats_query_range (native tumbling path) for sliding window without()") + _ = r.ParseForm() + statsBuckets = r.Form.Get("step") w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"data":{"resultType":"matrix","result":[]}}`)) + _, _ = w.Write([]byte(`{"status":"success","data":{"resultType":"matrix","result":[]}}`)) return } http.NotFound(w, r) @@ -1022,8 +1025,8 @@ func TestQueryRange_SlidingWindowWithout_UsesManualPath(t *testing.T) { p.RegisterRoutes(mux) mux.ServeHTTP(w, r) - if !manualPathHit { - t.Error("expected manual log-fetch path to be used for sliding window without()") + if manualPathHit || statsBuckets != "60s" { + t.Errorf("expected gcd(step, range) = 60s window buckets without a raw log fetch, got buckets %q raw=%v", statsBuckets, manualPathHit) } } @@ -1443,9 +1446,10 @@ func TestQueryRange_RateParserStageTumblingUsesVLNative(t *testing.T) { } func TestQueryRange_RateParserStageSlidingStaysManual(t *testing.T) { - // step=60 ≠ range=[5m]=300 → sliding window: slow path preserved. - base := time.Unix(1700000000, 0).UTC() - var slowCalled bool + // step=60 ≠ range=[5m]=300 → sliding window: anchored stats buckets keep the + // parser stage and the post-parser filter, with no raw log fetch. + base := time.Unix(1700000040, 0).UTC() // step-aligned, as Loki aligns metric ranges + var slowCalled, statsCalled bool vlBackend := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch r.URL.Path { @@ -1457,8 +1461,9 @@ func TestQueryRange_RateParserStageSlidingStaysManual(t *testing.T) { base.Format(time.RFC3339Nano), ) case "/select/logsql/stats_query_range": - t.Error("stats_query_range must NOT be called for sliding-window parser-stage rate") - w.WriteHeader(http.StatusInternalServerError) + statsCalled = true + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"status":"success","data":{"resultType":"matrix","result":[]}}`)) default: if r.URL.Path != "/metrics" { t.Logf("unhandled: %s", r.URL.Path) @@ -1481,7 +1486,7 @@ func TestQueryRange_RateParserStageSlidingStaysManual(t *testing.T) { if rec.Code != http.StatusOK { t.Fatalf("expected 200, got %d: %s", rec.Code, rec.Body.String()) } - if !slowCalled { - t.Error("expected slow /select/logsql/query for sliding-window parser-stage rate") + if slowCalled || !statsCalled { + t.Errorf("expected window buckets without a raw log fetch, got raw=%v stats=%v", slowCalled, statsCalled) } } diff --git a/internal/proxy/review_findings_test.go b/internal/proxy/review_findings_test.go index 46464ee5a..d59b1fdd9 100644 --- a/internal/proxy/review_findings_test.go +++ b/internal/proxy/review_findings_test.go @@ -276,11 +276,11 @@ func TestCollectRangeMetricSamples_RedactsBackendError(t *testing.T) { } } -// TestWindowedHits_GatedToDrilldownOrHighCard covers finding 3 (round 2): the -// window-sampled /hits rewrite (lossy per-window top-N sampling) must apply ONLY -// to Drilldown requests OR Grafana-sourced high-cardinality fields. Direct API -// clients must stay exact even for high-cardinality groupings. -func TestWindowedHits_GatedToDrilldownOrHighCard(t *testing.T) { +// TestWindowedHits_GatedToDrilldown: the window-sampled /hits rewrite (lossy +// per-window top-N sampling) applies ONLY to Logs Drilldown requests. Direct API +// clients and Grafana dashboards or Explore stay exact, like Loki, even for +// high-cardinality groupings. +func TestWindowedHits_GatedToDrilldown(t *testing.T) { p := &Proxy{} // 24h range so the >=2h gate would otherwise pass. mk := func(drilldown bool, grafanaUA bool) *http.Request { @@ -314,10 +314,11 @@ func TestWindowedHits_GatedToDrilldownOrHighCard(t *testing.T) { rejects("non-grafana high-card", mk(false, false), highCard) // Grafana dashboard/Explore (UA only, NOT Drilldown), low-card → exact. rejects("grafana dashboard low-card", mk(false, true), lowCard) + // Grafana dashboard/Explore, high-card → exact series, as Loki returns. + rejects("grafana dashboard high-card", mk(false, true), highCard) - // The gate ITSELF admits Drilldown (any field) and Grafana-sourced high-card - // fields. We assert the source/cardinality predicates rather than the full path - // (which needs a backend). + // The gate ITSELF admits Drilldown. We assert the source predicate rather than + // the full path (which needs a backend). if !isGrafanaDrilldownRequest(mk(true, true)) { t.Error("drilldown-tagged request must be recognized as Drilldown") } diff --git a/internal/proxy/sliding_window_parity_test.go b/internal/proxy/sliding_window_parity_test.go index c47979df0..e4be926bf 100644 --- a/internal/proxy/sliding_window_parity_test.go +++ b/internal/proxy/sliding_window_parity_test.go @@ -633,10 +633,12 @@ func TestSlidingRangeMetric_FineBucketsStayOnStats(t *testing.T) { // VictoriaLogs before v1.45 ignores the stats_query_range offset arg and aligns // buckets to the epoch, and a failed version probe leaves the release unknown. -// Both keep buckets only for epoch-aligned grids (a line exactly on a window -// edge then counts in the neighbouring window, a documented limit, so these -// fixtures keep lines off the edges); other grids use the raw evaluator instead -// of silently misaligned buckets. This includes the range == step topk path. +// A release known to be older keeps buckets only for epoch-aligned grids (a line +// exactly on a window edge then counts in the neighbouring window, a documented +// limit, so these fixtures keep lines off the edges); other grids use the raw +// evaluator instead of silently misaligned buckets. An unknown version stays on +// buckets: the arg is ignored at worst, while raw rows scale with the stored +// data. This includes the range == step topk path. func TestSlidingRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { s0 := time.Unix(1700000400, 0).UTC() var lines []slidingFixtureLine @@ -651,12 +653,13 @@ func TestSlidingRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { byApp bool window time.Duration start time.Time - wantRaw bool + unaligned bool + ranked bool // topk: one stats call with in-query _time buckets }{ {name: "sliding epoch-aligned grid", query: sliding, window: 2 * time.Minute, start: s0.Add(5 * time.Minute)}, - {name: "sliding unaligned grid", query: sliding, window: 2 * time.Minute, start: s0.Add(5*time.Minute + 30*time.Second), wantRaw: true}, - {name: "topk range equals step epoch-aligned grid", query: ranked, byApp: true, window: time.Minute, start: s0.Add(5 * time.Minute)}, - {name: "topk range equals step unaligned grid", query: ranked, byApp: true, window: time.Minute, start: s0.Add(5*time.Minute + 30*time.Second), wantRaw: true}, + {name: "sliding unaligned grid", query: sliding, window: 2 * time.Minute, start: s0.Add(5*time.Minute + 30*time.Second), unaligned: true}, + {name: "topk range equals step epoch-aligned grid", query: ranked, byApp: true, window: time.Minute, start: s0.Add(5 * time.Minute), ranked: true}, + {name: "topk range equals step unaligned grid", query: ranked, byApp: true, window: time.Minute, start: s0.Add(5*time.Minute + 30*time.Second), unaligned: true, ranked: true}, } { t.Run(fmt.Sprintf("version=%q/%s", version, tc.name), func(t *testing.T) { srv, fake := newSlidingFakeVL(t, lines) @@ -668,14 +671,34 @@ func TestSlidingRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { got := runSlidingQueryRange(t, p, tc.query, tc.start, end, time.Minute, nil) assertSlidingSeriesEqual(t, tc.query, lokiSlidingReference(lines, "count_over_time", tc.byApp, 0, tc.start, end, time.Minute, tc.window), got) calls, raw := fake.snapshot() - if tc.wantRaw { + if tc.unaligned && version != "" { if len(calls) != 0 || raw == 0 { - t.Fatalf("expected raw evaluator, got %+v stats calls and %d raw scans", calls, raw) + t.Fatalf("known old release, unaligned grid: expected the raw evaluator, got %+v stats calls and %d raw scans", calls, raw) } return } - if raw != 0 || len(calls) != 1 || calls[0].step != "60s" || calls[0].offset != "" { - t.Fatalf("expected one offset-free stats call with 60s buckets, got %+v and %d raw scans", calls, raw) + // The topk path buckets inside the query (stats by (_time:...)), so + // its anchor is the positive complement of the request offset. + wantStep, wantOffset := "60s", "" + if tc.ranked && version == "" { + // In-query _time buckets only where the anchor is honoured; + // a known-old release gets plain epoch buckets. + wantStep = "1m0s" + } + if version == "" { // unknown version: anchored buckets, arg ignored at worst + switch { + case tc.ranked && tc.unaligned: + wantOffset = "29999999999" + case tc.ranked: + wantOffset = "59999999999" + case tc.unaligned: + wantOffset = "-30000000001ns" + default: + wantOffset = "-1ns" + } + } + if raw != 0 || len(calls) != 1 || calls[0].step != wantStep || calls[0].offset != wantOffset { + t.Fatalf("expected one stats call with %s buckets and offset %q, got %+v and %d raw scans", wantStep, wantOffset, calls, raw) } }) } @@ -689,9 +712,9 @@ func newSlidingTestProxy(t *testing.T, backendURL string) *Proxy { return p } -// A failed startup version probe must not pin the raw evaluator: while the -// version is unknown the proxy retries the metrics probe in the background, and -// once it succeeds unaligned grids use anchored buckets. +// A failed startup version probe keeps anchored buckets (never a raw scan) and +// the proxy retries the metrics probe in the background until the version is +// known. func TestSlidingRangeMetric_VersionReprobeEnablesAnchoredBuckets(t *testing.T) { s0 := time.Unix(1700000400, 0).UTC() var lines []slidingFixtureLine @@ -727,10 +750,11 @@ func TestSlidingRangeMetric_VersionReprobeEnablesAnchoredBuckets(t *testing.T) { } } - // Unknown version within the probe interval: raw evaluator, no probe yet. + // Unknown version within the probe interval: anchored buckets, no raw scan + // and no probe yet. check(10) - if calls, raw := fake.snapshot(); len(calls) != 0 || raw != 1 { - t.Fatalf("unknown version must use the raw evaluator, got %+v stats calls and %d raw scans", calls, raw) + if calls, raw := fake.snapshot(); len(calls) != 1 || raw != 0 || calls[0].offset != "-30000000001ns" { + t.Fatalf("unknown version must use anchored buckets, got %+v stats calls and %d raw scans", calls, raw) } waitMetricsCalls(0) @@ -764,7 +788,7 @@ func TestSlidingRangeMetric_VersionReprobeEnablesAnchoredBuckets(t *testing.T) { check(12) waitMetricsCalls(2) calls, _ := fake.snapshot() - if len(calls) != 1 || calls[0].offset != "-30000000001ns" { + if len(calls) == 0 || calls[len(calls)-1].offset != "-30000000001ns" { t.Fatalf("expected one anchored stats call once the version is known, got %+v", calls) } } diff --git a/internal/proxy/tumbling_axis_parity_test.go b/internal/proxy/tumbling_axis_parity_test.go index 9b2fde393..5932b9d8b 100644 --- a/internal/proxy/tumbling_axis_parity_test.go +++ b/internal/proxy/tumbling_axis_parity_test.go @@ -338,16 +338,29 @@ func lokiTumblingReferenceParsed(lines []tumblingLine, fn string, by []string, s // the key. func runTumblingQuery(t *testing.T, p *Proxy, path string, params url.Values) map[string]map[int64]string { t.Helper() + rec := serveTumblingQuery(p, path, params, nil) + if rec.Code != http.StatusOK { + t.Fatalf("%s: expected 200, got %d: %s", params.Get("query"), rec.Code, rec.Body.String()) + } + return decodeTumblingSeries(t, params.Get("query"), rec.Body.Bytes()) +} + +func serveTumblingQuery(p *Proxy, path string, params url.Values, header http.Header) *httptest.ResponseRecorder { req := httptest.NewRequest(http.MethodGet, path+"?"+params.Encode(), nil) + for k, v := range header { + req.Header[k] = v + } rec := httptest.NewRecorder() if strings.HasSuffix(path, "query_range") { p.handleQueryRange(rec, req) } else { p.handleQuery(rec, req) } - if rec.Code != http.StatusOK { - t.Fatalf("%s: expected 200, got %d: %s", params.Get("query"), rec.Code, rec.Body.String()) - } + return rec +} + +func decodeTumblingSeries(t *testing.T, query string, body []byte) map[string]map[int64]string { + t.Helper() var resp struct { Data struct { Result []struct { @@ -357,8 +370,8 @@ func runTumblingQuery(t *testing.T, p *Proxy, path string, params url.Values) ma } `json:"result"` } `json:"data"` } - if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { - t.Fatalf("%s: decode: %v: %s", params.Get("query"), err, rec.Body.String()) + if err := json.Unmarshal(body, &resp); err != nil { + t.Fatalf("%s: decode: %v: %s", query, err, string(body)) } got := map[string]map[int64]string{} for _, series := range resp.Data.Result { @@ -366,7 +379,7 @@ func runTumblingQuery(t *testing.T, p *Proxy, path string, params url.Values) ma delete(series.Metric, "detected_level") key := canonicalLabelsKey(series.Metric) if got[key] != nil { - t.Errorf("%s: duplicate series %s: %s", params.Get("query"), key, rec.Body.String()) + t.Errorf("%s: duplicate series %s: %s", query, key, string(body)) } else { got[key] = map[int64]string{} } @@ -489,11 +502,10 @@ func TestTumblingRangeMetric_BucketsLabelledAtLokiEvaluationTime(t *testing.T) { } } -// From two hours on a grouped count is answered in two phases (a global top-N, -// then per-step counts for those values). The relabelled path keeps Loki's -// windows there too: anchored Phase 2 buckets, and the lines without the -// grouped label as the unlabelled series. -func TestTumblingRangeMetric_LongRangeTwoPhaseKeepsLokiWindows(t *testing.T) { +// A grouped count over hours is one anchored bucket scan: no ranking query +// ahead of it, Loki's windows, and the lines without the grouped label as the +// unlabelled series. +func TestTumblingRangeMetric_LongRangeSingleScanKeepsLokiWindows(t *testing.T) { base := time.Unix(1700000400, 0).UTC() lines := tumblingFixture(base) start, end, step := base, base.Add(3*time.Hour), 5*time.Minute @@ -504,16 +516,19 @@ func TestTumblingRangeMetric_LongRangeTwoPhaseKeepsLokiWindows(t *testing.T) { assertTumblingEqual(t, query, lokiTumblingReference(lines, "count_over_time", []string{"pod"}, start, end, step, 5*time.Minute), got) fake.mu.Lock() defer fake.mu.Unlock() - if len(fake.statsCalls) != 2 || !strings.Contains(fake.statsCalls[1].query, `or pod:"")`) || fake.statsCalls[1].offset == "" { - t.Fatalf("expected a top-N phase and an anchored phase 2 keeping unlabelled lines, got %+v", fake.statsCalls) + if len(fake.statsCalls) != 1 || fake.statsCalls[0].offset == "" || fake.rawCalls != 0 { + t.Fatalf("expected one anchored bucket scan, got %+v and %d raw scans", fake.statsCalls, fake.rawCalls) } } -// A grouped count over a field with more values than one bucket response can -// hold keeps the busiest values: a stats_query ranks them, and the bucket query -// restricted to them returns exact series. From two hours on the ranking runs -// first; below, only after the bucket response overflows its byte bound. -func TestShortRangeMetric_TooManySeriesKeepsBusiestValues(t *testing.T) { +// conformance: series-limits-and-partial-results, limits/series-limit-error, limits/drilldown-partial-with-warning +// conformance: loki_api_v1_query_range, loki_api_v1_query +// A grouped count with more series than -max-stats-query-series fails like +// Loki's max_query_series limit, naming the proxy flag. Logs Drilldown gets +// Loki's partial result instead: the busiest series, exact, and a warning. When +// the bucket response overflows its byte bound, a stats_query ranks the values +// and the bucket query restricted to them returns those series. +func TestShortRangeMetric_TooManySeriesFollowsLokiLimit(t *testing.T) { base := time.Unix(1700000400, 0).UTC() var lines []tumblingLine for i := 0; i < 400; i++ { @@ -531,14 +546,58 @@ func TestShortRangeMetric_TooManySeriesKeepsBusiestValues(t *testing.T) { } return sum } + t.Run("instant query", func(t *testing.T) { + srv, fake := newTumblingFakeVL(t, lines) + p := newSlidingTestProxy(t, srv.URL) + p.maxStatsQuerySeries = 150 + query := `sum by (pod) (count_over_time({app="top"}[5m]))` + params := url.Values{"query": {query}, "time": {strconv.FormatInt(base.Add(10*time.Minute).Unix(), 10)}} + + rec := serveTumblingQuery(p, "/loki/api/v1/query", params, nil) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "maximum number of series (150) reached for a single query") { + t.Fatalf("expected Loki's series limit error, got %d: %s", rec.Code, rec.Body.String()) + } + + rec = serveTumblingQuery(p, "/loki/api/v1/query", params, http.Header{"X-Query-Tags": {"Source=grafana-lokiexplore-app"}}) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"warnings":["maximum number of series (150) reached for a single query; returning partial results"]`) { + t.Fatalf("Drilldown: expected a partial vector with Loki's warning, got %d: %s", rec.Code, rec.Body.String()) + } + got := decodeTumblingSeries(t, query, rec.Body.Bytes()) + if len(got) != 150 { + t.Fatalf("Drilldown: expected 150 series, got %d", len(got)) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if fake.rawCalls != 0 { + t.Fatalf("the series limit must not fall back to raw scans, got %d", fake.rawCalls) + } + }) + + t.Run("range equals step", func(t *testing.T) { + srv, fake := newTumblingFakeVL(t, lines) + p := newSlidingTestProxy(t, srv.URL) + p.maxStatsQuerySeries = 150 + query := `sum by (pod) (count_over_time({app="top"}[5m]))` + rec := serveTumblingQuery(p, "/loki/api/v1/query_range", tumblingRangeParams(query, base.Add(5*time.Minute), base.Add(15*time.Minute), 5*time.Minute), nil) + // Loki's message, word for word: no proxy-internal text reaches Grafana. + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "maximum number of series (150) reached for a single query; consider reducing query cardinality") || strings.Contains(rec.Body.String(), "-max-stats-query-series") { + t.Fatalf("expected Loki's series limit error, got %d: %s", rec.Code, rec.Body.String()) + } + fake.mu.Lock() + defer fake.mu.Unlock() + if fake.rawCalls != 0 { + t.Fatalf("the series limit must not fall back to raw scans, got %d", fake.rawCalls) + } + }) for _, tc := range []struct { name string span time.Duration limit int64 wantCalls int + wantIn bool }{ - {"overflow below two hours", 15 * time.Minute, 40 << 10, 2}, - {"ranked first over hours", 3 * time.Hour, 64 << 20, 1}, + {"response overflow ranks values", 15 * time.Minute, 40 << 10, 2, true}, + {"series over the limit over hours", 3 * time.Hour, 64 << 20, 1, false}, } { t.Run(tc.name, func(t *testing.T) { old := maxStatsBucketResponseBytes @@ -550,7 +609,25 @@ func TestShortRangeMetric_TooManySeriesKeepsBusiestValues(t *testing.T) { srv, fake := newTumblingFakeVL(t, lines) p := newSlidingTestProxy(t, srv.URL) p.maxStatsQuerySeries = 150 // -max-stats-query-series - got := runTumblingQuery(t, p, "/loki/api/v1/query_range", tumblingRangeParams(query, start, end, step)) + params := tumblingRangeParams(query, start, end, step) + + rec := serveTumblingQuery(p, "/loki/api/v1/query_range", params, nil) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "maximum number of series (150) reached for a single query; consider reducing query cardinality") || strings.Contains(rec.Body.String(), "-max-stats-query-series") { + t.Fatalf("expected Loki's series limit error, got %d: %s", rec.Code, rec.Body.String()) + } + fake.mu.Lock() + rawBefore := fake.rawCalls + fake.statsCalls = nil + fake.mu.Unlock() + if rawBefore != 0 { + t.Fatalf("the series limit must not fall back to raw scans, got %d", rawBefore) + } + + rec = serveTumblingQuery(p, "/loki/api/v1/query_range", params, http.Header{"X-Query-Tags": {"Source=grafana-lokiexplore-app"}}) + if rec.Code != http.StatusOK || !strings.Contains(rec.Body.String(), `"warnings":["maximum number of series (150) reached for a single query; returning partial results"]`) { + t.Fatalf("Drilldown: expected a partial result with Loki's warning, got %d: %s", rec.Code, rec.Body.String()) + } + got := decodeTumblingSeries(t, query, rec.Body.Bytes()) want := lokiTumblingReference(lines, "count_over_time", []string{"pod"}, start, end, step, time.Minute) if want := p.resolvedMaxStatsQuerySeries(); len(got) != want { t.Fatalf("expected the %d busiest series (-max-stats-query-series), got %d", want, len(got)) @@ -573,19 +650,18 @@ func TestShortRangeMetric_TooManySeriesKeepsBusiestValues(t *testing.T) { fake.mu.Lock() defer fake.mu.Unlock() last := fake.statsCalls[len(fake.statsCalls)-1] - if fake.rawCalls != 0 || len(fake.statsCalls) != tc.wantCalls || !strings.Contains(last.query, "pod:in(") { + if fake.rawCalls != 0 || len(fake.statsCalls) != tc.wantCalls || strings.Contains(last.query, "pod:in(") != tc.wantIn { t.Fatalf("expected %d bucket calls ending with the top-value one and no raw scans, got %d raw scans and %d calls", tc.wantCalls, fake.rawCalls, len(fake.statsCalls)) } }) } } -// A backend without the stats_query_range offset arg (or with an unknown -// version, as while the startup probe is pending) has only epoch-aligned -// buckets. They serve the tumbling relabel when the start is aligned to the -// range; any other start would count lines of the wrong windows, so the exact -// evaluator answers. Lines sit off window edges, as epoch buckets are -// [T, T+range) while Loki's windows are (T-range, T]. +// A backend known to be without the stats_query_range offset arg has only +// epoch-aligned buckets. They serve the tumbling relabel when the start is +// aligned to the range; any other start would count lines of the wrong windows, +// so the window evaluator answers. Lines sit off window edges, as epoch buckets +// are [T, T+range) while Loki's windows are (T-range, T]. func TestTumblingRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { base := time.Unix(1700000400, 0).UTC() lines := tumblingFixture(base.Add(2500 * time.Millisecond)) @@ -596,6 +672,7 @@ func TestTumblingRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { start, end := base.Add(5*time.Minute+offset), base.Add(15*time.Minute+offset) srv, fake := newTumblingFakeVL(t, lines) p := newGapTestProxy(t, srv.URL) + p.storeBackendVersion("v1.44.0", "v1.44.0") got := runTumblingQuery(t, p, "/loki/api/v1/query_range", tumblingRangeParams(query, start, end, step)) assertTumblingEqual(t, query, lokiTumblingReference(lines, "count_over_time", []string{"pod"}, start, end, step, 5*time.Minute), got) fake.mu.Lock() @@ -607,7 +684,7 @@ func TestTumblingRangeMetric_BackendWithoutOffsetSupport(t *testing.T) { return } if len(fake.statsCalls) != 0 || fake.rawCalls != 1 { - t.Fatalf("unaligned start: expected the exact evaluator, got %+v and %d raw scans", fake.statsCalls, fake.rawCalls) + t.Fatalf("unaligned start: expected the window evaluator, got %+v and %d raw scans", fake.statsCalls, fake.rawCalls) } }) } @@ -779,3 +856,29 @@ func TestStatsMetricLabels_EmptyGroupValuesAreOmitted(t *testing.T) { assertTumblingEqual(t, query, want, got) }) } + +// conformance: window-bounds-and-step-alignment, semantics/offset-sample-timestamps +// conformance: loki_api_v1_query_range, loki_api_v1_query +// Loki evaluates `[5m] offset 5m` at T over (T-10m, T-5m] and stamps the sample +// at T, on range and instant queries alike. +func TestTumblingRangeMetric_OffsetStampsEvaluationTime(t *testing.T) { + base := time.Unix(1700000400, 0).UTC() + lines := tumblingFixture(base) + offset := 5 * time.Minute + shifted := make([]tumblingLine, len(lines)) // offset windows as plain windows + for i, line := range lines { + shifted[i] = line + shifted[i].ts += int64(offset) + } + query := `sum by (pod) (count_over_time({app="tumble"}[5m] offset 5m))` + start, end, step := base.Add(10*time.Minute), base.Add(25*time.Minute), 5*time.Minute + + srv, _ := newTumblingFakeVL(t, lines) + p := newSlidingTestProxy(t, srv.URL) + got := runTumblingQuery(t, p, "/loki/api/v1/query_range", tumblingRangeParams(query, start, end, step)) + assertTumblingEqual(t, query, lokiTumblingReference(shifted, "count_over_time", []string{"pod"}, start, end, step, 5*time.Minute), got) + + at := base.Add(17 * time.Minute) + got = runTumblingQuery(t, p, "/loki/api/v1/query", url.Values{"query": {query}, "time": {strconv.FormatInt(at.Unix(), 10)}}) + assertTumblingEqual(t, query+"@instant", lokiTumblingReference(shifted, "count_over_time", []string{"pod"}, at, at, time.Minute, 5*time.Minute), got) +} diff --git a/test/e2e-compat/docker-compose.yml b/test/e2e-compat/docker-compose.yml index 54b5f78d1..dc0dafb38 100644 --- a/test/e2e-compat/docker-compose.yml +++ b/test/e2e-compat/docker-compose.yml @@ -267,6 +267,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -335,6 +340,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -395,6 +405,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -455,6 +470,12 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Left at the built-in default (500, Loki's own default + # max_query_series) so the series-limit behaviour — Loki's error for a + # plain client, a partial result with a warning for Logs Drilldown — is + # exercised by the e2e suite. Every other proxy here matches this + # stack's Loki (max_query_series: 1000000) so parity tests compare + # answers rather than two different limits. - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -543,6 +564,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -586,6 +612,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -624,6 +655,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -662,6 +698,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=info" @@ -700,6 +741,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=debug" @@ -737,6 +783,11 @@ services: command: - "-listen=:3100" - "-backend=http://victorialogs:9428" + # Match this stack's Loki (max_query_series: 1000000 in + # loki-local-config.yaml): a query over the limit fails on both + # sides at the same cardinality, so parity tests compare answers + # rather than two different limits. + - "-max-stats-query-series=1000000" - "-ruler-backend=http://vmalert:8880" - "-alerts-backend=http://vmalert:8880" - "-log-level=debug" diff --git a/test/e2e-compat/range_metric_offset_series_limit_test.go b/test/e2e-compat/range_metric_offset_series_limit_test.go new file mode 100644 index 000000000..d03e0c228 --- /dev/null +++ b/test/e2e-compat/range_metric_offset_series_limit_test.go @@ -0,0 +1,256 @@ +//go:build e2e + +package e2e_compat + +import ( + "encoding/json" + "fmt" + "net/http" + "net/url" + "regexp" + "sort" + "strconv" + "strings" + "testing" + "time" +) + +// conformance: series-limits-and-partial-results, window-bounds-and-step-alignment, +// conformance: limits/series-limit-error, limits/drilldown-partial-with-warning, semantics/offset-sample-timestamps +// conformance: loki_api_v1_query_range, loki_api_v1_query +// TestRangeMetricCompatibilityOffsetAndSeriesLimit compares Loki and the proxy +// for `offset` (samples stay stamped at the evaluation time, not the shifted +// one) and for a grouping with many series. Over its series limit Loki fails +// the query with its max_query_series error and gives Logs Drilldown a partial +// result with a warning; the proxy does the same with -max-stats-query-series. +// The stack raises both limits well above this fixture, so the limit subtests +// run only where the proxy is configured below the fixture's cardinality (the +// default 500) and otherwise assert plain parity with Loki. +func TestRangeMetricCompatibilityOffsetAndSeriesLimit(t *testing.T) { + app := fmt.Sprintf("e2e-align-limit-%d", time.Now().UnixNano()) + base := time.Now().UTC().Truncate(10 * time.Minute).Add(-80 * time.Minute) + const span = time.Hour + const widePods = 520 + + type line struct { + ts time.Time + labels map[string]string + msg string + } + var lines []line + for _, s := range []struct { + pod string + every time.Duration + }{{"a", 10 * time.Second}, {"b", 25 * time.Second}} { + for i := 0; time.Duration(i)*s.every < span; i++ { + lines = append(lines, line{base.Add(time.Duration(i) * s.every), map[string]string{"app": app, "kind": "tick", "pod": s.pod}, "tick " + strconv.Itoa(i)}) + } + } + // One line per pod, 30s before a 5m mark, so every [1m] window at step 300 + // holds a sixth of the pods. + for i := 0; i < widePods; i++ { + pod := fmt.Sprintf("w%03d", i) + lines = append(lines, line{base.Add(time.Duration(i%6+1)*5*time.Minute - 30*time.Second), map[string]string{"app": app, "kind": "wide", "pod": pod}, "wide " + pod}) + } + + var vlRows strings.Builder + lokiStreams := map[string]map[string]any{} + for _, l := range lines { + fields := map[string]string{"_time": l.ts.Format(time.RFC3339Nano), "_msg": l.msg} + for k, v := range l.labels { + fields[k] = v + } + row, _ := json.Marshal(fields) + vlRows.Write(row) + vlRows.WriteByte('\n') + key := l.labels["kind"] + "/" + l.labels["pod"] + if lokiStreams[key] == nil { + lokiStreams[key] = map[string]any{"stream": l.labels, "values": [][]string{}} + } + lokiStreams[key]["values"] = append(lokiStreams[key]["values"].([][]string), []string{strconv.FormatInt(l.ts.UnixNano(), 10), l.msg}) + } + status, body := hardeningRequest(t, http.MethodPost, vlURL+"/insert/jsonline?_stream_fields=app,kind,pod", vlRows.String(), map[string]string{"Content-Type": "application/stream+json"}) + if status != http.StatusOK { + t.Fatalf("VL ingest: %d %s", status, body) + } + streams := make([]any, 0, len(lokiStreams)) + for _, s := range lokiStreams { + streams = append(streams, s) + } + payload, _ := json.Marshal(map[string]any{"streams": streams}) + status, body = hardeningRequest(t, http.MethodPost, lokiURL+"/loki/api/v1/push", string(payload), map[string]string{"Content-Type": "application/json", "X-Scope-OrgID": "0"}) + if status != http.StatusNoContent { + t.Fatalf("Loki ingest: %d %s", status, body) + } + forceVLFlush(t) + + tick := `{app="` + app + `", kind="tick"}` + wide := `{app="` + app + `", kind="wide"}` + waitForFixtureOnBothBackends(t, fmt.Sprintf("app:=%q", app), `{app="`+app+`"}`, base.Add(-time.Minute), base.Add(span), len(lines)) + waitForAlignLimitLokiMetrics(t, wide, base, widePods) + + t.Run("offset", func(t *testing.T) { + query := `sum by (pod) (count_over_time(` + tick + `[5m] offset 5m))` + start, end := base.Add(15*time.Minute), base.Add(45*time.Minute) + loki := slidingRangeSeries(t, lokiURL, query, start, end, 5*time.Minute, nil) + assertSlidingParity(t, query, loki, slidingRangeSeries(t, proxyURL, query, start, end, 5*time.Minute, nil)) + at := base.Add(32 * time.Minute) + lokiInstant := tumblingShortInstant(t, lokiURL, query, at) + if len(lokiInstant) != 2 { + t.Fatalf("Loki returned %d instant series, want 2: %v", len(lokiInstant), lokiInstant) + } + if proxyInstant := tumblingShortInstant(t, proxyURL, query, at); fmt.Sprint(proxyInstant) != fmt.Sprint(lokiInstant) { + t.Fatalf("instant %s: proxy %v, Loki %v", query, proxyInstant, lokiInstant) + } + }) + + // The series-limit subtests run against the proxy that keeps the built-in + // default (500): every other proxy in the stack matches its Loki, whose limit + // is far above this fixture. + limitProxyURL := envOr("PROXY_SERIES_LIMIT_URL", patternsAutodetectProxyURL) + start, end := base, base.Add(30*time.Minute) + limit := 0 // the proxy's -max-stats-query-series, read from its own answer + for _, window := range []string{"5m", "1m"} { + query := `sum by (pod) (count_over_time(` + wide + `[` + window + `]))` + t.Run("series limit/"+query, func(t *testing.T) { + loki := slidingRangeSeries(t, lokiURL, query, start, end, 5*time.Minute, nil) + if len(loki) != widePods { + t.Fatalf("Loki returned %d series, want %d", len(loki), widePods) + } + status, body, _ := alignLimitRangeQuery(t, limitProxyURL, query, start, end, nil) + if status == http.StatusOK { + // Configured above this fixture (the compose stack matches its + // Loki): the answer must simply be Loki's. + assertSlidingParity(t, query, loki, slidingRangeSeries(t, limitProxyURL, query, start, end, 5*time.Minute, nil)) + return + } + n := seriesLimitFromError(t, body) + if n >= widePods { + t.Fatalf("limit %d is not below the fixture's %d series, yet the query failed: %s", n, widePods, body) + } + limit = n + // Loki's message, word for word: Grafana renders it, so no + // proxy-internal text may appear in it. + if !strings.Contains(body, "consider reducing query cardinality by adding more specific stream selectors") || strings.Contains(body, "-max-stats-query-series") { + t.Fatalf("expected Loki's series limit message, got %s", body) + } + }) + } + + // Drilldown gets the partial result: at most the limit, every kept series + // exactly as Loki computes it, and Loki's warning. + t.Run("series limit/drilldown partial result", func(t *testing.T) { + if limit == 0 { + t.Skip("proxy series limit is above this fixture's cardinality") + } + query := `sum by (pod) (count_over_time(` + wide + `[1m]))` + loki := slidingRangeSeries(t, lokiURL, query, start, end, 5*time.Minute, nil) + status, body, resp := alignLimitRangeQuery(t, limitProxyURL, query, start, end, map[string]string{"X-Query-Tags": "Source=grafana-lokiexplore-app"}) + if status != http.StatusOK || resp["status"] != "success" { + t.Fatalf("Drilldown: expected 200, got %d %s", status, body) + } + warnings, _ := resp["warnings"].([]interface{}) + wantWarning := fmt.Sprintf("maximum number of series (%d) reached for a single query; returning partial results", limit) + if len(warnings) != 1 || warnings[0] != wantWarning { + t.Fatalf("Drilldown: expected Loki's partial result warning, got %v", resp["warnings"]) + } + proxy := alignLimitMatrix(t, resp) + if len(proxy) != limit { + t.Fatalf("Drilldown: expected %d series, got %d", limit, len(proxy)) + } + for key, got := range proxy { + want, ok := loki[key] + if !ok || fmt.Sprint(sortedPoints(got)) != fmt.Sprint(sortedPoints(want)) { + t.Errorf("Drilldown {%s}: proxy %v, Loki %v", key, sortedPoints(got), sortedPoints(want)) + } + } + }) +} + +// seriesLimitFromError reads N out of Loki's "maximum number of series (N) +// reached for a single query" error, failing the test on any other body. +func seriesLimitFromError(t *testing.T, body string) int { + t.Helper() + m := regexp.MustCompile(`maximum number of series \((\d+)\) reached for a single query`).FindStringSubmatch(body) + if m == nil { + t.Fatalf("expected Loki's series limit error, got %s", body) + } + n, err := strconv.Atoi(m[1]) + if err != nil { + t.Fatalf("series limit %q: %v", m[1], err) + } + return n +} + +func alignLimitRangeQuery(t *testing.T, baseURL, query string, start, end time.Time, headers map[string]string) (int, string, map[string]interface{}) { + t.Helper() + params := url.Values{ + "query": {query}, + "start": {strconv.FormatInt(start.UnixNano(), 10)}, + "end": {strconv.FormatInt(end.UnixNano(), 10)}, + "step": {"300"}, + } + all := map[string]string{"X-Scope-OrgID": "0"} + for k, v := range headers { + all[k] = v + } + return doJSONGET(t, baseURL+"/loki/api/v1/query_range?"+params.Encode(), all) +} + +func alignLimitMatrix(t *testing.T, resp map[string]interface{}) map[string]map[string]string { + t.Helper() + out := map[string]map[string]string{} + for _, item := range extractArray(extractMap(resp, "data"), "result") { + series, _ := item.(map[string]interface{}) + metric, _ := series["metric"].(map[string]interface{}) + keys := make([]string, 0, len(metric)) + for k, v := range metric { + keys = append(keys, fmt.Sprintf("%s=%v", k, v)) + } + sort.Strings(keys) + points := map[string]string{} + values, _ := series["values"].([]interface{}) + for _, raw := range values { + pair, _ := raw.([]interface{}) + if len(pair) == 2 { + ts, _ := pair[0].(float64) + points[strconv.FormatFloat(ts, 'f', -1, 64)] = fmt.Sprint(pair[1]) + } + } + out[strings.Join(keys, ",")] = points + } + return out +} + +func sortedPoints(points map[string]string) []string { + out := make([]string, 0, len(points)) + for ts, v := range points { + out = append(out, ts+"="+v) + } + sort.Strings(out) + return out +} + +// waitForAlignLimitLokiMetrics waits out Loki's fresh-stream metric blank window +// with a query shape no subtest compares, so no partial answer lands in Loki's +// results cache. +func waitForAlignLimitLokiMetrics(t *testing.T, selector string, base time.Time, series int) { + t.Helper() + params := url.Values{ + "query": {`count(count_over_time(` + selector + `[40m]))`}, + "start": {strconv.FormatInt(base.Add(40*time.Minute).Unix(), 10)}, + "end": {strconv.FormatInt(base.Add(40*time.Minute).Unix(), 10)}, + "step": {"600"}, // base+40m is on this grid, so Loki's alignment keeps the time + } + deadline := time.Now().Add(3 * time.Minute) + var last string + for time.Now().Before(deadline) { + status, body, resp := doJSONGET(t, lokiURL+"/loki/api/v1/query_range?"+params.Encode(), map[string]string{"X-Scope-OrgID": "0"}) + last = body + if status == http.StatusOK && strings.Contains(body, `"`+strconv.Itoa(series)+`"`) && resp["warnings"] == nil { + return + } + time.Sleep(time.Second) + } + t.Fatalf("Loki range metrics incomplete after 3m: %s", last) +} diff --git a/test/e2e-compat/raw_metric_limits_test.go b/test/e2e-compat/raw_metric_limits_test.go index 423d266df..c78023aae 100644 --- a/test/e2e-compat/raw_metric_limits_test.go +++ b/test/e2e-compat/raw_metric_limits_test.go @@ -55,7 +55,7 @@ func TestHardeningLive_RawMetricLimits(t *testing.T) { t.Run(mode, func(t *testing.T) { params := url.Values{"query": {`avg_over_time({app="` + app + `"}|json|unwrap value[5m])`}, "time": {evaluation.Format(time.RFC3339Nano)}, "start": {evaluation.Add(-time.Minute).Format(time.RFC3339Nano)}, "end": {evaluation.Format(time.RFC3339Nano)}, "step": {"60"}} status, body := hardeningRequest(t, http.MethodGet, server.URL+"/loki/api/v1/"+mode+"?"+params.Encode(), "", nil) - if status < 400 || !strings.Contains(string(body), "maximum metric series exceeded (2)") || strings.Contains(string(body), `"result"`) { + if status < 400 || !strings.Contains(string(body), "maximum number of series (2) reached") || strings.Contains(string(body), `"result"`) { t.Fatalf("overflow must be visible without partial data: %d %s", status, body) } params.Set("query", `avg_over_time({app="`+app+`",kind="0"}|json|unwrap value[5m])`) From 9f636cae1ea21c487641e86ce4fdaa9260369165 Mon Sep 17 00:00:00 2001 From: szibis Date: Wed, 23 Sep 2026 11:04:48 +0200 Subject: [PATCH 2/2] fix: size the partial-result warning response without length arithmetic CodeQL flagged the capacity passed to make() in addSeriesLimitWarning: len(body)+len(warning)+14 has no overflow guard. The body is bounded by the response caps long before that matters, but bytes.Join sizes the result once and checks the sum itself, so the response is still copied a single time and no unchecked addition is left here. Add a table test for the injected warning; it passes against both implementations. --- internal/proxy/manual_metric_budget.go | 10 ++++---- internal/proxy/manual_metric_budget_test.go | 28 +++++++++++++++++++++ 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/internal/proxy/manual_metric_budget.go b/internal/proxy/manual_metric_budget.go index 5be227126..539d5c818 100644 --- a/internal/proxy/manual_metric_budget.go +++ b/internal/proxy/manual_metric_budget.go @@ -266,13 +266,13 @@ func addSeriesLimitWarning(body []byte, scope *seriesLimitScope) []byte { return body } warning, _ := json.Marshal([]string{fmt.Sprintf("maximum number of series (%d) reached for a single query; returning partial results", limit)}) - out := make([]byte, 0, len(body)+len(warning)+14) - out = append(out, `{"warnings":`...) - out = append(out, warning...) + separator := []byte{} if len(body) > 2 { - out = append(out, ',') + separator = []byte{','} } - return append(out, body[1:]...) + // bytes.Join sizes the result once and checks the sum for overflow, so the + // response is not copied twice and no length arithmetic happens here. + return bytes.Join([][]byte{[]byte(`{"warnings":`), warning, separator, body[1:]}, nil) } // hasTopLevelWarnings reports whether the response object already carries a diff --git a/internal/proxy/manual_metric_budget_test.go b/internal/proxy/manual_metric_budget_test.go index dfee43a9d..60b53b218 100644 --- a/internal/proxy/manual_metric_budget_test.go +++ b/internal/proxy/manual_metric_budget_test.go @@ -136,3 +136,31 @@ func TestBareParserRawSampleWeightAppliesUnwrapConversion(t *testing.T) { }) } } + +func TestAddSeriesLimitWarning(t *testing.T) { + const want500 = `{"warnings":["maximum number of series (500) reached for a single query; returning partial results"]` + truncated := &seriesLimitScope{drilldown: true} + truncated.truncated.Store(500) + + cases := []struct { + name string + body string + scope *seriesLimitScope + want string + }{ + {"object with fields", `{"status":"success","data":{}}`, truncated, want500 + `,"status":"success","data":{}}`}, + {"empty object", `{}`, truncated, want500 + `}`}, + {"not truncated", `{"status":"success"}`, &seriesLimitScope{drilldown: true}, `{"status":"success"}`}, + {"no scope", `{"status":"success"}`, nil, `{"status":"success"}`}, + {"already warned", `{"warnings":["x"],"status":"success"}`, truncated, `{"warnings":["x"],"status":"success"}`}, + {"not an object", `[1,2]`, truncated, `[1,2]`}, + {"empty body", ``, truncated, ``}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := string(addSeriesLimitWarning([]byte(tc.body), tc.scope)); got != tc.want { + t.Fatalf("got %s\nwant %s", got, tc.want) + } + }) + } +}