diff --git a/README.md b/README.md index 4b214a13bd8..04a8ee2b321 100644 --- a/README.md +++ b/README.md @@ -93,23 +93,6 @@ Regardless of where you open the issue, someone at Datadog will try to help. If you would like to trace your bundled application then please read this page on [bundling and dd-trace](https://docs.datadoghq.com/tracing/trace_collection/automatic_instrumentation/dd_libraries/nodejs/#bundling). It includes information on how to use our ESBuild plugin and includes caveats for other bundlers. -When using the experimental OpenFeature provider, file-traced deployments can force the optional provider and -its dependencies into the output with a side-effect import before accessing `tracer.openfeature`: - -CommonJS: - -```js -require('dd-trace/openfeature') -``` - -ES modules: - -```js -import 'dd-trace/openfeature.js' -``` - -This is a fallback for build tools that do not recognize the provider's optional-require wrapper. - ## Security Vulnerabilities diff --git a/integration-tests/esbuild/build-and-test-openfeature.js b/integration-tests/esbuild/build-and-test-openfeature.js deleted file mode 100644 index 002fc001261..00000000000 --- a/integration-tests/esbuild/build-and-test-openfeature.js +++ /dev/null @@ -1,94 +0,0 @@ -#!/usr/bin/env node -'use strict' - -/* eslint-disable no-console */ - -// Regression test for #8980 under esbuild. When the optional peer -// `@datadog/openfeature-node-server` is installed, the dd-trace esbuild plugin bundles it -// into the output so feature flagging keeps working after the bundle is relocated to a tree -// without the peer on disk (standalone deploys). Without bundling, the opaque runtime require -// resolves from the bundle directory and falls back to the no-op provider. -// -// The complementary #8635 case (peer absent -> build must not follow the optional chain) is -// covered by `openfeature.spec.js`, whose sandbox does not install the peer. - -const fs = require('fs') -const os = require('os') -const path = require('path') -const assert = require('assert') -const { execFileSync } = require('child_process') -const esbuild = require('esbuild') -const ddPlugin = require('../../esbuild') // dd-trace/esbuild - -const OUTFILE = path.join(__dirname, 'openfeature-out.js') - -async function main () { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-openfeature-esbuild-')) - - try { - assert.strictEqual( - isResolvable('@datadog/openfeature-node-server', __dirname), - true, - 'the optional peer must be installed for this scenario; run `yarn install` with devDependencies' - ) - - await esbuild.build({ - entryPoints: [path.join(__dirname, 'openfeature-app.js')], - outfile: OUTFILE, - bundle: true, - platform: 'node', - target: 'node18', - plugins: [ddPlugin], - external: [ - 'pg', 'mysql2', 'better-sqlite3', 'sqlite3', 'mysql', 'mariadb', 'oracledb', 'pg-query-stream', 'tedious', - '@yaacovcr/transform', - '@datadog/native-appsec', '@datadog/native-iast-taint-tracking', '@datadog/native-metrics', - '@datadog/pprof', '@datadog/libdatadog', - ], - }) - - const bundle = fs.readFileSync(OUTFILE).toString() - assert( - !bundle.includes("requireOptionalPeer('@datadog/openfeature-node-server')"), - 'the opaque peer require survived; the plugin did not inline `@datadog/openfeature-node-server`' - ) - - const relocated = path.join(tmpDir, 'out.js') - fs.copyFileSync(OUTFILE, relocated) - assert.strictEqual( - isResolvable('@datadog/openfeature-node-server', tmpDir), - false, - 'the relocation dir must not resolve the peer, otherwise the test proves nothing' - ) - - const runOutput = execFileSync(process.execPath, [relocated], { encoding: 'utf8' }) - assert( - runOutput.includes('PROVIDER_OK'), - `relocated bundle did not load the real OpenFeature provider:\n${runOutput}` - ) - - console.log('ok') - } finally { - fs.rmSync(OUTFILE, { force: true }) - fs.rmSync(tmpDir, { recursive: true, force: true }) - } -} - -/** - * @param {string} request - Module specifier - * @param {string} fromDir - Directory to resolve from - * @returns {boolean} Whether the module resolves from `fromDir` - */ -function isResolvable (request, fromDir) { - try { - require.resolve(request, { paths: [fromDir] }) - return true - } catch { - return false - } -} - -main().catch((error) => { - console.error(error) - process.exitCode = 1 -}) diff --git a/integration-tests/esbuild/index.spec.js b/integration-tests/esbuild/index.spec.js index ad1a26ff037..ef69eaccfcc 100755 --- a/integration-tests/esbuild/index.spec.js +++ b/integration-tests/esbuild/index.spec.js @@ -95,12 +95,6 @@ esbuildVersions.forEach((version) => { }) }) - it('bundles the optional OpenFeature peer so it survives bundle relocation', () => { - execSync('node ./build-and-test-openfeature.js', { - timeout, - }) - }) - it('injects Git metadata into bundled applications', () => { execSync('node ./build-and-test-git-tags.js', { timeout, diff --git a/integration-tests/esbuild/openfeature-app.js b/integration-tests/esbuild/openfeature-app.js deleted file mode 100644 index 1ae0986a9a8..00000000000 --- a/integration-tests/esbuild/openfeature-app.js +++ /dev/null @@ -1,25 +0,0 @@ -#!/usr/bin/env node -'use strict' - -// Entry for `build-and-test-openfeature.js`. Enables the flagging provider through the -// public API so the bundled `flagging_provider.js` exercises the optional peer load at -// runtime. A broken resolution leaves `tracer.openfeature` on the no-op provider (#8980); -// a working one loads the real `FlaggingProvider`. - -const assert = require('assert') - -const tracer = require('../../').init({ // dd-trace - experimental: { flaggingProvider: { enabled: true } }, -}) - -const provider = tracer.openfeature - -assert.strictEqual( - provider?.constructor?.name, - 'FlaggingProvider', - `expected the real Datadog FlaggingProvider, got ${provider?.constructor?.name}` -) - -// eslint-disable-next-line no-console -console.log('PROVIDER_OK') -process.exit(0) diff --git a/integration-tests/webpack/build-and-test-openfeature.js b/integration-tests/webpack/build-and-test-openfeature.js deleted file mode 100644 index 28842ca6308..00000000000 --- a/integration-tests/webpack/build-and-test-openfeature.js +++ /dev/null @@ -1,151 +0,0 @@ -#!/usr/bin/env node -'use strict' - -/* eslint-disable no-console */ - -// End-to-end coverage for the OpenFeature optional peer chain -// `@datadog/openfeature-node-server` -> `@openfeature/server-sdk` -> `@openfeature/core` -// under webpack. Two scenarios pin the two failure modes: -// -// 1. #8635: without the dd-trace plugin, the require stays opaque so webpack never -// follows the optional chain. A user who bundles dd-trace without opting into -// feature flagging must not have their build fail on the missing chain. -// -// 2. #8980: with the dd-trace plugin and the peer installed, the plugin bundles the -// peer into the output. Feature flagging then survives the bundle being relocated -// to a tree where the peer is not on disk (e.g. a standalone deploy), instead of -// silently falling back to the no-op provider. - -const fs = require('fs') -const os = require('os') -const path = require('path') -const assert = require('assert') -const { execFileSync } = require('child_process') -const webpack = require('webpack') -const DatadogWebpackPlugin = require('../../webpack') // dd-trace/webpack -const experiments = require('./webpack-experiments') - -const ENTRY = path.join(__dirname, 'openfeature-app.js') -const FLAGGING_PROVIDER = path.join('openfeature', 'flagging_provider') -const EXTERNALS = [ - 'diagnostics_channel', - 'pg', 'mysql2', 'better-sqlite3', 'sqlite3', 'mysql', 'oracledb', 'pg-query-stream', 'tedious', - '@yaacovcr/transform', - // Optional native dd-trace modules (kept consistent with `build.js`). - '@datadog/native-appsec', '@datadog/native-iast-taint-tracking', '@datadog/native-metrics', - '@datadog/pprof', '@datadog/libdatadog', - // NOTE: `@datadog/openfeature-node-server` is deliberately absent. dd-trace must keep - // the require opaque without help from the user's webpack config. -] - -/** - * @param {string} outfile - Absolute path of the bundle to emit - * @param {Array} plugins - Webpack plugins to apply - * @returns {Promise} The webpack stats object - */ -function build (outfile, plugins) { - return new Promise((resolve, reject) => { - webpack({ - mode: 'development', - entry: ENTRY, - target: 'node', - externalsType: 'commonjs', - ...(experiments && { experiments }), - output: { filename: path.basename(outfile), path: path.dirname(outfile), hashFunction: 'sha256' }, - externals: EXTERNALS, - plugins, - }, (err, stats) => { - if (err) return reject(err) - if (stats.hasErrors()) return reject(new Error(stats.toString({ errors: true }))) - resolve(stats) - }) - }) -} - -/** - * @param {object} stats - Webpack stats object - * @returns {Array} `Critical dependency` warnings attributable to flagging_provider - */ -function flaggingProviderWarnings (stats) { - return stats.compilation.warnings.filter((warning) => - /Critical dependency/.test(warning.message) && - (String(warning.module?.resource).includes(FLAGGING_PROVIDER) || /flagging_provider/.test(warning.message)) - ) -} - -async function main () { - const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'dd-openfeature-')) - - try { - // Scenario 1 (#8635): no dd-trace plugin -> the require stays opaque. - const opaqueOut = path.join(__dirname, 'openfeature-out-opaque.js') - const opaqueStats = await build(opaqueOut, []) - try { - assert.strictEqual( - flaggingProviderWarnings(opaqueStats).length, - 0, - 'flagging_provider tripped the webpack expression-dependency path; resolve through ' + - '`__non_webpack_require__`, not bare `require.resolve`' - ) - const opaqueBundle = fs.readFileSync(opaqueOut).toString() - assert( - !opaqueBundle.includes('@datadog/flagging-core'), - 'bundle leaked `@datadog/flagging-core`; webpack must not statically follow the optional peer' - ) - assert( - !opaqueBundle.includes('node_modules/@openfeature/server-sdk'), - 'bundle leaked `@openfeature/server-sdk` paths; webpack must not statically follow the optional peer' - ) - } finally { - fs.rmSync(opaqueOut, { force: true }) - } - - // Scenario 2 (#8980): with the dd-trace plugin and the peer installed, the peer is - // bundled, so the relocated bundle loads the real provider instead of the no-op. - assert.strictEqual( - isResolvable('@datadog/openfeature-node-server', __dirname), - true, - 'the optional peer must be installed for this scenario; run `yarn install` with devDependencies' - ) - const bundledOut = path.join(__dirname, 'openfeature-out-bundled.js') - await build(bundledOut, [new DatadogWebpackPlugin()]) - const relocated = path.join(tmpDir, 'out.js') - fs.copyFileSync(bundledOut, relocated) - fs.rmSync(bundledOut, { force: true }) - - assert.strictEqual( - isResolvable('@datadog/openfeature-node-server', tmpDir), - false, - 'the relocation dir must not resolve the peer, otherwise the test proves nothing' - ) - - const runOutput = execFileSync(process.execPath, [relocated], { encoding: 'utf8' }) - assert( - runOutput.includes('PROVIDER_OK'), - `relocated bundle did not load the real OpenFeature provider:\n${runOutput}` - ) - - console.log('ok') - } finally { - fs.rmSync(tmpDir, { recursive: true, force: true }) - } -} - -/** - * @param {string} request - Module specifier - * @param {string} fromDir - Directory to resolve from - * @returns {boolean} Whether the module resolves from `fromDir` - */ -function isResolvable (request, fromDir) { - try { - require.resolve(request, { paths: [fromDir] }) - return true - } catch { - return false - } -} - -main().catch((error) => { - console.error(error) - process.exitCode = 1 -}) diff --git a/integration-tests/webpack/index.spec.js b/integration-tests/webpack/index.spec.js index d04632ae72c..ee821959021 100644 --- a/integration-tests/webpack/index.spec.js +++ b/integration-tests/webpack/index.spec.js @@ -58,10 +58,6 @@ webpackVersions.forEach((version) => { execSync('node ./build-and-test-skip-external.js', { timeout }) }) - it('does not follow `@datadog/openfeature-node-server` into its optional peer chain', () => { - execSync('node ./build-and-test-openfeature.js', { timeout }) - }) - it('injects Git metadata into bundled applications', () => { execSync('node ./build-and-test-git-tags.js', { timeout }) }) diff --git a/integration-tests/webpack/openfeature-app.js b/integration-tests/webpack/openfeature-app.js deleted file mode 100644 index 3ced0be8cf9..00000000000 --- a/integration-tests/webpack/openfeature-app.js +++ /dev/null @@ -1,25 +0,0 @@ -#!/usr/bin/env node -'use strict' - -// Entry for `build-and-test-openfeature.js`. Enables the flagging provider through -// the public API so the bundled `flagging_provider.js` exercises the bundler-opaque -// require at runtime. A broken resolution leaves `tracer.openfeature` on the no-op -// provider (see #8980); a working one loads the real `FlaggingProvider`. - -const assert = require('assert') - -const tracer = require('../../').init({ // dd-trace - experimental: { flaggingProvider: { enabled: true } }, -}) - -const provider = tracer.openfeature - -assert.strictEqual( - provider?.constructor?.name, - 'FlaggingProvider', - `expected the real Datadog FlaggingProvider, got ${provider?.constructor?.name}` -) - -// eslint-disable-next-line no-console -console.log('PROVIDER_OK') -process.exit(0) diff --git a/openfeature.js b/openfeature.js index 428ef77cf38..4d230e8b74d 100644 --- a/openfeature.js +++ b/openfeature.js @@ -1,4 +1,4 @@ 'use strict' -// Static fallback for file tracers that do not recognize the optional-peer wrapper. -require('@datadog/openfeature-node-server') +// Static fallback for file tracers that do not recognize the vendored provider's lazy require. +require('./vendor/dist/@datadog/openfeature-node-server') diff --git a/package.json b/package.json index 647441d1653..dc1743f1c10 100644 --- a/package.json +++ b/package.json @@ -184,7 +184,6 @@ "@datadog/native-appsec": "11.0.1", "@datadog/native-iast-taint-tracking": "4.2.0", "@datadog/native-metrics": "3.1.2", - "@datadog/openfeature-node-server": "2.0.2", "@datadog/pprof": "5.18.0", "@datadog/wasm-js-rewriter": "5.0.1", "@opentelemetry/api": ">=1.0.0 <1.10.0", @@ -197,6 +196,7 @@ "@babel/helpers": "^8.0.0", "@eslint/eslintrc": "^3.3.5", "@eslint/js": "^10.0.1", + "@datadog/openfeature-node-server": "2.1.0", "@msgpack/msgpack": "^3.1.3", "@openfeature/core": "^1.11.0", "@openfeature/server-sdk": "~1.22.0", diff --git a/packages/datadog-esbuild/index.js b/packages/datadog-esbuild/index.js index e819ce7f701..3aed61204e2 100644 --- a/packages/datadog-esbuild/index.js +++ b/packages/datadog-esbuild/index.js @@ -8,11 +8,6 @@ const { pathToFileURL, fileURLToPath } = require('node:url') const instrumentations = require('../datadog-instrumentations/src/helpers/instrumentations') const extractPackageAndModulePath = require('../datadog-instrumentations/src/helpers/extract-package-and-module-path') const hooks = require('../datadog-instrumentations/src/helpers/hooks') -const { - OPTIONAL_PEER_FILTER, - matchesOptionalPeerFile, - rewriteOptionalPeerLoads, -} = require('../datadog-instrumentations/src/helpers/optional-peer-bundler') const { processModule, isESMFile } = require('./src/utils') const log = require('./src/log') @@ -133,19 +128,6 @@ module.exports.setup = function (build) { ${build.initialOptions.banner.js}` } - // Keep the build from failing on the optional `@openfeature/core` peer of - // `@openfeature/server-sdk` when it is not installed (#8635). esbuild follows the chain - // whenever `@openfeature/server-sdk` is reachable -- the app importing it directly, or the - // bundled provider when the optional peer is present -- so mark `@openfeature/core` external - // when it is absent rather than erroring at bundle time. - try { - // eslint-disable-next-line n/no-unpublished-require - require.resolve('@openfeature/core') - } catch { - build.initialOptions.external ??= [] - build.initialOptions.external.push('@openfeature/core') - } - const esmBuild = isESMBuild(build) if ( esmBuild && @@ -182,22 +164,6 @@ ${build.initialOptions.banner.js}` log.warn('No git metadata available - skipping injection') } - // Rewrite optional-peer loads so installed peers get bundled and survive the bundle being - // relocated without them on disk (#8980). Registered before the generic onLoad so it wins for - // these files. Absent peers stay opaque, so a build that does not opt into the feature does not - // follow their dependency chain (#8635). - build.onLoad({ filter: OPTIONAL_PEER_FILTER }, args => { - const normalizedPath = args.path.replaceAll('\\', '/') - if (!matchesOptionalPeerFile(normalizedPath)) return - - log.debug('INLINE: optional-peer loader applied to %s', normalizedPath) - return { - contents: rewriteOptionalPeerLoads(fs.readFileSync(args.path, 'utf8'), path.dirname(args.path)), - loader: 'js', - resolveDir: path.dirname(args.path), - } - }) - // first time is intercepted, proxy should be created, next time the original should be loaded const interceptedESMModules = new Set() diff --git a/packages/datadog-esbuild/test/plugin.spec.js b/packages/datadog-esbuild/test/plugin.spec.js index a405a407f80..bd6e0ea53bd 100644 --- a/packages/datadog-esbuild/test/plugin.spec.js +++ b/packages/datadog-esbuild/test/plugin.spec.js @@ -5,18 +5,6 @@ const { describe, it } = require('mocha') const ddPlugin = require('../index') -function captureOptionalPeerOnLoad () { - let onLoad - ddPlugin.setup({ - initialOptions: {}, - onResolve () {}, - onLoad (options, callback) { - if (options.filter.source.includes('require-provider')) onLoad = callback - }, - }) - return onLoad -} - function captureOnResolve () { let onResolve ddPlugin.setup({ @@ -47,25 +35,4 @@ describe('datadog-esbuild plugin', () => { assert.strictEqual(result, undefined) }) - - describe('optional peer bundling', () => { - it('rewrites the installed peer load in require-provider into a literal require', () => { - const onLoad = captureOptionalPeerOnLoad() - const providerPath = require.resolve('../../dd-trace/src/openfeature/require-provider') - - const result = onLoad({ path: providerPath }) - - assert.ok(result.contents.includes("require('@datadog/openfeature-node-server')"), 'should inline the peer') - assert.ok( - !result.contents.includes("requireOptionalPeer('@datadog/openfeature-node-server')"), - 'should drop the opaque load' - ) - }) - - it('ignores files that match the filter but are not an optional-peer file', () => { - const onLoad = captureOptionalPeerOnLoad() - - assert.strictEqual(onLoad({ path: '/somewhere/else/require-provider.js' }), undefined) - }) - }) }) diff --git a/packages/datadog-instrumentations/src/helpers/optional-peer-bundler.js b/packages/datadog-instrumentations/src/helpers/optional-peer-bundler.js deleted file mode 100644 index 031d9236ba6..00000000000 --- a/packages/datadog-instrumentations/src/helpers/optional-peer-bundler.js +++ /dev/null @@ -1,65 +0,0 @@ -'use strict' - -const path = require('node:path') - -// Build-time half of the optional-peer mechanism shared by the webpack and esbuild plugins. -// -// Runtime files load an optional peer through a local `requireOptionalPeer('name')` wrapper. -// File tracers recognize its bound-require shape, while bundlers cannot follow the dynamic -// argument, so a build that does not opt into the feature never pulls in the peer's dependency -// chain (#8635). When the peer is installed at build time the user has opted in, so the plugins -// rewrite the call into a literal `require('name')` and let the bundler inline the peer, which -// keeps it working after the bundle is relocated without the peer on disk (#8980). Peers that -// are absent at build time stay opaque, so the rewrite is a no-op and the #8635 guarantee holds. - -// Files that load an optional peer this way, as suffixes of the resolved module path. The same -// suffix matches the repo layout and `node_modules/dd-trace`. Add a file here to extend the -// mechanism to a new optional peer; no plugin change is needed. -const OPTIONAL_PEER_FILES = [ - 'packages/dd-trace/src/openfeature/require-provider.js', -] - -// Captures the peer name from `requireOptionalPeer('name')` / `requireOptionalPeer("name")`. -const OPTIONAL_PEER_LOAD = /requireOptionalPeer\((['"])(.+?)\1\)/g - -// esbuild's `onLoad` needs a path filter; match the basenames so the callback only fires for -// the candidate files, then `matchesOptionalPeerFile` confirms the full suffix on a normalized -// path (basenames carry no separators, so the filter is OS-agnostic). -const OPTIONAL_PEER_FILTER = new RegExp( - `(?:${OPTIONAL_PEER_FILES.map((file) => path.basename(file).replaceAll(/[.*+?^${}()|[\]\\]/g, String.raw`\$&`)).join('|')})$` -) - -/** - * @param {string} normalizedResource - Resolved module path with forward slashes - * @returns {boolean} Whether the module is one of the optional-peer loader files - */ -function matchesOptionalPeerFile (normalizedResource) { - return OPTIONAL_PEER_FILES.some((suffix) => normalizedResource.endsWith(suffix)) -} - -/** - * Rewrites each `requireOptionalPeer('name')` whose peer resolves from `fromDir` into a literal - * `require('name')` so the bundler inlines it. Peers that do not resolve stay opaque, so a build - * without the peer keeps the #8635 guarantee. - * - * @param {string} source - Source of an optional-peer loader file - * @param {string} fromDir - Directory to resolve the optional peers from - * @returns {string} Source with installed optional peers turned into literal requires - */ -function rewriteOptionalPeerLoads (source, fromDir) { - return source.replaceAll(OPTIONAL_PEER_LOAD, (match, _quote, request) => { - try { - require.resolve(request, { paths: [fromDir] }) - } catch { - return match - } - return `require('${request}')` - }) -} - -module.exports = { - OPTIONAL_PEER_FILES, - OPTIONAL_PEER_FILTER, - matchesOptionalPeerFile, - rewriteOptionalPeerLoads, -} diff --git a/packages/datadog-instrumentations/test/helpers/optional-peer-bundler.spec.js b/packages/datadog-instrumentations/test/helpers/optional-peer-bundler.spec.js deleted file mode 100644 index 712d9ee909b..00000000000 --- a/packages/datadog-instrumentations/test/helpers/optional-peer-bundler.spec.js +++ /dev/null @@ -1,59 +0,0 @@ -'use strict' - -const assert = require('node:assert/strict') -const path = require('node:path') - -const { describe, it } = require('mocha') - -const { - OPTIONAL_PEER_FILES, - OPTIONAL_PEER_FILTER, - matchesOptionalPeerFile, - rewriteOptionalPeerLoads, -} = require('../../src/helpers/optional-peer-bundler') - -describe('optional-peer-bundler', () => { - describe('rewriteOptionalPeerLoads', () => { - it('turns an installed peer load into a literal require', () => { - const source = "const x = requireOptionalPeer('@datadog/openfeature-node-server')" - - assert.strictEqual( - rewriteOptionalPeerLoads(source, __dirname), - "const x = require('@datadog/openfeature-node-server')" - ) - }) - - it('leaves an absent peer load opaque', () => { - const source = "const x = requireOptionalPeer('@datadog/this-peer-is-not-installed')" - - assert.strictEqual(rewriteOptionalPeerLoads(source, __dirname), source) - }) - - it('leaves source without an optional-peer load unchanged', () => { - const source = "const fs = require('node:fs')" - - assert.strictEqual(rewriteOptionalPeerLoads(source, __dirname), source) - }) - }) - - describe('matchesOptionalPeerFile', () => { - it('matches a registered optional-peer file suffix', () => { - assert.strictEqual( - matchesOptionalPeerFile('/app/node_modules/dd-trace/packages/dd-trace/src/openfeature/require-provider.js'), - true - ) - }) - - it('does not match an unrelated module in the same directory', () => { - assert.strictEqual(matchesOptionalPeerFile('/app/packages/dd-trace/src/openfeature/index.js'), false) - }) - }) - - describe('OPTIONAL_PEER_FILTER', () => { - it('matches the basename of every registered file', () => { - for (const file of OPTIONAL_PEER_FILES) { - assert.match(path.basename(file), OPTIONAL_PEER_FILTER) - } - }) - }) -}) diff --git a/packages/datadog-plugin-dd-trace-api/src/index.js b/packages/datadog-plugin-dd-trace-api/src/index.js index 029373b0926..79d629a6e68 100644 --- a/packages/datadog-plugin-dd-trace-api/src/index.js +++ b/packages/datadog-plugin-dd-trace-api/src/index.js @@ -78,6 +78,10 @@ module.exports = class DdTraceApiPlugin extends Plugin { } // handleEvent('configure') + // No handoff for `openfeature` yet: an application that calls into the tracer + // exclusively through the `dd-trace-api` shim (instead of `require('dd-trace')`) cannot + // reach `tracer.openfeature` through that shim. This does not affect SSI itself -- + // auto-injection and vendored-provider resolution work regardless of this gap. handleEvent('startSpan') handleEvent('wrap') handleEvent('trace') diff --git a/packages/datadog-webpack/index.js b/packages/datadog-webpack/index.js index 728a29b6702..33bb840be5b 100644 --- a/packages/datadog-webpack/index.js +++ b/packages/datadog-webpack/index.js @@ -6,7 +6,6 @@ const fs = require('node:fs') const instrumentations = require('../datadog-instrumentations/src/helpers/instrumentations') const extractPackageAndModulePath = require('../datadog-instrumentations/src/helpers/extract-package-and-module-path') const hooks = require('../datadog-instrumentations/src/helpers/hooks') -const { matchesOptionalPeerFile } = require('../datadog-instrumentations/src/helpers/optional-peer-bundler') const { isESMFile } = require('../datadog-esbuild/src/utils') const log = require('./src/log') @@ -137,16 +136,6 @@ class DatadogWebpackPlugin { const normalizedResource = resource.replaceAll('\\', '/') - // Rewrite optional-peer loads so installed peers get bundled and survive relocation - // (#8980); absent peers stay opaque, so a build that does not opt into the feature does - // not follow their dependency chain (#8635). - if (matchesOptionalPeerFile(normalizedResource)) { - createData.loaders ||= [] - createData.loaders.push({ loader: require.resolve('./src/optional-peer-loader') }) - log.debug('INLINE: optional-peer loader applied to %s', normalizedResource) - return - } - if (!resource.includes('node_modules')) { return } diff --git a/packages/datadog-webpack/src/optional-peer-loader.js b/packages/datadog-webpack/src/optional-peer-loader.js deleted file mode 100644 index dcf7cb7d48b..00000000000 --- a/packages/datadog-webpack/src/optional-peer-loader.js +++ /dev/null @@ -1,17 +0,0 @@ -'use strict' - -const { rewriteOptionalPeerLoads } = require('../../datadog-instrumentations/src/helpers/optional-peer-bundler') - -/** - * Webpack loader applied to the optional-peer loader files. Rewrites each - * `requireOptionalPeer('name')` whose peer is installed at build time into a literal - * `require('name')` so webpack bundles the peer (#8980). Peers that are absent stay opaque, so - * builds that do not opt into the feature keep the #8635 guarantee. - * - * @param {string} source - * @returns {string} - */ -module.exports = function optionalPeerLoader (source) { - this.cacheable(false) - return rewriteOptionalPeerLoads(source, this.context) -} diff --git a/packages/datadog-webpack/test/plugin.spec.js b/packages/datadog-webpack/test/plugin.spec.js index a58353e48ea..6b7daf4362c 100644 --- a/packages/datadog-webpack/test/plugin.spec.js +++ b/packages/datadog-webpack/test/plugin.spec.js @@ -5,7 +5,6 @@ const { describe, it } = require('mocha') const DatadogWebpackPlugin = require('../index') const loader = require('../src/loader') -const optionalPeerLoader = require('../src/optional-peer-loader') describe('DatadogWebpackPlugin', () => { describe('apply', () => { @@ -50,51 +49,6 @@ describe('DatadogWebpackPlugin', () => { assert.equal(tapped[0], 'DatadogWebpackPlugin') }) }) - - describe('optional peer bundling', () => { - function captureAfterResolve () { - const plugin = new DatadogWebpackPlugin() - let afterResolve - plugin.apply({ - options: { optimization: {} }, - hooks: { - environment: { tap: () => {} }, - thisCompilation: { tap: () => {} }, - normalModuleFactory: { - tap: (name, fn) => fn({ hooks: { afterResolve: { tap: (n, f) => { afterResolve = f } } } }), - }, - }, - }) - return afterResolve - } - - it('applies the optional-peer loader to require-provider', () => { - const createData = { resource: require.resolve('../../dd-trace/src/openfeature/require-provider') } - - captureAfterResolve()({ createData }) - - assert.ok( - createData.loaders?.some((entry) => entry.loader.includes('optional-peer-loader')), - 'the optional-peer loader should be applied' - ) - }) - - it('does not apply the optional-peer loader to unrelated modules', () => { - const createData = { resource: '/app/packages/dd-trace/src/openfeature/index.js' } - - captureAfterResolve()({ createData }) - - assert.strictEqual(createData.loaders, undefined) - }) - - it('ignores modules without a resolved resource', () => { - const createData = {} - - captureAfterResolve()({ createData }) - - assert.strictEqual(createData.loaders, undefined) - }) - }) }) describe('loader', () => { @@ -138,14 +92,3 @@ describe('loader', () => { assert.ok(result.includes('__dd_payload'), 'should use __dd_payload variable') }) }) - -describe('optionalPeerLoader', () => { - it('rewrites an installed optional-peer load into a literal require', () => { - const source = "const { DatadogNodeServerProvider } = requireOptionalPeer('@datadog/openfeature-node-server')" - - const result = optionalPeerLoader.call({ cacheable: () => {}, context: __dirname }, source) - - assert.ok(result.includes("require('@datadog/openfeature-node-server')"), 'should use a literal require') - assert.ok(!result.includes('requireOptionalPeer('), 'should drop the opaque call') - }) -}) diff --git a/packages/dd-trace/index.js b/packages/dd-trace/index.js index ba6740669ef..68c787a5a95 100644 --- a/packages/dd-trace/index.js +++ b/packages/dd-trace/index.js @@ -1,4 +1,3 @@ 'use strict' -require('./src/openfeature/register') module.exports = require('./src/bootstrap') diff --git a/packages/dd-trace/src/feature-registry.js b/packages/dd-trace/src/feature-registry.js deleted file mode 100644 index 5be999ae057..00000000000 --- a/packages/dd-trace/src/feature-registry.js +++ /dev/null @@ -1,29 +0,0 @@ -'use strict' - -/** - * @typedef {{ enable: (config: import('./config/config-base')) => void, disable: () => void }} FeatureModule - * @typedef {new (tracer: import('./tracer'), config: import('./config/config-base')) => object} FeatureProvider - */ - -/** - * @typedef {object} Feature - * @property {string} name - * @property {object} noop - * @property {() => FeatureModule} factory - * @property {(config: import('./config/config-base')) => boolean} isEnabled - * @property {() => FeatureProvider} provider - * @property {(rc: import('./remote_config'), config: import('./config/config-base'), - * proxy: import('./proxy')) => void} [remoteConfig] - */ - -/** @type {{ [name: string]: Feature }} */ -const features = {} - -/** - * @param {Feature} feature - */ -function registerFeature (feature) { - features[feature.name] = feature -} - -module.exports = { features, registerFeature } diff --git a/packages/dd-trace/src/noop/proxy.js b/packages/dd-trace/src/noop/proxy.js index 6f6feaa1d99..9e2c30092f0 100644 --- a/packages/dd-trace/src/noop/proxy.js +++ b/packages/dd-trace/src/noop/proxy.js @@ -1,9 +1,9 @@ 'use strict' -const { features } = require('../feature-registry') const NoopAppsecSdk = require('../appsec/sdk/noop') const NoopLLMObsSDK = require('../llmobs/noop') const NoopAIGuardSDK = require('../aiguard/noop') +const NoopFlaggingProvider = require('../openfeature/noop') const NoopDogStatsDClient = require('./dogstatsd') const NoopTracer = require('./tracer') @@ -12,6 +12,7 @@ const noopAppsec = new NoopAppsecSdk() const noopDogStatsDClient = new NoopDogStatsDClient() const noopLLMObs = new NoopLLMObsSDK(noop) const noopAIGuard = new NoopAIGuardSDK() +const noopFlaggingProviderInstance = new NoopFlaggingProvider() const noopProfiling = { setCustomLabelKeys () {}, runWithLabels (labels, fn) { return fn() }, @@ -25,9 +26,7 @@ class NoopProxy { this.dogstatsd = noopDogStatsDClient this.llmobs = noopLLMObs this.aiguard = noopAIGuard - for (const { name, noop } of Object.values(features)) { - this[name] = noop - } + this.openfeature = noopFlaggingProviderInstance this.setBaggageItem = (key, value) => {} this.getBaggageItem = (key) => {} this.getAllBaggageItems = () => {} diff --git a/packages/dd-trace/src/openfeature/flagging_provider.js b/packages/dd-trace/src/openfeature/flagging_provider.js index 8d3d434c9eb..c4e7057fb48 100644 --- a/packages/dd-trace/src/openfeature/flagging_provider.js +++ b/packages/dd-trace/src/openfeature/flagging_provider.js @@ -1,14 +1,14 @@ 'use strict' const { channel } = require('dc-polyfill') + +const { DatadogNodeServerProvider } = require('../../../../vendor/dist/@datadog/openfeature-node-server') const log = require('../log') const configurationSource = require('./configuration_source') const { EXPOSURE_CHANNEL } = require('./constants/constants') const EvalMetricsHook = require('./eval-metrics-hook') const SpanEnrichmentHook = require('./span-enrichment-hook') -const { DatadogNodeServerProvider } = require('./require-provider') - /** * OpenFeature provider that integrates with Datadog's feature flagging system. * Extends DatadogNodeServerProvider to add tracer integration and configuration management. @@ -48,6 +48,22 @@ class FlaggingProvider extends DatadogNodeServerProvider { this.#configurationSource?.start() } + /** + * @param {import('@openfeature/core').EvaluationContext} [context] + * @returns {Promise} + */ + initialize (context) { + const promise = super.initialize(context) + + // `DatadogNodeServerProvider#initialize` starts a timer that is never unref'd, which would + // otherwise keep an idle process (a short script, a serverless handler) alive for up to + // `initializationTimeoutMs` while waiting for configuration to arrive. + // TODO: remove once `@datadog/openfeature-node-server` unrefs this timer itself. + this.initController?.timeoutId?.unref?.() + + return promise + } + /** * Called when the provider is shut down. * Cleans up resources including channel subscriptions. diff --git a/packages/dd-trace/src/openfeature/register.js b/packages/dd-trace/src/openfeature/register.js deleted file mode 100644 index 81153babaef..00000000000 --- a/packages/dd-trace/src/openfeature/register.js +++ /dev/null @@ -1,35 +0,0 @@ -'use strict' - -const { registerFeature } = require('../feature-registry') - -const noop = new (require('./noop'))() - -/** @typedef {import('../proxy') & { openfeature: object }} OpenFeatureProxy */ - -registerFeature({ - name: 'openfeature', - noop, - factory: () => require('./index'), - provider: () => require('./flagging_provider'), - - /** @param {import('../config/config-base')} config */ - isEnabled (config) { - return config.featureFlags.DD_FEATURE_FLAGS_ENABLED - }, - - /** - * @param {import('../remote_config')} rc - RemoteConfig instance - * @param {import('../config/config-base')} config - * @param {OpenFeatureProxy} proxy - */ - remoteConfig (rc, config, proxy) { - const openfeatureRemoteConfig = require('./remote_config') - const subscribe = config.featureFlags.DD_FEATURE_FLAGS_ENABLED && - config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE === 'remote_config' - openfeatureRemoteConfig.enable( - rc, - () => proxy.openfeature, - subscribe - ) - }, -}) diff --git a/packages/dd-trace/src/openfeature/remote_config.js b/packages/dd-trace/src/openfeature/remote_config.js index 2f41e541ec3..6bc2be754df 100644 --- a/packages/dd-trace/src/openfeature/remote_config.js +++ b/packages/dd-trace/src/openfeature/remote_config.js @@ -6,7 +6,7 @@ const RemoteConfigCapabilities = require('../remote_config/capabilities') * Configures remote config handlers for openfeature feature flagging * * @param {import('../remote_config')} rc - RemoteConfig instance - * @param {() => import('./flagging_provider')} getOpenfeatureProxy + * @param {() => InstanceType>} getOpenfeatureProxy * @param {boolean} subscribe - Whether Agent Remote Config owns UFC delivery */ function enable (rc, getOpenfeatureProxy, subscribe) { diff --git a/packages/dd-trace/src/openfeature/require-provider.js b/packages/dd-trace/src/openfeature/require-provider.js deleted file mode 100644 index 424d2e0d69b..00000000000 --- a/packages/dd-trace/src/openfeature/require-provider.js +++ /dev/null @@ -1,18 +0,0 @@ -'use strict' - -/** @type {(request: string) => typeof import('@datadog/openfeature-node-server')} */ -let requireOptionalPeer - -// @ts-expect-error webpack exposes this escape hatch as a free variable. -// eslint-disable-next-line camelcase -if (typeof __non_webpack_require__ === 'function') { - // eslint-disable-next-line camelcase, no-undef - requireOptionalPeer = __non_webpack_require__ -} else { - // nft recognizes createRequire through a binding named `module`. - const module = require('node:module') - const runtimeRequire = module.createRequire(__filename) - requireOptionalPeer = runtimeRequire -} - -module.exports = requireOptionalPeer('@datadog/openfeature-node-server') diff --git a/packages/dd-trace/src/proxy.js b/packages/dd-trace/src/proxy.js index 443bc3a4872..abea3f6cf43 100644 --- a/packages/dd-trace/src/proxy.js +++ b/packages/dd-trace/src/proxy.js @@ -1,7 +1,6 @@ 'use strict' const NoopProxy = require('./noop/proxy') -const { features } = require('./feature-registry') const DatadogTracer = require('./tracer') const getConfig = require('./config') const { getEnvironmentVariable } = require('./config/helper') @@ -38,9 +37,9 @@ const OFFLINE_VALIDATION_EXPORTERS = new Set([ 'playwright_worker', 'vitest_worker', ]) -const FEATURE_STATE_NOOP = 0 -const FEATURE_STATE_LAZY = 1 -const FEATURE_STATE_ACTIVE = 2 +const OPENFEATURE_STATE_NOOP = 0 +const OPENFEATURE_STATE_LAZY = 1 +const OPENFEATURE_STATE_ACTIVE = 2 class LazyModule { constructor (provider) { @@ -91,8 +90,7 @@ function defineLazily (obj, property, getClass, ...args) { } class Tracer extends NoopProxy { - /** @type {Record | undefined} */ - #featureStates + #openfeatureState = OPENFEATURE_STATE_NOOP constructor () { super() @@ -115,12 +113,9 @@ class Tracer extends NoopProxy { aiguard: new LazyModule(() => require('./aiguard')), iast: new LazyModule(() => require('./appsec/iast')), llmobs: new LazyModule(() => require('./llmobs')), + openfeature: new LazyModule(() => require('./openfeature')), rewriter: new LazyModule(() => require('./appsec/iast/taint-tracking/rewriter')), } - - for (const feature of Object.values(features)) { - this._modules[feature.name] = new LazyModule(feature.factory) - } } /** @@ -213,9 +208,10 @@ class Tracer extends NoopProxy { DynamicInstrumentation.start(config, rc) } - for (const feature of Object.values(features)) { - feature.remoteConfig?.(rc, config, this) - } + const openfeatureRemoteConfig = require('./openfeature/remote_config') + const subscribeOpenfeatureToRemoteConfig = config.featureFlags.DD_FEATURE_FLAGS_ENABLED && + config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE === 'remote_config' + openfeatureRemoteConfig.enable(rc, () => this.openfeature, subscribeOpenfeatureToRemoteConfig) } if (config.profiling.DD_PROFILING_ENABLED === 'true') { @@ -305,28 +301,32 @@ class Tracer extends NoopProxy { } /** - * @param {(typeof features)[string]} feature + * `tracer.openfeature` is only reachable through this proxy. SSI itself (auto-injecting + * the tracer, resolving the vendored provider regardless of the customer's own + * `node_modules` layout) is unaffected -- that is what this PR fixes. The remaining gap is + * narrower: there is no `dd-trace-api` handoff for `openfeature` yet (see + * `packages/datadog-plugin-dd-trace-api`), so an application that calls into the tracer + * exclusively through the `dd-trace-api` shim -- instead of `require('dd-trace')` -- cannot + * reach the flagging provider through that shim. + * * @param {import('./config/config-base')} config */ - #enableFeature (feature, config) { - const states = this.#featureStates ??= {} - const state = states[feature.name] ?? FEATURE_STATE_NOOP - - if (state === FEATURE_STATE_ACTIVE || state === FEATURE_STATE_LAZY) return - states[feature.name] = FEATURE_STATE_LAZY + #enableOpenfeature (config) { + if (this.#openfeatureState !== OPENFEATURE_STATE_NOOP) return + this.#openfeatureState = OPENFEATURE_STATE_LAZY - Reflect.defineProperty(this, feature.name, { + Reflect.defineProperty(this, 'openfeature', { get: () => { - const Provider = feature.provider() - const provider = new Provider(this._tracer, config) + const FlaggingProvider = require('./openfeature/flagging_provider') + const provider = new FlaggingProvider(this._tracer, config) - this._modules[feature.name].enable(config) - Reflect.defineProperty(this, feature.name, { + this._modules.openfeature.enable(config) + Reflect.defineProperty(this, 'openfeature', { value: provider, configurable: true, enumerable: true, }) - states[feature.name] = FEATURE_STATE_ACTIVE + this.#openfeatureState = OPENFEATURE_STATE_ACTIVE return provider }, configurable: true, @@ -373,9 +373,7 @@ class Tracer extends NoopProxy { this._modules.llmobs.disable() } - for (const feature of Object.values(features)) { - if (feature.isEnabled(config)) this.#enableFeature(feature, config) - } + if (config.featureFlags.DD_FEATURE_FLAGS_ENABLED) this.#enableOpenfeature(config) if (this._tracingInitialized) { this._tracer.configure(config) diff --git a/packages/dd-trace/test/openfeature/file-tracing.spec.js b/packages/dd-trace/test/openfeature/file-tracing.spec.js index 1dce1f9817d..78a69f89e27 100644 --- a/packages/dd-trace/test/openfeature/file-tracing.spec.js +++ b/packages/dd-trace/test/openfeature/file-tracing.spec.js @@ -2,8 +2,6 @@ const assert = require('node:assert/strict') const { spawnSync } = require('node:child_process') -const { mkdirSync, mkdtempSync, rmSync, symlinkSync } = require('node:fs') -const { tmpdir } = require('node:os') const path = require('node:path') const { describe, it } = require('mocha') @@ -11,10 +9,8 @@ const { describe, it } = require('mocha') const { NODE_MAJOR } = require('../../../../version') const repoRoot = path.resolve(__dirname, '../../../..') -const expectedPackageFiles = [ - 'node_modules/@datadog/openfeature-node-server/package.json', - 'node_modules/@datadog/flagging-core/package.json', - 'node_modules/spark-md5/package.json', +const expectedTracedFiles = [ + 'vendor/dist/@datadog/openfeature-node-server/index.js', ] if (NODE_MAJOR < 20) { @@ -25,52 +21,86 @@ if (NODE_MAJOR < 20) { // eslint-disable-next-line import/order const { nodeFileTrace } = require('@vercel/nft') -/** - * @param {string} entrypoint - */ -async function assertTracesProvider (entrypoint) { - const { fileList } = await nodeFileTrace([entrypoint], { base: repoRoot }) - - for (const expectedPackageFile of expectedPackageFiles) { - assert.ok(fileList.has(expectedPackageFile), `Expected trace to include ${expectedPackageFile}`) - } -} - describe('OpenFeature file tracing', () => { - it('traces the provider dependency tree through the runtime wrapper', async () => { - await assertTracesProvider(path.join(repoRoot, 'packages/dd-trace/src/openfeature/flagging_provider.js')) - }) + it('traces the provider dependency tree through tracer.openfeature', async function () { + this.timeout(30000) + const entrypoint = path.join(repoRoot, 'packages/dd-trace/src/proxy.js') + const { fileList } = await nodeFileTrace([entrypoint], { base: repoRoot }) - it('traces the provider dependency tree through the explicit entrypoint', async () => { - await assertTracesProvider(path.join(repoRoot, 'openfeature.js')) + for (const expectedTracedFile of expectedTracedFiles) { + assert.ok(fileList.has(expectedTracedFile), `Expected trace to include ${expectedTracedFile}`) + } }) - it('loads the provider through the explicit entrypoint', () => { - require(path.join(repoRoot, 'openfeature.js')) + it('loads the provider through tracer.openfeature', () => { + const tracerPath = JSON.stringify(path.join(repoRoot, 'packages/dd-trace')) + const result = spawnSync( + process.execPath, + [ + '--eval', + `const tracer = require(${tracerPath}); tracer.init({ plugins: false }); ` + + "require('@openfeature/server-sdk'); if (!tracer.openfeature) throw new Error('no provider')", + ], + { encoding: 'utf8' } + ) + + assert.strictEqual(result.status, 0, result.stderr) }) - it('loads the explicit entrypoint as a CommonJS and ESM package subpath', () => { - const fixtureRoot = mkdtempSync(path.join(tmpdir(), 'dd-trace-openfeature-')) - const nodeModulesPath = path.join(fixtureRoot, 'node_modules') + it('does not throw when accessed and registered before `@openfeature/server-sdk` is loaded', () => { + const tracerPath = JSON.stringify(path.join(repoRoot, 'packages/dd-trace')) + const result = spawnSync( + process.execPath, + [ + '--eval', + `const tracer = require(${tracerPath}); tracer.init({ plugins: false }); ` + + 'const provider = tracer.openfeature; ' + + "const { OpenFeature } = require('@openfeature/server-sdk'); " + + 'OpenFeature.setProvider(provider)', + ], + { encoding: 'utf8' } + ) + + assert.strictEqual(result.status, 0, result.stderr) + }) - try { - mkdirSync(nodeModulesPath) - symlinkSync(repoRoot, path.join(nodeModulesPath, 'dd-trace'), 'junction') - const commonJsResult = spawnSync( - process.execPath, - ['--eval', "require('dd-trace/openfeature')"], - { cwd: fixtureRoot, encoding: 'utf8' } - ) - assert.strictEqual(commonJsResult.status, 0, commonJsResult.stderr) + it('does not load active OpenFeature modules before application access', () => { + const packagePath = path.join(repoRoot, 'packages/dd-trace') + const script = ` + const tracer = require(${JSON.stringify(packagePath)}) + tracer.init() + const modules = [ + require.resolve(${JSON.stringify(path.join(packagePath, 'src/exporters/common/client-library-headers'))}), + require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/index'))}), + require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/writers/exposures'))}), + require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/flagging_provider'))}), + require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/configuration_source'))}), + require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/agentless_configuration_source'))}), + require.resolve(${JSON.stringify(path.join(repoRoot, 'vendor/dist/@datadog/openfeature-node-server'))}), + require.resolve('@openfeature/server-sdk'), + require.resolve('@openfeature/core') + ] + process.stdout.write(JSON.stringify(modules.map(module => require.cache[module] !== undefined))) + ` + for (const featureFlagsEnabled of ['false', 'true']) { + for (const remoteConfigurationEnabled of ['false', 'true']) { + for (const tracingEnabled of ['false', 'true']) { + const result = spawnSync(process.execPath, ['-e', script], { + encoding: 'utf8', + env: { + ...process.env, + DD_FEATURE_FLAGS_ENABLED: featureFlagsEnabled, + DD_INSTRUMENTATION_TELEMETRY_ENABLED: 'false', + DD_REMOTE_CONFIGURATION_ENABLED: remoteConfigurationEnabled, + DD_TRACE_ENABLED: tracingEnabled, + DD_TRACE_STARTUP_LOGS: 'false', + }, + }) - const esmResult = spawnSync( - process.execPath, - ['--input-type=module', '--eval', "import 'dd-trace/openfeature.js'"], - { cwd: fixtureRoot, encoding: 'utf8' } - ) - assert.strictEqual(esmResult.status, 0, esmResult.stderr) - } finally { - rmSync(fixtureRoot, { recursive: true, force: true }) + assert.strictEqual(result.status, 0, result.stderr) + assert.deepStrictEqual(JSON.parse(result.stdout), Array(9).fill(false)) + } + } } }) }) diff --git a/packages/dd-trace/test/openfeature/flagging_provider.spec.js b/packages/dd-trace/test/openfeature/flagging_provider.spec.js index ba83736731a..c4183e3039e 100644 --- a/packages/dd-trace/test/openfeature/flagging_provider.spec.js +++ b/packages/dd-trace/test/openfeature/flagging_provider.spec.js @@ -1,9 +1,9 @@ 'use strict' const assert = require('node:assert/strict') -const fs = require('node:fs') -const { describe, it, beforeEach, afterEach } = require('mocha') +const { DatadogNodeServerProvider } = require('@datadog/openfeature-node-server') +const { describe, it, beforeEach } = require('mocha') const sinon = require('sinon') const proxyquire = require('proxyquire') @@ -76,6 +76,7 @@ describe('FlaggingProvider', () => { './configuration_source': configurationSource, './eval-metrics-hook': mockEvalMetricsHookClass, './span-enrichment-hook': mockSpanEnrichmentHookClass, + '../../../../vendor/dist/@datadog/openfeature-node-server': { DatadogNodeServerProvider }, }) }) @@ -208,82 +209,9 @@ describe('FlaggingProvider', () => { describe('inheritance', () => { it('should extend DatadogNodeServerProvider', () => { - const { DatadogNodeServerProvider } = require('@datadog/openfeature-node-server') const provider = new FlaggingProvider(mockTracer, mockConfig) assert.ok(provider instanceof DatadogNodeServerProvider) }) }) - - // Pins the optional-peer gate against leaking the provider chain into customer bundles (#8635). - // `file-tracing.spec.js` covers the same wrapper's nft contract. - describe('optional-peer gate', () => { - const modulePath = require.resolve('../../src/openfeature/flagging_provider') - const providerModulePath = require.resolve('../../src/openfeature/require-provider') - const peer = '@datadog/openfeature-node-server' - - afterEach(() => { - delete require.cache[modulePath] - delete require.cache[providerModulePath] - delete globalThis.__webpack_require__ - delete globalThis.__non_webpack_require__ - }) - - it('uses `require` outside a bundler', () => { - assert.strictEqual(typeof globalThis.__webpack_require__, 'undefined') - delete require.cache[modulePath] - delete require.cache[providerModulePath] - - const ReloadedFlaggingProvider = require(modulePath) - - assert.strictEqual(typeof ReloadedFlaggingProvider, 'function') - assert.strictEqual(ReloadedFlaggingProvider.name, 'FlaggingProvider') - }) - - it('uses `__non_webpack_require__`, never `__webpack_require__`, under webpack', () => { - const loadCalls = [] - globalThis.__webpack_require__ = () => { - throw new Error('webpack require must not run for an optional peer') - } - /** @param {string} request */ - globalThis.__non_webpack_require__ = (request) => { - loadCalls.push(request) - return require(request) - } - - delete require.cache[modulePath] - delete require.cache[providerModulePath] - const ReloadedFlaggingProvider = require(modulePath) - - assert.deepStrictEqual(loadCalls, [peer]) - assert.strictEqual(typeof ReloadedFlaggingProvider, 'function') - }) - - it('falls back to `require` when `__non_webpack_require__` is absent', () => { - globalThis.__webpack_require__ = () => { - throw new Error('webpack require must not run for an optional peer') - } - - delete require.cache[modulePath] - delete require.cache[providerModulePath] - const ReloadedFlaggingProvider = require(modulePath) - - assert.strictEqual(typeof ReloadedFlaggingProvider, 'function') - }) - - it('keeps the provider load opaque to bundlers', () => { - const source = fs.readFileSync(providerModulePath, 'utf8') - - assert.doesNotMatch( - source, - /require\(\s*['"]@datadog\/openfeature-node-server['"]\s*\)/, - 'a literal require would let bundlers resolve the optional peer chain at build time' - ) - assert.doesNotMatch( - source, - /\brequire\(\s*[^'"\s]/, - 'a dynamic require would create a webpack expression dependency' - ) - }) - }) }) diff --git a/packages/dd-trace/test/openfeature/flagging_provider_timeout.spec.js b/packages/dd-trace/test/openfeature/flagging_provider_timeout.spec.js index 721ad85cc7d..408ae381f80 100644 --- a/packages/dd-trace/test/openfeature/flagging_provider_timeout.spec.js +++ b/packages/dd-trace/test/openfeature/flagging_provider_timeout.spec.js @@ -3,6 +3,7 @@ const assert = require('node:assert/strict') const { inspect } = require('node:util') +const { DatadogNodeServerProvider } = require('@datadog/openfeature-node-server') const { ProviderEvents } = require('@openfeature/server-sdk') const { afterEach, beforeEach, describe, it } = require('mocha') const proxyquire = require('proxyquire') @@ -60,6 +61,7 @@ describe('FlaggingProvider Initialization Timeout', () => { './configuration_source': { create: sinon.stub(), }, + '../../../../vendor/dist/@datadog/openfeature-node-server': { DatadogNodeServerProvider }, }) }) @@ -93,6 +95,21 @@ describe('FlaggingProvider Initialization Timeout', () => { assert.strictEqual(provider.initController.isInitializing(), false) }) + it('does not keep the process alive while waiting for configuration', async () => { + const provider = new FlaggingProvider(mockTracer, mockConfig) + + const initPromise = provider.initialize() + + initPromise.catch(() => { + // Expected to reject on timeout + }) + + assert.strictEqual(provider.initController.timeoutId.hasRef(), false) + + await clock.tickAsync(30000) + await initPromise.catch(() => {}) + }) + it('should not timeout if configuration is set before 30 seconds', async () => { const provider = new FlaggingProvider(mockTracer, mockConfig) diff --git a/packages/dd-trace/test/openfeature/register.spec.js b/packages/dd-trace/test/openfeature/register.spec.js deleted file mode 100644 index 13d67209ed7..00000000000 --- a/packages/dd-trace/test/openfeature/register.spec.js +++ /dev/null @@ -1,141 +0,0 @@ -'use strict' - -const assert = require('node:assert/strict') -const { spawnSync } = require('node:child_process') -const path = require('node:path') - -const { beforeEach, describe, it } = require('mocha') -const proxyquire = require('proxyquire') -const sinon = require('sinon') - -require('../setup/core') - -describe('OpenFeature register', () => { - let config - let feature - let FlaggingProvider - let openfeatureModule - let openfeatureRemoteConfig - let proxy - let registerFeature - - function NoopFlaggingProvider () {} - - beforeEach(() => { - /** @param {object} registeredFeature */ - const register = (registeredFeature) => { - feature = registeredFeature - } - registerFeature = sinon.spy(register) - openfeatureModule = { - enable: sinon.spy(), - disable: sinon.spy(), - } - openfeatureRemoteConfig = { - enable: sinon.spy(), - } - FlaggingProvider = function () {} - - delete require.cache[require.resolve('../../src/openfeature/register')] - proxyquire('../../src/openfeature/register', { - '../feature-registry': { registerFeature }, - './flagging_provider': FlaggingProvider, - './remote_config': openfeatureRemoteConfig, - './index': openfeatureModule, - './noop': NoopFlaggingProvider, - }) - - config = { - featureFlags: { - DD_FEATURE_FLAGS_CONFIGURATION_SOURCE: 'agentless', - DD_FEATURE_FLAGS_ENABLED: true, - }, - } - proxy = { openfeature: feature.noop } - }) - - it('registers the OpenFeature feature boundaries', () => { - sinon.assert.calledOnce(registerFeature) - - assert.strictEqual(feature.name, 'openfeature') - assert.ok(feature.noop instanceof NoopFlaggingProvider) - assert.strictEqual(feature.factory(), openfeatureModule) - assert.strictEqual(feature.provider(), FlaggingProvider) - }) - - it('does not load active OpenFeature modules before application access', () => { - const packagePath = path.join(__dirname, '../..') - const script = ` - const tracer = require(${JSON.stringify(packagePath)}) - tracer.init() - const modules = [ - require.resolve(${JSON.stringify(path.join(packagePath, 'src/exporters/common/client-library-headers'))}), - require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/index'))}), - require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/writers/exposures'))}), - require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/flagging_provider'))}), - require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/require-provider'))}), - require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/configuration_source'))}), - require.resolve(${JSON.stringify(path.join(packagePath, 'src/openfeature/agentless_configuration_source'))}), - require.resolve('@datadog/openfeature-node-server'), - require.resolve('@openfeature/server-sdk'), - require.resolve('@openfeature/core') - ] - process.stdout.write(JSON.stringify(modules.map(module => require.cache[module] !== undefined))) - ` - for (const featureFlagsEnabled of ['false', 'true']) { - for (const remoteConfigurationEnabled of ['false', 'true']) { - for (const tracingEnabled of ['false', 'true']) { - const result = spawnSync(process.execPath, ['-e', script], { - encoding: 'utf8', - env: { - ...process.env, - DD_FEATURE_FLAGS_ENABLED: featureFlagsEnabled, - DD_INSTRUMENTATION_TELEMETRY_ENABLED: 'false', - DD_REMOTE_CONFIGURATION_ENABLED: remoteConfigurationEnabled, - DD_TRACE_ENABLED: tracingEnabled, - DD_TRACE_STARTUP_LOGS: 'false', - }, - }) - - assert.strictEqual(result.status, 0, result.stderr) - assert.deepStrictEqual(JSON.parse(result.stdout), Array(10).fill(false)) - } - } - } - }) - - it('selects the provider from the calculated Feature Flags state', () => { - assert.strictEqual(feature.isEnabled(config), true) - - config.featureFlags.DD_FEATURE_FLAGS_ENABLED = false - - assert.strictEqual(feature.isEnabled(config), false) - }) - - it('installs Remote Config delivery when selected', () => { - const rc = {} - config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE = 'remote_config' - - feature.remoteConfig(rc, config, proxy) - - sinon.assert.calledOnceWithExactly(openfeatureRemoteConfig.enable, rc, sinon.match.func, true) - assert.strictEqual(openfeatureRemoteConfig.enable.firstCall.args[1](), proxy.openfeature) - }) - - it('does not install Remote Config delivery when disabled', () => { - const rc = {} - config.featureFlags.DD_FEATURE_FLAGS_ENABLED = false - - feature.remoteConfig(rc, config, proxy) - - sinon.assert.calledOnceWithExactly(openfeatureRemoteConfig.enable, rc, sinon.match.func, false) - }) - - it('does not install Remote Config delivery for the default agentless source', () => { - const rc = {} - - feature.remoteConfig(rc, config, proxy) - - sinon.assert.calledOnceWithExactly(openfeatureRemoteConfig.enable, rc, sinon.match.func, false) - }) -}) diff --git a/packages/dd-trace/test/proxy.spec.js b/packages/dd-trace/test/proxy.spec.js index 2e35f114a81..bd24e4454c0 100644 --- a/packages/dd-trace/test/proxy.spec.js +++ b/packages/dd-trace/test/proxy.spec.js @@ -6,7 +6,6 @@ const { inspect } = require('node:util') const { describe, it, beforeEach, afterEach } = require('mocha') const sinon = require('sinon') const proxyquire = require('proxyquire') -const featureRegistry = require('../src/feature-registry') const RemoteConfigCapabilities = require('../src/remote_config/capabilities') require('./setup/core') @@ -268,25 +267,8 @@ describe('TracerProxy', () => { './dogstatsd': dogStatsD, './noop/dogstatsd': NoopDogStatsDClient, './flare': flare, - }) - - const { enable: openfeatureRcEnable } = require('../src/openfeature/remote_config') - const noopOpenfeature = {} - - featureRegistry.registerFeature({ - name: 'openfeature', - noop: noopOpenfeature, - factory: () => openfeature, - provider: () => OpenFeatureProvider, - /** @param {object} config */ - isEnabled (config) { - return config.featureFlags.DD_FEATURE_FLAGS_ENABLED - }, - remoteConfig (rc, config, proxy) { - const subscribe = config.featureFlags.DD_FEATURE_FLAGS_ENABLED && - config.featureFlags.DD_FEATURE_FLAGS_CONFIGURATION_SOURCE === 'remote_config' - openfeatureRcEnable(rc, () => proxy.openfeature, subscribe) - }, + './openfeature': openfeature, + './openfeature/flagging_provider': OpenFeatureProvider, }) proxy = new ProxyClass() diff --git a/scripts/check_licenses.js b/scripts/check_licenses.js index 13fb872a120..15d260507ad 100644 --- a/scripts/check_licenses.js +++ b/scripts/check_licenses.js @@ -188,7 +188,9 @@ function addNpmProductionDependencies (dependencies, packageLockPath) { if (!packages) throw new Error('package-lock.json does not contain package metadata') for (const [packagePath, dependency] of Object.entries(packages)) { - if (!packagePath || dependency.link || (dependency.dev && !dependency.devOptional)) continue + // A peer dependency is supplied by the consumer, not shipped by this package, the same + // way addYarnProductionDependencies excludes peerDependencies from its graph walk. + if (!packagePath || dependency.link || dependency.peer || (dependency.dev && !dependency.devOptional)) continue dependencies.add(dependency.name ?? getNameFromPackagePath(packagePath)) } diff --git a/scripts/check_licenses.spec.mjs b/scripts/check_licenses.spec.mjs index 0de99d82832..c6b7c9ce168 100644 --- a/scripts/check_licenses.spec.mjs +++ b/scripts/check_licenses.spec.mjs @@ -21,7 +21,6 @@ const expectedDependencies = [ 'source-package', 'transitive-source', 'unversioned-target', - 'vendor-peer', 'vendor-regular', 'vendor-source', ] diff --git a/scripts/electron-package-excludes.json b/scripts/electron-package-excludes.json index a27c78da6a2..e87589069ff 100644 --- a/scripts/electron-package-excludes.json +++ b/scripts/electron-package-excludes.json @@ -4,7 +4,6 @@ "@datadog/native-appsec", "@datadog/native-iast-taint-tracking", "@datadog/native-metrics", - "@datadog/openfeature-node-server", "@datadog/pprof", "@datadog/wasm-js-rewriter", "oxc-parser" diff --git a/vendor/package-lock.json b/vendor/package-lock.json index f6bde2233b2..72edc90abba 100644 --- a/vendor/package-lock.json +++ b/vendor/package-lock.json @@ -8,6 +8,7 @@ "license": "(Apache-2.0 OR BSD-3-Clause)", "dependencies": { "@apm-js-collab/code-transformer": "^0.18.1", + "@datadog/openfeature-node-server": "2.1.0", "@datadog/sketches-js": "2.1.1", "@datadog/source-map": "npm:source-map@^0.6.0", "@isaacs/ttlcache": "^2.1.5", @@ -66,6 +67,27 @@ "node": ">=0.10.0" } }, + "node_modules/@datadog/flagging-core": { + "version": "2.0.2", + "resolved": "https://registry.npmjs.org/@datadog/flagging-core/-/flagging-core-2.0.2.tgz", + "integrity": "sha512-2+oWyqz/EMNXtsgyW3NtueFgL0TciWInyMg/6bEUZhfr7UgPzCQ88Nag9FGoPCig19F/tHXE5hLiQccjkRUMWQ==", + "license": "Apache-2.0", + "dependencies": { + "spark-md5": "^3.0.2" + } + }, + "node_modules/@datadog/openfeature-node-server": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/@datadog/openfeature-node-server/-/openfeature-node-server-2.1.0.tgz", + "integrity": "sha512-6nzVv7d5budwJTqh/k52L7cUcl3bNsg/zYrWAniHVBlf2cyYH2dH20xwZ40kEkBVfhoaJRelQIg0qbNeoVmJlw==", + "license": "Apache-2.0", + "dependencies": { + "@datadog/flagging-core": "2.0.2" + }, + "engines": { + "node": ">=18.0.0" + } + }, "node_modules/@datadog/sketches-js": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/@datadog/sketches-js/-/sketches-js-2.1.1.tgz", @@ -731,6 +753,12 @@ "node": ">= 12" } }, + "node_modules/spark-md5": { + "version": "3.0.2", + "resolved": "https://registry.npmjs.org/spark-md5/-/spark-md5-3.0.2.tgz", + "integrity": "sha512-wcFzz9cDfbuqe0FZzfi2or1sgyIrsDwmPwfZC4hiNidPdPINjeUwNfv5kldczoEAcjl9Y1L3SM7Uz2PUEQzxQw==", + "license": "(WTFPL OR MIT)" + }, "node_modules/tlhunter-sorted-set": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/tlhunter-sorted-set/-/tlhunter-sorted-set-0.1.0.tgz", diff --git a/vendor/package.json b/vendor/package.json index f575e1cf948..6e1ed0e12f3 100644 --- a/vendor/package.json +++ b/vendor/package.json @@ -5,6 +5,7 @@ }, "dependencies": { "@apm-js-collab/code-transformer": "^0.18.1", + "@datadog/openfeature-node-server": "2.1.0", "@datadog/sketches-js": "2.1.1", "@datadog/source-map": "npm:source-map@^0.6.0", "@isaacs/ttlcache": "^2.1.5", diff --git a/vendor/rspack.config.js b/vendor/rspack.config.js index deb1137308d..d0f84ce6589 100644 --- a/vendor/rspack.config.js +++ b/vendor/rspack.config.js @@ -2,12 +2,6 @@ // TODO: Stop depending on `@opentelemetry/api` and instead intercept the user // version with an instrumentation. -// TODO: Stop depending on `@openfeature/server-sdk` and `@openfeature/core` and -// instead intercept the user version with an instrumentation. -// TODO: Vendor `@datadog/openfeature-node-server` when the above has been -// addressed. Until then, `packages/dd-trace/src/openfeature/flagging_provider.js` -// loads it through a bundler-opaque require so customer bundles do not -// follow the optional peer-of-peer chain (see #8635). // TODO: Fix `import-in-the-middle` so that it doesn't interfere with the global // object or switch to our own internal loader and remove the dependency. // TODO: Vendor `dc-polyfill` and figure out why it fails the tests. @@ -67,9 +61,9 @@ module.exports = { }), ], }, - // These are shared between dd-trace and users, so they need to be external. + // This is shared between dd-trace and users, so it needs to be external. externals: { - '@opentelemetry/api': '@opentelemetry/api' + '@opentelemetry/api': '@opentelemetry/api', }, plugins: [ new LicenseWebpackPlugin({ diff --git a/yarn.lock b/yarn.lock index d5bf8141ed4..c30d163be58 100644 --- a/yarn.lock +++ b/yarn.lock @@ -279,10 +279,10 @@ node-addon-api "^6.1.0" node-gyp-build "^3.9.0" -"@datadog/openfeature-node-server@2.0.2": - version "2.0.2" - resolved "https://registry.yarnpkg.com/@datadog/openfeature-node-server/-/openfeature-node-server-2.0.2.tgz#304b93f12fe63619d5bb140023dfcecf56f38017" - integrity sha512-647eJuiOVzCEk50His94wjSlOgJCbHIJv4cAW425eW3GqQJC2Y73qGBYV1/s9oBsAmsUNa90tc+ETunfUH2hKQ== +"@datadog/openfeature-node-server@2.1.0": + version "2.1.0" + resolved "https://registry.yarnpkg.com/@datadog/openfeature-node-server/-/openfeature-node-server-2.1.0.tgz#28a6eba85bfda1ad4c7564812a19364c255b14cf" + integrity sha512-6nzVv7d5budwJTqh/k52L7cUcl3bNsg/zYrWAniHVBlf2cyYH2dH20xwZ40kEkBVfhoaJRelQIg0qbNeoVmJlw== dependencies: "@datadog/flagging-core" "2.0.2"