AlertFiltering: prohibit partial filtering #18197
Conversation
This documentation-only commit clarifies that a query should either ignore restrictAlertsTo completely or apply restrictAlertsTo filtering to all alerts. This update eliminates the ambiguity on whether a query may choose to apply restrictAlertsTo filtering to only some alerts but not others (it may not).
There was a problem hiding this comment.
LGTM.
If I were to nitpick, I might say that "completely ignore this predicate" is slightly too strong a requirement. The query is allowed to "use" the extensible in a way where it generates the same alerts using different internal computations, depending on whether a diff is specified.
But I have no idea to which degree that is even a realistic opportunity, so I'm okay with not attempting to explain that in this context.
There was a problem hiding this comment.
LGTM. I'll just say here, for those wondering now or in the future why we're adding the requirement, that the purpose is to avoid alert wobble on PRs. If a small code change can cause unrelated alerts to come and go, that will make the timeline view in code scanning chaotic. This is why #17648 was needed.
Yes. And I realized that my PR description is incomplete:
In addition to that, the new requirement is also intended to ensure that a query cannot decide whether to perform filtering based on the |
|
Hmm, that raises another question: can a query decide to filter or not based on what's in the database? (I'd assume no, otherwise the rule wouldn't solve any alert wobble problem). |
This documentation-only PR clarifies that a query should either ignore
restrictAlertsTocompletely or applyrestrictAlertsTofiltering to all alerts. This update eliminates the ambiguity on whether a query may choose to applyrestrictAlertsTofiltering to only some alerts but not others (it may not).