fix: a division or modulo by zero is NaN, like Loki by szibis · Pull Request #745 · ReliablyObserve/loki-vl-proxy

szibis · 2026-10-12T11:45:56Z

Summary

New gap: semantics/binary-division-by-zero-nan.

Loki's rule. In Loki 3.7.7, syntax.MergeBinOp (pkg/logql/syntax/ast.go:1978-2005) returns NaN for a zero right operand of / and %, whatever the numerator. This applies:

in literal folding: reduceBinOp at ast.go:1918, so 1 / 0 is NaN;
in LiteralStepEvaluator: evaluator.go:1179;
in vectorBinop: evaluator.go:1020.

What the proxy did.

binarySampleValue computed a / b, so x / 0, -1 / 0 and Inf / 0 gave ±Inf. Only 0 / 0 gave NaN.
The fallback applyOp returned 0 for both / and %.

Fix. Both now return NaN for / and % when the divisor is 0 or -0. Comparisons (with and without bool), ^, and NaN/Inf operands already matched MergeBinOp; the edge table now pins them.

Tests

TestBinarySampleValueMatchesLokiMergeBinOp: 12 operators × 9 operands squared (0, -0, ±1, 2.5, -7, NaN, ±Inf), with and without bool, against a direct port of MergeBinOp.
TestBinaryMatchersMatchLokiMergeBinOp: the same table through the scalar-right, scalar-left and vector-vector matchers.
TestBinaryScalarFoldMatchesLokiReduceBinOp.
TestApplyOpDivisionAndModuloByZeroAreNaN.
TestBinaryVectorDivisionByZeroHandlers: query_range and query.
TestBinaryConstantVectorDivisionByZeroInstant.
Tests that pinned the old answers now expect NaN. One of them asserted 0, which a NaN satisfied vacuously.
e2e vs Loki: TestCompat_BinaryDivisionByZeroIsNaN, 12 shapes on query and query_range.

Pending before ready

e2e against Loki on VictoriaLogs v1.52.0.
A short control-set A/B.
Expected conflicts:
- fix: binary vector matching raises Loki's error text at Loki's step #742: same file.
- fix: an instant query keeps its failure status and always answers a resultType #735: also makes literal 1/0 NaN; drop the duplicate test row.
- fix: an aggregation over a binary expression aggregates the whole binary, like Loki #741: registers this row as open; keep this fixed version.

…-nan

github-actions · 2026-10-12T11:47:36Z

⚪ Performance A/B: no change beyond noise

8 shape×range — 8 ⚪ same · results vs Loki: 7 same, 0 new difference(s), 1 pre-existing

Loki profiles compared (like for like):

aligned: not run (no build supports -align-queries-with-step, so every aligned row would be not like-for-like)
loki-defaults: 8 shape×range; proxy flag base off, pr off

Every shape is within noise of the base build (±30% and 100 ms).

8 unchanged shape×range (within noise)

	shape	range	Loki profile	base p50	PR p50	change	Loki p50	PR ÷ Loki	result vs Loki
⚪	drilldown service landing	1h	defaults	21 ms	22 ms	⚪ +5%	229 ms	0.10×	✅ same
⚪	instant ungrouped rate	instant	defaults	5 ms	5 ms	⚪ +0%	159 ms	0.03×	✅ same
⚪	json rate by level (no filter, no drop)	1h	defaults	400	400	⚪ +0%	400	—	✅ same
⚪	json volume no filters	1h	defaults	102 ms	93 ms	⚪ -9%	126 ms	0.74×	✅ same
⚪	logs plain	1h	defaults	85 ms	81 ms	⚪ -5%	290 ms	0.28×	✅ same
⚪	logs \| json	1h	defaults	128 ms	131 ms	⚪ +2%	903 ms	0.15×	✅ same
⚪	stream-label count by app	1h	defaults	10 ms	10 ms	⚪ +0%	99 ms	0.10×	✅ same
⚪	topk rate by app	1h	defaults	68 ms	69 ms	⚪ +1%	210 ms	0.33×	➖ differs (as base)

Cold (first-run) timings — what a user opening a panel sees

