feat(processor): per-destination visibility at the destination-filter boundary - #7283
feat(processor): per-destination visibility at the destination-filter boundary#7283mihir20 wants to merge 9 commits into
Conversation
|
Stack (merge bottom-up)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7283 +/- ##
==========================================
- Coverage 80.12% 79.79% -0.34%
==========================================
Files 601 601
Lines 67217 67301 +84
==========================================
- Hits 53857 53701 -156
- Misses 10184 10434 +250
+ Partials 3176 3166 -10 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
17bcee0 to
5445674
Compare
2155a5b to
63dfc97
Compare
63dfc97 to
d4c03d8
Compare
| DrainEventCode = 410 | ||
| SuccessEventCode = 200 | ||
| DeliveredWithWarningCode = 296 | ||
| ConsentDeniedEventCode = 297 |
There was a problem hiding this comment.
why does this require a new status code?
There was a problem hiding this comment.
It's as per the original design proposal 😅. We are distinguishing filtering at a stage with different status codes for different statuses.
There was a problem hiding this comment.
I am still trying to remember why we need this, I mean we are using different statuses already
There was a problem hiding this comment.
I guess I was influenced by the reportingv2 solution where counter labels are stable and we classify through special category codes. But not sure there is any benefit in the current design by inventing yet another status code.
… boundary Add classifyDestinations as the single classification site at fan-out, narrowing the candidate set to the RETL-stamped destination before classification, and emit per-candidate destination_enter succeeded/200 rows (Reporting.destinationEnterMetrics.enabled) plus per-destination filtered_integration/298 and filtered_consent/297 destination_filter rows (Reporting.perDestinationFilterMetrics.enabled), with a filtered_no_destination/298 carve-out for zero-candidate events. Processor.earlyDestinationFilter (default true) gates the reorder: when false the preprocess destination-filter guard is skipped so all events reach source hydration and tracking-plan validation, fan-out becomes the drop point, and the inPU chain labels are rewired to the new stage order (source_hydration=gateway, tracking_plan_validator=srchyd|gateway, user_transformer=destination_filter). The flag is snapshotted once per batch and threaded through the stage messages so one in-flight batch never observes mixed values. All flags are reloadable and default to byte-identical-to-today behavior. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🔒 Scanned for secrets using gitleaks 8.28.0
The old two-level per-destType fan-out loop was collapsed into a single loop over availableDestinations, but the inner block's indentation and closing brace were left behind, breaking the build at processor.go:2577. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> 🔒 Scanned for secrets using gitleaks 8.28.0
🔒 Scanned for secrets using gitleaks 8.28.0
🔒 Scanned for secrets using gitleaks 8.28.0
…ctions and update related logic 🔒 Scanned for secrets using gitleaks 8.28.0
…lter handling in reporting 🔒 Scanned for secrets using gitleaks 8.28.0
🔒 Scanned for secrets using gitleaks 8.28.0
… destination classification 🔒 Scanned for secrets using gitleaks 8.28.0
… rehydration and update related logic 🔒 Scanned for secrets using gitleaks 8.28.0
cb7f42b to
ef26068
Compare
| // so the reused transform stages operate on them unchanged. | ||
| // passed them. | ||
| // | ||
| // Config drift between fan-out and consume: a destination deleted from the config |
There was a problem hiding this comment.
In practice destinations are soft deleted, never hard deleted & soft deleted destinations arrive to rudder-server as disabled
| // survive filtering (available) and the ones excluded, with reason and status code, in the same | ||
| // order and applying the same filters the fan-out uses today: client-integration filtering first, | ||
| // then consent filtering. When specificDestID is set (RETL), the candidate set is narrowed to that | ||
| // single destination before classification — sibling destinations are never candidates and produce |
There was a problem hiding this comment.
what are sibling destinations?
| DrainEventCode = 410 | ||
| SuccessEventCode = 200 | ||
| DeliveredWithWarningCode = 296 | ||
| ConsentDeniedEventCode = 297 |
There was a problem hiding this comment.
I guess I was influenced by the reportingv2 solution where counter labels are stable and we classify through special category codes. But not sure there is any benefit in the current design by inventing yet another status code.
Description
Part of Pipeline Inspector: gives per-destination visibility at the destination-filter boundary so fully- and partially-dropped events stop vanishing from the reporting funnel without a trace.
What changed
Single classification site. The per-destType destination discovery in the fan-out loop is replaced by one
classifyDestinationscall that splits a source's candidate destinations intoavailableandexcluded(with the exclusion reason and status code). A RETL-stampeddestination_idnarrows the candidate set before classification.isDestinationAvailable(the preprocess guard) andclassifyDestinationsagree by construction, which is pinned by tests.Per-source destination snapshot.
classifyDestinationsruns per event, so it now consumes asourceDestinationssnapshot (destTypesDisplayName → definition forFilterClientIntegrations,byTypeNamedefinition-name → enabled destinations in backend-config order) built once per source under a single config RLock, instead of taking two RLocks and re-scanning the source's destination list for every event. Only the event-dependent work (client-integration filtering, consent filtering) remains per event. Filtering semantics, ordering and RETL narrowing are unchanged.One flag:
Processor.earlyDestinationFilter(reloadable, defaulttrue).true(default): behavior is byte-identical to today. Events with no surviving destination drop at the preprocess guard (pre tracking-plan) with the existing source-leveldestination_filterfiltered/298row. No new rows are emitted anywhere.false: the preprocess guard is skipped, so every event reaches source hydration and tracking-plan validation, and the destination-filter decision moves to fan-out — which becomes the single drop point. Per-destination visibility comes with it unconditionally:destination_entersucceeded/200 for every candidate destination (survivors and excluded alike), sodestination_enter − filtered_* = user_transformer inputholds per destination;destination_filterrows for excluded candidates:filtered_integration/298(integrations opt-out) andfiltered_consent/297(consent denied);filtered_no_destination/298(source-level, emptydestination_id) for zero-candidate events — a source with no destinations, or a RETL event whose stamped destination is unavailable. Without this carve-out those events would disappear from the funnel entirely.The flag is snapshotted once per batch in
preprocessStageand threaded through the stage messages, so an in-flight batch never observes mixed values when the reloadable config flips.Hard cutover, no dual-emit. With the flag
false, the old source-levelfilteredrow stops entirely — there is no intermediate state where the drop moves to fan-out but keeps the old row shape. Consumers of that row (reporting-servicefilteredEvents, dashboards) must be ready before the flip.in_puis left empty on all new rows. The field is slated for deprecation, so no chain value is computed for the rows introduced here, and the inPU values of pre-existing rows (gateway, tracking-plan, source-hydration, user-transformer) are untouched regardless of the flag.Proc rebuild stage: disabled destinations keep flowing (review follow-up).
DESTINATION_ENTERis recorded at fan-out, but a forked event whose destination got disabled before the proc rebuild stage consumed it used to be dropped there with no reporting row — leaving the reporting chain dangling. The rebuild stage now re-hydrates a disabled destination as-is and lets the event continue through the pipeline, so it reaches the router/batchrouter queue and is aborted there with the usual drain reporting ("destination is disabled"). A destination deleted from the config is still dropped at rebuild to a terminalFilteredstatus: without its config there is no destination type, hence no transformation to run and no router queue to store to.New reloadable flag
Processor.DestinationIsolation.dropEventsForDisabledDestAtProcRebuild(defaultfalse) opts back into the early drop for disabled destinations — useful for draining a proc-table backlog quickly after the user disables a misbehaving destination.Behavior summary for a fully-dropped event
Processor.earlyDestinationFiltertrue(default)filtered/298only — identical to todayfalsedestination_enter×N +filtered_*×N; zero-candidate events emitfiltered_no_destination/298Side effects of flipping to
false, for rollout notes: zero-destination events now incur source-hydration/TP transformer cost for sources that have those configured, and TP incoming/violation counts increase accordingly (arguably a fix — TP health no longer silently excludes events whose user opted out of every destination).Tests
TestClassifyDestinations: unit coverage of the classifier — integrations/consent exclusion, RETL narrowing, ordering, disabled destinations excluded from the candidate set, and an invariant subtest thatlen(available) > 0agrees withisDestinationAvailableacross a case table.TestDestinationVisibilityReporting: stage-level coverage of the fan-out reporting — enter/filter row shapes, the zero-candidate and RETL carve-outs, forked-destination enter rows, both flag states, and mid-flight flag flips on the same handle.TestProcRebuildStage: config-drift coverage of the rebuild stage — a disabled destination flows through re-hydrated as-is by default, is dropped asFilteredwhendropEventsForDisabledDestAtProcRebuildis set, and a deleted destination is always dropped.integration_test/reporting_dropped_events: an end-to-end scenario driving one server through both flag states against a real Postgres reports table (preprocess row byte-compat, enter rows, partial exclusion, zero-candidate).Linear Ticket
pipe-3286
Security