Skip to content

fix: keep parser captures in the series of a metric query - #587

Merged
szibis merged 1 commit into
mainfrom
fix/parser-capture-series-identity
Sep 23, 2026
Merged

szibis merged 1 commit into
mainfrom
fix/parser-capture-series-identity

Conversation

@szibis

@szibis szibis commented Sep 22, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Loki names a metric series with the stream labels plus every label the pipeline extracted. | regexp and | pattern name their captures in the query itself, so the proxy can keep them without reading a single extra row — it did not, and collapsed them into the stream:

query (1 minute of app="worker-service", e2e stack) Loki main this build
count_over_time({…} | regexp "job_id=(?P<jid>\w+)" [1m]) 450 series, jid present 30 series, no jid 450 series, jid present
count_over_time({…} | pattern "<_> job_id=<jid> <_>" [1m]) 450 30 450
count_over_time({…} | regexp … [2m]), range, 10 min 5400 357 5355

The captures now join the stream in the series identity: the stats-bucket route groups by them in VictoriaLogs, the row-evaluated route keeps them as labels, and the /hits route — which groups by a single field — is skipped for those queries. The aggregation stays in the backend.

(The range figures come from a build run with the compat stack's -max-stats-query-series=1000000, the value #581 sets for the compose proxies; the code default stays 500, which is Loki's own default. With 500 the same query is capped as before.)

The | json / | logfmt half, and a new opt-in flag

Their key set is known only once a line is read, so matching Loki's identity means evaluating the query from rows instead of pushing it down to VictoriaLogs stats. That is off by default:

  • Default: unchanged fast paths, and the difference is now written down in docs/KNOWN_ISSUES.md with the measured numbers (| logfmt: Loki 450 series, proxy 30; | json: Loki 1050 on a busier app).
  • -exact-parser-series-identity=true (Helm extraArgs.exact-parser-series-identity, docs row in configuration.md) names those series with the parsed labels, as Loki does, and routes them to the row evaluator bounded by the manual row budget. Measured on the stack: count_over_time({app="worker-service"} | logfmt [1m]) returns Loki's 450 series with the flag on, 69 without.

Verification

  • go test ./internal/... ./cmd/..., go vet, gofmt and golangci-lint (v2.13.2, Go 1.27.1) are clean. New unit tests cover the capture extraction (| regexp named and unnamed groups, | pattern placeholders, | json/| logfmt, unparsable queries), the emitted grouping, and the bucket route turning capture fields into labels.
  • New e2e-compat test TestCompat_ParserCaptureSeriesIdentity (in the core shard) pushes three job ids to Loki and VictoriaLogs and compares | regexp, | pattern and bytes_over_time series with Loki's own answers. It passes on this build and fails on main.
  • Three existing sliding-window parity cases used | regexp "(?P<m>.)"/| pattern "<m>" as stand-ins for "a parser"; with captures in the identity their fake backend no longer models the grouping, so they now use | logfmt, which is what they were testing (route selection), and the capture behaviour has its own tests.

Grouping sweep (15 shapes, instant and range, 15m/1h/6h/24h)

VictoriaLogs RestartCount 3 before and 3 after.

range queries exact (main) exact (this build) labels match (main) labels (this build)
15m 46 36 36 38 38
1h 46 29 28 31 30
6h 46 24 24 26 26
24h 46 15 14 27 26

None of the sweep's shapes uses | regexp or | pattern, so the two builds should be equal, and they are except for topk(3, …), where places 3-5 are tied and membership is arbitrary on both sides.

Backend safety

12 concurrent 24h/7d queries for 120s against this build with the code defaults: VictoriaLogs restart count 3 before and 3 after, /labels probe 200 on all 120 samples, 740 × 200 and 105 × 400 (Loki-parity query errors), peak VictoriaLogs memory 2812 MiB. PASS.

One finding for #581: the same run with -max-stats-query-series=1000000 fails, because sum by (pod) (rate({service_name="api-gateway"}[5m])) over 24h/7d then exceeds the 64 MB manual response cap and returns manual metric response exceeds 67108864 bytes, which names no operator flag. Raising the series cap on the compose proxies without raising (or naming) that byte cap turns the old truncation into a hard 502 on wide by (pod) queries.

Conformance registry

Item Kind State Served by Tests declaring it
loki_api_v1_query endpoint proven hybrid 1
loki_api_v1_query_range endpoint proven hybrid 1
parsed-label-series-identity behaviour partial — 3

