Skip to content

Switch from dd-trace to dd-trace-electron - #186

Open
rochdev wants to merge 3 commits into
mainfrom
rochdev/dd-trace-electron
Open

Switch from dd-trace to dd-trace-electron#186
rochdev wants to merge 3 commits into
mainfrom
rochdev/dd-trace-electron

Conversation

@rochdev

@rochdev rochdev commented Jul 27, 2026

Copy link
Copy Markdown
Member

Summary

  • Replace the dd-trace dependency with dd-trace-electron, an internal-use build of dd-trace published from dd-trace-js specifically for this SDK
  • Update instrument-prelude.ts, instrument.ts, and Tracing.ts to require/type dd-trace-electron instead of dd-trace
  • Update all three bundler plugins (Vite, Webpack, esbuild) to externalize and copyPackageTree for dd-trace-electron
  • Update rollup.config.mjs external list and the browserWindow.spec.ts mock accordingly
  • Update docs/ARCHITECTURE.md to describe the new dependency and clarify that the copyPackageTree optional-dependency filtering is now defense-in-depth rather than a required exclusion, since dd-trace-electron's package.json doesn't declare OpenTelemetry/OpenFeature/ASM/IAST/profiling native modules as dependencies at all
  • Allowlist dd-trace-electron (all versions) in .yarnrc.yml across the root, e2e/app, playground, and the e2e/integration test apps, to bypass Yarn's npmMinimalAgeGate for this internal-only package

Why

The SDK previously bundled the public dd-trace package, which pulls in a large surface of optional dependencies it never needs for Electron (OpenFeature, ASM, IAST, profiling native modules). This meant bundler externalization carried extra weight: the Vite/Webpack/esbuild plugins' copyPackageTree helper had to explicitly skip optionalDependencies when copying dd-trace's tree into packaged apps, to avoid dragging in ~84 MB of unused native modules. Also on load, dd-trace's entrypoint ran the openfeature/register step, which cost a Yarn PnP-unfriendly resolution and could error when OpenFeature deps aren't installed.

dd-trace-electron is a build published from the dd-trace-js repo specifically for this SDK's use case ("Internal use only... intended to be used by the Datadog Electron SDK"). Its entrypoint skips openfeature/register entirely, and its package.json declares none of dd-trace's OpenTelemetry/OpenFeature/ASM/IAST/profiling optional dependencies — only the minimal runtime deps the Electron exporter needs (dc-polyfill, opentracing, import-in-the-middle). Switching to it removes the need to special-case optionalDependencies in the bundler plugins' package-copying logic (now defense-in-depth rather than load-bearing), and shrinks what gets copied into packaged apps from ~84 MB of optional native modules down to nothing extra.

This PR was generated by Claude Code.

Test plan

  • yarn build — dist output verified clean of stray dd-trace literal references (only legitimate descriptive comments about dd-trace's product behavior remain)
  • yarn typecheck — no errors in any touched files
  • yarn eslint — clean on all touched files
  • yarn vitest run — affected unit tests (browserWindow, Tracing) pass
  • Manual smoke test in a sample Electron app (not yet done)

dd-trace-electron is an internal-use build of dd-trace published from
dd-trace-js specifically for this SDK. Its entrypoint skips the
openfeature/register step and its package.json declares no
OpenTelemetry/OpenFeature/ASM/IAST/profiling native modules at all, so
the bundler plugins' optional-dependency filtering (copyPackageTree
only walking `dependencies`) is now defense-in-depth rather than a
required exclusion mechanism. Using a package dedicated to this SDK
also means it can never be deduplicated with a customer's own direct
dd-trace dependency.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@datadog-datadog-prod-us1

This comment has been minimized.

rochdev and others added 2 commits July 27, 2026 18:58
Prettier formatting was off in the dd-trace-electron doc update,
failing CI's format check. e2e/app and playground reference
@datadog/electron-sdk via portal, so their lockfiles needed
regenerating to reflect the dd-trace -> dd-trace-electron swap.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
dd-trace-electron is internal-use-only, published from dd-trace-js
specifically for this SDK, and is never a supply-chain risk. Pinning
the allowlist to one version meant every future bump would require
re-editing 10 .yarnrc.yml files to dodge Yarn's npmMinimalAgeGate.
Dropping the version suffix matches it against any version. Also
extends the allowlist to e2e/app, playground, and the e2e/integration
test apps' own .yarnrc.yml, whose installs hit the same quarantine
error.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@rochdev
rochdev marked this pull request as ready for review July 27, 2026 23:14
@rochdev
rochdev requested a review from a team as a code owner July 27, 2026 23:14

