feat(openfeature): vendor the flagging provider instead of an optional peer dependency - #9570
Merged
Merged
Conversation
Contributor
Overall package sizeSelf size: 8.07 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.3 | 125.43 kB | 441.68 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: a64371e | Docs | Datadog PR Page | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-13 03:18:13 Comparing candidate commit a64371e in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2325 metrics, 33 unstable metrics.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9570 +/- ##
==========================================
- Coverage 98.56% 98.52% -0.05%
==========================================
Files 972 967 -5
Lines 140837 140644 -193
Branches 12982 12031 -951
==========================================
- Hits 138821 138570 -251
- Misses 2016 2074 +58
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rochdev
changed the title
rochdev
force-pushed
the
rochdev/openfeature-bundler-entrypoint
branch
from
August 1, 2026 01:08
9faa31a to
8b21280
Compare
rochdev
marked this pull request as ready for review
August 3, 2026 03:50
rochdev
requested review from
dd-oleksii,
leoromanovsky and
tlhunter
and removed request for
a team
August 3, 2026 03:50
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2db427c4c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: caa112c44a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
BridgeAR
reviewed
Aug 3, 2026
BridgeAR
left a comment
BridgeAR
left a comment
Member
There was a problem hiding this comment.
Please keep the bundler tests, since they verify things are working as expected.
| 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 |
Member
There was a problem hiding this comment.
We could actually still have a bundled part, even if it is a peer dependency
Member
Author
There was a problem hiding this comment.
Not sure I understand the above, please clarify.
Member
Author
They were validating the old approach explicitly and were replaced by new tests that test the behaviour instead. |
Member
I had a similar issue when I built dc-polyfill. I solved it by creating a global registry that can be shared across package versions. Is that something we can do here as well? Maybe not as every single version of dc-polyfill had such a concept but your package won't... |
tlhunter
reviewed
Aug 3, 2026
leoromanovsky
requested changes
Aug 4, 2026
leoromanovsky
left a comment
leoromanovsky
left a comment
Contributor
There was a problem hiding this comment.
- Fix the test command.
On the exact PR head, npm run test:openfeature reported:
Cannot find any files matching pattern "packages/datadog-instrumentations/test/openfeature-server-sdk.spec.{js,mjs}"
The suite reported 209 passing tests, but the newly added instrumentation spec did not run.
- Retain real black-box tests. The PR deletes the dedicated esbuild and Webpack
OpenFeature tests. I manually verified CJS, documented ESM loading, and esbuild at head caa112c, but unit-testing the bridge is not durable protection for the customer integration.
- Either add the dd-trace-api/SSI provider handoff or explicitly state that end-to-end FFE through pure SSI remains future work.
|
@rochdev Let's assume we are able to remove That would allow for security updates in But as a stop-gap, I'd be fine with moving this forward so long as @leoromanovsky's concerns are addressed. |
Member
Author
What would be the benefit of the registry over an instrumentation? (assuming it would be possible which it may not be as you pointed out because of version support) |
Member
Author
|
@greghuels Assuming the dependency could be removed completely that would definitely be the best case scenario, but it's unclear whether that would be achievable. By vendoring and defining an external, we're basically removing the dependency by force externally and then replacing it with a placeholder that is then replaced by the real dependency from the user at import time, but if there was no dependency to being with that wouldn't be needed. |
Member
Author
|
@leoromanovsky Everything should be addressed! |
@rochdev Yeah, I think there needs to be an investigation on our end to see if we can do this without causing breaking changes. In the meantime, I think your change here makes sense. |
greghuels
added a commit
to DataDog/openfeature-js-client
that referenced
this pull request
Aug 7, 2026
Adds a pre-publish script that fails if @openfeature/server-sdk or @openfeature/core appear in dependencies/peerDependencies, or if the compiled .js files contain any runtime @openfeature imports. This prevents accidentally reintroducing the runtime dependency that would break SSI compatibility with dd-trace-js, where dd-trace is installed outside the application's node_modules tree and would get a separate copy of the SDK with a different event emitter identity. See DataDog/dd-trace-js#9570 for context.
greghuels
added a commit
to DataDog/openfeature-js-client
that referenced
this pull request
Aug 7, 2026
* Custom EventEmitter * Bundle type declarations to remove @openfeature devDependencies from .d.ts output Add dts-bundle-generator to inline all @openfeature/core and @openfeature/server-sdk types into a single bundled index.d.ts. This eliminates unresolved import type references in the published .d.ts files, which previously caused 'Cannot find module' errors for TypeScript consumers since neither package is declared as a runtime dependency or peerDependency. Changes: - package.json: types field now points to bundled index.d.ts - package.json: build script runs build:types after tsc builds - package.json: added dts-bundle-generator to devDependencies - index.d.ts: generated bundled type declarations (642 lines, zero @openfeature references, node:diagnostics_channel imported externally) * Fix yarn install: pin TypeScript to 5.9.3 via resolutions dts-bundle-generator declares typescript >=5.0.2 as a regular dependency, causing yarn to resolve TypeScript 7.0.2 for its nested copy. The built-in compat/typescript patch then fails with ENOENT trying to lstat an incorrect path. Adding a resolutions field in the root package.json forces all TypeScript resolutions to 5.9.3, matching the existing devDependency constraint. Also includes yarn.lock update for dts-bundle-generator and devDependencies reordering by yarn. * Don't export inlined OpenFeature types from bundled declarations Use --export-referenced-types false so inlined types like Provider, ResolutionDetails, ServerProviderEvents, ProviderEventEmitter, etc. are used in DatadogNodeServerProvider's method signatures but not exported as standalone types. This prevents consumers from accidentally importing our bundled copy of these types instead of getting them from their own @openfeature/server-sdk installation. The bundled index.d.ts now exports only: - UniversalFlagConfigurationV1 - DatadogNodeServerProviderOptions - DatadogNodeServerProvider All OpenFeature types remain inlined (zero @openfeature references) for SSI compatibility, but are invisible to consumers' import statements. TypeScript still performs structural comparison when the customer passes the provider to their own OpenFeature SDK. * Fix dependency build metadata * Fix build:types temp file cleanup and add index.d.ts to clean script - Use trap EXIT in build:types to ensure src/.dts-entry.ts is removed even if dts-bundle-generator fails - Add src/.dts-entry.ts to .gitignore as a safety net - Add index.d.ts to the clean script so all generated artifacts are removed consistently * Add prepack guard to prevent @openfeature/server-sdk runtime dependency Adds a pre-publish script that fails if @openfeature/server-sdk or @openfeature/core appear in dependencies/peerDependencies, or if the compiled .js files contain any runtime @openfeature imports. This prevents accidentally reintroducing the runtime dependency that would break SSI compatibility with dd-trace-js, where dd-trace is installed outside the application's node_modules tree and would get a separate copy of the SDK with a different event emitter identity. See DataDog/dd-trace-js#9570 for context. * Fix duplicate resolutions block in root package.json The earlier TypeScript resolution was added as a second resolutions block instead of merging with the existing one, causing yarn to ignore the typescript@npm:>=5.0.2 pin and crash on TS 7.0.2's compat patch. Merged into a single resolutions block. * Add comment explaining why build:types is needed * formatting * test: add TypeScript consumer test for node-server package The node-server package's bundled index.d.ts (generated by dts-bundle-generator) had no TypeScript consumer test to verify it compiles correctly from a consumer's perspective. The existing test-app-node/test.js only tested runtime require() behavior. This adds: - test-app-node/tsconfig.json: consumer-style TypeScript config - test-app-node/typecheck.ts: imports and exercises the public API (DatadogNodeServerProvider, UniversalFlagConfigurationV1, etc.) - test-app-node/package.json: adds typescript + @types/node devDeps - scripts/test-node-package-install.sh: runs tsc --noEmit before runtime tests The type check runs in both scenarios: 1. Without @openfeature/* installed (SSI/dd-trace) — verifies the bundled index.d.ts is self-contained with no dangling references 2. With @openfeature/* installed (normal consumer) — verifies type compatibility with the real OpenFeature SDK * test: add tests for NodeProviderEventEmitter Cover emit/addHandler, handler error isolation (incl. async rejections and a throwing logger), removeHandler LIFO semantics, removeAllHandlers scoping, getHandlers, and setLogger chaining/current-logger routing. * format * test: verify OpenFeature.setProvider(provider) type-checks against minimum SDK version Add Test 11 to typecheck.ts that calls OpenFeature.setProvider(provider) to catch type drift between the bundled index.d.ts and the Provider interface expected by @openfeature/server-sdk. Uses @ts-ignore on the import so the file still compiles in the SSI/dd-trace scenario where @openfeature/server-sdk is not installed. Add CI step to run the node install test against the minimum supported OpenFeature SDK versions (server-sdk 1.15.0, core 1.3.0) per COMPATIBILITY.md, ensuring the type check catches regressions at the lower bound of the supported range.
greghuels
requested changes
Aug 7, 2026
There was a problem hiding this comment.
@rochdev @datadog/openfeature-node-server should no longer have a dependency on @openfeature/server-sdk. The change to remove @openfeature/server-sdk as a dependency was merged here and released in version 2.1.0.
So it might make sense to close this PR out and simply add @datadog/openfeature-node-server as a patch-range dependency (i.e. "~2.1.0").
Member
Author
|
Updated to remove all the hackery around the OpenFeature SDK. Still vendoring our own library as ideally we would do that with everything for consistency, to avoid weird issues when mixing bundlers and ESM, to reduce package size, and reduce complexity and time of linking dependencies, and eventually be self-contained to maximize portability. Any library not done yet is because they have some incompatibility with being vendored, not by choice. @leoromanovsky @greghuels Should be ready for re-review! |
| "@babel/helpers": "^8.0.0", | ||
| "@eslint/eslintrc": "^3.3.5", | ||
| "@eslint/js": "^10.0.1", | ||
| "@datadog/openfeature-node-server": "2.1.0", |
Contributor
There was a problem hiding this comment.
exact version pin needed?
Member
Author
There was a problem hiding this comment.
We started doing this in #5995. Unfortunately the PR doesn't explain the reasoning, but I prefer to keep it the same so that it stays consistent. The good news is that it doesn't really change anything because the vendoring is done from the lockfile, so pin or no pin is effectively the same there, and it makes sense to keep the dev dependency in line with that. This is also automatically updated weekly by Dependabot.
There was a problem hiding this comment.
The good news is that it doesn't really change anything because the vendoring is done from the lockfile
Agreed that with vendoring there's no difference. It might be worth documenting why vendoring is necessary though (or pointing us to existing documentation). Getting to a place where transient packages can be upgraded by customers without a version bump of dd-trace has the benefit of allowing customers to pull in hotfixes and security patches of transient dependencies via yarn upgrade or npm update, even if they are many versions behind latest dd-trace. If the idea is to mitigate zero-day supply chain exploits, then that's a legit reason that should at least be documented.
Member
Author
There was a problem hiding this comment.
It also does the opposite in that it allows zero-day exploits to actually land to begin with. So vendoring with a 5 days lag allows us to never ship the exploit to begin with. If the exploit is discovered more than 5 days later, then we can expedite a fix as well without waiting for Dependabot. Trade-offs all around, but given the other benefits of vendoring and the fact that we already do this for everything else (except native extensions because it's not possible to vendor them and we instead avoid installing entirely where not supported) I'd rather keep things consistent. Happy to revisit any past decisions later on but that's out of scope of this PR.
There was a problem hiding this comment.
Yeah, that's what I was calling out by referencing zero day attacks. It's a good reason to keep vendoring. Like you said, tradeoffs either way. It's just unclear on my end if that was motivation behind vendoring. In any case, you should be good to go with this change.
greghuels
previously approved these changes
Aug 7, 2026
rochdev
force-pushed
the
rochdev/openfeature-bundler-entrypoint
branch
from
August 10, 2026 02:59
9ea2bf5 to
f479b46
Compare
leoromanovsky
previously approved these changes
Aug 11, 2026
…point dd-trace/openfeature previously only existed as a side-effect require for file tracers (#9324). Turn it into the real public entrypoint: it now returns a usable FlaggingProvider instance after tracer.init(), and tracer.openfeature is deprecated in its favor since it doesn't work in bundled applications. Extract the base provider class into a factory so it can be constructed from either the bundler-opaque require-provider wrapper (legacy tracer.openfeature) or a plain require (the new entrypoint) without duplicating the class body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
openfeature.js now pulls in the full tracer package to check tracer.init() state, so nodeFileTrace has a much larger dependency tree to walk. The default 5s mocha timeout was tight enough that CI runners under load exceeded it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…l peer `@datadog/openfeature-node-server` was an optional peer dependency resolved through a bundler-opaque require, requiring dedicated esbuild/webpack plugin code to keep the require invisible when the peer wasn't installed (#8635) and to inline it so bundles survived relocation when it was (#8980). Vendoring the provider into `vendor/dist/` removes the need for a peer at all, so all of that machinery (`feature-registry.js`, `register.js`, `require-provider.js`, the optional-peer esbuild/webpack loaders, and their integration tests) is deleted in favor of a single lazy `tracer.openfeature` property. The vendored provider's only remaining external dependency is `@openfeature/server-sdk`, used solely for `OpenFeatureEventEmitter` and `ProviderEvents`. Bundling our own copy would give those a different identity than the customer's, so it's externalized to a small bridge module (`server-sdk-bridge.js`) that a new `openfeature-server-sdk` instrumentation fills in from the customer's own `require()`. `FlaggingProvider#initialize` also unrefs the vendored provider's initialization timer, which otherwise keeps an idle process (a short script, a serverless handler) alive for up to `initializationTimeoutMs` while waiting for configuration to arrive. TODO left to remove this once `@datadog/openfeature-node-server` unrefs it upstream. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
scripts/check_licenses.js treated every non-dev entry in vendor/package-lock.json as requiring license attribution, including `peer: true` entries. Those record an unresolved peer dependency range that npm never actually installs, so nothing is shipped for them. The official dd-license-attribution tool used in CI already excludes these, so the local script disagreed with CI and had two extraneous rows (@openfeature/core, @openfeature/server-sdk) added to satisfy it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The prior fix's comment claimed npm never installs peer entries, which is false under npm 7+ auto-install-peers - vendor/node_modules physically has @openfeature/core and @openfeature/server-sdk installed. The real reason they don't need attribution is that peer dependencies are supplied by the consumer rather than shipped by this package, matching how the yarn.lock walk already excludes peerDependencies. The pre-existing vendor-peer test fixture assumed the opposite and was never validated against real CI behavior for a peer case, so it broke once the npm-lock scan started excluding peers too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… deferred bridge emitter @openfeature/core's transferListeners always calls oldProvider.events?.removeHandler(...) on every subsequent OpenFeature.setProvider() call for a domain, regardless of whether the app registered any handlers itself. The deferred emitter only implemented addHandler/emit, so replacing the Datadog OpenFeature provider with another provider at runtime threw "oldProvider.events.removeHandler is not a function", crashing the customer's app. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Fix test:openfeature script so the instrumentation spec actually runs: mocha treats a brace-only glob segment as a literal filename, so add a wildcard segment that both mocha and verify-exercised-tests resolve. - Add real esbuild/webpack black-box tests that bundle @openfeature/server-sdk (not marked external) and assert dd-trace's bundler-instrumentation mechanism still bridges the real event emitter into the vendored provider. - Correct comments in proxy.js and the dd-trace-api plugin: SSI itself is fixed by this PR's vendoring; the only remaining gap is that the dd-trace-api shim has no openfeature handoff. Generated with Claude Code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@datadog/openfeature-node-server 2.1.0 ships its own ProviderEventEmitter and no longer depends on @openfeature/server-sdk at all (neither as a dependency nor a peer), so the identity-preserving bridge this PR introduced is no longer needed: - Bump @datadog/openfeature-node-server to 2.1.0 everywhere it's pinned (vendor, root devDependencies, plugin versions matrix) and rebuild vendor/dist. - Remove the @openfeature/server-sdk external redirect from vendor/rspack.config.js -- the vendored provider no longer references that package at all. - Delete server-sdk-bridge.js, the openfeature-server-sdk require-hook instrumentation, and their specs. - Remove the now-obsolete esbuild/webpack black-box tests that verified the bridge survived bundling -- there's no bridge left to verify. - Update file-tracing.spec.js's expected traced files and loaded-modules list to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…erver root package.json's devDependency was bumped to 2.1.0 but yarn.lock still pinned 2.0.2, which made bun's yarn.lock-migration fail to resolve the package in plugin test CI.
rochdev
force-pushed
the
rochdev/openfeature-bundler-entrypoint
branch
from
August 13, 2026 03:04
f479b46 to
a64371e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a64371e3f2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
leoromanovsky
approved these changes
Aug 13, 2026
dd-octo-sts Bot
pushed a commit
that referenced
this pull request
Aug 13, 2026
…l peer dependency (#9570) * feat(openfeature): return a ready provider from the openfeature entrypoint dd-trace/openfeature previously only existed as a side-effect require for file tracers (#9324). Turn it into the real public entrypoint: it now returns a usable FlaggingProvider instance after tracer.init(), and tracer.openfeature is deprecated in its favor since it doesn't work in bundled applications. Extract the base provider class into a factory so it can be constructed from either the bundler-opaque require-provider wrapper (legacy tracer.openfeature) or a plain require (the new entrypoint) without duplicating the class body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(openfeature): raise nft trace timeout for the entrypoint test openfeature.js now pulls in the full tracer package to check tracer.init() state, so nodeFileTrace has a much larger dependency tree to walk. The default 5s mocha timeout was tight enough that CI runners under load exceeded it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): vendor the flagging provider instead of an optional peer `@datadog/openfeature-node-server` was an optional peer dependency resolved through a bundler-opaque require, requiring dedicated esbuild/webpack plugin code to keep the require invisible when the peer wasn't installed (#8635) and to inline it so bundles survived relocation when it was (#8980). Vendoring the provider into `vendor/dist/` removes the need for a peer at all, so all of that machinery (`feature-registry.js`, `register.js`, `require-provider.js`, the optional-peer esbuild/webpack loaders, and their integration tests) is deleted in favor of a single lazy `tracer.openfeature` property. The vendored provider's only remaining external dependency is `@openfeature/server-sdk`, used solely for `OpenFeatureEventEmitter` and `ProviderEvents`. Bundling our own copy would give those a different identity than the customer's, so it's externalized to a small bridge module (`server-sdk-bridge.js`) that a new `openfeature-server-sdk` instrumentation fills in from the customer's own `require()`. `FlaggingProvider#initialize` also unrefs the vendored provider's initialization timer, which otherwise keeps an idle process (a short script, a serverless handler) alive for up to `initializationTimeoutMs` while waiting for configuration to arrive. TODO left to remove this once `@datadog/openfeature-node-server` unrefs it upstream. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(build): exclude unresolved peer entries from license check scripts/check_licenses.js treated every non-dev entry in vendor/package-lock.json as requiring license attribution, including `peer: true` entries. Those record an unresolved peer dependency range that npm never actually installs, so nothing is shipped for them. The official dd-license-attribution tool used in CI already excludes these, so the local script disagreed with CI and had two extraneous rows (@openfeature/core, @openfeature/server-sdk) added to satisfy it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(licenses): stop expecting npm peer dependencies to need attribution The prior fix's comment claimed npm never installs peer entries, which is false under npm 7+ auto-install-peers - vendor/node_modules physically has @openfeature/core and @openfeature/server-sdk installed. The real reason they don't need attribution is that peer dependencies are supplied by the consumer rather than shipped by this package, matching how the yarn.lock walk already excludes peerDependencies. The pre-existing vendor-peer test fixture assumed the opposite and was never validated against real CI behavior for a peer case, so it broke once the npm-lock scan started excluding peers too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): implement full ProviderEventEmitter contract in the deferred bridge emitter @openfeature/core's transferListeners always calls oldProvider.events?.removeHandler(...) on every subsequent OpenFeature.setProvider() call for a domain, regardless of whether the app registered any handlers itself. The deferred emitter only implemented addHandler/emit, so replacing the Datadog OpenFeature provider with another provider at runtime threw "oldProvider.events.removeHandler is not a function", crashing the customer's app. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): address review feedback on tests and SSI scope - Fix test:openfeature script so the instrumentation spec actually runs: mocha treats a brace-only glob segment as a literal filename, so add a wildcard segment that both mocha and verify-exercised-tests resolve. - Add real esbuild/webpack black-box tests that bundle @openfeature/server-sdk (not marked external) and assert dd-trace's bundler-instrumentation mechanism still bridges the real event emitter into the vendored provider. - Correct comments in proxy.js and the dd-trace-api plugin: SSI itself is fixed by this PR's vendoring; the only remaining gap is that the dd-trace-api shim has no openfeature handoff. Generated with Claude Code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): drop the server-sdk event-emitter bridge @datadog/openfeature-node-server 2.1.0 ships its own ProviderEventEmitter and no longer depends on @openfeature/server-sdk at all (neither as a dependency nor a peer), so the identity-preserving bridge this PR introduced is no longer needed: - Bump @datadog/openfeature-node-server to 2.1.0 everywhere it's pinned (vendor, root devDependencies, plugin versions matrix) and rebuild vendor/dist. - Remove the @openfeature/server-sdk external redirect from vendor/rspack.config.js -- the vendored provider no longer references that package at all. - Delete server-sdk-bridge.js, the openfeature-server-sdk require-hook instrumentation, and their specs. - Remove the now-obsolete esbuild/webpack black-box tests that verified the bridge survived bundling -- there's no bridge left to verify. - Update file-tracing.spec.js's expected traced files and loaded-modules list to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): update stale yarn.lock entry for openfeature-node-server root package.json's devDependency was bumped to 2.1.0 but yarn.lock still pinned 2.0.2, which made bun's yarn.lock-migration fail to resolve the package in plugin test CI. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Merged
dd-octo-sts Bot
pushed a commit
that referenced
this pull request
Aug 13, 2026
…l peer dependency (#9570) * feat(openfeature): return a ready provider from the openfeature entrypoint dd-trace/openfeature previously only existed as a side-effect require for file tracers (#9324). Turn it into the real public entrypoint: it now returns a usable FlaggingProvider instance after tracer.init(), and tracer.openfeature is deprecated in its favor since it doesn't work in bundled applications. Extract the base provider class into a factory so it can be constructed from either the bundler-opaque require-provider wrapper (legacy tracer.openfeature) or a plain require (the new entrypoint) without duplicating the class body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(openfeature): raise nft trace timeout for the entrypoint test openfeature.js now pulls in the full tracer package to check tracer.init() state, so nodeFileTrace has a much larger dependency tree to walk. The default 5s mocha timeout was tight enough that CI runners under load exceeded it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): vendor the flagging provider instead of an optional peer `@datadog/openfeature-node-server` was an optional peer dependency resolved through a bundler-opaque require, requiring dedicated esbuild/webpack plugin code to keep the require invisible when the peer wasn't installed (#8635) and to inline it so bundles survived relocation when it was (#8980). Vendoring the provider into `vendor/dist/` removes the need for a peer at all, so all of that machinery (`feature-registry.js`, `register.js`, `require-provider.js`, the optional-peer esbuild/webpack loaders, and their integration tests) is deleted in favor of a single lazy `tracer.openfeature` property. The vendored provider's only remaining external dependency is `@openfeature/server-sdk`, used solely for `OpenFeatureEventEmitter` and `ProviderEvents`. Bundling our own copy would give those a different identity than the customer's, so it's externalized to a small bridge module (`server-sdk-bridge.js`) that a new `openfeature-server-sdk` instrumentation fills in from the customer's own `require()`. `FlaggingProvider#initialize` also unrefs the vendored provider's initialization timer, which otherwise keeps an idle process (a short script, a serverless handler) alive for up to `initializationTimeoutMs` while waiting for configuration to arrive. TODO left to remove this once `@datadog/openfeature-node-server` unrefs it upstream. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(build): exclude unresolved peer entries from license check scripts/check_licenses.js treated every non-dev entry in vendor/package-lock.json as requiring license attribution, including `peer: true` entries. Those record an unresolved peer dependency range that npm never actually installs, so nothing is shipped for them. The official dd-license-attribution tool used in CI already excludes these, so the local script disagreed with CI and had two extraneous rows (@openfeature/core, @openfeature/server-sdk) added to satisfy it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(licenses): stop expecting npm peer dependencies to need attribution The prior fix's comment claimed npm never installs peer entries, which is false under npm 7+ auto-install-peers - vendor/node_modules physically has @openfeature/core and @openfeature/server-sdk installed. The real reason they don't need attribution is that peer dependencies are supplied by the consumer rather than shipped by this package, matching how the yarn.lock walk already excludes peerDependencies. The pre-existing vendor-peer test fixture assumed the opposite and was never validated against real CI behavior for a peer case, so it broke once the npm-lock scan started excluding peers too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): implement full ProviderEventEmitter contract in the deferred bridge emitter @openfeature/core's transferListeners always calls oldProvider.events?.removeHandler(...) on every subsequent OpenFeature.setProvider() call for a domain, regardless of whether the app registered any handlers itself. The deferred emitter only implemented addHandler/emit, so replacing the Datadog OpenFeature provider with another provider at runtime threw "oldProvider.events.removeHandler is not a function", crashing the customer's app. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): address review feedback on tests and SSI scope - Fix test:openfeature script so the instrumentation spec actually runs: mocha treats a brace-only glob segment as a literal filename, so add a wildcard segment that both mocha and verify-exercised-tests resolve. - Add real esbuild/webpack black-box tests that bundle @openfeature/server-sdk (not marked external) and assert dd-trace's bundler-instrumentation mechanism still bridges the real event emitter into the vendored provider. - Correct comments in proxy.js and the dd-trace-api plugin: SSI itself is fixed by this PR's vendoring; the only remaining gap is that the dd-trace-api shim has no openfeature handoff. Generated with Claude Code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): drop the server-sdk event-emitter bridge @datadog/openfeature-node-server 2.1.0 ships its own ProviderEventEmitter and no longer depends on @openfeature/server-sdk at all (neither as a dependency nor a peer), so the identity-preserving bridge this PR introduced is no longer needed: - Bump @datadog/openfeature-node-server to 2.1.0 everywhere it's pinned (vendor, root devDependencies, plugin versions matrix) and rebuild vendor/dist. - Remove the @openfeature/server-sdk external redirect from vendor/rspack.config.js -- the vendored provider no longer references that package at all. - Delete server-sdk-bridge.js, the openfeature-server-sdk require-hook instrumentation, and their specs. - Remove the now-obsolete esbuild/webpack black-box tests that verified the bridge survived bundling -- there's no bridge left to verify. - Update file-tracing.spec.js's expected traced files and loaded-modules list to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): update stale yarn.lock entry for openfeature-node-server root package.json's devDependency was bumped to 2.1.0 but yarn.lock still pinned 2.0.2, which made bun's yarn.lock-migration fail to resolve the package in plugin test CI. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Merged
pabloerhard
pushed a commit
that referenced
this pull request
Aug 17, 2026
…l peer dependency (#9570) * feat(openfeature): return a ready provider from the openfeature entrypoint dd-trace/openfeature previously only existed as a side-effect require for file tracers (#9324). Turn it into the real public entrypoint: it now returns a usable FlaggingProvider instance after tracer.init(), and tracer.openfeature is deprecated in its favor since it doesn't work in bundled applications. Extract the base provider class into a factory so it can be constructed from either the bundler-opaque require-provider wrapper (legacy tracer.openfeature) or a plain require (the new entrypoint) without duplicating the class body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(openfeature): raise nft trace timeout for the entrypoint test openfeature.js now pulls in the full tracer package to check tracer.init() state, so nodeFileTrace has a much larger dependency tree to walk. The default 5s mocha timeout was tight enough that CI runners under load exceeded it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): vendor the flagging provider instead of an optional peer `@datadog/openfeature-node-server` was an optional peer dependency resolved through a bundler-opaque require, requiring dedicated esbuild/webpack plugin code to keep the require invisible when the peer wasn't installed (#8635) and to inline it so bundles survived relocation when it was (#8980). Vendoring the provider into `vendor/dist/` removes the need for a peer at all, so all of that machinery (`feature-registry.js`, `register.js`, `require-provider.js`, the optional-peer esbuild/webpack loaders, and their integration tests) is deleted in favor of a single lazy `tracer.openfeature` property. The vendored provider's only remaining external dependency is `@openfeature/server-sdk`, used solely for `OpenFeatureEventEmitter` and `ProviderEvents`. Bundling our own copy would give those a different identity than the customer's, so it's externalized to a small bridge module (`server-sdk-bridge.js`) that a new `openfeature-server-sdk` instrumentation fills in from the customer's own `require()`. `FlaggingProvider#initialize` also unrefs the vendored provider's initialization timer, which otherwise keeps an idle process (a short script, a serverless handler) alive for up to `initializationTimeoutMs` while waiting for configuration to arrive. TODO left to remove this once `@datadog/openfeature-node-server` unrefs it upstream. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(build): exclude unresolved peer entries from license check scripts/check_licenses.js treated every non-dev entry in vendor/package-lock.json as requiring license attribution, including `peer: true` entries. Those record an unresolved peer dependency range that npm never actually installs, so nothing is shipped for them. The official dd-license-attribution tool used in CI already excludes these, so the local script disagreed with CI and had two extraneous rows (@openfeature/core, @openfeature/server-sdk) added to satisfy it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(licenses): stop expecting npm peer dependencies to need attribution The prior fix's comment claimed npm never installs peer entries, which is false under npm 7+ auto-install-peers - vendor/node_modules physically has @openfeature/core and @openfeature/server-sdk installed. The real reason they don't need attribution is that peer dependencies are supplied by the consumer rather than shipped by this package, matching how the yarn.lock walk already excludes peerDependencies. The pre-existing vendor-peer test fixture assumed the opposite and was never validated against real CI behavior for a peer case, so it broke once the npm-lock scan started excluding peers too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): implement full ProviderEventEmitter contract in the deferred bridge emitter @openfeature/core's transferListeners always calls oldProvider.events?.removeHandler(...) on every subsequent OpenFeature.setProvider() call for a domain, regardless of whether the app registered any handlers itself. The deferred emitter only implemented addHandler/emit, so replacing the Datadog OpenFeature provider with another provider at runtime threw "oldProvider.events.removeHandler is not a function", crashing the customer's app. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): address review feedback on tests and SSI scope - Fix test:openfeature script so the instrumentation spec actually runs: mocha treats a brace-only glob segment as a literal filename, so add a wildcard segment that both mocha and verify-exercised-tests resolve. - Add real esbuild/webpack black-box tests that bundle @openfeature/server-sdk (not marked external) and assert dd-trace's bundler-instrumentation mechanism still bridges the real event emitter into the vendored provider. - Correct comments in proxy.js and the dd-trace-api plugin: SSI itself is fixed by this PR's vendoring; the only remaining gap is that the dd-trace-api shim has no openfeature handoff. Generated with Claude Code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): drop the server-sdk event-emitter bridge @datadog/openfeature-node-server 2.1.0 ships its own ProviderEventEmitter and no longer depends on @openfeature/server-sdk at all (neither as a dependency nor a peer), so the identity-preserving bridge this PR introduced is no longer needed: - Bump @datadog/openfeature-node-server to 2.1.0 everywhere it's pinned (vendor, root devDependencies, plugin versions matrix) and rebuild vendor/dist. - Remove the @openfeature/server-sdk external redirect from vendor/rspack.config.js -- the vendored provider no longer references that package at all. - Delete server-sdk-bridge.js, the openfeature-server-sdk require-hook instrumentation, and their specs. - Remove the now-obsolete esbuild/webpack black-box tests that verified the bridge survived bundling -- there's no bridge left to verify. - Update file-tracing.spec.js's expected traced files and loaded-modules list to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): update stale yarn.lock entry for openfeature-node-server root package.json's devDependency was bumped to 2.1.0 but yarn.lock still pinned 2.0.2, which made bun's yarn.lock-migration fail to resolve the package in plugin test CI. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
pabloerhard
pushed a commit
that referenced
this pull request
Aug 17, 2026
…l peer dependency (#9570) * feat(openfeature): return a ready provider from the openfeature entrypoint dd-trace/openfeature previously only existed as a side-effect require for file tracers (#9324). Turn it into the real public entrypoint: it now returns a usable FlaggingProvider instance after tracer.init(), and tracer.openfeature is deprecated in its favor since it doesn't work in bundled applications. Extract the base provider class into a factory so it can be constructed from either the bundler-opaque require-provider wrapper (legacy tracer.openfeature) or a plain require (the new entrypoint) without duplicating the class body. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(openfeature): raise nft trace timeout for the entrypoint test openfeature.js now pulls in the full tracer package to check tracer.init() state, so nodeFileTrace has a much larger dependency tree to walk. The default 5s mocha timeout was tight enough that CI runners under load exceeded it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): vendor the flagging provider instead of an optional peer `@datadog/openfeature-node-server` was an optional peer dependency resolved through a bundler-opaque require, requiring dedicated esbuild/webpack plugin code to keep the require invisible when the peer wasn't installed (#8635) and to inline it so bundles survived relocation when it was (#8980). Vendoring the provider into `vendor/dist/` removes the need for a peer at all, so all of that machinery (`feature-registry.js`, `register.js`, `require-provider.js`, the optional-peer esbuild/webpack loaders, and their integration tests) is deleted in favor of a single lazy `tracer.openfeature` property. The vendored provider's only remaining external dependency is `@openfeature/server-sdk`, used solely for `OpenFeatureEventEmitter` and `ProviderEvents`. Bundling our own copy would give those a different identity than the customer's, so it's externalized to a small bridge module (`server-sdk-bridge.js`) that a new `openfeature-server-sdk` instrumentation fills in from the customer's own `require()`. `FlaggingProvider#initialize` also unrefs the vendored provider's initialization timer, which otherwise keeps an idle process (a short script, a serverless handler) alive for up to `initializationTimeoutMs` while waiting for configuration to arrive. TODO left to remove this once `@datadog/openfeature-node-server` unrefs it upstream. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(build): exclude unresolved peer entries from license check scripts/check_licenses.js treated every non-dev entry in vendor/package-lock.json as requiring license attribution, including `peer: true` entries. Those record an unresolved peer dependency range that npm never actually installs, so nothing is shipped for them. The official dd-license-attribution tool used in CI already excludes these, so the local script disagreed with CI and had two extraneous rows (@openfeature/core, @openfeature/server-sdk) added to satisfy it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(licenses): stop expecting npm peer dependencies to need attribution The prior fix's comment claimed npm never installs peer entries, which is false under npm 7+ auto-install-peers - vendor/node_modules physically has @openfeature/core and @openfeature/server-sdk installed. The real reason they don't need attribution is that peer dependencies are supplied by the consumer rather than shipped by this package, matching how the yarn.lock walk already excludes peerDependencies. The pre-existing vendor-peer test fixture assumed the opposite and was never validated against real CI behavior for a peer case, so it broke once the npm-lock scan started excluding peers too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): implement full ProviderEventEmitter contract in the deferred bridge emitter @openfeature/core's transferListeners always calls oldProvider.events?.removeHandler(...) on every subsequent OpenFeature.setProvider() call for a domain, regardless of whether the app registered any handlers itself. The deferred emitter only implemented addHandler/emit, so replacing the Datadog OpenFeature provider with another provider at runtime threw "oldProvider.events.removeHandler is not a function", crashing the customer's app. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): address review feedback on tests and SSI scope - Fix test:openfeature script so the instrumentation spec actually runs: mocha treats a brace-only glob segment as a literal filename, so add a wildcard segment that both mocha and verify-exercised-tests resolve. - Add real esbuild/webpack black-box tests that bundle @openfeature/server-sdk (not marked external) and assert dd-trace's bundler-instrumentation mechanism still bridges the real event emitter into the vendored provider. - Correct comments in proxy.js and the dd-trace-api plugin: SSI itself is fixed by this PR's vendoring; the only remaining gap is that the dd-trace-api shim has no openfeature handoff. Generated with Claude Code. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(openfeature): drop the server-sdk event-emitter bridge @datadog/openfeature-node-server 2.1.0 ships its own ProviderEventEmitter and no longer depends on @openfeature/server-sdk at all (neither as a dependency nor a peer), so the identity-preserving bridge this PR introduced is no longer needed: - Bump @datadog/openfeature-node-server to 2.1.0 everywhere it's pinned (vendor, root devDependencies, plugin versions matrix) and rebuild vendor/dist. - Remove the @openfeature/server-sdk external redirect from vendor/rspack.config.js -- the vendored provider no longer references that package at all. - Delete server-sdk-bridge.js, the openfeature-server-sdk require-hook instrumentation, and their specs. - Remove the now-obsolete esbuild/webpack black-box tests that verified the bridge survived bundling -- there's no bridge left to verify. - Update file-tracing.spec.js's expected traced files and loaded-modules list to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(openfeature): update stale yarn.lock entry for openfeature-node-server root package.json's devDependency was bumped to 2.1.0 but yarn.lock still pinned 2.0.2, which made bun's yarn.lock-migration fail to resolve the package in plugin test CI. --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Vendors
@datadog/openfeature-node-server, Datadog's OpenFeature provider, directly into the tracer package (bundled via rspack intovendor/dist/) instead of requiring the application to install it as a peer dependency.tracer.openfeatureis the single, lazy entrypoint for the provider — it can be accessed and registered with@openfeature/server-sdkin either order.As of
@datadog/openfeature-node-server2.1.0, the vendored provider no longer depends on@openfeature/server-sdkat all — it ships its own event emitter (NodeProviderEventEmitter) instead of importingOpenFeatureEventEmitter/ProviderEventsfrom the SDK. That's the only reason this needed special handling: OpenFeature's own event wiring (provider.events.addHandler(eventType, handler)) is duck-typed on event name strings, not emitter class identity, so the vendored provider and the application's own copy of@openfeature/server-sdkinteroperate correctly without any shared dependency, bridge module, or dedicated instrumentation.This also removes the generic multi-feature "feature registry" abstraction (
feature-registry.js,openfeature/register.js,openfeature/require-provider.js) and the esbuild/webpackrequireOptionalPeerrewriting machinery it depended on, since a fully self-contained vendored dependency doesn't need either:tracer.openfeatureis now wired directly as a lazy module on the proxy, the same way the tracer's other lazy modules are.FlaggingProvider#initializealso unrefs the vendored provider's initialization timer (with a TODO to remove once fixed upstream), so a short script or serverless handler isn't kept alive for up toinitializationTimeoutMsjust because it's waiting on remote configuration.Motivation
Requiring
@datadog/openfeature-node-serveras an optional peer dependency assumed dd-trace and the application shared the samenode_modulestree. That doesn't hold for Single Step Instrumentation (SSI), where dd-trace is installed outside the application's own directory: resolving the peer viamodule.createRequire(__filename)walks upnode_modulesfrom dd-trace's own install location, so it could never reach the application'snode_moduleswhere the provider was actually installed. Vendoring the provider removes the dependency on the application installing anything at all, so it's available regardless of where dd-trace itself lives.Treating
@openfeature/server-sdkandtracer.openfeatureas independent optional peer dependencies also required dedicated esbuild/webpack plugin support to keep bundlers from breaking the optional requires, and behavior depended on the order the two were accessed relative to each other. Vendoring removes those constraints too: it behaves the same whether or not the application is bundled, no longer depends on access order, and — since the provider no longer has any runtime dependency of its own to reconcile with the application's copy — needs no bundler-specific support at all.Additional Notes
This PR was generated with Claude Code.