parsed-label-series-identity is new: it carries the measured numbers, records the | json / | logfmt deviation as an owner decision with the waiver parsed-identity-dynamic-keys (expiry 2027-03-31) and names -exact-parser-series-identity as the opt-in. python3 scripts/ci/check_conformance.py passes.

Flag documentation

-exact-parser-series-identity is in docs/configuration.md, docs/KNOWN_ISSUES.md, examples/loki-vl-proxy-full.env (flag-only list), examples/k8s/proxy-config-configmap.yaml (commented arg), charts/loki-vl-proxy/README.md and charts/loki-vl-proxy/values.yaml (commented example only — the chart does not pin false defaults). scripts/ci/validate_helm_flags.sh passes. docs/logql-parser.md and docs/translation-reference.md describe the series identity and link the deviation instead of repeating it, and docs/real-window-compatibility-gaps.md lists the | regexp / | pattern change as a correction that returns more series than before.

Sizing guidance: -max-stats-query-series and capture cardinality

Keeping | regexp and | pattern captures in the series identity multiplies a
query's series count by the number of distinct captured values, so the default
-max-stats-query-series of 500 is reached sooner than before on
capture-bearing queries. This is the intended Loki behaviour — Loki returns one
series per captured value — but it changes the sizing arithmetic:

  • Before, count_over_time({app="x"} | regexp "…(?P<jid>…)" [1m]) cost roughly
    one series per stream.
  • Now it costs streams × distinct capture values. A capture over a
    request id, job id or trace id is effectively unbounded and will hit the cap.

Measured on the e2e stack (15m, step 15s,
count_over_time({app="api-gateway"} | regexp "(?P<mth>GET|POST|PUT)" [5m])):
Loki returns 6 927 series; at the default cap both this build and main return
maximum metric series exceeded (500). The cap bites identically on both, since
the generator's rotating pod names already exceed 500 streams — but with
captures it will bite at a proportionally smaller stream count.

Operator guidance: size -max-stats-query-series as
expected streams × expected distinct capture values, and prefer captures with
bounded value sets (HTTP method, status class, route template) over unbounded
ones (ids) in metric queries. The limit is per-query and configurable via the
flag and extraArgs.max-stats-query-series; hitting it returns a Loki-style
error naming the flag rather than silently truncating. Raising it on wide
by (pod) queries should be done together with the manual response byte cap
noted in the backend-safety section above, or the truncation becomes a 502.

@github-actions github-actions Bot added size/L Large change scope/proxy Proxy core scope/helm Helm chart scope/docs Documentation scope/cli CLI flags scope/tests Tests bugfix Bug fix labels Sep 22, 2026
@github-actions

github-actions Bot commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

PR Quality Report

Compared against base branch main.

Coverage and tests

Signal Base PR Delta
Test count 7400 7406 6
Coverage 89.9% 89.9% 0.0% (stable)

Compatibility

Track Base PR Delta
Loki API 100.0% 11/11 (100.0%) 0.0% (stable)
Logs Drilldown 100.0% 17/17 (100.0%) 0.0% (stable)
VictoriaLogs 100.0% 11/11 (100.0%) 0.0% (stable)

Performance smoke

Lower CPU cost (ns/op) is better. Lower benchmark memory cost (B/op, allocs/op) is better. Higher throughput is better. Lower load-test memory growth is better. Benchmark rows are medians from repeated samples.

Signal Base PR Delta
QueryRange cache-hit CPU cost 3242.0 ns/op 3329.0 ns/op +2.7% (stable)
QueryRange cache-hit memory 1240.0 B/op 1240.0 B/op 0.0% (stable)
QueryRange cache-hit allocations 27.0 allocs/op 27.0 allocs/op 0.0% (stable)
QueryRange cache-bypass CPU cost 3929.0 ns/op 3936.0 ns/op +0.2% (stable)
QueryRange cache-bypass memory 1298.0 B/op 1298.0 B/op 0.0% (stable)
QueryRange cache-bypass allocations 27.0 allocs/op 27.0 allocs/op 0.0% (stable)
Labels cache-hit CPU cost 833.2 ns/op 855.6 ns/op +2.7% (stable)
Labels cache-hit memory 48.0 B/op 48.0 B/op 0.0% (stable)
Labels cache-hit allocations 3.0 allocs/op 3.0 allocs/op 0.0% (stable)
Labels cache-bypass CPU cost 1445.0 ns/op 1391.0 ns/op -3.7% (stable)
Labels cache-bypass memory 392.0 B/op 392.0 B/op 0.0% (stable)
Labels cache-bypass allocations 14.0 allocs/op 14.0 allocs/op 0.0% (stable)

