Skip to content

[RUM-15104] Integrate dd-trace#95

Open
cdn34dd wants to merge 46 commits into
mainfrom
carlosnogueira/RUM-15104/integrate-dd-trace
Open

[RUM-15104] Integrate dd-trace#95
cdn34dd wants to merge 46 commits into
mainfrom
carlosnogueira/RUM-15104/integrate-dd-trace

Conversation

@cdn34dd
Copy link
Copy Markdown
Collaborator

@cdn34dd cdn34dd commented Apr 13, 2026

Motivation

Capture main-process HTTP requests as RUM resource events with trace/span IDs, enabling RUM-APM correlation without requiring a local Datadog Agent. Automatically inject the renderer bridge preload via dd-trace's BrowserWindow wrapping, replacing the SDK's own registerPreload() approach.

Changes

Core tracing integration

  • Integrate dd-trace (added as a production dependency) to instrument fetch and Electron's net module. Spans are published via dd-trace's electron exporter to a diagnostics channel, converted to RUM resources by ResourceConverter, and sent through the existing SDK transport.
  • Add ResourceConverter to convert dd-trace HTTP spans into RUM resource events (trace IDs as decimal for intake compatibility). All spans are also forwarded to the spans intake with electron context enrichment.
  • Add Tracing class that configures dd-trace plugins (electron, http), with graceful degradation if dd-trace is unavailable.
  • Add RawRumResource type and SPANS transport track.

Entry point and preload injection

  • Add @datadog/electron-sdk/instrument entry point — must be imported before electron so dd-trace can hook require('electron') and wrap BrowserWindow for automatic preload injection.
  • Remove registerPreload() — preload injection is now fully handled by dd-trace's BrowserWindow wrapping.
  • Remove tracing configuration option — tracing is enabled by default when dd-trace is available.

Bundler plugins

At runtime, dd-trace hooks require('electron') which breaks when bundlers alter module loading order or when the preload file from dd-trace isn't available in packaged apps. Two bundler plugins are provided:

  • Add @datadog/electron-sdk/vite-plugin (datadogVitePlugin) — externalizes dd-trace to prevent Vite's require hoisting from breaking hook registration, prepends dd-trace initialization before hoisted requires, copies dd-trace and its transitive dependencies into the build output for packaged apps, and emits dd-trace's preload script at the fallback path.
  • Add @datadog/electron-sdk/webpack-plugin (DatadogWebpackPlugin) — copies dd-trace's preload script into the webpack output at the fallback path dd-trace expects in packaged apps, and automatically excludes dd-trace from @vercel/webpack-asset-relocator-loader which would otherwise crash on dd-trace's dynamic require patterns.

Build and packaging

  • Add @rollup/plugin-json (required by dd-trace)
  • Move dd-trace from devDependencies to dependencies so it's installed transitively in consumer apps
  • Add rollup build configs, package exports, and type declarations for both plugins

Integration test updates

  • Update all integration app main entry points to use import '@datadog/electron-sdk/instrument' before electron
  • Remove manual import '@datadog/electron-sdk/preload' from all integration app preloads
  • Add datadogVitePlugin() to forge-vite, electron-vite, and electron-builder-vite configs
  • Add DatadogWebpackPlugin to forge-webpack config

Checklist

  • Tested locally (playground)
  • Added unit tests for this change.
  • Added e2e/integration tests for this change.
  • Updated related documentation.

cdn34dd added 7 commits April 23, 2026 10:13
…jection

- Bundle dd-trace to capture main-process HTTP requests (fetch, net) as
RUM resource events with trace/span IDs for APM correlation.
- Uses dd-trace's electron exporter to publish spans via diagnostics
channel instead of requiring a local Datadog Agent.
- Add @datadog/electron-sdk/init entry point that must be imported
before electron —  so it can wrap BrowserWindow for automatic preload
injection in both bundled and non-bundled environments.
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-15104/integrate-dd-trace branch 3 times, most recently from 528c305 to 147cba6 Compare April 27, 2026 12:07
cdn34dd added 7 commits April 30, 2026 11:13
dd-trace must be available at runtime in packaged Electron apps for
preload injection and tracing to work. Moving it to dependencies ensures
it's installed transitively when the SDK is consumed.

Also updates dd-trace.tgz with latest changes (preload path resolution
fix, flagging_provider optional dependency fix, BigInt coercion fix).
Vite hoists all top-level require() calls to the start of the bundle,
breaking dd-trace's module hooking which requires require('electron')
to run after dd-trace's hooks are registered.

The datadogVitePlugin:
- Externalizes dd-trace and @datadog/electron-sdk to prevent hoisting
- Prepends dd-trace initialization before any hoisted requires
- Copies dd-trace + transitive deps into build output for packaged apps
- Emits preload script at dd-trace's fallback path
In packaged Electron apps, dd-trace's preload file resolution fails
because node_modules is absent. dd-trace falls back to
path.join(__dirname, 'electron', 'preload.js').

