Repository navigation
feat(node): support immediate per-subject experimentation opt-out - #420
leoromanovsky wants to merge 13 commits into
Conversation
OpenFeature Browser Provider Bundle SizesMeasured from the Vite production output after installing packed Both scenarios initialize an OpenFeature provider, evaluate a boolean flag, change context, and evaluate again. Telemetry is disabled for DatadogProvider; no tracking hooks are registered for DatadogCoreProvider. DatadogProvider fetches precomputed assignments for each context; DatadogCoreProvider receives rules from fetchRulesConfiguration once and evaluates locally. Sizes include OpenFeature and the same small scenario harness. Configuration responses are supplied by Playwright and are not bundled. These are complete scenario JS sizes, not configuration payload sizes or isolated provider/Protobuf costs; the difference between rows is not a decoder-only delta.
Dependency checks passed: no Protobuf markers in default/precomputed scenarios; markers present in rules-based scenarios. OpenFeature Browser Tracking Hook Bundle SizesSynthetic entrypoints import and call tracking hook factories from the packed
OpenFeature Node Server Bundle SizesIndependent Webpack production builds of the installed core and Node SDK tarballs, targeting Node 18 with ESM (
Adding SSR helpers to the provider adds 79.7 KiB raw / 22.2 KiB gzip in this build. This includes fetching, parsing, and serialization, not just the decoder. SSR helpers fetch rules for the browser, not for configuring that provider (which uses its existing JSON API). These are bundler sizes, not npm install sizes, unbundled Node require/import costs, payload sizes, or latency. Gzip compares compressed artifacts. The browser scenarios include OpenFeature and a different harness, so their sizes are not directly comparable. Dependency checks passed: no Protobuf runtime or generated schema modules in the provider-only bundle; present in both SSR bundles (checked using Webpack module statistics). This report shows current PR artifact sizes only; it does not compare against the base branch. |
Validate activation timestamps when JSON configurations are received and protobuf responses are decoded. Reuse parsed instants during evaluation while preserving nanosecond precision, per-flag errors in every mode, and raw core API compatibility. Environment: Datadog workspace
Make doLog an explicit boolean in the UFC allocation type, matching the backend schema. Update test allocations and regenerate the Node package declarations. Environment: Datadog workspace
Use one mode-and-cutoff cache key scheme and require an explicit acknowledgement policy. Receive the edited browser rules fixture again so activation preparation uses its final timestamp. Environment: Datadog workspace
Explain immediate exclusion and the literal activation cutoff used by future-only mode. Environment: Datadog workspace
Stack Node immediate opt-out on the shared browser implementation, reuse its timestamp validation, and leave environment configuration to dd-trace-js. Environment: Datadog workspace
Apply the repository Prettier formatting required by CI. Environment: Datadog workspace
Keep the merged core and browser implementation, preserve the Node opt-out option, and regenerate the public declarations. Environment: Datadog workspace
aarsilv
left a comment
aarsilv
left a comment
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Heads up: main pins the fixture submodule to 1c831fed, which DataDog/ffe-system-test-data#52 replaced when it was squash-merged as 0781ce2. Could you re-pin, and update the two case counts in packages/core/test/evaluation/flags-v1.spec.ts?
1c831fed is not on the fixture repository's main; GitHub reports it as diverged (2 ahead, 5 behind). This PR does not change the pin, but it is the open PR in this area.
The merged fixtures add two immediate and two future-mode cases. So a pin bump alone fails flags-v1.spec.ts: toBe(26) receives 28, and after that fix toHaveLength(42) receives 44. With both counts updated, every core test passes, including the capability cases.
Evidence — core tests with the pin moved
| Fixture commit | Counts | core | node-server |
|---|---|---|---|
1c831fed (current pin) |
26 / 42 | 1031/1031 | 546/546 |
0781ce2 (merged) |
26 / 42 | 1 failed, 1048 passed | 560/560 |
0781ce2 (merged) |
28 / 44 | 1049/1049 | 560/560 |
f20ab80 (fixture main when I ran it) |
28 / 44 | 1053/1053 | 564/564 |
1c831fed (control) |
28 / 44 | 1 failed, 1030 passed | — |
Suggested fix — re-pin and update the counts
diff --git a/packages/core/test/evaluation/flags-v1.spec.ts b/packages/core/test/evaluation/flags-v1.spec.ts
index 6787537..a0e75df 100644
--- a/packages/core/test/evaluation/flags-v1.spec.ts
+++ b/packages/core/test/evaluation/flags-v1.spec.ts
@@ -141,9 +141,9 @@ describe('Universal Flag Configuration V1', () => {
it('loads the immediate opt-out capability cases', () => {
const cases = getTestCaseFileNames().flatMap(getTestCases)
const immediateCases = cases.filter((testCase) => testCase.capabilities?.experimentationOptOutImmediate)
- expect(immediateCases.length).toBe(26)
+ expect(immediateCases.length).toBe(28)
expect(cases.filter((testCase) => testCase.capabilities?.experimentationOptOutFutureExperimentsOnly)).toHaveLength(
- 42
+ 44
)
expect(new Set(immediateCases.map((testCase) => testCase.variationType))).toEqual(
new Set(['BOOLEAN', 'STRING', 'INTEGER', 'NUMERIC', 'JSON'])
diff --git a/packages/core/test/ffe-system-test-data b/packages/core/test/ffe-system-test-data
index 1c831fe..0781ce2 160000
--- a/packages/core/test/ffe-system-test-data
+++ b/packages/core/test/ffe-system-test-data
@@ -1 +1 @@
-Subproject commit 1c831fed02a428a1b0a4ac412da9b773e22df257
+Subproject commit 0781ce2c55f9da7f9290035c768bbc46570556c0| }, | ||
| } as unknown as UniversalFlagConfigurationV1 | ||
|
|
||
| describe('immediate experimentation opt-out', () => { |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] Run the shared immediate-mode fixture cases through DatadogNodeServerProvider, not only through core.
test/ffe-system.spec.ts builds the provider without experimentationOptOutMode, so through this provider the shared cases run only in disabled mode. Core flags-v1.spec.ts checks their experimentationOptOutImmediate expectations, but through the core evaluator. Your all-value-types test here already catches a provider that drops the mode.
The shared cases add rows that the Node tests do not have: an unknown flag with a malformed timestamp (INVALID_CONTEXT, not FLAG_NOT_FOUND), a subject with no targeting key, and non-string attribute values (booleans, null, a number, an object, an array). The suite below passes all 26 immediate cases at this head; with the mode dropped at the four evaluate calls, 22 fail.
Suggested test — a second suite in test/ffe-system.spec.ts
diff --git a/packages/node-server/test/ffe-system.spec.ts b/packages/node-server/test/ffe-system.spec.ts
index 81d3657..a4cabe0 100644
--- a/packages/node-server/test/ffe-system.spec.ts
+++ b/packages/node-server/test/ffe-system.spec.ts
@@ -4,6 +4,7 @@ import path from 'node:path'
import type { ExposureEvent } from '@datadog/flagging-core'
import type { EvaluationContext, FlagValue, JsonValue, Logger, ResolutionDetails } from '@openfeature/core'
import type { UniversalFlagConfigurationV1, VariantType } from '../src/configuration/ufc-v1'
+import { ExperimentationOptOutMode } from '../src/experimentation-opt-out'
import { DatadogNodeServerProvider } from '../src/provider'
type SystemTestCase = {
@@ -16,6 +17,9 @@ type SystemTestCase = {
value: FlagValue
reason: string
}
+ capabilities?: {
+ experimentationOptOutImmediate?: { value: FlagValue; reason: string; errorCode?: string }
+ }
}
const fixtureDirectory = path.join(__dirname, '../../core/test/ffe-system-test-data')
@@ -52,8 +56,11 @@ function getTestCases(testCaseFileName: string): SystemTestCase[] {
) as SystemTestCase[]
}
-async function evaluateTestCase(testCase: SystemTestCase): Promise<ResolutionDetails<FlagValue>> {
- const provider = new DatadogNodeServerProvider({ exposureChannel })
+async function evaluateTestCase(
+ testCase: SystemTestCase,
+ experimentationOptOutMode?: ExperimentationOptOutMode
+): Promise<ResolutionDetails<FlagValue>> {
+ const provider = new DatadogNodeServerProvider({ exposureChannel, experimentationOptOutMode })
provider.setConfiguration(configuration)
const context = {
@@ -86,3 +93,16 @@ describe('FFE system tests through DatadogNodeServerProvider', () => {
})
})
})
+
+describe('FFE immediate opt-out capability cases through DatadogNodeServerProvider', () => {
+ const testCases = getTestCaseFileNames()
+ .flatMap(getTestCases)
+ .filter((testCase) => testCase.capabilities?.experimentationOptOutImmediate)
+ it.each(testCases)('with context $targetingKey and flag $flag', async (testCase) => {
+ const expected = testCase.capabilities?.experimentationOptOutImmediate
+ const details = await evaluateTestCase(testCase, ExperimentationOptOutMode.Immediate)
+ expect(details.value).toEqual(expected?.value)
+ expect(details.reason).toEqual(expected?.reason)
+ expect(details.errorCode).toEqual(expected?.errorCode)
+ })
+})Verification — what I ran
- At this head (fixture pin
1c831fed): the new suite passes 26/26. - Mutant (mode replaced by
undefinedat the fourevaluatecalls): 22 of the 26 fail. - Together with the fixture re-pin I suggest in this review's summary (and nothing else): node-server 588/588 (28 immediate cases), core 1049/1049.
| defaultValue, | ||
| context, | ||
| _logger, | ||
| this.experimentationOptOutMode |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Heads up: in the default Disabled mode, each Node evaluation that reaches the allocation loop still does the core opt-out work. Could the core skip the activation lookup and the exclusion call when the policy is inactive?
evaluateForSubject looks up prepared activations and calls excludesExperimentAt for every allocation it visits, also when the policy is false. That code came with #423, not this PR, but Node evaluations in the default Disabled mode that reach the loop pay it. Decision D017 in DataDog/ffe-system-test-data#55 records "inactive effective opt-out skips allocation-exclusion operations"; it was adopted for Go, and other SDKs are still listed as proposed.
In my benchmark, this head is 1.8–4.6% slower per evaluation than before opt-out support, and the guard below brings it within 1.7% (table). Its test, which checks that disabled mode reads no activation metadata, fails at this head.
Measurements — median ns per evaluation, disabled mode, Node 24
| Allocations per flag | Before opt-out support (4573204) | This head | This head + guard |
|---|---|---|---|
| 1 | 239 | 250 | 243 |
| 5 | 1123 | 1144 | 1130 |
| 20 | 4264 | 4339 | 4264 |
Ten alternating rounds of 200,000 evaluations each, with a fresh context per call. Every allocation except the last has a rule that does not match, so the loop visits all of them.
// Per-evaluation cost of evaluateRulesBasedConfiguration in disabled mode. Usage: node bench-core.js <label=cjsDir>...
const variants = process.argv.slice(2).map((a) => { const [label, dir] = a.split('='); return { label, ev: require(`${dir}/index.js`).evaluateRulesBasedConfiguration } })
const logger = { debug() {}, info() {}, warn() {}, error() {} }
function config(k) {
const allocations = []
for (let i = 0; i < k - 1; i++) allocations.push({ key: `a${i}`, doLog: i % 2 === 0, randomizationActivatedAt: '2026-09-01T00:00:00Z',
rules: [{ conditions: [{ attribute: 'country', operator: 'ONE_OF', value: ['XX'] }] }], splits: [{ variationKey: 'on', shards: [] }] })
allocations.push({ key: 'last', doLog: false, splits: [{ variationKey: 'on', shards: [] }] })
return { createdAt: '2026-01-01T00:00:00Z', format: 'SERVER', environment: { name: 'test' },
flags: { f: { key: 'f', enabled: true, variationType: 'BOOLEAN', variations: { on: { key: 'on', value: true } }, allocations, totalShards: 10000 } } }
}
const N = 200000, ROUNDS = 10
for (const k of [1, 5, 20]) {
const cfg = config(k)
const samples = Object.fromEntries(variants.map((v) => [v.label, []]))
for (const v of variants) { // warm up and sanity check
const r = v.ev(cfg, 'boolean', 'f', false, { targetingKey: 'u', country: 'US' }, logger, 'disabled')
if (r.value !== true || r.errorCode) throw new Error(`${v.label} k=${k}: ${JSON.stringify(r)}`)
for (let i = 0; i < N; i++) v.ev(cfg, 'boolean', 'f', false, { targetingKey: 'u' + (i % 1000), country: 'US' }, logger, 'disabled')
}
for (let round = 0; round < ROUNDS; round++) {
for (const v of round % 2 ? [...variants].reverse() : variants) {
const t0 = process.hrtime.bigint()
for (let i = 0; i < N; i++) v.ev(cfg, 'boolean', 'f', false, { targetingKey: 'u' + (i % 1000), country: 'US' }, logger, 'disabled')
samples[v.label].push(Number(process.hrtime.bigint() - t0) / N)
}
}
const med = (a) => { const s = [...a].sort((x, y) => x - y); return (s[4] + s[5]) / 2 }
console.log(`allocations=${k}: ` + variants.map((v) => `${v.label} ${med(samples[v.label]).toFixed(0)} ns/op`).join(' | '))
}Suggested fix — skip opt-out work for an inactive policy
diff --git a/packages/core/src/evaluation/evaluateForSubject.ts b/packages/core/src/evaluation/evaluateForSubject.ts
index e7e97d6..9f66114 100644
--- a/packages/core/src/evaluation/evaluateForSubject.ts
+++ b/packages/core/src/evaluation/evaluateForSubject.ts
@@ -61,12 +61,18 @@ export function evaluateForSubject<T extends FlagValueType>(
}
const now = new Date(evaluationTimestampMs)
- const prepared = prepareJsonFlagActivations(flag)
- if (!prepared.valid && needsActivation(excludeExperiments)) {
+ // An inactive policy does no activation lookup or exclusion work.
+ const prepared = excludeExperiments ? prepareJsonFlagActivations(flag) : undefined
+ if (prepared && !prepared.valid && needsActivation(excludeExperiments)) {
return { value: defaultValue, reason: 'ERROR', errorCode: 'PARSE_ERROR' as ErrorCode }
}
- for (const { allocation, activation } of prepared.allocations) {
- if (excludesExperimentAt(excludeExperiments, allocation.doLog !== false, activation)) continue
+ for (let index = 0; index < flag.allocations.length; index++) {
+ const allocation = flag.allocations[index]
+ if (
+ prepared &&
+ excludesExperimentAt(excludeExperiments, allocation.doLog !== false, prepared.allocations[index].activation)
+ )
+ continue
if (allocation.startAt && now < new Date(allocation.startAt)) {
logger.debug(`allocation before start date`, {
flagKey: flag.key,Test — fails at this head, passes with the fix
diff --git a/packages/core/src/evaluation/experimentation-opt-out-future.test.ts b/packages/core/src/evaluation/experimentation-opt-out-future.test.ts
index 997a065..b5936f9 100644
--- a/packages/core/src/evaluation/experimentation-opt-out-future.test.ts
+++ b/packages/core/src/evaluation/experimentation-opt-out-future.test.ts
@@ -322,3 +322,27 @@ it.each([
const acknowledged = parseOptOutInstant(timestamp)
expect(!!acknowledged && compareOptOutInstants(parseOptOutInstant(cutoff)!, acknowledged) === 0).toBe(expected)
})
+
+it.each([undefined, false, 'disabled'] as const)(
+ 'does no activation work for an inactive policy (mode %s)',
+ (optOutMode) => {
+ const configuration = jsonRules('2026-09-01T00:00:00Z')
+ const allocation = configuration.flags.flag.allocations[0]
+ const activation = allocation.randomizationActivatedAt
+ const read = jest.fn(() => activation)
+ Object.defineProperty(allocation, 'randomizationActivatedAt', { get: read })
+ for (const targetingKey of ['first', 'second']) {
+ const result = evaluateRulesBasedConfiguration(
+ configuration,
+ 'boolean',
+ 'flag',
+ false,
+ { ...context, targetingKey },
+ logger,
+ optOutMode
+ )
+ expect(result).toMatchObject({ value: true })
+ }
+ expect(read).not.toHaveBeenCalled()
+ }
+)Verification — what I ran
- New test at this head: 3 failed (the getter is read once, by the lazy preparation). With the guard: 3 passed. The test checks activation reads; it does not count
excludesExperimentAtcalls. - In Node,
setConfigurationalready prepares activations (provider.ts:87), so the per-evaluation part is the cached lookup and the exclusion calls. - With the guard: core 1034/1034, node-server 546/546, browser 528/528; tsc, biome and prettier clean.
| } | ||
| ``` | ||
|
|
||
| Use an RFC3339 timestamp string. In immediate mode, any valid timestamp excludes the subject immediately, including a future timestamp. |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Could this section describe the supported timestamp subset, and say that Node does not offer FutureExperimentsOnly?
"Use an RFC3339 timestamp string" is broader than the shared parser accepts. In immediate mode, this provider returns INVALID_CONTEXT for 2016-12-31T23:59:60Z, which RFC 3339's time-second allows. The browser README already lists the subset.
The constructor also throws for future_experiments_only, but the README does not say that only Disabled and Immediate exist.
Provider results — immediate mode at this head
dd_experimentation_opt_out_at |
Result |
|---|---|
2016-12-31T23:59:60Z |
default, ERROR, INVALID_CONTEXT |
2026-10-19 12:00:00Z |
default, ERROR, INVALID_CONTEXT |
2026-10-19t12:00:00z |
excluded from the experiment allocation, no error |
2026-10-19T12:00:00.1234567890Z |
excluded from the experiment allocation, no error |
Suggested text — the browser wording, adapted
diff --git a/packages/node-server/README.md b/packages/node-server/README.md
index 56ced5a..67efc7b 100644
--- a/packages/node-server/README.md
+++ b/packages/node-server/README.md
@@ -50,6 +50,7 @@ Applications own retries, polling, storage, tracking, and fallback.
## Per-subject experimentation opt-out
Set the `experimentationOptOutMode` provider option to `ExperimentationOptOutMode.Immediate`.
+Node supports `Disabled` and `Immediate`; `FutureExperimentsOnly` is not available in this provider.
The default is `Disabled`. Unsupported mode values throw during provider construction.
The provider uses the explicit option only. When using `dd-trace-js`, the tracer owns
`DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE` and passes the resolved mode to this provider.
@@ -68,7 +69,11 @@ const context = {
}
```
-Use an RFC3339 timestamp string. In immediate mode, any valid timestamp excludes the subject immediately, including a future timestamp.
+Use a timestamp in the supported RFC3339 subset: `YYYY-MM-DDTHH:mm:ss` with an optional decimal fraction, followed by `Z`
+or a numeric `+HH:mm` or `-HH:mm` offset. Lowercase `t` and `z` are accepted. Fractional digits beyond nanoseconds are
+truncated without rounding. Use valid calendar dates and seconds `00–59`. Both the written date and its UTC instant must
+have years `0000–9999`. Whitespace, named timezones, Unicode signs, and leap seconds are rejected.
+In immediate mode, any valid timestamp excludes the subject immediately, including a future timestamp.
The evaluator skips exposure-logging allocations before targeting and randomization, then tries ordinary allocations in order.
If none match, it returns the application's default value.
An invalid attribute returns `INVALID_CONTEXT` and the application default.
|
🤖 Comment from Claude working with Aaron Silverman: Friendly ping on the review from Oct 24: the four items below are still open, with no reply yet. The Android and iOS companion PRs have addressed their review rounds, so this is the remaining SDK in the set.
Each comment includes a tested patch. If you'd rather handle any of them in a follow-up PR, a short reply saying so is enough. |
aarsilv
left a comment
aarsilv
left a comment
There was a problem hiding this comment.
Approving with some low-priority findings you can take or leave as you see fit!
FFE reviewer selectionReviewers: @typotter, @sameerank Why: openfeature architecture — The PR changes the Node OpenFeature provider's public contract. Reviewers come from the matched pod. 🤖 Automated selection. Selection detailsInitiative: fallback / openfeature-architecture Why this match: The PR changes the Node OpenFeature provider's public contract. It adds a new constructor option, experimentationOptOutMode (Disabled/Immediate, validated when the provider is built), and passes it to the shared evaluator from all four resolve methods. As a result, subjects that opted out skip experiment allocations, and invalid opt-out input returns INVALID_CONTEXT. Exposure and event handling stay the same. That falls under ufc-compliance's exclusion for allocation-eligibility changes, which sends this work to the provider/evaluation SME. No other registered project covers per-subject experiment opt-out. The browser-ts paths don't include packages/node-server.
The PR author was excluded. Existing reviewers and teams were preserved. Debug or fix a problemWorkflow run · Bits Code session
Evidence and run details✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head. Generated asynchronously by the FFE reviewer-routing workflow. Changed paths: Evaluated head: Registry revision: Pinned skill source: Instructions: workflow-injected snapshot (no plugin loaded) Instruction snapshot SHA-256: |
Motivation
Node applications need to exclude selected subjects from experiments while retaining ordinary feature delivery. The Node provider needs an explicit policy option; tracer configuration and environment precedence belong in
dd-trace-js.Changes
With the shared evaluator and timestamp parser merged in #423, this PR adds only the Node provider option, wiring, declarations, documentation, and tests.
Disabledis the default.Immediateskips experiment allocations when the evaluation context contains a validdd_experimentation_opt_out_at, including a future timestamp. Ordinary allocations remain eligible. Invalid enabled input returnsINVALID_CONTEXT; unsupported provider modes throw at construction. Exposure deduplication and evaluation telemetry are unchanged.dd-trace-js #10826 owns
DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODEand passes the resolved mode to this provider. The provider does not read environment variables. Future-only mode remains outside this Node API change. The tracer integration requires a core/Node provider release and a vendored dependency update.Test instructions
After merging
mainat58bc213b51eb539d58ee4a5c7719ccf500f09b66, all three package builds, unit test suites, and type checks passed, along with Node test type checking, lint, formatting, package-version validation, and all 124 build-script tests. Node tests cover explicit/default modes, independence from the environment, all four evaluation methods, shared timestamp validation, and exposure deduplication.These are local SDK checks on Node 22.15.0. Package-install smoke tests, tracer system tests, and staging were not rerun.
Checklist
FFE reviewer selection
Reviewers: @sameerank, @greghuels
Why: Global fallback — This PR adds an experimentationOptOutMode option (Disabled/Immediate) to the Node provider. Global reviewers were selected because no project or SME pod matched.
🤖 Automated selection.
Selection details
Initiative: global / global
Why this match: This PR adds an experimentationOptOutMode option (Disabled/Immediate) to the Node provider. The provider passes it into evaluate() for all four evaluation methods, so opted-out subjects skip experiment allocations. Exposure handling and telemetry are unchanged. No registered initiative matches this per-subject experiment exclusion during evaluation. ufc-compliance covers suppressing evaluation/exposure event egress, which this PR does not change. experimentation-decisioning covers layers and mutual exclusion, and holdouts and bandits do not apply. The only fallback, browser-ts, covers packages/browser, core and react, not packages/node-server. So global ownership applies.
The PR author was excluded. Existing reviewers and teams were preserved.
Debug or fix a problem
Workflow run · Bits Code session
dry_run=false. Unchanged completed runs are skipped. If the receipt saysneeds_reconciliationor a write outcome is uncertain, ask the workflow owner to reconcile GitHub and the receipt before retrying; do not delete the lock blindly.Evidence and run details
✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head.
Generated asynchronously by the FFE reviewer-routing workflow.
Changed paths:
packages/node-server/src/experimentation-opt-out.ts,packages/node-server/src/provider.ts,packages/node-server/src/index.ts,packages/node-server/index.d.tsEvaluated head:
dc9086f6c1004f512a436ce84be71a11ce488744Registry revision:
revision:1946aa2a-530e-487a-84a7-83aff44a1089Pinned skill source:
d214cfc65ae154b97d39af5cf32caf48aaec9bf0Instructions: workflow-injected snapshot (no plugin loaded)
Instruction snapshot SHA-256:
f23b2a88c13c0da90e2dd58246a14f257af91e2195c555039bd555b52e6f3516