Skip to content

Commit 20d518a

Browse files
committed
fix(otel): share the application's @opentelemetry/api-logs copy
The logs pipeline registers a global provider through @opentelemetry/api-logs the same way the tracer bridge registers through @opentelemetry/api, so dd-trace bundling its own copy risks the first-writer-wins global diverging from the copy the application reads. Resolve the application's copy instead and stop shipping our own, which also drops a package from the install for serverless. 1. The api loader is generalized to any OpenTelemetry API package via forPackage(); the logs provider loads api-logs through it. 2. @opentelemetry/api-logs becomes an optional peerDependency instead of a bundled optionalDependency. 3. The peer-dep config guard is now per-flag: DD_METRICS_OTEL_ENABLED needs @opentelemetry/api; DD_LOGS_OTEL_ENABLED also needs @opentelemetry/api-logs, so a missing api-logs disables OTel logs and leaves DD log injection on instead of crashing the host.
1 parent ef0827b commit 20d518a

6 files changed

Lines changed: 231 additions & 104 deletions

File tree

package.json

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,15 @@
168168
"opentracing": ">=0.14.7"
169169
},
170170
"peerDependencies": {
171-
"@opentelemetry/api": ">=1.0.0 <1.10.0"
171+
"@opentelemetry/api": ">=1.0.0 <1.10.0",
172+
"@opentelemetry/api-logs": "<1.0.0"
172173
},
173174
"peerDependenciesMeta": {
174175
"@opentelemetry/api": {
175176
"optional": true
177+
},
178+
"@opentelemetry/api-logs": {
179+
"optional": true
176180
}
177181
},
178182
"optionalDependencies": {
@@ -183,7 +187,6 @@
183187
"@datadog/openfeature-node-server": "2.0.0",
184188
"@datadog/pprof": "5.15.0",
185189
"@datadog/wasm-js-rewriter": "5.0.1",
186-
"@opentelemetry/api-logs": "<1.0.0",
187190
"oxc-parser": "^0.132.0"
188191
},
189192
"devDependencies": {
@@ -196,6 +199,7 @@
196199
"@openfeature/core": "^1.11.0",
197200
"@openfeature/server-sdk": "~1.22.0",
198201
"@opentelemetry/api": ">=1.0.0 <1.10.0",
202+
"@opentelemetry/api-logs": "<1.0.0",
199203
"@stylistic/eslint-plugin": "^5.10.0",
200204
"@types/mocha": "^10.0.10",
201205
"@types/node": "^18.19.106",

packages/dd-trace/src/config/index.js

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -342,16 +342,25 @@ class Config extends ConfigBase {
342342
if (!trackedConfigOrigins.has('dogstatsd.hostname')) {
343343
setAndTrack(this, 'dogstatsd.hostname', agentHostname)
344344
}
345-
// Both OTel pipelines require the optional @opentelemetry/api peer dep. Disable them before
346-
// the log-injection mutual exclusion below, so a missing module leaves DD log injection on.
347-
// Warn only when the customer turned the flag on; a default-on flag is disabled silently so
348-
// changing a default later does not warn every app that never opted in.
349-
if ((this.DD_LOGS_OTEL_ENABLED || this.DD_METRICS_OTEL_ENABLED) &&
350-
!require('../opentelemetry/api').isAvailable()) {
351-
for (const name of ['DD_LOGS_OTEL_ENABLED', 'DD_METRICS_OTEL_ENABLED']) {
352-
if (this[name]) {
345+
// Each OTel pipeline requires its optional peer deps. The metrics pipeline needs
346+
// @opentelemetry/api; the logs pipeline also registers a global provider through
347+
// @opentelemetry/api-logs. Disable a flag whose peer is missing before the
348+
// log-injection mutual exclusion below, so a missing module leaves DD log injection
349+
// on instead of crashing the host. Warn only when the customer turned the flag on; a
350+
// default-on flag is disabled silently so changing a default later does not warn
351+
// every app that never opted in.
352+
if (this.DD_LOGS_OTEL_ENABLED || this.DD_METRICS_OTEL_ENABLED) {
353+
const otelApi = require('../opentelemetry/api')
354+
const requiredPeers = {
355+
DD_METRICS_OTEL_ENABLED: ['@opentelemetry/api'],
356+
DD_LOGS_OTEL_ENABLED: ['@opentelemetry/api', '@opentelemetry/api-logs'],
357+
}
358+
for (const [name, peers] of Object.entries(requiredPeers)) {
359+
if (!this[name]) continue
360+
const missing = peers.find(peer => !otelApi.forPackage(peer).isAvailable())
361+
if (missing) {
353362
if (trackedConfigOrigins.has(name)) {
354-
log.warn('@opentelemetry/api is not installed; disabling %s', name)
363+
log.warn('%s is not installed; disabling %s', missing, name)
355364
}
356365
setAndTrack(this, name, false)
357366
}

packages/dd-trace/src/opentelemetry/api.js

Lines changed: 106 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,13 @@ const { DD_MAJOR } = require('../../../../version')
44
const satisfies = require('../../../../vendor/dist/semifies')
55
const log = require('../log')
66

7-
const PACKAGE_NAME = '@opentelemetry/api'
8-
9-
// undefined: not resolved yet; null: resolved and absent; object: resolved api.
10-
/** @type {typeof import('@opentelemetry/api') | null | undefined} */
11-
let cachedApi
12-
// Absolute path of the resolved package entrypoint, used to read its version lazily.
13-
/** @type {string | undefined} */
14-
let cachedEntry
15-
let warned = false
7+
// The consequence of resolving an unsupported version differs per package: an
8+
// out-of-range @opentelemetry/api silently downgrades spans to no-ops (issue #6882),
9+
// while @opentelemetry/api-logs only drops the records it cannot serialize.
10+
const UNSUPPORTED_CONSEQUENCE = {
11+
'@opentelemetry/api': 'OpenTelemetry spans may run as no-ops.',
12+
'@opentelemetry/api-logs': 'OpenTelemetry log records may be dropped.',
13+
}
1614

1715
// Node builtins are required lazily inside the helpers below rather than at module
1816
// scope. This module is loaded from the config initialization path to answer
@@ -23,7 +21,7 @@ let warned = false
2321

2422
/**
2523
* `createRequire` rooted at the application entrypoint, used to resolve the copy
26-
* of `@opentelemetry/api` the user's code loads rather than dd-trace's own.
24+
* of the package the user's code loads rather than dd-trace's own.
2725
*
2826
* @returns {NodeRequire | undefined}
2927
*/
@@ -38,59 +36,55 @@ function applicationRequire () {
3836

3937
/**
4038
* @param {NodeRequire} req
41-
* @returns {{ api: typeof import('@opentelemetry/api'), entry: string } | undefined}
39+
* @param {string} packageName
40+
* @returns {{ api: object, entry: string } | undefined}
4241
*/
43-
function resolveFrom (req) {
42+
function resolveFrom (req, packageName) {
4443
try {
45-
return { api: req(PACKAGE_NAME), entry: req.resolve(PACKAGE_NAME) }
44+
return { api: req(packageName), entry: req.resolve(packageName) }
4645
} catch {}
4746
}
4847

4948
/**
50-
* @returns {{ api: typeof import('@opentelemetry/api'), entry: string } | undefined}
49+
* @param {string} packageName
50+
* @returns {{ api: object, entry: string } | undefined}
5151
*/
52-
function resolveApi () {
53-
// v6 declares @opentelemetry/api as an optional peer dependency, so a single
54-
// shared copy lives in the application and dd-trace's own require resolves it.
52+
function resolveApi (packageName) {
53+
// v6 declares the OpenTelemetry API packages as optional peer dependencies, so a
54+
// single shared copy lives in the application and dd-trace's own require resolves it.
5555
if (DD_MAJOR >= 6) {
56-
return resolveFrom(require)
56+
return resolveFrom(require, packageName)
5757
}
58-
// v5 bundles @opentelemetry/api as an optional dependency, so dd-trace's own
59-
// require can resolve its bundled (older) copy instead of the application's.
60-
// The OTel global API rejects a provider registered by a copy older than the
61-
// reader's, which silently downgrades every span to a no-op (issue #6882).
62-
// Prefer the application's copy and fall back to dd-trace's bundled one.
58+
// v5 bundles the OpenTelemetry API packages as optional dependencies, so dd-trace's
59+
// own require can resolve its bundled (older) copy instead of the application's. The
60+
// OTel global API rejects a provider registered by a copy older than the reader's,
61+
// which silently downgrades every span to a no-op (issue #6882). Prefer the
62+
// application's copy and fall back to dd-trace's bundled one.
6363
const appRequire = applicationRequire()
64-
return (appRequire && resolveFrom(appRequire)) || resolveFrom(require)
65-
}
66-
67-
function ensureResolved () {
68-
if (cachedApi !== undefined) return
69-
const resolved = resolveApi()
70-
cachedApi = resolved?.api ?? null
71-
cachedEntry = resolved?.entry
64+
return (appRequire && resolveFrom(appRequire, packageName)) || resolveFrom(require, packageName)
7265
}
7366

7467
/**
7568
* Reads the package version by walking up from the resolved entrypoint. The
76-
* package blocks `require('@opentelemetry/api/package.json')` through its
77-
* `exports` map, so the version is read from disk instead. `require-package-json`
78-
* resolves against a module's `module.paths`, which on v5 points at dd-trace's copy
79-
* rather than the application copy this entrypoint was resolved from, so it cannot
80-
* answer "which copy did we share"; the walk anchors on the resolved entry instead.
69+
* package blocks `require('<pkg>/package.json')` through its `exports` map, so the
70+
* version is read from disk instead. `require-package-json` resolves against a
71+
* module's `module.paths`, which on v5 points at dd-trace's copy rather than the
72+
* application copy this entrypoint was resolved from, so it cannot answer "which
73+
* copy did we share"; the walk anchors on the resolved entry instead.
8174
*
8275
* @param {string} entry - Absolute path to the resolved package entrypoint.
76+
* @param {string} packageName
8377
* @returns {string | undefined}
8478
*/
85-
function readVersionNear (entry) {
79+
function readVersionNear (entry, packageName) {
8680
const { readFileSync } = require('node:fs')
8781
const { dirname, join, parse } = require('node:path')
8882
let dir = dirname(entry)
8983
const { root } = parse(dir)
9084
while (dir !== root) {
9185
try {
9286
const pkg = JSON.parse(readFileSync(join(dir, 'package.json'), 'utf8'))
93-
if (pkg.name === PACKAGE_NAME) return pkg.version
87+
if (pkg.name === packageName) return pkg.version
9488
} catch {}
9589
dir = dirname(dir)
9690
}
@@ -100,50 +94,97 @@ function readVersionNear (entry) {
10094
* Warns when the resolved version is outside dd-trace's declared range, read from
10195
* dd-trace's own package.json so the threshold stays in lockstep with the declaration.
10296
*
103-
* @param {string | undefined} version - Resolved `@opentelemetry/api` version.
97+
* @param {string} packageName
98+
* @param {string | undefined} version - Resolved package version.
10499
*/
105-
function warnIfUnsupported (version) {
100+
function warnIfUnsupported (packageName, version) {
106101
const pkg = require('../../../../package.json')
107-
const range = pkg.peerDependencies?.[PACKAGE_NAME] ?? pkg.optionalDependencies?.[PACKAGE_NAME]
102+
const range = pkg.peerDependencies?.[packageName] ?? pkg.optionalDependencies?.[packageName]
108103
if (version && range && !satisfies(version, range)) {
109104
log.warn(
110-
'@opentelemetry/api@%s is outside the range dd-trace supports (%s); OpenTelemetry spans may run as no-ops.',
111-
version, range
105+
'%s@%s is outside the range dd-trace supports (%s); %s',
106+
packageName, version, range, UNSUPPORTED_CONSEQUENCE[packageName]
112107
)
113108
}
114109
}
115110

116111
/**
117-
* Returns the `@opentelemetry/api` the bridge must share with the application,
118-
* throwing a clear error when it is not installed. The first successful load
119-
* warns once if the resolved version is outside dd-trace's supported range.
112+
* Builds a loader that shares a single copy of an OpenTelemetry API package between
113+
* dd-trace and the application. Each package gets its own cached state so resolution
114+
* and the once-only version warning stay independent.
120115
*
121-
* @returns {typeof import('@opentelemetry/api')}
116+
* @param {string} packageName - The package to resolve (e.g. `@opentelemetry/api`).
117+
* @returns {{ load: () => object, isAvailable: () => boolean }}
122118
*/
123-
function load () {
124-
ensureResolved()
125-
if (cachedApi === null) {
126-
throw new Error(
127-
`${PACKAGE_NAME} is not installed but is required to use the OpenTelemetry bridge ` +
128-
'(tracer.TracerProvider). Add it as a dependency of your application to enable the bridge.'
129-
)
119+
function createLoader (packageName) {
120+
// undefined: not resolved yet; null: resolved and absent; object: resolved api.
121+
/** @type {object | null | undefined} */
122+
let cachedApi
123+
// Absolute path of the resolved package entrypoint, used to read its version lazily.
124+
/** @type {string | undefined} */
125+
let cachedEntry
126+
let warned = false
127+
128+
function ensureResolved () {
129+
if (cachedApi !== undefined) return
130+
const resolved = resolveApi(packageName)
131+
cachedApi = resolved?.api ?? null
132+
cachedEntry = resolved?.entry
130133
}
131-
if (!warned) {
132-
warned = true
133-
warnIfUnsupported(cachedEntry && readVersionNear(cachedEntry))
134+
135+
return {
136+
/**
137+
* Returns the package the bridge must share with the application, throwing a clear
138+
* error when it is not installed. The first successful load warns once if the
139+
* resolved version is outside dd-trace's supported range.
140+
*
141+
* @returns {object}
142+
*/
143+
load () {
144+
ensureResolved()
145+
if (cachedApi === null) {
146+
throw new Error(
147+
`${packageName} is not installed but is required to use the OpenTelemetry bridge ` +
148+
'(tracer.TracerProvider). Add it as a dependency of your application to enable the bridge.'
149+
)
150+
}
151+
if (!warned) {
152+
warned = true
153+
warnIfUnsupported(packageName, cachedEntry && readVersionNear(cachedEntry, packageName))
154+
}
155+
return cachedApi
156+
},
157+
158+
/**
159+
* Whether the package can be resolved. Kept free of filesystem access so it is
160+
* safe to call from the config initialization path.
161+
*
162+
* @returns {boolean}
163+
*/
164+
isAvailable () {
165+
ensureResolved()
166+
return cachedApi !== null
167+
},
134168
}
135-
return cachedApi
136169
}
137170

171+
const apiLoader = createLoader('@opentelemetry/api')
172+
const loaders = new Map([['@opentelemetry/api', apiLoader]])
173+
138174
/**
139-
* Whether `@opentelemetry/api` can be resolved. Kept free of filesystem access so
140-
* it is safe to call from the config initialization path.
175+
* Returns the shared loader for an OpenTelemetry API package, reusing the cached
176+
* loader so every consumer of a package shares one resolution and one warning.
141177
*
142-
* @returns {boolean}
178+
* @param {string} packageName
179+
* @returns {{ load: () => object, isAvailable: () => boolean }}
143180
*/
144-
function isAvailable () {
145-
ensureResolved()
146-
return cachedApi !== null
181+
function forPackage (packageName) {
182+
let loader = loaders.get(packageName)
183+
if (!loader) {
184+
loader = createLoader(packageName)
185+
loaders.set(packageName, loader)
186+
}
187+
return loader
147188
}
148189

149-
module.exports = { load, isAvailable }
190+
module.exports = { load: apiLoader.load, isAvailable: apiLoader.isAvailable, forPackage }

packages/dd-trace/src/opentelemetry/logs/logger_provider.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict'
2-
const { logs } = require('@opentelemetry/api-logs')
3-
const { context } = require('../api').load()
2+
const otelApi = require('../api')
3+
const { logs } = otelApi.forPackage('@opentelemetry/api-logs').load()
4+
const { context } = otelApi.load()
45
const log = require('../../log')
56
const ContextManager = require('../context_manager')
67
const Logger = require('./logger')

0 commit comments

Comments
 (0)