✨ add beforeSend event filtering and scrubbing - #180
Conversation
|
@codex review |
There was a problem hiding this comment.
Stale comment
PR Review — Score: 4.6 / 5
This is a well-scoped addition of main-process
beforeSendfiltering and scrubbing.RumEventMapperis wired at the right boundary — after format hooks assemble the event and beforeServerRumEventemission — with an allowlist + clone pattern that protects identity fields, blocks view/crash drops, and fails open on callback errors. Documentation, playground controls, and unit/E2E coverage are thorough. I would approve.Why 4.6: Correct pipeline placement, strong allowlist enforcement with sanitization, protected view/crash semantics, clear public API typing (
RumBeforeSend,MainRumEvent), and meaningful tests across mapper, assembly, config, e2e, and playground.Why not 5: Minor consistency gap when scrubbing
service/version(ddtags not updated), plus small UX/perf nits around string clearing and per-event cloning.
Findings
- [Minor] ddtags not synced with scrubbed service/version — Allowlisted
service/versionchanges do not update theddtagsstring set bycommonContext.- [Nit] String fields cannot be cleared — Setting an allowlisted string to
null/undefinedsilently keeps the original value.- [Nit] Per-event deepClone cost — Every main-process RUM event is deep-cloned when
beforeSendis configured.
Architectural flow
LoadingsequenceDiagram participant Coll as RUM collection participant EM as EventManager participant MA as MainAssembly participant Hooks as FormatHooks participant Map as RumEventMapper participant VC as ViewCollection participant T as Transport Coll->>EM: RawRumEvent EM->>MA: handle MA->>Hooks: triggerRum Hooks-->>MA: session, view, service, ddtags MA->>MA: combine raw + hooks MA->>Map: map assembled event Map->>Map: deepClone and beforeSend alt discarded Map-->>MA: undefined MA-->>EM: no ServerEvent else kept or scrubbed Map-->>MA: modified event MA->>EM: ServerRumEvent EM->>VC: increment counters EM->>T: batch for intake endBefore: Main-process
RawRumEvents were enriched via format hooks and emitted directly asServerRumEvents. Renderer events already used the Browser SDKbeforeSendbefore crossing the bridge.After:
MainAssemblyrunsRumEventMapperon fully assembled main-process RUM events. Customers can scrub allowlisted fields or returnfalseto drop non-view, non-crash events. Discarded events never becomeServerRumEvents, so view counters and transport stay consistent. Telemetry, profiles, spans, and renderer RUM events are unchanged.Sent by Cursor Automation: electron-sdk reviews
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d879cfa0f2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b3ae277cbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40e95a4680
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const modifiableFieldPaths = MODIFIABLE_FIELD_PATHS_BY_EVENT[event.type]; | ||
| const result = limitModification(event, modifiableFieldPaths, (modifiableEvent) => { |
There was a problem hiding this comment.
Fail open for unknown renderer event types
When a newer Browser SDK sends a RUM event type that this Electron SDK does not yet know, this lookup returns undefined and objectEntries(modifiableFieldPaths) throws before the callback runs. Because RendererPipeline calls apply() for every renderer RUM event whenever beforeSendRum is configured, a bridge/SDK version mismatch drops all events of the new type through the monitored IPC handler; use the common modifiable paths or otherwise pass unknown types through unchanged.
AGENTS.md reference: AGENTS.md:L3-L6
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
PR Review — Score: 4.7 / 5
This is a solid beforeSendRum implementation wired at the right assembly boundary for both main and renderer RUM events. The allowlist + clone pattern protects identity fields, blocks view/crash drops, fails open on callback errors, and exposes source for process-aware filtering. Follow-up commits addressed prior naming, test structure, README examples, crash-protection documentation, and renderer coverage. I would approve.
Why 4.7: Correct pipeline placement on MainAssembly and RendererPipeline, comprehensive allowlists for main and renderer event shapes, clear public API (beforeSendRum, BeforeSendContext), and meaningful unit/E2E/playground coverage.
Why not 5: A forward-compat gap for unrecognized event.type values can throw before the callback runs, plus small API ergonomics (callback return typing) and the inherent per-event clone cost.
Findings
- [Minor] Unknown event types — Unrecognized
event.typemakes field-path lookup undefined and can throw beforebeforeSendRumruns. - [Minor] Strict callback return type — Runtime keeps events on void/undefined but
RumBeforeSendrequiresboolean. - [Nit] Per-event deepClone — Every RUM event is deep-cloned when
beforeSendRumis configured.
Architectural flow
sequenceDiagram
participant Coll as RUM collection
participant Bridge as Renderer bridge
participant EM as EventManager
participant MA as MainAssembly
participant RP as RendererPipeline
participant Hooks as FormatHooks
participant BS as BeforeSend
participant T as Transport
Coll->>EM: RawRumEvent
EM->>MA: handle
MA->>Hooks: triggerRum
Hooks-->>MA: session, view, service, ddtags
MA->>MA: combine raw + hooks
MA->>BS: apply assembled event (source main)
alt discarded
BS-->>MA: undefined
MA-->>EM: no ServerEvent
else kept or scrubbed
BS-->>MA: modified event
MA->>EM: ServerRumEvent
end
Bridge->>RP: bridge RUM message
RP->>Hooks: triggerRum (renderer)
Hooks-->>RP: session, container view, overrides
RP->>RP: combine renderer event + overrides
RP->>BS: apply (source renderer)
alt discarded
BS-->>RP: undefined
else kept or scrubbed
BS-->>RP: modified event
RP->>EM: ServerRumEvent
end
EM->>T: batch for intake
Before: Main and renderer RUM events were enriched via format hooks and emitted directly as ServerRumEvents. Renderer events could already be filtered by the Browser SDK beforeSend before crossing the bridge.
After: MainAssembly and RendererPipeline run the shared BeforeSend helper after Electron enrichment. Customers can scrub allowlisted fields or return false to drop non-view, non-crash events from either process. Discarded events never become ServerRumEvents. Telemetry, profiles, spans, and logs remain outside this callback.
Sent by Cursor Automation: electron-sdk reviews
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40e95a4680
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".


Motivation
Allow Electron applications to filter or scrub main-process RUM events before they are sent.
Changes
beforeSendcallback for fully assembled main-process RUM events.Note :
I kept beforeSend separate because renderer events already go through the Browser SDK callback before crossing the bridge
Running the Electron callback afterward would add a second filtering step . Browser therefore handles renderer events, while Electron handles main-process events. Android follow the same path with webview, there is no "global" before send
Test instructions
yarn test:unityarn typecheckyarn buildyarn playwright test -c e2e --project=e2e e2e/scenarios/before-send.scenario.tsyarn --cwd playground test before-send.scenario.ts