Repository navigation
test: carry a resource difference beyond the Loki window when it adds nothing new against Loki - #911
Merged
Merged
Conversation
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
… nothing new against Loki A base-vs-PR difference at a range Loki does not hold (6h) passed only when every difference on the row was a query the page proved at a shorter range. A resource request (Explore's /resources/index/stats?query=...) has no query id, so it was never carried, and every page showing a selector fix also changes index/stats: the explore multivalue-regexp pages of the selector fix had to be pinned to 15m and 1h. A resource difference at such a range is now carried when the page proved at least one query and, at every shorter range Loki holds, the same resource (path and query, time parameters ignored) shows up and the PR adds no difference from Loki on it that the base did not have (compare.py's loki_new). A resource that differs from Loki in a new way on the PR, one that does not show up at a shorter range, and one on a page with no proven query stay uncarried. Base vs PR at the same range stays exact. Replayed over 52 recorded runs (1076 captures): the only changed verdicts are the two 6h captures of the selector-fix run (explore-logs-multivalue-regexp and explore-volume-multivalue-regexp, fail -> improved); all other verdicts are equal.
szibis
force-pushed
the
test/visual-carry-resources
branch
from
October 28, 2026 14:54
9b49b81 to
661b19e
Compare
Contributor
✅ Visual smoke: passedBase
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 Core set montages (5): base | PR | Lokidd-fields-1h
dd-landing-1h
dd-service-logs-1h
explore-logs-plain-1h
explore-metric-plain-1h
All montages of this run: |
Contributor
PR Quality ReportCompared against base branch Coverage and tests
Compatibility
Performance smokePerformance smoke was skipped for this PR because no perf-sensitive paths changed. State
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Bench-only (
bench/visual): no CHANGELOG entry, no release, no product change, so the e2e, visual, A/B and load proofs do not apply.Problem
At a range Loki does not hold (6h) a base-vs-PR difference passes only when every difference on the row is a query the same page proved at a shorter range (#897). A resource request (no query id, e.g. Explore's
/resources/index/stats?query=...) was never carried, deliberately. Every page that shows a selector fix also changesindex/stats, so those pages could not be captured at 6h (theexplore-*-multivalue-regexppages of #909 had to be pinned to 15m and 1h).Rule
A resource difference at such a range is carried when
start/end/from/to/time/since/until/step/interval/tsignored) shows up, and the PR adds nothing new against Loki on it: its vs-Loki differences are a subset of the base's (compare.py'sloki_newholds none for it), or it has none; andA resource that differs from Loki in a new way on the PR at a shorter range, one that does not show up at a shorter range, and one on a page with no proven query are not carried. Every other failure condition and base-vs-PR at the same range are unchanged.
Change
visual_comment.py:resource_id/resource_ids(identity without time parameters; the diff text carries the URL cut at 90 characters, a trailing partial parameter is dropped),proven_resources(per page and Loki-held range: resources seen and resources new against Loki),resource_carried;carriedandassesstake the newresourcesargument,evaluatepasses it. A row withoutloki_newcounts every vs-Loki difference as new.test_a_resource_difference_at_6h_is_never_carriedis renamedtest_a_resource_with_a_new_difference_from_loki_is_never_carried, plus: the fix: regexp label matchers select what Loki selects, on selectors and label filters #909 shape carries; a resource new vs Loki at 1h is not carried; a resource absent at the shorter ranges (nowhere, or at 1h only) is not carried; a resource the PR brought to Loki is carried; a page with no proven query is not carried; a carried resource does not carry a new one next to it; resource identity ignores time parameters and a cut parameter.bench/visual/README.md: one paragraph.Replay of recorded runs
52 compare.json artifacts (50 earlier visual-smoke runs, the #909 run, the #736 run; 1076 captures) evaluated with the base gate and the new gate, each with the run's own plan.json, pixeldiff.json and flipped list from meta.json. For the 50 earlier runs the base gate's exit equals the CI conclusion of that run (46 success = exit 0, 4 failure = exit 1), so the replay is faithful. Every capture verdict is unchanged except the two 6h captures of the #909 run, the only 6h rows with a resource difference:
The #909 run's own exit stays 1 for an unrelated reason: its fix proof
profiles/label-filter-regexp-anchoringis "still differs" (index/stats differs from Loki on both builds), which this change does not touch.Known limit
The resource identity comes from the difference text, which carries the URL cut at 90 characters (about 37 encoded characters of an index/stats selector). Two selectors of one page sharing that prefix would be treated as one resource. Follow-up: have compare.py tag resource differences with their full normalised id, as it does for
qid.Changelog (Required)
None: bench-only, no releasable change (the changelog gate reports "skipped (no releasable changes detected)").
Validation
python3 -m unittest discover -s bench/visual/tests: 144 tests ok; the new tests fail on the base gate where they should (carry cases) and pass where they should not carry.python3 scripts/ci/check_conformance.py: conformance gate ok. Registry: none (bench-only).Update: rebased onto main after #909
explore-logs-multivalue-regexpandexplore-volume-multivalue-regexpcover 6h again: the"ranges": ["15m", "1h"]pin from fix: regexp label matchers select what Loki selects, on selectors and label filters #909 is removed, so this PR's own visual-smoke is their first live 6h run under the new rule. The README sentence that told authors to pin such pages now describes the carry instead.test_an_index_stats_difference_beside_a_proven_query_is_not_carried_to_6hpinned the old rule. It is now..._carries_to_6hand expects the carry, since the PR adds nothing new against Loki on index/stats at 15m and 1h.+, as the code-quality review asked.python3 -m unittest discover -s bench/visual/testsruns 146 tests, OK.plan.py --checkpasses, and the conformance gate passes.