shape	range	Loki profile	base cold	PR cold	Loki cold	PR ÷ Loki cold
drilldown service landing	1h	defaults	63 ms	70 ms	1.06 s	0.07×
instant ungrouped rate	instant	defaults	14 ms	11 ms	258 ms	0.04×
json rate by level (no filter, no drop)	1h	defaults	400	400	400	—
json volume no filters	1h	defaults	252 ms	218 ms	607 ms	0.36×
logs plain	1h	defaults	90 ms	71 ms	308 ms	0.23×
logs \| json	1h	defaults	230 ms	246 ms	1.20 s	0.20×
stream-label count by app	1h	defaults	25 ms	22 ms	433 ms	0.05×
topk rate by app	1h	defaults	50 ms	48 ms	645 ms	0.07×

Why these shapes

control smoke → 8: control/drilldown service landing, control/instant ungrouped rate, control/json rate by level (no filter, no drop), control/json volume no filters, control/logs plain, control/logs | json, control/stream-label count by app, control/topk rate by app

_{base 2f36266c86 → PR aba5ff92d5 · 4 runs (first is cold), warm p50 · ranges 1h · noise ±30% and 100 ms · VictoriaLogs restarts 0→0 · 1.5 min · phases stack_up 36s, seed_and_build 28s, loki-defaults:loki_profile 0s, loki-defaults:proxies 2s, loki-defaults:matrix 21s. Same runner, one fresh stack, targets interleaved per request. How to read and reproduce: bench/ab/README.md.}

github-actions · 2026-10-12T11:53:11Z

✅ Visual smoke: passed

Base 2f36266c86 vs PR f7ca503460, Grafana through the proxy builds and Loki on one fresh stack; run.
Core set: 5 capture(s) at 1h. Detailed set: 2 entries (explore-unwrap-max-over-time-end-sample, explore-unwrap-sum-over-time-sliding-window). 11 capture(s), 15m/1h/6h, 24h and 7d stay out of PR runs.

page	range	set	base = PR (data)	vs Loki (PR)	pixel diff	result
dd-fields	1h	core	✅ identical	identical (4 explained, 1 history-dependent)	0.11%	✅
dd-landing	1h	core	✅ identical	identical (3 explained)	0.14%	✅
dd-service-logs	1h	core	✅ identical	identical (3 explained, 1 history-dependent)	0.11%	✅
explore-logs-plain	1h	core	✅ identical	identical (1 explained)	0.07%	✅
explore-metric-plain	1h	core	✅ identical	identical (1 explained)	0.04%	✅
explore-unwrap-max-over-time-end-sample	15m	detailed	✅ identical	identical (1 explained)	0.04%	✅
explore-unwrap-max-over-time-end-sample	1h	detailed	✅ identical	identical (1 explained)	0.04%	✅
explore-unwrap-max-over-time-end-sample	6h	detailed	✅ identical	n/a (beyond the Loki window)	0.04%	✅
explore-unwrap-sum-over-time-sliding-window	15m	detailed	✅ identical	identical (1 explained)	0.04%	✅
explore-unwrap-sum-over-time-sliding-window	1h	detailed	✅ identical	identical (1 explained)	0.04%	✅
explore-unwrap-sum-over-time-sliding-window	6h	detailed	✅ identical	n/a (beyond the Loki window)	0.04%	✅

Gate: a base-vs-PR data difference passes when the PR is closer to Loki, and fails when the PR diverges from Loki or Loki cannot decide it (at a range Loki does not hold, a difference passes when the same page proved that query to answer like Loki at a shorter range; otherwise label the pull request visual-change-expected to accept an intended change). Also failing: a panel empty on the PR but not on the base, an error banner, panel error, error answer or new "No data" panel on the PR, a PR side that never settled, a difference that did not reproduce on the recapture. Pixel differences only warn.

Core set montages (5): base | PR | Loki

dd-fields-1h

dd-landing-1h

dd-service-logs-1h

explore-logs-plain-1h

explore-metric-plain-1h

All montages of this run: pr-visuals/pr-745.

github-actions · 2026-10-12T11:57:16Z

PR Quality Report

Compared against base branch main.

Coverage and tests