@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.5 / 5

Solid dependency swap with consistent updates across bundler plugins, instrumentation entry points, lockfiles, and integration-app Yarn config. The expanded ARCHITECTURE.md tracing section clearly explains why dd-trace-electron exists and how copyPackageTree behavior changes. I would approve this PR; the remaining gaps are documentation polish and the manual smoke test the author already flagged.

Why 4.5: Mechanical rename is thorough (all three bundler plugins, rollup externals, prelude/instrument, Tracing.ts, unit-test mock, e2e .yarnrc.yml allowlists). The architecture write-up captures the deduplication benefit and the defense-in-depth shift for optional-dependency filtering. npmPreapprovedPackages is the right tool for a freshly published internal package under Renovate's age gate.

Why not 5: Customer-facing README.md still describes dd-trace throughout (setup diagram, bundler section). The Monitoring Architecture diagram at the top of ARCHITECTURE.md was not updated to match the new tracing section. No CHANGELOG entry yet for a notable runtime dependency change, and the manual smoke test in the PR test plan is still unchecked.


Findings

  • [Minor] README still references dd-trace — The primary getting-started doc, architecture diagram, and bundler-plugin section all still name dd-trace instead of dd-trace-electron.
  • [Minor] ARCHITECTURE overview diagram stale — The Monitoring Architecture mermaid block at the top of docs/ARCHITECTURE.md (and its narrative) still labels the tracer as dd-trace while the tracing section below was updated.
  • [Minor] Missing CHANGELOG entry — Swapping the core tracing dependency is customer-visible; worth a note under the next release.
  • [Nit] Residual dd-trace mentionse2e/integration/README.md and instrument-prelude.spec.ts test titles still say dd-trace.

Architectural flow

sequenceDiagram
    participant App as Main app
    participant Plugin as Bundler plugin
    participant Inst as instrument entry
    participant DTE as dd-trace-electron
    participant Patches as SDK patches
    participant Chan as diagnostics_channel
    participant SP as SpanProcessor

    App->>Plugin: build main bundle
    Plugin->>Plugin: externalize dd-trace-electron
    Plugin->>Plugin: copyPackageTree minimal deps

    App->>Inst: import instrument first
    Inst->>DTE: require (skips OpenFeature register)
    Inst->>DTE: tracer.init electron exporter
    Inst->>Patches: patch BrowserWindow, ipc, net

    Patches->>DTE: startSpan / inject / extract
    DTE->>Chan: datadog apm electron export
    Chan->>SP: finished spans
    SP->>SP: enrich and convert to RUM resources
Loading

Before: The SDK depended on the public dd-trace package. Its entrypoint ran openfeature/register on load, and its package.json declared large optional native modules. Bundler plugins had to externalize dd-trace and rely on copyPackageTree walking only dependencies to keep ~84 MB of optional modules out of packaged apps.

After: The SDK depends on dd-trace-electron, a purpose-built build with a slimmer entrypoint and no OpenFeature/ASM/IAST/profiling deps in its tree. Bundler plugins externalize and copy dd-trace-electron instead; optional-dependency filtering remains as defense-in-depth. Using a distinct package name also prevents Yarn/npm from deduplicating the SDK tracer with a customer's own direct dd-trace install.

Open in Web View Automation 

Sent by Cursor Automation: electron-sdk reviews

Comment thread docs/ARCHITECTURE.md
### dd-trace-electron as a bundled dependency

dd-trace is declared as a **direct runtime dependency** in `package.json`, not as an optional or peer dependency. When customers install `@datadog/electron-sdk`, they get dd-trace automatically.
The SDK depends on [`dd-trace-electron`](https://www.npmjs.com/package/dd-trace-electron), an

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The new tracing dependency section is thorough and well reasoned. For consistency, consider also updating the Monitoring Architecture diagram and narrative at the top of this file (still labeled dd-trace on lines 15 and 48) and the customer-facing README.md, which still describes dd-trace in setup and bundler docs.

Comment thread package.json
},
"dependencies": {
"dd-trace": "5.109.0"
"dd-trace-electron": "5.118.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pinning 5.118.0 keeps the SDK on the same major line as the previous dd-trace@5.109.0, but this still spans nine dd-trace minor releases. Worth a brief sanity check that nothing in the 5.1105.118 range affects the electron exporter or diagnostics channel contract the SDK relies on (the unchecked manual smoke test in the PR description would cover this).

@sbarrio
sbarrio requested a review from cdn34dd July 28, 2026 07:08

@cdn34dd cdn34dd left a comment

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.

Overall it's a good improvement, left a couple of comments regarding the new library and some questions regarding how future features will be handled.

Comment thread e2e/app/.yarnrc.yml
Comment on lines +3 to +5

npmPreapprovedPackages:
- dd-trace-electron

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.

Running yarn test:integration:init && yarn test:integration on the repo root, seems to leave hanging .yarnrc.yml instead of a clean tree, we should check what's causing it.

Comment thread package.json
},
"dependencies": {
"dd-trace": "5.109.0"
"dd-trace-electron": "5.118.0"

@cdn34dd cdn34dd Aug 3, 2026

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.

When looking at the dependencie each variant of dd-trace has, there is a drastic reduction on the number of dependencies.

npm view dd-trace@5.109.0 dependencies optionalDependencies peerDependencies --json
{
  "dependencies": {
    "dc-polyfill": "^0.1.11",
    "opentracing": ">=0.14.7",
    "import-in-the-middle": "^3.0.1"
  },
  "optionalDependencies": {
    "oxc-parser": "^0.132.0",
    "@datadog/pprof": "5.15.0",
    "@opentelemetry/api": ">=1.0.0 <1.10.0",
    "@datadog/libdatadog": "0.9.4",
    "@datadog/native-appsec": "11.0.1",
    "@datadog/native-metrics": "3.1.2",
    "@opentelemetry/api-logs": "<1.0.0",
    "@datadog/wasm-js-rewriter": "5.0.1",
    "@datadog/openfeature-node-server": "2.0.0",
    "@datadog/native-iast-taint-tracking": "4.2.0"
  }
}

when compared to the new electron variant:

npm view dd-trace-electron@5.118.0 dependencies optionalDependencies peerDependencies --json
{
  "dependencies": {
    "dc-polyfill": "^0.1.11",
    "import-in-the-middle": "^3.3.2",
    "opentracing": ">=0.14.7"
  },
  "optionalDependencies": {
    "@opentelemetry/api": ">=1.0.0 <1.10.0",
    "@opentelemetry/api-logs": "<1.0.0"
  }
}

but is this as lean as this new variant can be, for instance, does it really need to include the optionalDependencies ?

Comment thread package.json
},
"dependencies": {
"dd-trace": "5.109.0"
"dd-trace-electron": "5.118.0"

@cdn34dd cdn34dd Aug 3, 2026

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.

Let's say at some point we need to add more feature to dd-trace-electron, for instance main process profiling, what is the current plan ? Is it to include it in this package or simply create a new package (dd-trace-electron-profiling) ?

Comment thread package.json
},
"dependencies": {
"dd-trace": "5.109.0"
"dd-trace-electron": "5.118.0"

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.

Coming back to the package dependencies, now that the electron-sdk owns the net and IPC instrumentation and dd-trace is used for span creation, context and the electron exporter, could we double-check whether all its regular runtime dependencies are still required?

  "dependencies": {
    "dc-polyfill": "^0.1.11",
    "import-in-the-middle": "^3.3.2",
    "opentracing": ">=0.14.7"
  }

In particular, it would be useful to understand which runtime paths still require each dependency and whether any could be removed from an Electron-specific build.

I'm asking because we’re trying to simplify the bundler plugins and evaluating whether electron-sdk and potentially dd-trace-electron could stop being externalized. Removing these runtime dependencies could make bundling both packages much simpler.

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