Conversation
Code Review ✅ Approved🟡 Medium risk Backport to 1.13 fixes column-level test cases matching in table-scoped alerts by reading the table from the test case's OptionsDisplay: compact → Counting what did not apply, without listing it. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
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.
Backport of the column-level part of #33393 (Fixes #33388) to
1.13.Problem
An observability alert can be narrowed to "test cases of these tables". On
1.13that filter compares the test case'sentityFQN, which for a column-level test is<table>.<column>, not the table. So an alert scoped to a table never fires for the column tests on it (not null, unique, values in set), which are most of a real suite. Nothing reports the miss.Why it happened
#27987 made alert filters match names exactly instead of by pattern. Before it, this filter pattern-matched the table name inside the test case's own full name, which starts with its table's full name, so column tests matched. #27987 switched it to an exact comparison with
entityFQN, which for a column test also carries the column, so they stopped matching. #27987 was backported, so every release from 1.13.0 to 1.13.6 has this, and anyone upgrading from 1.12 silently lost these alerts.What this PR changes
The filter reads the table from the test case's
entityLink, which is how search and permission checks already find it. That is one method inAlertsRuleEvaluator.Not in this PR
#33393 also fixed how the test case and data contract filters handle comments (#31330, #33389). Those fixes build on #30571, which makes every scoping filter look at what a comment is about. #30571 is not on
1.13: here, all scoping filters let comment events through on purpose. Changing only these two would make them behave differently from the rest, so they are left as they are. For the same reason, the integration test that expects an event that is not a test case to pass the filter is unchanged. The2.0backport, which has #30571, takes all three fixes: #33569.What users will notice
Alerts that use the table filter start receiving column-level test events again, as they did on 1.12.
How it was tested
spotless:checkclean.AlertsRuleEvaluatorTableFilterTest, plusAlertUtilTest: 39 green.1.13's current evaluator, the column-level test fails.AlertsRuleEvaluatorResourceITrun locally on this branch: 40 run, 0 failures, 1 skipped (already skipped before this change).The PR appears safe to merge and restores the intended table-filter behavior without changing unrelated alert semantics.
Summary
This backport corrects table-scoped alert evaluation for column-level test cases by deriving the parent table FQN from the test case’s entity link rather than its column-qualified
entityFQN.Reviews (1) · Last reviewed commit: "Backport #33393 to 1.13: match column-le..."