Signal	Base	PR	Delta
Test count	15619	15642	23
Coverage	88.6%	88.6%	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	3299.0 ns/op	3328.0 ns/op	+0.9% (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	3881.0 ns/op	4016.0 ns/op	+3.5% (stable)
QueryRange cache-bypass memory	1302.0 B/op	1301.0 B/op	-0.1% (stable)
QueryRange cache-bypass allocations	27.0 allocs/op	27.0 allocs/op	0.0% (stable)
Labels cache-hit CPU cost	891.8 ns/op	897.2 ns/op	+0.6% (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	1464.0 ns/op	1455.0 ns/op	-0.6% (stable)
Labels cache-bypass memory	399.0 B/op	399.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.

github-actions · 2026-10-12T11:57:21Z

PR Quality Report

Compared against base branch main.

Coverage and tests

Signal	Base	PR	Delta
Test count	8648	8671	23
Coverage	87.7%	87.8%	+0.1% (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	3279.0 ns/op	3288.0 ns/op	+0.3% (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	3895.0 ns/op	4037.0 ns/op	+3.6% (stable)
QueryRange cache-bypass memory	1300.0 B/op	1297.0 B/op	-0.2% (stable)
QueryRange cache-bypass allocations	27.0 allocs/op	27.0 allocs/op	0.0% (stable)
Labels cache-hit CPU cost	879.2 ns/op	878.9 ns/op	-0.0% (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	1432.0 ns/op	1409.0 ns/op	-1.6% (stable)
Labels cache-bypass memory	399.0 B/op	399.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.

Loki's MergeBinOp returns NaN for a zero right operand of / and % whatever the numerator is, for literal folds, vector against literal and vector against vector. The proxy computed IEEE a/b, so 1 / 0 and sum(rate(...)) / 0 were +Inf or -Inf, and the fallback arithmetic answered 0.

…-nan

…he compose Loki times out on literal-only queries

github-actions Bot added size/XL Extra large change scope/proxy Proxy core scope/docs Documentation scope/tests Tests bugfix Bug fix labels Oct 12, 2026

szibis added a commit that referenced this pull request Oct 12, 2026

chore(conformance): record #745 as the fix of binary-division-by-zero…

8e0c807

…-nan

github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Oct 12, 2026

szibis mentioned this pull request Oct 12, 2026

fix: a scalar answer carries its time in milliseconds, like Loki #752

Merged

szibis added 3 commits October 28, 2026 12:17

chore(conformance): record #745 as the fix of binary-division-by-zero…

9a4c351

…-nan

test: compare literal division by zero in the unit tests, not live; t…

f7ca503

…he compose Loki times out on literal-only queries

szibis force-pushed the fix/binary-division-by-zero-nan branch from e2bcf6e to f7ca503 Compare October 28, 2026 11:20

github-actions Bot added size/XL Extra large change and removed size/XL Extra large change labels Oct 28, 2026

szibis marked this pull request as ready for review October 28, 2026 11:26

szibis merged commit 24b9593 into main Oct 28, 2026
73 checks passed

szibis deleted the fix/binary-division-by-zero-nan branch October 28, 2026 11:48

szibis mentioned this pull request Oct 28, 2026

fix: an aggregation over a binary expression aggregates the whole binary, like Loki #741

Merged

Navigation Menu

Repository navigation

fix: a division or modulo by zero is NaN, like Loki - #745

fix: a division or modulo by zero is NaN, like Loki#745
szibis merged 3 commits into
mainfrom
fix/binary-division-by-zero-nan

szibis commented Oct 12, 2026

Uh oh!

github-actions Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 12, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

szibis commented Oct 12, 2026

Summary

Tests

Pending before ready

Uh oh!

github-actions Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚪ Performance A/B: no change beyond noise

Uh oh!

github-actions Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Visual smoke: passed

Uh oh!

github-actions Bot commented Oct 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Quality Report

Coverage and tests

Compatibility

Performance smoke

State

Uh oh!

github-actions Bot commented Oct 12, 2026

PR Quality Report

Coverage and tests

Compatibility

Performance smoke

State

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented Oct 12, 2026 •

edited

Loading

github-actions Bot commented Oct 12, 2026 •

edited

Loading

github-actions Bot commented Oct 12, 2026 •

edited

Loading