Skip to content

✨ forward logs from the renderer process - #212

Open
kikoveiga wants to merge 1 commit into
mainfrom
kikoveiga/renderer-logs
Open

✨ forward logs from the renderer process#212
kikoveiga wants to merge 1 commit into
mainfrom
kikoveiga/renderer-logs

Conversation

@kikoveiga

@kikoveiga kikoveiga commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Motivation

The Electron bridge already receives log events from the renderer Browser Logs SDK, but RendererPipeline discarded them. In bridge mode the Browser SDK sends logs only to the host, so installing the Electron SDK alongside @datadog/browser-logs could silently lose every renderer log.

Ticket: RUM-15536

Changes

Forward and enrich renderer logs

  • Forward valid Browser Logs payloads as ServerLogsEvent objects on a dedicated LOGS track.
  • Preserve renderer-owned fields such as date, message, status, service, ddtags, view, and action context.
  • Add Electron application and session ids, and fill user/account context only when the renderer did not provide it.
  • Keep forwarding logs that have no covering RUM session, with the Browser SDK stub session ids removed.
  • Upload with ddsource=browser while retaining DD-EVP-ORIGIN: electron for the uploader identity.

Sample logs in the host SDK

Browser Logs uses an always-tracked session stub in bridge mode, so renderer DD_LOGS.init({ sessionSampleRate }) does not sample logs sent over the bridge. This adds logsSampleRate to Electron SDK configuration:

  • accepts 0100 and defaults to 100;
  • samples each valid renderer log independently before enrichment;
  • stays independent from RUM sessionSampleRate;
  • matches the Android and iOS WebView integrations;
  • does not impose a separate relay cap on sampled-in logs.

Close reliability and API gaps

  • Initialize all transport handlers before opening the renderer IPC listener, preventing startup-time event loss.
  • Rotate LOGS batches at 1,000 entries as well as the shared byte limit, respecting the Logs intake array limit without dropping events.
  • Handle null renderer user/account context safely.
  • Reuse the common renderer emission path for RUM, telemetry, and logs.
  • Align public nested log types with Browser Logs payloads and export LogsAssembleParams from the assembly barrel.
  • Document renderer-log setup and sampling in README.md, and keep the architecture overview concise.

Verification

yarn typecheck
yarn test:unit
yarn test:e2e:init
yarn playwright test -c e2e --project=e2e e2e/scenarios/logs.scenario.ts
  • 56 unit-test files / 1,061 tests passed.
  • All 5 renderer Logs E2E scenarios passed, including logsSampleRate: 0 and a 1,001-log upload split into requests of 1,000 and 1.
  • ESLint, Prettier, and git diff --check passed for the changed files.

Checklist

  • Added unit tests.
  • Added end-to-end coverage.
  • Updated public and architecture documentation.
  • Addressed or documented the agentic review findings.

Base automatically changed from kikoveiga/renderer-telemetry to main September 3, 2026 08:30
@kikoveiga
kikoveiga force-pushed the kikoveiga/renderer-logs branch 3 times, most recently from 5357937 to 02097f5 Compare September 3, 2026 11:51
@kikoveiga
kikoveiga marked this pull request as ready for review September 3, 2026 11:54
@kikoveiga
kikoveiga requested a review from a team as a code owner September 3, 2026 11:54

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review — Score: 4.8 / 5

This is a well-executed feature PR that closes an important gap: once the Browser Logs SDK is in bridge mode, renderer logs have no other path to Datadog, and this change makes relay reliable end-to-end. The implementation follows established SDK patterns (format hooks, customer-context precedence, mobile WebView parity for logsSampleRate), fixes the init-order race by registering transport before opening the IPC listener, and is backed by thorough unit and e2e coverage including the 1,000-entry intake split. I would approve.

Why 4.8: Correct boundary validation, observable failures via telemetry, monitor() on the IPC path, intentional context-merging rules, unconditional LOGS track registration for recovery, batch rotation at the intake limit, and documentation/README updates that explain the bridge-mode sampling contract clearly.

