fix(alerts): scope thread events by their parent entity instead of bypassing filters - #30571
fix(alerts): scope thread events by their parent entity instead of bypassing filters#30571manerow wants to merge 3 commits into
Conversation
Code Review ✅ Approved 2 resolved / 2 findingsReplaces permissive thread-event rule bypasses with parent entity reference evaluation in AlertsRuleEvaluator, correctly scoping notifications by FQN, ID, source, owner, and domain. No issues found. ✅ 2 resolved✅ Edge Case: Malformed filterByEntityId value throws and aborts thread batch
✅ Performance: Parent entity re-resolved per matcher for thread events
OptionsDisplay: compact → Showing less information. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Gitar | Powered by Gitar — free for open source |
✅ Playwright Results — workflow succeededValidated commit ✅ 537 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 5 skipped · 🧰 0 lifecycle flaky PerformanceBlocking targets: ✅ met · Optimization targets: 🟡 in progress Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting. 🕒 Full workflow signal wall (to summary) 52m 8s ⏱️ Max setup 3m 1s · max shard execution 17m 44s · max shard-job elapsed before upload 21m 35s · reporting 5s 🌐 202.59 requests/attempt · 2.86 app boots/UI scenario · 4.32% common-shard skew Optimization targets still in progress:
How to debug locally# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip # view trace |
Fixes #30555
Problem
An alert scoped to one entity receives conversations about every entity of that type. Reproduced on
a clean
maindeploy: an alert withresource: glossaryTerm,filterByEventType: [threadCreated, postCreated]and
filterByFqn: [<term T2>]received both events for a conversation started on T1.The same holds for Owner, Domain, Entity Id and Source: the filter is stored, shown in the UI, and
has no effect.
Root cause
A thread change event carries
entityType = THREADand aThreadpayload rather than the entity thethread is about, so five matchers in
AlertsRuleEvaluatorshort-circuited:Returning
truefrom a filter that cannot be evaluated means "deliver", not "does not match". Thatwas unreachable for entity resources until #28122 correctly routed thread events to the alert of the
entity the thread is about, at which point
glossaryTerm,tableand friends, which do offer thesefilters, started hitting the bypass.
Fix
The event already carries the answer:
Thread.entityRefpoints at the parent entity. Each bypass nowevaluates the same question against that reference.
THREADeventmatchAnyEntityFqnmatchesFqnOrDescendant(thread.entityRef.fullyQualifiedName, ...), so ancestor scoping from #28833 keeps workingmatchAnyEntityIdthread.entityRef.idmatchAnySourcethread.entityRef.typematchAnyOwnerNameownersand reuses the existingmatchOwners(...)matchAnyDomaindomainsthrough a new sharedmatchDomains(...), also used by the entity pathOwner and domain resolve the parent through the existing
Entity.getEntityOrNull(ref, fields, NON_DELETED),so a null reference, a deleted parent or an unknown entity type yield
falseinstead of throwing. Thatmatters here: an escaping exception in a matcher aborts the whole change-event batch, the failure mode
fixed in #28304.
Matchers that genuinely cannot apply to a thread (test result, pipeline state, ingestion state) already
return
falsesince #29112 and are untouched.Also drops the dead
announcemententry fromAlertUtil.THREAD_TYPE_RESOURCES. Announcement has been afirst-class entity since #25894, its events carry
entityType = announcementand take the generic path,and
FeedResource.rejectLegacyAnnouncementAccessblocks the legacy door.taskstays until the lastlegacy thread-task writer is migrated (#30559).
Behaviour change worth noting in release notes
Alerts that were receiving every thread event because their scoping filter was ignored will now receive
only what they scoped. No migration is required: no schema change, and the filter vocabulary is identical
between 1.13 and
main(same eleven filter function names, same per-resourcesupportedFilters), soevery stored rule keeps parsing and evaluating. Only the meaning for
THREADevents changes.Tests
AlertsRuleEvaluatorThreadScopeTest(new, 9 cases): FQN exact / ancestor / sibling-prefix / non-match, entity id, source, and a thread with noentityRefwhere every scoping filter must returnfalse.AlertsRuleEvaluatorResourceIT(4 new cases): owner and domain against real entities, FQN, and an unresolvable parent. These fail against the pre-fix build and pass after it.AlertUtilTest: the announcement assertions now cover the post-redesign behaviour, plus a case proving anannouncementresource still matches realentityType=announcementevents.Verified end to end on a local deploy
filterByFqn: [T2]filterByOwnerName: [admin], T1 is admin-ownedfilterByFqn: [nonexistent]The last two cover an edge case worth knowing: the observability trigger section is a form list with no
minimum, so an observability alert can be saved with zero triggers, and the #29112 guard only rejects
thread events when
actionsis non-empty. Such an alert does receive thread events, and now honours itsfilters too.
Also driven through the UI: building the alert in Settings > Notifications > Alerts, commenting on both
glossary terms, and confirming the alert's Recent Events tab shows
Total Events: 1for the in-scopecomment only.
Greptile Summary
Thread-event alerts now evaluate entity, owner, domain, FQN, ID, and source filters against the referenced parent entity.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Event as Thread ChangeEvent participant Alert as AlertUtil participant Rules as AlertsRuleEvaluator participant Parent as Parent Entity Event->>Alert: Evaluate subscription Alert->>Rules: Apply configured filters Rules->>Event: Read Thread.entityRef alt FQN, ID, or source filter Rules->>Rules: Match reference fields else Owner or domain filter Rules->>Parent: Resolve referenced entity fields Parent-->>Rules: Owners or domains Rules->>Rules: Match configured scope end Rules-->>Alert: Match or non-matchReviews (3): Last reviewed commit: "refactor(alerts): drop the per-field sub..." | Re-trigger Greptile