The DatadogWebpackPlugin:
- Copies dd-trace's preload script to that fallback path in webpack output
- Auto-excludes dd-trace from @vercel/webpack-asset-relocator-loader which
  crashes on dd-trace's dynamic require patterns
Add rollup build entries, TypeScript declarations, package.json exports,
typesVersions, and files entries for both vite-plugin and webpack-plugin.

Update instrument.ts JSDoc to reference both bundler plugins.
- forge-vite, electron-vite, electron-builder-vite: add datadogVitePlugin()
- forge-webpack: add DatadogWebpackPlugin, remove manual asset-relocator exclude
- README: add Bundler Plugins section with Vite and Webpack usage examples
- ARCHITECTURE: rewrite preload injection section to explain both plugins
- Integration README: mention both plugins in toolchain overview
Preload injection is now fully owned by dd-trace's BrowserWindow wrapping.
Remove the SDK's own preload entry point, bridge implementation, and tests.
Clean up rollup build config, package.json exports, and stale comments.
@cdn34dd cdn34dd force-pushed the carlosnogueira/RUM-15104/integrate-dd-trace branch from 147cba6 to d347711 Compare April 30, 2026 12:02
cdn34dd added 2 commits April 30, 2026 17:35
- Replace broad localhost/127.0.0.1 filtering with precise hostname
  matching against the configured intake (browser-intake-{site}) and
  proxy URL. User HTTP requests to localhost are no longer dropped.
- Override span service name with the SDK config value so all spans
  use the same service as RUM events, instead of dd-trace's default
  (which picks up the package.json name).
- Extract ResourceConverterConfig interface to reduce constructor args.
@cdn34dd
Copy link
Copy Markdown
Collaborator Author

cdn34dd commented Apr 30, 2026

@codex review

Copy link
Copy Markdown

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

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: 393ef1e466

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

Comment thread src/domain/tracing/Tracing.ts Outdated
Comment thread src/domain/tracing/ResourceConverter.ts Outdated
Comment thread src/domain/tracing/ResourceConverter.ts Outdated
Comment thread LICENSE-3rdparty.csv Outdated
Comment thread README.md Outdated
Comment thread README.md
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread src/domain/rum/view/ViewContext.ts
Comment thread src/assembly/hooks.ts Outdated
Comment thread e2e/integration/apps/electron-builder-vite/src/main.ts Outdated
Comment thread src/entries/vite-plugin.ts
Comment thread src/entries/webpack-plugin.ts
cdn34dd added 4 commits May 20, 2026 09:05
The .mjs builds use bare require('dd-trace') which doesn't exist in ESM
modules. When loaded via import (e.g. Electron Forge with webpack, or
esbuild ESM output), the calls silently fail in try/catch blocks
preventing dd-trace initialization and disabling the ResourceConverter.

Use createRequire(import.meta.url) for ESM context in both instrument.ts
and Tracing.ts, matching the pattern already used in the vite plugin.
esbuild hoists ESM import statements before module body code, so import
'@datadog/electron-sdk/instrument' cannot guarantee execution before
import 'electron'. The plugin prepends a banner that initializes
dd-trace via synchronous require(), and externalizes dd-trace and
@datadog/electron-sdk.
The published dd-trace has optional peer dependencies (e.g.
@openfeature/server-sdk) that webpack fails to resolve when bundling.

Externalize dd-trace and @datadog/electron-sdk so webpack leaves them as
runtime require() calls, and copy their node_modules into the webpack
output so they are available in packaged apps where the project's
node_modules is absent.
@cdn34dd cdn34dd marked this pull request as ready for review May 20, 2026 08:48
@cdn34dd cdn34dd requested a review from a team as a code owner May 20, 2026 08:48
Copy link
Copy Markdown

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

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: 8fa4262c1b

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

Comment thread src/entries/vite-plugin.ts Outdated
@bcaudan bcaudan force-pushed the carlosnogueira/RUM-15104/integrate-dd-trace branch from 080a3f6 to 2ad720f Compare May 20, 2026 12:08
Comment thread docs/ARCHITECTURE.md

- **Vite** hoists all `require()` calls to the top of the bundle, breaking import order. The `datadogVitePlugin` (`@datadog/electron-sdk/vite-plugin`) fixes this by externalizing dd-trace, prepending initialization before hoisted requires, and copying dd-trace's runtime dependencies into the build output for packaged apps.
- **Webpack** preserves module execution order (lazy evaluation via `__webpack_require__`), so the import order in source code is maintained. The `DatadogWebpackPlugin` (`@datadog/electron-sdk/webpack-plugin`) copies dd-trace's preload script into the webpack output at the fallback path dd-trace expects in packaged apps.
- **ESBuild** TODO
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@cdn34dd I'll let you add the details for esbuild