Why not 5: The top-level monitoring architecture diagram still depicts only RUM over the bridge (logs get a prose section but not a diagram edge), and configuration telemetry has no schema field for logsSampleRate yet so usage of non-default sampling is invisible in SDK configuration events (consistent with existing telemetry constraints, but still a small observability gap).


Findings

  • [Nit] Architecture diagram omits logs — The new Logs section is accurate, but the overview Mermaid chart still labels the bridge as RUM-only; a second edge (or relabel) would match the new behavior.

Architectural flow

sequenceDiagram
    participant BL as Browser Logs SDK
    participant Bridge as DatadogEventBridge
    participant RP as RendererPipeline
    participant Hooks as Format Hooks
    participant EM as EventManager
    participant T as Transport LOGS track
    participant Intake as Datadog Logs Intake

    BL->>Bridge: assembled log event
    Bridge->>RP: IPC log message
    RP->>RP: validate date message status
    RP->>RP: apply logsSampleRate
    RP->>Hooks: triggerLogs enrichment
    Hooks-->>RP: application session usr account
    RP->>EM: ServerLogsEvent on LOGS track
    EM->>T: post to batch producer
    T->>Intake: POST /api/v2/logs with ddsource=browser
Loading

Before: Bridge log events were accepted over IPC but dropped in RendererPipeline (TODO), so renderer logs in bridge mode were lost after the Browser Logs SDK stopped talking to intake directly.

After: Valid logs are sampled via logsSampleRate, enriched with main-process session/application/user/account context while preserving renderer-owned fields (service, ddtags, view, etc.), batched on a dedicated LOGS track (splitting at 1,000 events), and uploaded with ddsource=browser and DD-EVP-ORIGIN: electron. Transport handlers are registered before the IPC listener opens so events cannot fall through an unhandled gap during init().

Open in Web View Automation 

Sent by Cursor Automation: electron-sdk reviews

Comment thread src/index.ts
Comment thread src/assembly/RendererPipeline.ts
Comment thread src/transport/batch/standard/StandardBatchProducer.ts
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-07T09:36:47.370554Z 1296494 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@kikoveiga
kikoveiga force-pushed the kikoveiga/renderer-logs branch from 02097f5 to 626f0d8 Compare September 3, 2026 12:55
@sbarrio
sbarrio requested a review from bcaudan September 4, 2026 07:04
Forward Browser Logs events received over the renderer bridge on a dedicated LOGS track so enabling the bridge no longer drops them.

Preserve browser attribution and renderer-owned fields while enriching logs with Electron application, session, user, and account context. Add host-side per-log sampling through logsSampleRate, matching the mobile WebView integrations.

Register transport before renderer IPC, split Logs uploads at the 1,000-entry intake limit, align the public payload types and exports, and cover forwarding, sampling, context, and batching with unit and end-to-end tests.

Ticket: RUM-15536

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
@kikoveiga
kikoveiga force-pushed the kikoveiga/renderer-logs branch from 626f0d8 to 1296494 Compare September 7, 2026 09:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1296494983

ℹ️ 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".

* the main process reads or replaces are named here; everything else the renderer sends is carried
* through untouched, so a field browser-core adds does not need a change on this side.
*/
export interface LogsEvent {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Add an example for the exported LogsEvent API

The new LogsEvent type is exported from src/index.ts, making it a public API, but its JSDoc has no @example. docs/REVIEW.md explicitly requires every new or modified public API to include one; add a representative Browser Logs payload example so the generated declarations meet the repository's API documentation standard.

AGENTS.md reference: AGENTS.md:L3-L6

Useful? React with 👍 / 👎.

Comment thread README.md
Comment on lines +90 to +91
Logs assembled by the Browser Logs SDK are forwarded through the Electron bridge and uploaded by the
main process. Configure renderer-log sampling on the Electron SDK:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Document the renderer-side Logs SDK initialization

When a customer follows this new section, it only shows configuring the main-process Electron SDK and never tells them to install and initialize @datadog/browser-logs. The preceding setup link is specifically for Browser RUM, and logsSampleRate alone cannot produce renderer log events, so the advertised feature silently sends nothing unless users infer this separate step; include the renderer-side Browser Logs setup or a direct link to it.

AGENTS.md reference: AGENTS.md:L3-L6

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant