[RUM-14998] IPC Renderer process support#38
Conversation
This comment has been minimized.
This comment has been minimized.
ad16b19 to
b0594e7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0594e75f3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
059773f to
267768a
Compare
|
|
||
| ### Renderer Process Support | ||
|
|
||
| The SDK automatically registers a preload script that exposes a `DatadogEventBridge` to every renderer process. When `@datadog/browser-rum` is initialized in a renderer, it detects the bridge and routes events through IPC to the main process instead of posting directly to the intake. |
There was a problem hiding this comment.
💬 suggestion: adding a note on the need to have browser SDK
| The SDK automatically registers a preload script that exposes a `DatadogEventBridge` to every renderer process. When `@datadog/browser-rum` is initialized in a renderer, it detects the bridge and routes events through IPC to the main process instead of posting directly to the intake. | |
| In order to monitor the renderer process, the [Browser SDK](https://docs.datadoghq.com/real_user_monitoring/application_monitoring/browser/setup/) must be setup in pages loaded by the renderer. | |
| The Electron SDK automatically registers a preload script that exposes a `DatadogEventBridge` to every renderer process. When present, the Browser SDK detects the bridge and routes events through IPC to the Electron SDK instead of sending them directly to Datadog servers. |
|
|
||
| Both `contextIsolation: true` (default) and `contextIsolation: false` are supported. | ||
|
|
||
| > **Note:** Only the automatic preload mode is currently supported. Manual bridge setup is not yet available. |
There was a problem hiding this comment.
🥜 nitpick: Unless there is a specific reason to mention it, let's not mention manual bridge setup
|
|
||
| The SDK automatically registers a preload script that exposes a `DatadogEventBridge` to every renderer process. When `@datadog/browser-rum` is initialized in a renderer, it detects the bridge and routes events through IPC to the main process instead of posting directly to the intake. | ||
|
|
||
| Both `contextIsolation: true` (default) and `contextIsolation: false` are supported. |
There was a problem hiding this comment.
❓ question: Does it really works with contextIsolation: false?
There was a problem hiding this comment.
Yes it does, I tested on the playground app to make sure.
There was a problem hiding this comment.
ok, let's add a test for it then and maybe remove this mention.
| new BridgeHandler(eventManager, { | ||
| defaultPrivacyLevel: config.defaultPrivacyLevel, | ||
| allowedWebViewHosts: config.allowedWebViewHosts, | ||
| }); |
There was a problem hiding this comment.
🥜 nitpick: we could use config directly as a dependency, it would avoid the noise here and the need to update this file if BridgeHandler need an extra config parameter
| const currentDir = typeof __dirname !== 'undefined' ? __dirname : path.dirname(fileURLToPath(import.meta.url)); | ||
| const preloadPath = path.join(currentDir, 'preload-auto.cjs'); | ||
| session.defaultSession.registerPreloadScript({ | ||
| type: 'frame', | ||
| filePath: preloadPath, | ||
| }); |
There was a problem hiding this comment.
💬 suggestion: what about extracting that to a different file?
It would keep the init function at a higher level of abstraction.
| if (startTime !== undefined) { | ||
| await this.page.evaluate( | ||
| (ts) => (globalThis as unknown as ElectronAppWindow).electronAPI.generateManualError(ts), | ||
| startTime | ||
| ); | ||
| } else { | ||
| await this.page.locator('#generate-manual-error').click(); | ||
| } |
There was a problem hiding this comment.
❓ question: What is the need there?
There was a problem hiding this comment.
The main reason is test, the 'click' option is what most e2e test use, the startTime override is required for test that assert timing.
There was a problem hiding this comment.
Is it related to this PR though?
FMU, it was working by calling directly the electronAPI all the time and the button was not really needed, so not sure why introducing the button.
| async openFileWindow() { | ||
| await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.openFileWindow()); | ||
| } | ||
|
|
||
| async openHttpWindow() { | ||
| await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.openHttpWindow()); | ||
| } |
There was a problem hiding this comment.
💬 suggestion: It could be less confusing to have all this setup mentioning renderer test, something like:
| async openFileWindow() { | |
| await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.openFileWindow()); | |
| } | |
| async openHttpWindow() { | |
| await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.openHttpWindow()); | |
| } | |
| async openRendererTestWindowOverFile() { | |
| await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.openRendererTestWindowOverFile()); | |
| } | |
| async openRendererTestWindowOverHttp() { | |
| await this.page.evaluate(() => (globalThis as unknown as ElectronAppWindow).electronAPI.openRendererTestWindowOverHttp()); | |
| } |
There was a problem hiding this comment.
What exactly is confusing about them, I think the suggested names aren't any better.
There was a problem hiding this comment.
When discovering how things are working for the e2e tests, it is not obvious why we have different browser windows and which files / APIs are related to those.
Having something in the name that mention renderer would make that clearer IMO.
There was a problem hiding this comment.
Ok, I'll change to have renderer in the name, but to something a little less verbose.
| this.eventManager.notify({ | ||
| kind: EventKind.RAW, | ||
| source: EventSource.RENDERER, | ||
| format: EventFormat.RUM, | ||
| data: bridgeEvent.event, | ||
| } as RawRumEvent); |
| const { session, application } = (hookResult ?? {}) as MainProcessAttributes; | ||
|
|
||
| return { | ||
| kind: EventKind.SERVER, | ||
| track: EventTrack.RUM, | ||
| data: combine(event.data, { | ||
| session: { id: session?.id }, | ||
| application: { id: application?.id }, | ||
| }) as RumEvent, | ||
| }; |
There was a problem hiding this comment.
💬 suggestion: I was a bit confused by the MainProcessAttributes type with the optional properties.
What about removing the type and make things clearer in the implementation:
| const { session, application } = (hookResult ?? {}) as MainProcessAttributes; | |
| return { | |
| kind: EventKind.SERVER, | |
| track: EventTrack.RUM, | |
| data: combine(event.data, { | |
| session: { id: session?.id }, | |
| application: { id: application?.id }, | |
| }) as RumEvent, | |
| }; | |
| const { session, application } = hookResult ?? {}; | |
| const mainProcessAttributes = { | |
| session: { id: session?.id }, | |
| application: { id: application?.id }, | |
| }; | |
| return { | |
| kind: EventKind.SERVER, | |
| track: EventTrack.RUM, | |
| // override some renderer event attributes by main process attributes | |
| data: combine(event.data, mainProcessAttributes) as RumEvent, | |
| }; |
- Integrate browser SDK - Track requests and user actions
267768a to
8dccb25
Compare
8dccb25 to
ae817d6
Compare

Motivation
Adds support for capturing RUM events from renderer processes by bridging the browser SDK to the main process event pipeline.
Changes
src/preload/bridge.ts) that exposes aDatadogEventBridgeto renderer processes, allowing the browser RUM SDK (@datadog/browser-rum) to send events through IPC instead of posting directly to the intakeBridgeHandlerin the main process to receive renderer events via IPC and route them through the existingEventManagerpipelinedist/preload-auto.cjsand automatically registered viasession.registerPreloadScript()duringinit()@datadog/browser-rumin the renderer, with UI controls for fetch demos and error generationChecklist