cdn34dd and others added 20 commits May 21, 2026 07:58
Update vite and webpack plugins to initialize dd-trace automatically via
a banner, matching the esbuild plugin's approach. Users no longer need
to  add `import '@datadog/electron-sdk/instrument'` to their source
code.
- new resource.scenario.ts covering fetch, http.request, net.request
- TestServer fixture as a controllable request target
- host env allowlist for the Electron child process
- dd-trace exporter flushed from _flushTransport
- `computeIntakeUrlForTrack` now appends `?ddforward=%2Fapi%2Fv2%2F{track}` when proxy is set
- e2e proxy config updated to base URL only
- unit tests updated accordingly
Detect the Rollup output format in renderChunk and use an ESM-safe
banner when format is 'es'. In ESM, dd-trace's IITM hooks cannot wrap
BrowserWindow because static imports are loaded before module code
evaluates, so the ESM banner registers the preload script directly via
session.defaultSession.registerPreloadScript() on app ready.

This is needed for electron-vite which, unlike Forge's vite plugin,
allows configuring ESM output for the main process.
Copy of the electron-vite integration app configured with ESM output
(format: 'es') to verify the vite plugin's ESM banner and preload
registration work in both dev and packaged modes.
- forge-esbuild-cjs: esbuild → CJS, exercises the SDK plugin's CJS banner + dd-trace IITM require('electron') wrap
- forge-esbuild-esm: esbuild → ESM main, CJS preload, ESM renderer; exercises the plugin's ESM branch (createRequire + session.registerPreloadScript)
- both packaged via Electron Forge (no Forge bundler plugin) and wired into INTEGRATION_APPS for auto-discovery
- Fix intake hostname normalization for subdomain sites (e.g.
us3.datadoghq.com → browser-intake-us3-datadoghq.com) by extracting
computeIntakeHostname and reusing it in ResourceConverter
- Use either proxy or intake hostname for SDK request filtering, not
both
- Set startTime on RUM resource events from span.start so Assembly
resolves view/session context against the request start time
- Replace manual try/catch + addError with monitor() wrapper
- Replace ResourceConverterConfig with Configuration
- Use branded TimeStamp and ServerDuration types in RawRumResource
- Remove displayInfo debug logs from ResourceConverter
- Add console.warn when dd-trace is not found in bundler plugins and
instrument entry point
- Add console.warn when copyPackageTree fails in vite and webpack
plugins
- Import computeIntakeHostname from transport barrel instead of internal
module
- Rename class and files to reflect broader responsibility (filtering,
enriching, emitting resources, and forwarding span envelopes)
- Extract onMessage handler into  private methods: processTrace,
isSdkRequest, enrichSpan, emitResource, emitTraceEnvelope
- Update references in index.ts, Tracing.ts, and ARCHITECTURE.md
Replaces the `sw_vers` subprocess call with Electron's
`process.getSystemVersion()`, which is synchronous and in-process.
Makes `getUserAgent` synchronous and avoids polluting customer traces
with an SDK-internal `child_process` span.
+ span processor namings
+ span hooks
@bcaudan bcaudan force-pushed the carlosnogueira/RUM-15104/integrate-dd-trace branch from be5bd00 to f4b6afb Compare May 22, 2026 14:39
Comment on lines +153 to +154
copyPackageTree('dd-trace');
copyPackageTree('@datadog/electron-sdk');
Copy link
Copy Markdown
Collaborator

@bcaudan bcaudan May 22, 2026

Choose a reason for hiding this comment

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

💭 thought:‏copyPackageTree('dd-trace', ...) makes sense, dd-trace needs to be available at runtime in packaged apps where node_modules isn't included.

copyPackageTree('@datadog/electron-sdk', ...) feels a bit odd though: it looks like the SDK ends up copying itself into its own output. IIUC, this is a consequence of externalizing @datadog/electron-sdk so the banner require("@datadog/electron-sdk/instrument") can resolve at runtime.

One option worth considering: inlining the instrument.ts logic directly into the CJS banner:

const CJS_BANNER = 'try{require("dd-trace").default.init({experimental:{exporter:"electron"}})}catch{}';

This would remove @datadog/electron-sdk from both the external list and copyPackageTree, since the banner no longer needs to resolve it at runtime, only dd-trace would need special treatment.

The main tradeoff is that the banner and instrument.ts would duplicate the init logic, so if the init options ever expand both would need updating. Is instrument.ts expected to stay this minimal long-term? If so, inlining might be worth it to avoid the self-copy.
Wdyt?

Comment thread src/index.ts

if (tracing.enabled) {
new SpanProcessor(eventManager, hooks, config);
displayInfo('Tracing enabled');
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💬 suggestion: ‏do we need to keep it?


switch (bridgeEvent.eventType) {
case 'rum':
displayInfo('Bridge received renderer RUM event:', (bridgeEvent.event as { type?: string })?.type);
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

💬 suggestion: ‏do we need to keep it?

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.

2 participants