State

  • Coverage, compatibility, and sampled performance are reported here from the same PR workflow.
  • This is a delta report, not a release gate by itself. Required checks still decide merge safety.
  • Performance is a smoke comparison, not a full benchmark lab run.
  • Delta states use the same noise guards as the quality gate (percent + absolute + low-baseline checks), so report labels match merge-gate behavior.

@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from ac3e8d7 to 1c8981b Compare September 22, 2026 10:04
@github-actions github-actions Bot added size/XL Extra large change and removed size/L Large change labels Sep 22, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from 1c8981b to 2dfaa23 Compare September 22, 2026 11:29
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 22, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from 2dfaa23 to 93d5ca3 Compare September 22, 2026 11:42
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 22, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from 93d5ca3 to 44a6532 Compare September 22, 2026 13:29
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 22, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from 44a6532 to f18c576 Compare September 22, 2026 13:52
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 22, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from f18c576 to fe7de25 Compare September 22, 2026 13:59
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 22, 2026
@szibis

szibis commented Sep 22, 2026

Copy link
Copy Markdown
Collaborator Author

Live verification on the shared e2e stack (head fe7de25e)

Run against the compose stack with a proxy built from this branch, on a port whose listener PID was asserted to be the one launched (lsof -t == child PID) before any query — a leftover proxy from another session squats the default admin listener 127.0.0.1:3101, which makes a second proxy exit at startup while the squatter keeps answering, so an unverified port produces confident, reproducible and wrong results.

Range matrix, proxy vs Loki direct

40 checks: 8 query shapes × 15m / 1h / 6h / 24h, range and instant, tenant 0.

p587: label sets match Loki on 38/40 checks
VL restarts before=0 after=0

The two differences are the same query at 6h and 24h — sum by (service_name) (count_over_time({namespace="prod"}[5m])), 21 series from Loki and 22 from the proxy. The extra series is service_name="otel-api-service", and it is a fixture asymmetry, not a translation difference:

{namespace="prod", service_name="otel-api-service"} over 24h
  loki : 0 streams, 0 entries
  proxy: 1 stream,  5 entries
Loki /series index for {namespace="prod"}: otel-api-service absent

VictoriaLogs holds rows Loki never received, so the proxy reports one service more. Every other label set and series count matches Loki exactly, including by (detected_level), by (level, detected_level), topk, rate and bytes_over_time.

Backend safety under concurrent long-range load

16 workers, 5 minutes, 24h and 7d windows, parser-heavy shapes, with the stack proxy's own rate-limit settings (-rate-limit-per-second=0 -rate-limit-burst=0):

VL restart count before=0 after=0
load responses by status: {400: 10305, 200: 72087}
/labels probe by status: {200: 51}
peak VictoriaLogs container memory: 3679 MiB
RESULT: PASS

No 502, no 503, /labels answering for the whole run, and the VictoriaLogs restart count unchanged. The 400s are Loki-parity pipeline errors on non-JSON lines.

@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 23, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from b755d99 to 5d28695 Compare September 23, 2026 09:18
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 23, 2026
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from 5d28695 to fe5a577 Compare September 23, 2026 09:45
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 23, 2026
Loki names a metric series with the stream labels plus every label the
pipeline extracted, so count_over_time({app="x"} | regexp
"job_id=(?P<jid>\w+)" [1m]) returns one series per captured value (450 on
the e2e generator) where the proxy returned one per stream (30), without
the label. `| pattern` had the same gap.

Both parsers name their captures in the query, so the captures now join
the stream in the series identity: the stats-bucket route groups by them
in VictoriaLogs, the row-evaluated route keeps them as labels, and the
/hits route, which groups by one field, is skipped for such queries. No
extra backend work: the grouping stays inside VictoriaLogs.

The keys of | json and | logfmt are known only once a line is read, so
their labels can only join the identity by evaluating the query from
rows. That stays off by default and is documented as the remaining
deviation; -exact-parser-series-identity opts in per deployment.
@szibis
szibis force-pushed the fix/parser-capture-series-identity branch from fe5a577 to 1e8857d Compare September 23, 2026 10:07
@github-actions github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Sep 23, 2026
@szibis
szibis merged commit 6a55974 into main Sep 23, 2026
56 checks passed
@szibis
szibis deleted the fix/parser-capture-series-identity branch September 23, 2026 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Bug fix scope/cli CLI flags scope/docs Documentation scope/helm Helm chart scope/proxy Proxy core scope/tests Tests size/XL Extra large change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant