Skip to content

Commit 63247fa

Browse files
committed
fix(esbuild,webpack): bundle the OpenTelemetry API fallback when the app has no copy
The plugins marked @opentelemetry/api and @opentelemetry/api-logs external unconditionally, so a bundle that used the OpenTelemetry bridge but was deployed without the packages in node_modules threw at runtime on the fallback require. Externalizing is only needed when the application owns a copy the instrumentation must capture and share; a package the application does not declare has no competing copy to protect. Externalize a package only when the application's package.json declares it, and otherwise leave it in the bundle so dd-trace's own copy is inlined and the bundle stays self-contained. When the manifest cannot be read, keep externalizing so the shared-copy correctness is never traded for the self-contained-bundle optimization. Fixes: #6882
1 parent 37cd972 commit 63247fa

7 files changed

Lines changed: 193 additions & 22 deletions

File tree

docs/API.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,8 @@ const tracerProvider = new tracer.TracerProvider()
398398
tracerProvider.register()
399399
```
400400

401+
dd-trace binds to the application's own `@opentelemetry/api` copy when it is present and falls back to the copy it bundles otherwise. If you require `@opentelemetry/api` yourself, require it before creating the `TracerProvider` so the bridge registers on the same copy your code reads. The `dd-trace/esbuild` and `dd-trace/webpack` plugins mark a package external only when your application depends on it, so its single runtime copy is shared with the bridge; keep those resolvable at runtime. A package your application does not depend on is bundled from dd-trace's own copy, so no extra runtime dependency is needed.
402+
401403
The following attributes are available to override Datadog-specific options:
402404

403405
* `service.name`: The service name to be used for this span. The service name from the tracer will be used if this is not provided.

packages/datadog-esbuild/index.js

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const {
1313
matchesOptionalPeerFile,
1414
rewriteOptionalPeerLoads,
1515
} = require('../datadog-instrumentations/src/helpers/optional-peer-bundler')
16+
const { otelApiPackagesToExternalize } = require('../datadog-instrumentations/src/helpers/otel-api-externals')
1617
const { processModule, isESMFile } = require('./src/utils')
1718
const log = require('./src/log')
1819

@@ -146,12 +147,14 @@ ${build.initialOptions.banner.js}`
146147
build.initialOptions.external.push('@openfeature/core')
147148
}
148149

149-
// The OpenTelemetry API packages are optional peers the application owns. The bridge captures
150-
// the application's copy through the `@opentelemetry/api` instrumentation, which only fires on a
151-
// runtime require. Bundling them would inline a second copy that the instrumentation never sees,
152-
// so the bridge would register its provider on the wrong copy and silently downgrade every span
153-
// to a no-op (issue #6882). Mark them external so the require survives for interception.
154-
for (const otelApiPackage of ['@opentelemetry/api', '@opentelemetry/api-logs']) {
150+
// Keep an OpenTelemetry API package external only when the application declares its own copy: the
151+
// bridge captures that copy through the `@opentelemetry/api` instrumentation, which fires on a
152+
// runtime require, so bundling it would inline a second copy the instrumentation never sees and the
153+
// bridge would register on the wrong one, downgrading every span to a no-op (issue #6882). A
154+
// package the application does not declare is left to bundle, so dd-trace's own fallback copy is
155+
// inlined and the bundle stays self-contained.
156+
const workingDir = build.initialOptions.absWorkingDir || process.cwd()
157+
for (const otelApiPackage of otelApiPackagesToExternalize(workingDir)) {
155158
build.initialOptions.external ??= []
156159
build.initialOptions.external.push(otelApiPackage)
157160
externalModules.add(otelApiPackage)

packages/datadog-esbuild/test/plugin.spec.js

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict'
22

33
const assert = require('node:assert/strict')
4+
const fs = require('node:fs')
5+
const os = require('node:os')
6+
const path = require('node:path')
47
const { describe, it } = require('mocha')
58

69
const ddPlugin = require('../index')
@@ -24,6 +27,8 @@ function setupExternal (initialOptions = {}) {
2427

2528
describe('datadog-esbuild plugin', () => {
2629
describe('OpenTelemetry API externalization', () => {
30+
// The repo's own package.json declares both packages, so a build run from the repo root
31+
// externalizes both (they are the application's declared copies).
2732
it('marks both OpenTelemetry API peers external so the bundle shares the application copy', () => {
2833
const external = setupExternal()
2934

@@ -38,6 +43,29 @@ describe('datadog-esbuild plugin', () => {
3843
assert.ok(external.includes('@opentelemetry/api'), 'should externalize @opentelemetry/api')
3944
assert.ok(external.includes('@opentelemetry/api-logs'), 'should externalize @opentelemetry/api-logs')
4045
})
46+
47+
it('bundles a package the application does not depend on so the bundle stays self-contained', () => {
48+
const workingDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-esbuild-otel-'))
49+
fs.writeFileSync(path.join(workingDir, 'package.json'), JSON.stringify({ name: 'app', dependencies: {} }))
50+
51+
const external = setupExternal({ absWorkingDir: workingDir }) ?? []
52+
53+
assert.ok(!external.includes('@opentelemetry/api'), 'should bundle @opentelemetry/api')
54+
assert.ok(!external.includes('@opentelemetry/api-logs'), 'should bundle @opentelemetry/api-logs')
55+
})
56+
57+
it('externalizes only the package the application declares', () => {
58+
const workingDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-esbuild-otel-'))
59+
fs.writeFileSync(
60+
path.join(workingDir, 'package.json'),
61+
JSON.stringify({ name: 'app', dependencies: { '@opentelemetry/api': '^1.9.0' } })
62+
)
63+
64+
const external = setupExternal({ absWorkingDir: workingDir }) ?? []
65+
66+
assert.ok(external.includes('@opentelemetry/api'), 'should externalize the declared copy')
67+
assert.ok(!external.includes('@opentelemetry/api-logs'), 'should bundle the undeclared copy')
68+
})
4169
})
4270

4371
describe('optional peer bundling', () => {
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
'use strict'
2+
3+
const fs = require('node:fs')
4+
const path = require('node:path')
5+
6+
const OTEL_API_PACKAGES = ['@opentelemetry/api', '@opentelemetry/api-logs']
7+
8+
/**
9+
* Decide which OpenTelemetry API packages a bundle must keep external.
10+
*
11+
* The bridge captures the application's own copy through require interception, which only fires on a
12+
* runtime require. Bundling a copy the application also owns would inline a second copy the
13+
* interception never sees, so the bridge would register its provider on the wrong copy and silently
14+
* downgrade every span to a no-op (issue #6882). A package the application does not declare has no
15+
* competing copy, so it is left to bundle: dd-trace's own fallback copy is inlined and the bundle
16+
* stays self-contained, needing no `@opentelemetry/api` in `node_modules` at runtime.
17+
*
18+
* @param {string} workingDir Directory whose `package.json` lists the application's dependencies.
19+
* @returns {string[]} The subset of `OTEL_API_PACKAGES` to mark external.
20+
*/
21+
function otelApiPackagesToExternalize (workingDir) {
22+
const declared = readDeclaredDependencies(workingDir)
23+
// A missing or unreadable manifest is inconclusive, so err toward external: sharing the
24+
// application's copy is the correctness-preserving default and only costs the self-contained-bundle
25+
// optimization when the application in fact owns no copy.
26+
if (!declared) return OTEL_API_PACKAGES
27+
return OTEL_API_PACKAGES.filter(name => declared.has(name))
28+
}
29+
30+
/**
31+
* @param {string} workingDir
32+
* @returns {Set<string> | undefined} All declared dependency names, or `undefined` when the manifest
33+
* cannot be read.
34+
*/
35+
function readDeclaredDependencies (workingDir) {
36+
let manifest
37+
try {
38+
manifest = JSON.parse(fs.readFileSync(path.join(workingDir, 'package.json'), 'utf8'))
39+
} catch {
40+
return
41+
}
42+
return new Set([
43+
...Object.keys(manifest.dependencies ?? {}),
44+
...Object.keys(manifest.devDependencies ?? {}),
45+
...Object.keys(manifest.optionalDependencies ?? {}),
46+
...Object.keys(manifest.peerDependencies ?? {}),
47+
])
48+
}
49+
50+
module.exports = { OTEL_API_PACKAGES, otelApiPackagesToExternalize }
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
'use strict'
2+
3+
const assert = require('node:assert/strict')
4+
const fs = require('node:fs')
5+
const os = require('node:os')
6+
const path = require('node:path')
7+
8+
const { describe, it, beforeEach } = require('mocha')
9+
10+
const { OTEL_API_PACKAGES, otelApiPackagesToExternalize } = require('../src/helpers/otel-api-externals')
11+
12+
describe('otel-api-externals', () => {
13+
let workingDir
14+
15+
beforeEach(() => {
16+
workingDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-otel-externals-'))
17+
})
18+
19+
/**
20+
* @param {object} manifest
21+
*/
22+
function writeManifest (manifest) {
23+
fs.writeFileSync(path.join(workingDir, 'package.json'), JSON.stringify(manifest))
24+
}
25+
26+
it('externalizes only the packages the application declares', () => {
27+
writeManifest({ name: 'app', dependencies: { '@opentelemetry/api': '^1.9.0' } })
28+
29+
assert.deepStrictEqual(otelApiPackagesToExternalize(workingDir), ['@opentelemetry/api'])
30+
})
31+
32+
it('bundles every package the application does not declare', () => {
33+
writeManifest({ name: 'app', dependencies: { express: '^4.0.0' } })
34+
35+
assert.deepStrictEqual(otelApiPackagesToExternalize(workingDir), [])
36+
})
37+
38+
it('detects the packages across every dependency field', () => {
39+
writeManifest({
40+
name: 'app',
41+
devDependencies: { '@opentelemetry/api': '^1.9.0' },
42+
peerDependencies: { '@opentelemetry/api-logs': '<1.0.0' },
43+
})
44+
45+
assert.deepStrictEqual(otelApiPackagesToExternalize(workingDir), OTEL_API_PACKAGES)
46+
})
47+
48+
it('errs toward external when the manifest cannot be read', () => {
49+
// No package.json written: sharing the application copy is the correctness-preserving default.
50+
assert.deepStrictEqual(otelApiPackagesToExternalize(workingDir), OTEL_API_PACKAGES)
51+
})
52+
})

packages/datadog-webpack/index.js

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const instrumentations = require('../datadog-instrumentations/src/helpers/instru
77
const extractPackageAndModulePath = require('../datadog-instrumentations/src/helpers/extract-package-and-module-path')
88
const hooks = require('../datadog-instrumentations/src/helpers/hooks')
99
const { matchesOptionalPeerFile } = require('../datadog-instrumentations/src/helpers/optional-peer-bundler')
10+
const { otelApiPackagesToExternalize } = require('../datadog-instrumentations/src/helpers/otel-api-externals')
1011
const { isESMFile } = require('../datadog-esbuild/src/utils')
1112
const log = require('./src/log')
1213

@@ -81,20 +82,25 @@ class DatadogWebpackPlugin {
8182
}
8283
})
8384

84-
// The OpenTelemetry API packages are optional peers the application owns. The bridge captures
85-
// the application's copy through the @opentelemetry/api instrumentation, which only fires on a
86-
// runtime require. Bundling them would inline a second copy the instrumentation never sees, so
87-
// the bridge would register its provider on the wrong copy and silently downgrade every span to
88-
// a no-op (issue #6882). Mark them external so the require survives for interception. The
89-
// `commonjs` type forces a CommonJS require regardless of the user's externalsType.
90-
const externals = Array.isArray(compiler.options.externals)
91-
? compiler.options.externals
92-
: [compiler.options.externals].filter(Boolean)
93-
externals.push({
94-
'@opentelemetry/api': 'commonjs @opentelemetry/api',
95-
'@opentelemetry/api-logs': 'commonjs @opentelemetry/api-logs',
96-
})
97-
compiler.options.externals = externals
85+
// Keep an OpenTelemetry API package external only when the application declares its own copy: the
86+
// bridge captures that copy through the @opentelemetry/api instrumentation, which fires on a
87+
// runtime require, so bundling it would inline a second copy the instrumentation never sees and
88+
// the bridge would register on the wrong one, downgrading every span to a no-op (issue #6882). A
89+
// package the application does not declare is left to bundle, so dd-trace's own fallback copy is
90+
// inlined and the bundle stays self-contained. The `commonjs` type forces a CommonJS require
91+
// regardless of the user's externalsType.
92+
const workingDir = compiler.options.context || process.cwd()
93+
const otelApiExternals = {}
94+
for (const otelApiPackage of otelApiPackagesToExternalize(workingDir)) {
95+
otelApiExternals[otelApiPackage] = `commonjs ${otelApiPackage}`
96+
}
97+
if (otelApiExternals['@opentelemetry/api'] || otelApiExternals['@opentelemetry/api-logs']) {
98+
const externals = Array.isArray(compiler.options.externals)
99+
? compiler.options.externals
100+
: [compiler.options.externals].filter(Boolean)
101+
externals.push(otelApiExternals)
102+
compiler.options.externals = externals
103+
}
98104

99105
const gitMetadata = getGitMetadata()
100106
if (gitMetadata.repositoryURL || gitMetadata.commitSHA) {

packages/datadog-webpack/test/plugin.spec.js

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
'use strict'
22

33
const assert = require('node:assert/strict')
4+
const fs = require('node:fs')
5+
const os = require('node:os')
6+
const path = require('node:path')
47
const { describe, it } = require('mocha')
58

69
const DatadogWebpackPlugin = require('../index')
@@ -50,9 +53,9 @@ describe('DatadogWebpackPlugin', () => {
5053
assert.equal(tapped[0], 'DatadogWebpackPlugin')
5154
})
5255

53-
function applyToExternals (externals) {
56+
function applyToExternals (externals, context) {
5457
const compiler = {
55-
options: { optimization: {}, externals },
58+
options: { optimization: {}, externals, context },
5659
hooks: {
5760
environment: { tap: () => {} },
5861
thisCompilation: { tap: () => {} },
@@ -63,6 +66,17 @@ describe('DatadogWebpackPlugin', () => {
6366
return compiler.options.externals
6467
}
6568

69+
function manifestDir (manifest) {
70+
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-webpack-otel-'))
71+
fs.writeFileSync(path.join(dir, 'package.json'), JSON.stringify(manifest))
72+
return dir
73+
}
74+
75+
function otelExternalEntry (externals) {
76+
if (!Array.isArray(externals)) return undefined
77+
return externals.find(entry => entry && typeof entry === 'object' && '@opentelemetry/api' in entry)
78+
}
79+
6680
it('externalizes both OpenTelemetry API peers as a commonjs require', () => {
6781
const externals = applyToExternals()
6882

@@ -85,6 +99,22 @@ describe('DatadogWebpackPlugin', () => {
8599
assert.ok(externals.includes('pg'), 'should preserve the user external')
86100
assert.ok('@opentelemetry/api' in externals.at(-1), 'should externalize @opentelemetry/api')
87101
})
102+
103+
it('bundles a package the application does not depend on so the bundle stays self-contained', () => {
104+
const context = manifestDir({ name: 'app', dependencies: {} })
105+
106+
const externals = applyToExternals(undefined, context)
107+
108+
assert.strictEqual(otelExternalEntry(externals), undefined)
109+
})
110+
111+
it('externalizes only the package the application declares', () => {
112+
const context = manifestDir({ name: 'app', dependencies: { '@opentelemetry/api': '^1.9.0' } })
113+
114+
const entry = otelExternalEntry(applyToExternals(undefined, context))
115+
116+
assert.deepStrictEqual(entry, { '@opentelemetry/api': 'commonjs @opentelemetry/api' })
117+
})
88118
})
89119

90120
describe('optional peer bundling', () => {

0 commit comments

Comments
 (0)