Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 67 additions & 0 deletions dd-openfeature/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
# Datadog OpenFeature provider

## Per-subject experimentation opt-out

Set `DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE=immediate` before constructing the provider.
The default is `disabled`. Existing configuration delivery and credentials are still required.

```java
import com.datadog.openfeature.Provider;
import dev.openfeature.sdk.MutableContext;
import dev.openfeature.sdk.OpenFeatureAPI;

OpenFeatureAPI api = OpenFeatureAPI.getInstance();
api.setProviderAndWait(new Provider());
boolean enabled = api.getClient().getBooleanValue("new-checkout", false,
new MutableContext("subject-123")
.add("dd_experimentation_opt_out_at", "2026-10-19T12:00:00Z"));
```

In immediate mode, a valid RFC3339 timestamp excludes the subject from allocations that record experiment exposures.
Evaluation continues through ordinary feature allocations. If none match, the provider returns the application's default.
A future timestamp also takes effect immediately. It does not schedule exclusion.

Omit the attribute for subjects who can participate.
Invalid, null, empty, or non-string values return the application's default with `INVALID_CONTEXT` and a diagnostic.
Timestamp validation uses the same parser as allocation `startAt` and `endAt`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

[P2] Clarify the README in four places: the timestamp grammar, the scope of INVALID_CONTEXT, unsupported modes, and the backend dependencies.

  1. "a valid RFC3339 timestamp" and "Timestamp validation uses the same parser as allocation startAt and endAt" describe ISO_OFFSET_DATE_TIME, which is not the shared grammar (see my parser comment). State the grammar instead of naming the parser.
  2. "Invalid, null, empty, or non-string values return the application's default with INVALID_CONTEXT" is true. Once configuration has loaded, it applies to every flag evaluated with that context, including flags without experiments. Say so explicitly.
  3. "Unsupported values throw IllegalArgumentException" changes if the warn-and-disable suggestion is adopted.
  4. Line 65 covers part of the backend dependency for future mode. Draft decisions D012 and D018 in Add a Lean proof prototype and shared SDK decision history ffe-system-test-data#55 record both dependencies:
    • If a completed winner rollout still carries doLog=true, it is skipped for opted-out subjects: always in immediate mode, and in future mode unless its activation is before the cutoff. Those subjects get the next matching allocation or the default.
    • If randomizationActivatedAt is absent, future mode skips every doLog=true allocation for an opted-out subject, the same as immediate mode.
Suggested text — assumes the parser and warn-and-disable suggestions are adopted
`dd_experimentation_opt_out_at` must be a `String`: a four-digit year, a full date and time with
seconds (`00`–`59`), `T` or `t`, optional fractional seconds (digits after the ninth are ignored),
and `Z`, `z`, or a `±HH:MM` offset. The instant must fall in UTC years 0000–9999.
For example, `2026-10-19T12:00:00Z`. `java.time.Instant` values are rejected.

In an enabled mode, once configuration has loaded, an invalid value makes every flag evaluation
for that context return the application's default with `INVALID_CONTEXT`. This includes flags without experiments and flags
that do not exist.

An unsupported mode logs a warning and disables experimentation opt-out.

Until the configuration service marks completed winner rollouts `doLog=false`, opted-out subjects
can skip those rollouts and receive the next matching allocation or the default. Future mode also
needs `randomizationActivatedAt`. Until the configuration service supplies it, future mode skips
every `doLog=true` allocation for an opted-out subject, the same as immediate mode.

Disabled mode ignores the attribute, including invalid values.

An explicit option overrides configuration:

```java
import com.datadog.openfeature.ExperimentationOptOutMode;

Provider provider = new Provider(new Provider.Options()
.experimentationOptOutMode(ExperimentationOptOutMode.DISABLED));
```

`DEFAULT` reads the normal SDK configuration sources, including
`-Ddd.feature.flags.experimentation.opt.out.mode=immediate`.
The provider resolves the mode at construction. Unsupported values throw `IllegalArgumentException`.
The deprecated `datadog.trace.api.openfeature.Provider` alias uses the same evaluator
and accepts the same `com.datadog.openfeature.ExperimentationOptOutMode` options.

This setting is independent of evaluation telemetry opt-out.
Immediate withdrawal can change an enrolled subject's experience. Account for this change in experiment analysis.

Use `future_experiments_only` or `ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY` to exclude newer experiments.
Supply the same stored `dd_experimentation_opt_out_at` timestamp with each evaluation.
For exposure-logging allocations, the provider compares `randomizationActivatedAt` with that timestamp:

| Allocation activation | Behavior |
| --- | --- |
| Before the subject's opt-out time | Evaluate normally, including exposure logging |
| Equal to or after the opt-out time | Skip the allocation and continue ordinary delivery |
| Missing or null | Skip the allocation because its activation time is unknown |

Ordinary allocations remain available. Missing subject attributes preserve normal evaluation.
Both enabled modes reject invalid subject timestamps.
Malformed activation metadata returns `PARSE_ERROR` for the affected flag only in future mode with a valid subject timestamp.
Disabled mode, immediate mode, and evaluations without a subject timestamp preserve ordinary evaluation.
Comparisons retain nanosecond precision and account for timezone offsets.
The cutoff does not depend on the SDK clock. It is not a scheduled change or proof of prior enrollment.
An older allocation can still assign a subject for the first time.

Enable future mode after the configuration service supplies stable activation timestamps and preserves ordinary fallback allocations.
Keep the subject's stored opt-out time stable across sessions and SDK instances.
The provider stores no consent history and emits no new event type.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

[P3] Add two facts to the README: exposure events carry dd_experimentation_opt_out_at, and the invocation context cannot remove the attribute when it is set on a shared context.

  1. "The provider stores no consent history and emits no new event type." is true. But exposure events copy the context attributes into subject.attributes (DDEvaluator.exposureEvent calls flattenContext(context)), and that includes the opt-out timestamp. A subject with the attribute can still produce exposures in disabled mode and, in future mode, for experiments activated before its cutoff. With full evaluation data on, flag evaluation events can also include it. A reader of this line can think the timestamp stays in the process.
  2. OpenFeature merges the API, transaction, client and invocation contexts. If an application sets the attribute on the API, transaction or client context, every evaluation through it sees that value unless a higher-priority context sets its own. The invocation context cannot remove it: .add(ATTRIBUTE, (String) null) is an invalid value, so in an enabled mode, once configuration has loaded, every flag returns INVALID_CONTEXT. "Omit the attribute for subjects who can participate" does not say this.
Evidence
  • Executed against this PR: DDEvaluator.exposureEvent with a context that has the attribute: subject.attributes={dd_experimentation_opt_out_at=2026-10-19T12:00:00Z, targetingKey=subject}.
  • Executed with my other suggested changes applied: immediate mode, attribute 2026-10-19T12:00:00Z on the API context of a LayeredEvaluationContext, and .add(ATTRIBUTE, (String) null) on the invocation context. The evaluation returns INVALID_CONTEXT. The merged keySet() contains the attribute, and getValue returns a Value whose isNull() is true. This PR's cutoff() gives the same result, because the key is present and the value is not a string.
  • In SDK 1.23.0, LayeredEvaluationContext.getValue asks the hook, invocation, client, transaction and API contexts in that order and returns the first non-null result. I read this in the bytecode; it is not a separate run.
Suggested text — insert the first block after line 25; replace line 67 with the second
OpenFeature merges the API, transaction, client, and invocation contexts. The invocation context
cannot remove an attribute set on the API, transaction, or client context. Set
`dd_experimentation_opt_out_at` only on the context of the subject it describes. Overriding it
with `null` is an invalid value, not an omission.
The provider stores no consent history and emits no new event type. Like other context
attributes, `dd_experimentation_opt_out_at` is included in the subject attributes of exposure
events and, when full evaluation data is enabled, can be included in flag evaluation events.

Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
package com.datadog.openfeature;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
import static java.util.concurrent.TimeUnit.NANOSECONDS;
import static java.util.concurrent.TimeUnit.SECONDS;

import com.datadog.openfeature.internal.bridge.v1.AgentBridge;
import com.datadog.openfeature.internal.config.Settings;
import com.datadog.openfeature.internal.ufc.Allocation;
import com.datadog.openfeature.internal.ufc.Flag;
import com.datadog.openfeature.internal.ufc.ServerConfiguration;
import com.datadog.openfeature.internal.ufc.Split;
import com.datadog.openfeature.internal.ufc.ValueType;
import com.datadog.openfeature.internal.ufc.Variant;
import dev.openfeature.sdk.MutableContext;
import dev.openfeature.sdk.ProviderEvaluation;
import java.util.HashMap;
import java.util.Map;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Param;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.TearDown;
import org.openjdk.jmh.annotations.Warmup;

/**
* Measures provider resolution, including timestamp validation and ordinary allocation fallback.
*/
@State(Scope.Thread)
@Warmup(iterations = 3, time = 1, timeUnit = SECONDS)
@Measurement(iterations = 5, time = 1, timeUnit = SECONDS)
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(NANOSECONDS)
@Fork(1)
public class ExperimentationOptOutBenchmark {
@Param({"DISABLED", "IMMEDIATE", "FUTURE_EXPERIMENTS_ONLY"})
public ExperimentationOptOutMode mode;

@Param({"false", "true"})
public boolean optedOut;

private Provider provider;
private MutableContext context;

@Setup
public void setUp() {
final DDEvaluator evaluator =
new DDEvaluator(() -> {}, AgentBridge.NONE, Settings.load(AgentBridge.NONE), mode);
final Map<String, Variant> variants = new HashMap<>();
variants.put("experiment", new Variant("experiment", false));
variants.put("ordinary", new Variant("ordinary", true));
final Flag flag =
new Flag(
"flag",
true,
ValueType.BOOLEAN,
variants,
asList(allocation("experiment", true), allocation("ordinary", false)));
evaluator.accept(new ServerConfiguration("", "", false, null, singletonMap("flag", flag)));
provider =
new Provider(new Provider.Options().experimentationOptOutMode(mode), evaluator, false);
context = new MutableContext("subject");
if (optedOut) {
context.add("dd_experimentation_opt_out_at", "2026-10-19T12:00:00Z");
}
final ProviderEvaluation<Boolean> result = evaluate();
final boolean excluded = mode != ExperimentationOptOutMode.DISABLED && optedOut;
if (result.getErrorCode() != null
|| result.getValue() != excluded
|| !(excluded ? "ordinary" : "experiment")
.equals(result.getFlagMetadata().getString("allocationKey"))) {
throw new IllegalStateException("Benchmark did not resolve the expected allocation");
}
}

private static Allocation allocation(final String key, final boolean doLog) {
return new Allocation(
key, null, null, null, singletonList(new Split(emptyList(), key, emptyMap(), 1)), doLog);
}

@Benchmark
public ProviderEvaluation<Boolean> evaluate() {
return provider.getBooleanEvaluation("flag", false, context);
}

@TearDown
public void tearDown() {
provider.shutdown();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,12 +84,22 @@ class DDEvaluator implements Evaluator, Consumer<ServerConfiguration> {
private volatile FeatureFlagsRuntime runtime;

DDEvaluator(final Runnable configCallback, final AgentBridge bridge, final Settings settings) {
this(configCallback, bridge, settings, ExperimentationOptOutMode.DISABLED);
}

DDEvaluator(
final Runnable configCallback,
final AgentBridge bridge,
final Settings settings,
final ExperimentationOptOutMode experimentationOptOutMode) {
this.configCallback = configCallback;
this.bridge = bridge;
this.settings = settings;
// When span enrichment is off, the __dd_* span-enrichment metadata is attached only to
// evaluations of allocations logging exposures, for the exposure hook.
this.core = new FlagEvaluator(settings.isSpanEnrichmentEnabled(), this::isExposed);
this.core =
new FlagEvaluator(
settings.isSpanEnrichmentEnabled(), this::isExposed, experimentationOptOutMode);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.datadog.openfeature;

/** Controls per-subject exclusion from experimentation. */
public enum ExperimentationOptOutMode {
/** Reads DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE, which defaults to disabled. */
DEFAULT,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

[P2] Consider removing the public DEFAULT member and modeling "unset" as a null option, as enableEvaluationTelemetryOptOutParameter already does.

DEFAULT is an instruction to read configuration, not a mode. Provider resolves it first, but the internal FlagEvaluator and DDEvaluator constructors still accept it and treat it as disabled. Customers can also write experimentationOptOutMode(DEFAULT). Once released, removing the member breaks source and binary compatibility. The sibling option uses Boolean null for "unset", and Ruby uses nil.

Sketch — not compiled; it also touches two tests and the README
// ExperimentationOptOutMode: delete DEFAULT.

// Provider.Options
private ExperimentationOptOutMode experimentationOptOutMode; // null: read configuration

/**
 * Sets per-subject experimentation opt-out. Overrides
 * {@code DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE}. Without an explicit option, the provider
 * reads configuration at construction. Disabled by default.
 */
public Options experimentationOptOutMode(final ExperimentationOptOutMode mode) {
  this.experimentationOptOutMode = requireNonNull(mode, "mode");
  return this;
}

// ExperimentationOptOut.resolve: drop requireNonNull(override, …), then
if (override != null) {
  return override;
}

requireNonNull needs import static java.util.Objects.requireNonNull;. Tests that use DEFAULT today: ExperimentationOptOutProviderTest (to reset) and ExperimentationOptOutTest (resolve(DEFAULT, …)). README line 38 also names it.

/** Ignores dd_experimentation_opt_out_at, overriding the environment setting. */
DISABLED,
/**
* Skips exposure-logging allocations when dd_experimentation_opt_out_at is an RFC3339 timestamp.
* Applies immediately, including to experiments already in progress.
*/
IMMEDIATE,
/**
* Skips exposure-logging allocations activated at or after dd_experimentation_opt_out_at. Unknown
* activation is skipped. Older allocations continue normally, including exposures. This is an
* allocation activation cutoff, not a record of prior subject enrollment.
*/
FUTURE_EXPERIMENTS_ONLY
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import com.datadog.openfeature.internal.bridge.v1.AgentBridge;
import com.datadog.openfeature.internal.bridge.v1.AgentBridges;
import com.datadog.openfeature.internal.config.Settings;
import com.datadog.openfeature.internal.evaluation.ExperimentationOptOut;
import com.datadog.openfeature.internal.flagevaluation.FlagEvaluationWriter;
import dev.openfeature.sdk.ErrorCode;
import dev.openfeature.sdk.EvaluationContext;
Expand Down Expand Up @@ -39,6 +40,7 @@ public class Provider extends EventProvider implements Metadata {
private final Options options;
private final AgentBridge bridge;
private final Settings settings;
private final ExperimentationOptOutMode experimentationOptOutMode;
private final AtomicReference<InitializationState> initializationState =
new AtomicReference<>(InitializationState.NOT_STARTED);
private final FlagEvalMetrics flagEvalMetrics;
Expand Down Expand Up @@ -79,6 +81,8 @@ public Provider(final Options options) {
this.evaluator = evaluator;
this.bridge = AgentBridges.detect();
this.settings = Settings.load(this.bridge);
this.experimentationOptOutMode =
ExperimentationOptOut.resolve(options.experimentationOptOutMode, this.settings);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

[P3] Optional: log the resolved opt-out mode once at construction.

The mode can come from the explicit option, the environment, a system property, or agent configuration. Today nothing in the log shows which mode the provider uses. The constructor already announces the span-enrichment state ("span enrichment enabled" or "span enrichment disabled"). The evaluation-telemetry opt-out does not log its state, so the module has no single convention here. Take it or leave it.

Suggested line, after this assignment (log and METADATA already exist in Provider):

    log.info(
        "{} experimentation opt-out mode {}",
        METADATA,
        this.experimentationOptOutMode.name().toLowerCase(java.util.Locale.ROOT));

This prints the configuration spelling, for example future_experiments_only. It is a sketch; I did not compile it.

FlagEvalMetrics metrics = null;
FlagEvalMetricsHook hook = null;
try {
Expand Down Expand Up @@ -242,7 +246,8 @@ private Evaluator buildEvaluator() {
if (evaluator != null) {
return evaluator;
}
return new DDEvaluator(this::onConfigurationChange, this.bridge, this.settings);
return new DDEvaluator(
this::onConfigurationChange, this.bridge, this.settings, this.experimentationOptOutMode);
}

private FlagEvaluationWriter flagEvaluationPipeline() {
Expand Down Expand Up @@ -335,6 +340,18 @@ public static class Options {
private long timeout = 30;
private TimeUnit unit = SECONDS;
private Boolean evaluationTelemetryOptOutParameterEnabled;
private ExperimentationOptOutMode experimentationOptOutMode = ExperimentationOptOutMode.DEFAULT;

/**
* Sets per-subject experimentation opt-out. Overrides configuration when not DEFAULT.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

[P3] Expand this public Javadoc to name the context attribute, its required type, and what happens to an invalid value.

Provider.Options and ExperimentationOptOutMode are public API. Today the option's Javadoc does not say which attribute the mode reads, that the value must be a timestamp String, that a java.time.Instant value is rejected, or that a null mode throws NullPointerException. A user who reads only the Javadoc in an IDE can pass an Instant and get INVALID_CONTEXT for every flag.

Suggested text — :dd-openfeature:javadoc passes

If the DEFAULT member is removed, as suggested in the comment on ExperimentationOptOutMode, drop the unless clause.

--- a/dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java
@@ -343,10 +348,19 @@ public class Provider extends EventProvider implements Metadata {
     private ExperimentationOptOutMode experimentationOptOutMode = ExperimentationOptOutMode.DEFAULT;
 
     /**
-     * Sets per-subject experimentation opt-out. Overrides configuration when not DEFAULT.
+     * Sets per-subject experimentation opt-out through the {@code dd_experimentation_opt_out_at}
+     * evaluation context attribute. Overrides {@code DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE}
+     * unless {@link ExperimentationOptOutMode#DEFAULT}. The provider resolves the mode at
+     * construction. Disabled by default.
      *
-     * @param mode the mode; configure before constructing the provider.
+     * <p>In an enabled mode, supply the attribute as a timestamp {@code String}, such as {@code
+     * Instant.toString()}. Once configuration has loaded, a present value that is not a valid
+     * timestamp string, including a {@code java.time.Instant} value, makes every flag evaluation
+     * with that context return the default with {@code INVALID_CONTEXT}.
+     *
+     * @param mode the mode, not null; configure before constructing the provider.
      * @return these options.
+     * @throws NullPointerException if {@code mode} is null.
      */
     public Options experimentationOptOutMode(final ExperimentationOptOutMode mode) {
       this.experimentationOptOutMode = java.util.Objects.requireNonNull(mode, "mode");
--- a/dd-openfeature/src/main/java/com/datadog/openfeature/ExperimentationOptOutMode.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/ExperimentationOptOutMode.java
@@ -4,11 +4,13 @@ package com.datadog.openfeature;
 public enum ExperimentationOptOutMode {
   /** Reads DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE, which defaults to disabled. */
   DEFAULT,
-  /** Ignores dd_experimentation_opt_out_at, overriding the environment setting. */
+  /** Ignores dd_experimentation_opt_out_at, including invalid values. */
   DISABLED,
   /**
-   * Skips exposure-logging allocations when dd_experimentation_opt_out_at is an RFC3339 timestamp.
-   * Applies immediately, including to experiments already in progress.
+   * Skips exposure-logging allocations when dd_experimentation_opt_out_at is an RFC3339 timestamp
+   * string, including a future one. Applies immediately, including to experiments already in
+   * progress. Once configuration has loaded, a present value that is not a valid timestamp string
+   * returns the default with INVALID_CONTEXT.
    */
   IMMEDIATE,
   /**

*
* @param mode the mode; configure before constructing the provider.
* @return these options.
*/
public Options experimentationOptOutMode(final ExperimentationOptOutMode mode) {
this.experimentationOptOutMode = java.util.Objects.requireNonNull(mode, "mode");
return this;
}

/**
* Enables per-evaluation telemetry opt-out through the {@code dd_evaluation_telemetry_opt_out}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,12 @@ public boolean isEvaluationTelemetryPerSubjectOptOutEnabled() {
return Boolean.TRUE.equals(getBoolean(EVALUATION_TELEMETRY_PER_SUBJECT_OPT_OUT_ENABLED));
}

/** Returns the configured experimentation opt-out mode, disabled by default. */
public String experimentationOptOutMode() {
final String mode = get("feature.flags.experimentation.opt.out.mode");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

[P3] A few small cleanups, take or leave.

  • Setting key. This is the only inline key literal in Settings. Every other key is a constant (EVALUATION_TELEMETRY_PER_SUBJECT_OPT_OUT_ENABLED and others). Add static final String EXPERIMENTATION_OPT_OUT_MODE = "feature.flags.experimentation.opt.out.mode"; and reuse it in SettingsTest.
  • requireNonNull. Provider.Options writes java.util.Objects.requireNonNull inline. The module imports it statically elsewhere (ExposureHook).
  • Activation validity. Allocation carries invalidRandomizationActivation per allocation and has three constructors, and hasInvalidActivation walks the allocations (up to all of them) on each opted-out future-mode evaluation. The only consumer asks a flag-level question. Compute it once in the parser and store it on Flag. Then the 8-argument constructor and the per-allocation boolean go away. The scan costs about 6–10 ns at 20 allocations, so this is about shape, not speed.
  • Comments that could be more precise:
    • Allocation: "Invalid activation is retained for future-mode evaluation errors." Only a boolean is kept, not the value. Suggest: "True when randomizationActivatedAt was present but unparseable. Future mode with a valid subject cutoff then fails the flag with PARSE_ERROR."
    • Allocation: "null means unknown". It is also null when the value is invalid.
    • ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY: "Unknown activation is skipped." That holds for missing or null. A malformed value fails the whole flag with PARSE_ERROR when the subject has a valid cutoff.
    • ExperimentationOptOut: "Validates the subject's opt-out timestamp before allocation selection." The class also resolves the mode and decides exclusion.

return mode == null ? "disabled" : mode;
}

/**
* @return the Datadog API key, or {@code null} if not set.
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
package com.datadog.openfeature.internal.evaluation;

import com.datadog.openfeature.ExperimentationOptOutMode;
import com.datadog.openfeature.internal.config.Settings;
import com.datadog.openfeature.internal.ufc.Allocation;
import dev.openfeature.sdk.EvaluationContext;
import dev.openfeature.sdk.Value;
import java.time.DateTimeException;
import java.time.Instant;
import java.time.format.DateTimeFormatter;
import java.util.List;
import java.util.Locale;
import java.util.Objects;

/** Validates the subject's opt-out timestamp before allocation selection. */
public final class ExperimentationOptOut {
public static final String ATTRIBUTE = "dd_experimentation_opt_out_at";
public static final String INVALID_TIMESTAMP =
ATTRIBUTE
+ " must be an RFC3339 timestamp string; supply a valid timestamp to opt out,"
+ " or omit the attribute to permit experimentation";

private ExperimentationOptOut() {}

/** Resolves the provider option before reading central configuration. */
public static ExperimentationOptOutMode resolve(
final ExperimentationOptOutMode override, final Settings settings) {
Objects.requireNonNull(override, "experimentationOptOutMode");
if (override != ExperimentationOptOutMode.DEFAULT) {
return override;
}
final String configured = settings.experimentationOptOutMode();
switch (configured.trim().toLowerCase(Locale.ROOT)) {
case "":

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

[P3] Remove case "":, which cannot be reached, and replace the "" row in providerConfigurationPrecedence, which depends on the environment of the test process.

Settings.get skips blank values from every source, and experimentationOptOutMode() maps null to "disabled". So resolve never receives a blank string. The "" row sets the system property to an empty string. Settings.get then skips it and reads the next source, DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE. That row fails when the environment sets an enabled or unsupported mode.

Evidence

With DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE=immediate in the environment of the Gradle test run, providerConfigurationPrecedence row [1] (the "" row) fails. No other row of ExperimentationOptOutTest fails.

The SettingsTest row below covers the same blank-skip behaviour with injected sources. It passes, and it fails when Settings.get is changed to return blank values.

Suggested change — passes all :dd-openfeature:test tests with the rest of the review patch
--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
@@ -31,7 +31,6 @@ public final class ExperimentationOptOut {
     }
     final String configured = settings.experimentationOptOutMode();
     switch (configured.trim().toLowerCase(Locale.ROOT)) {
-      case "":
       case "disabled":
         return ExperimentationOptOutMode.DISABLED;
       case "immediate":
--- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOutTest.java
+++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOutTest.java
@@ -31,7 +35,6 @@ class ExperimentationOptOutTest {
   @ParameterizedTest
   @ValueSource(
       strings = {
-        "",
         "disabled",
         "immediate",
         " IMMEDIATE ",
--- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/config/SettingsTest.java
+++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/config/SettingsTest.java
@@ -98,7 +98,8 @@ class SettingsTest {
     "environment enables  |           |             | immediate   | immediate  ",
     "property wins        |           | disabled    | immediate   | disabled   ",
     "agent wins           | immediate | disabled    | disabled    | immediate  ",
-    "invalid is preserved |           | unsupported | immediate   | unsupported"
+    "invalid is preserved |           | unsupported | immediate   | unsupported",
+    "blank property skips |           | ' '         | immediate   | immediate  "
   })
   void resolvesExperimentationOptOutSource(
       final String agent, final String property, final String environment, final String expected) {

case "disabled":
return ExperimentationOptOutMode.DISABLED;
case "immediate":
return ExperimentationOptOutMode.IMMEDIATE;
case "future_experiments_only":
return ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY;
default:
throw new IllegalArgumentException(
"DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE must be disabled, immediate, or future_experiments_only");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

[P1] On an unsupported mode, log a warning and fall back to disabled instead of throwing from the Provider constructor.

The mode is read through the agent's config layer, so it can come from the environment, a system property, or fleet and local stable config. A typo there throws IllegalArgumentException from new Provider(). It throws even with DD_FEATURE_FLAGS_ENABLED=false. In usual startup code, such as the smoke app's Spring @Bean, the application then fails to start. Other bad settings in this module log a warning instead: an invalid boolean reads as false, an invalid positive integer falls back to the next source or the default, and an unsupported configuration source disables Feature Flags. Falling back to disabled removes this constructor exception. The trade-off is that, until the typo is fixed, no subject is opted out; the warning names the bad value.

This departs from draft decision D015 ("enabled providers reject unsupported modes") in DataDog/ffe-system-test-data#55, so the contract needs the same change.

Evidence — executed against this PR
Configuration Result
DD_FEATURE_FLAGS_ENABLED=false, mode immedaite IllegalArgumentException from new Provider()
mode future-experiments-only (hyphens) IllegalArgumentException

OpenFeatureAPI.setProviderAndWait(...) never receives the provider, so nothing in the OpenFeature SDK catches the exception. In the smoke app's Spring @Bean, bean creation fails.

Suggested fix — warn and disable; compiles on its own against this PR
--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
@@ -11,9 +11,12 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.Objects;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 /** Validates the subject's opt-out timestamp before allocation selection. */
 public final class ExperimentationOptOut {
+  private static final Logger LOGGER = LoggerFactory.getLogger(ExperimentationOptOut.class);
   public static final String ATTRIBUTE = "dd_experimentation_opt_out_at";
   public static final String INVALID_TIMESTAMP =
       ATTRIBUTE
@@ -39,8 +42,12 @@
       case "future_experiments_only":
         return ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY;
       default:
-        throw new IllegalArgumentException(
-            "DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE must be disabled, immediate, or future_experiments_only");
+        // A typo must not stop the application. Without a valid mode, no subject is opted out.
+        LOGGER.warn(
+            "Unsupported experimentation opt-out mode '{}'; expected disabled, immediate, or"
+                + " future_experiments_only. Experimentation opt-out is disabled.",
+            configured);
+        return ExperimentationOptOutMode.DISABLED;
     }
   }
 

providerConfigurationPrecedence["unsupported"] then expects DISABLED instead of a throw. A test that pins the disabled case, from the test class in my comment on the exclusion loop:

  // An unsupported mode does not stop construction when Feature Flags is disabled.
  @Test
  void disabledFeatureFlagsDoNotValidateMode() {
    final String[][] properties = {
      {"dd.feature.flags.enabled", "false"},
      {"dd.feature.flags.experimentation.opt.out.mode", "unsupported"}
    };
    final String[] previous = new String[properties.length];
    for (int i = 0; i < properties.length; i++) {
      previous[i] = System.setProperty(properties[i][0], properties[i][1]);
    }
    try {
      assertDoesNotThrow(() -> new Provider().shutdown());
    } finally {
      for (int i = 0; i < properties.length; i++) {
        if (previous[i] == null) {
          System.clearProperty(properties[i][0]);
        } else {
          System.setProperty(properties[i][0], previous[i]);
        }
      }
    }
  }

With all the fixes from my comments applied, the 1325 module tests pass. Restoring the throw turns this test and providerConfigurationPrecedence["unsupported"] red.

}
}

/** Parses the cutoff once per evaluation; null means no exclusion. */
static Instant cutoff(final ExperimentationOptOutMode mode, final EvaluationContext context) {
if ((mode != ExperimentationOptOutMode.IMMEDIATE
&& mode != ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY)
|| !context.keySet().contains(ATTRIBUTE)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

[P2] Use context.getValue(ATTRIBUTE) alone here, without keySet(), so an enabled mode does not copy every context key on each evaluation.

In OpenFeature SDK 1.23.0, the client builds a new LayeredEvaluationContext for each flag evaluation. The first keySet() call on it copies the keys of every layer into a new set. So in an enabled mode, every evaluation that reaches cutoff() pays for that copy, including subjects without the attribute. getValue looks the key up layer by layer and builds no key set. ExperimentationOptOutBenchmark passes a flat MutableContext straight to Provider, so it does not show the cost.

Measured allocation per evaluation — 200,000 DDEvaluator.evaluate calls, no opt-out attribute, one-allocation flag

The context is new LayeredEvaluationContext(ImmutableContext.EMPTY, ImmutableContext.EMPTY, null, invocation): empty API and transaction layers, and the subject's attributes on the invocation layer. Bytes per evaluation from ThreadMXBean.getThreadAllocatedBytes:

Context disabled immediate, this PR immediate, with the fix
layered, 0 other attributes 544 1312 632
layered, 16 other attributes 544 2560 544
flat MutableContext, 0 or 16 attributes 384 384 384

The copy grows with the number of attributes. The layered context keeps the set after the first call. So when a later step in the same evaluation calls keySet() anyway, such as exposure logging on a new exposure, the fix saves no key-set copy for that evaluation. Evaluations where no later step calls keySet(), such as a repeat exposure with full evaluation data off, save the whole copy.

Suggested fix — passes all 1270 :dd-openfeature:test tests with the rest of the review patch

This hunk also adds the INVALID_CUTOFF sentinel from my comment on line 63, with its Javadoc. To take only this change, apply just the removal of the keySet() check and the new value == null return, and keep the rest of cutoff(), including its Javadoc and the final throw, as it is.

--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
@@ -44,15 +43,32 @@ public final class ExperimentationOptOut {
     }
   }
 
-  /** Parses the cutoff once per evaluation; null means no exclusion. */
+  /**
+   * Returned by {@link #cutoff} for a present but invalid attribute. Compare by identity: {@code
+   * Instant.MIN} is earlier than any timestamp the formatter accepts, so a parsed cutoff is never
+   * this instance.
+   */
+  static final Instant INVALID_CUTOFF = Instant.MIN;
+
+  /**
+   * Parses the cutoff once per evaluation.
+   *
+   * @return {@code null} when no exclusion applies, {@link #INVALID_CUTOFF} when the attribute is
+   *     present but invalid, otherwise the cutoff.
+   */
   static Instant cutoff(final ExperimentationOptOutMode mode, final EvaluationContext context) {
-    if ((mode != ExperimentationOptOutMode.IMMEDIATE
-            && mode != ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY)
-        || !context.keySet().contains(ATTRIBUTE)) {
+    if (mode != ExperimentationOptOutMode.IMMEDIATE
+        && mode != ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY) {
       return null;
     }
+    // getValue is a lookup per context layer. keySet() on the LayeredEvaluationContext that the
+    // OpenFeature client passes copies every key of every layer on each evaluation. An OpenFeature
+    // null (new Value()) is non-null here and stays invalid below.
     final Value value = context.getValue(ATTRIBUTE);
-    if (value != null && value.isString()) {
+    if (value == null) {
+      return null;
+    }
+    if (value.isString()) {
       try {
         // Use the same timestamp parser as allocation startAt/endAt.
         return DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(value.asString(), Instant::from);

One behaviour changes. A raw Java null map entry (attributes.put(ATTRIBUTE, null) on an ImmutableContext) now reads as absent, because getValue cannot tell it from a missing key. An OpenFeature null, such as MutableContext.add(ATTRIBUTE, (String) null), is still a Value and stays invalid. A test that pins this, for ExperimentationOptOutTest:

@Test
void rawJavaNullEntryIsTreatedAsAbsent() {
  final Map<String, Value> attributes = new HashMap<>();
  attributes.put(ExperimentationOptOut.ATTRIBUTE, null);
  for (final ExperimentationOptOutMode mode : ExperimentationOptOutMode.values()) {
    assertNull(ExperimentationOptOut.cutoff(mode, new ImmutableContext("subject", attributes)));
  }
}

It needs import dev.openfeature.sdk.ImmutableContext;, java.util.HashMap and java.util.Map. It fails on this PR and passes with the fix.

To make the benchmark show the cost, build the layered context in the benchmark method. This compiles (compileJmhJava); I did not run JMH.

--- a/dd-openfeature/src/jmh/java/com/datadog/openfeature/ExperimentationOptOutBenchmark.java
+++ b/dd-openfeature/src/jmh/java/com/datadog/openfeature/ExperimentationOptOutBenchmark.java
@@ -16,8 +16,11 @@ import com.datadog.openfeature.internal.ufc.ServerConfiguration;
 import com.datadog.openfeature.internal.ufc.Split;
 import com.datadog.openfeature.internal.ufc.ValueType;
 import com.datadog.openfeature.internal.ufc.Variant;
-import dev.openfeature.sdk.MutableContext;
+import dev.openfeature.sdk.EvaluationContext;
+import dev.openfeature.sdk.ImmutableContext;
+import dev.openfeature.sdk.LayeredEvaluationContext;
 import dev.openfeature.sdk.ProviderEvaluation;
+import dev.openfeature.sdk.Value;
 import java.util.HashMap;
 import java.util.Map;
 import org.openjdk.jmh.annotations.Benchmark;
@@ -49,8 +52,12 @@ public class ExperimentationOptOutBenchmark {
   @Param({"false", "true"})
   public boolean optedOut;
 
+  /** Ordinary attributes next to the opt-out attribute. */
+  @Param({"0", "16"})
+  public int extraAttributes;
+
   private Provider provider;
-  private MutableContext context;
+  private ImmutableContext invocation;
 
   @Setup
   public void setUp() {
@@ -69,10 +76,14 @@ public class ExperimentationOptOutBenchmark {
     evaluator.accept(new ServerConfiguration("", "", false, null, singletonMap("flag", flag)));
     provider =
         new Provider(new Provider.Options().experimentationOptOutMode(mode), evaluator, false);
-    context = new MutableContext("subject");
+    final Map<String, Value> attributes = new HashMap<>();
+    for (int i = 0; i < extraAttributes; i++) {
+      attributes.put("attribute" + i, new Value("value" + i));
+    }
     if (optedOut) {
-      context.add("dd_experimentation_opt_out_at", "2026-10-19T12:00:00Z");
+      attributes.put("dd_experimentation_opt_out_at", new Value("2026-10-19T12:00:00Z"));
     }
+    invocation = new ImmutableContext("subject", attributes);
     final ProviderEvaluation<Boolean> result = evaluate();
     final boolean excluded = mode != ExperimentationOptOutMode.DISABLED && optedOut;
     if (result.getErrorCode() != null
@@ -90,6 +101,11 @@ public class ExperimentationOptOutBenchmark {
 
   @Benchmark
   public ProviderEvaluation<Boolean> evaluate() {
+    // The OpenFeature client builds a new layered context for every flag evaluation. Here the API
+    // and transaction layers are empty and there is no client layer.
+    final EvaluationContext context =
+        new LayeredEvaluationContext(
+            ImmutableContext.EMPTY, ImmutableContext.EMPTY, null, invocation);
     return provider.getBooleanEvaluation("flag", false, context);
   }
 

return null;
}
final Value value = context.getValue(ATTRIBUTE);
if (value != null && value.isString()) {
try {
// Use the same timestamp parser as allocation startAt/endAt.
return DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(value.asString(), Instant::from);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

[P1] Replace ISO_OFFSET_DATE_TIME with a parser for the shared timestamp grammar, and test it against the shared corpus. Suggest matching the iOS and Android opt-out PRs, which use a hand-written parser.

ISO_OFFSET_DATE_TIME is not the grammar in draft decisions D005–D007. On the 50-row shared timestamp corpus, Java disagrees on 8 rows. libdatadog and the iOS, Android, and browser opt-out parsers agree on all 50. So one stored cutoff can opt a subject out in a Java service and return INVALID_CONTEXT on every flag in a Ruby service. randomizationActivatedAt uses the same formatter (UniversalFlagConfigParser.java:239), so the future-mode PARSE_ERROR rule also fires on different inputs.

timestampValidationMatchesAllocationTimestamps cannot catch this, because it takes its expected result from ISO_OFFSET_DATE_TIME itself.

1. The 8 corpus rows where Java disagrees — JDK 21, this PR's cutoff(); ids without the timestamp/ prefix
Corpus id Input Contract Java
ten-digits-zero 2026-10-19T12:00:00.1234567890Z accept, truncate to 9 digits reject
ten-digits-nine 2026-10-19T12:00:00.1234567899Z accept reject
twelve-digits 2026-10-19T12:00:00.123456789999Z accept reject
fraction-zero 2026-01-01T00:00:00.000000000000Z accept reject
offset-max 2026-01-01T00:00:00+23:59 accept reject (java.time caps offsets at ±18:00)
utc-year-underflow 0000-01-01T00:00:00+01:00 reject accept
utc-year-overflow 9999-12-31T23:59:59-01:00 reject accept
empty-fraction 2026-01-01T00:00:00.Z reject accept

Outside the corpus, Java also accepts 2026-10-19T12:00Z (no seconds), +01:00:30 (seconds in the offset), +01 (hour-only offset), and +10000-01-01T00:00:00Z. The contract rejects all four. The first is what OffsetDateTime.toString() prints when the seconds and nanoseconds are zero.

2. The other opt-out parsers I checked — the same 50 rows, run through a replica of each parser
SDK Parser Rows that disagree
libdatadog (Ruby, Python, PHP) hand-written 0
iOS, DataDog/dd-sdk-ios#3338 hand-written regex and calendar checks 0
Android, DataDog/dd-sdk-android#4024 regex, then non-lenient SimpleDateFormat 0 (desktop JDK replica; not run on an Android runtime)
browser, DataDog/openfeature-js-client#423 hand-written 0
Java, this PR ISO_OFFSET_DATE_TIME 8
Go, DataDog/dd-trace-go#5666 (merged) time.Parse(time.RFC3339Nano, …) 10

Go rejects lowercase t/z. It accepts a comma fraction, +24:00, +01:60, a one-digit hour, and out-of-range UTC years. That is outside this PR, but it is the same class of gap. Apart from libdatadog's own suite, none of these SDKs' tests run the corpus.

3. Suggested fix — a port of libdatadog's parser, wired into both call sites
package com.datadog.openfeature.internal.ufc;

import java.time.DateTimeException;
import java.time.Instant;
import java.time.LocalDate;
import java.time.ZoneOffset;
import javax.annotation.Nullable;

/**
 * Contract timestamp grammar (D005-D007) shared by the subject cutoff and allocation activation.
 */
public final class OptOutTimestamps {
  private OptOutTimestamps() {}

  /** Returns the instant, or null when {@code s} is outside the contract grammar. */
  @Nullable
  public static Instant parse(final String s) {
    final int n = s.length();
    if (n < 20
        || s.charAt(4) != '-'
        || s.charAt(7) != '-'
        || (s.charAt(10) != 'T' && s.charAt(10) != 't')
        || s.charAt(13) != ':'
        || s.charAt(16) != ':') {
      return null;
    }
    final int year = digits(s, 0, 4);
    final int month = digits(s, 5, 7);
    final int day = digits(s, 8, 10);
    final int hour = digits(s, 11, 13);
    final int minute = digits(s, 14, 16);
    final int second = digits(s, 17, 19);
    if (year < 0
        || month < 0
        || day < 0
        || hour < 0
        || hour > 23
        || minute < 0
        || minute > 59
        || second < 0
        || second > 59) {
      return null;
    }
    int i = 19;
    int nanos = 0;
    if (s.charAt(i) == '.') {
      final int start = ++i;
      int scale = 100_000_000;
      while (i < n && s.charAt(i) >= '0' && s.charAt(i) <= '9') {
        nanos += (s.charAt(i) - '0') * scale; // truncate beyond 9 digits
        scale /= 10;
        i++;
      }
      if (i == start) {
        return null;
      }
    }
    final int offsetSeconds;
    final int rest = n - i;
    if (rest == 1 && (s.charAt(i) == 'Z' || s.charAt(i) == 'z')) {
      offsetSeconds = 0;
    } else if (rest == 6 && (s.charAt(i) == '+' || s.charAt(i) == '-') && s.charAt(i + 3) == ':') {
      final int oh = digits(s, i + 1, i + 3);
      final int om = digits(s, i + 4, i + 6);
      if (oh < 0 || oh > 23 || om < 0 || om > 59) {
        return null;
      }
      offsetSeconds = (s.charAt(i) == '-' ? -1 : 1) * (oh * 3600 + om * 60);
    } else {
      return null;
    }
    final Instant instant;
    try {
      instant =
          LocalDate.of(year, month, day)
              .atTime(hour, minute, second, nanos)
              .toInstant(ZoneOffset.UTC)
              .minusSeconds(offsetSeconds);
    } catch (final DateTimeException e) {
      return null;
    }
    final int utcYear = instant.atOffset(ZoneOffset.UTC).getYear();
    return utcYear < 0 || utcYear > 9999 ? null : instant;
  }

  private static int digits(final String s, final int from, final int to) {
    int v = 0;
    for (int i = from; i < to; i++) {
      final char c = s.charAt(i);
      if (c < '0' || c > '9') {
        return -1;
      }
      v = v * 10 + (c - '0');
    }
    return v;
  }
}
--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java
@@ -236,7 +236,8 @@ public final class UniversalFlagConfigParser {
     }
     final Map<?, ?> json = asObject(value, "allocation");
     final Object activation = json.get("randomizationActivatedAt");
-    final Instant parsedActivation = activation instanceof String ? toInstant(activation) : null;
+    final Instant parsedActivation =
+        activation instanceof String ? OptOutTimestamps.parse((String) activation) : null;
     return new Allocation(
         asString(json.get("key"), "key"),
         toList(json.get("rules"), "rules", UniversalFlagConfigParser::toRule),

In ExperimentationOptOut.cutoff, with import com.datadog.openfeature.internal.ufc.OptOutTimestamps;:

    if (value != null && value.isString()) {
      final Instant cutoff = OptOutTimestamps.parse(value.asString());
      if (cutoff != null) {
        return cutoff;
      }
    }
    throw new IllegalArgumentException(INVALID_TIMESTAMP);

startAt and endAt keep their current parser.

4. Corpus-driven test — replaces timestampValidationMatchesAllocationTimestamps

I copied the corpus into src/test/resources/experimentation-opt-out-timestamps.jsonl from libdatadog's libdd-ffe-test-suite/tests/data/experimentation-opt-out-timestamps.jsonl. The same 50 rows are the "kind": "timestamp" lines of contracts/experimentation-opt-out/corpus/browser-proposal.jsonl in ffe-system-test-data. Once DataDog/ffe-system-test-data#55 merges, the test can read them from the existing submodule instead of a copy.

  static Stream<Arguments> timestampCorpus() throws IOException {
    final List<Arguments> cases = new ArrayList<>();
    final byte[] corpus;
    try (InputStream in =
        ExperimentationOptOutTest.class.getResourceAsStream(
            "/experimentation-opt-out-timestamps.jsonl")) {
      corpus = readAll(in);
    }
    for (final String line : new String(corpus, UTF_8).split("\n")) {
      if (!line.trim().isEmpty()) {
        final Map<String, Object> row = JsonReading.readObject(line);
        cases.add(Arguments.of(row.get("id"), row.get("input"), row.get("expected")));
      }
    }
    return cases.stream();
  }

  private static byte[] readAll(final InputStream in) throws IOException {
    final ByteArrayOutputStream out = new ByteArrayOutputStream();
    final byte[] buffer = new byte[8192];
    for (int n; (n = in.read(buffer)) != -1; ) {
      out.write(buffer, 0, n);
    }
    return out.toByteArray();
  }

  @ParameterizedTest(name = "{0}")
  @MethodSource("timestampCorpus")
  void timestampGrammarMatchesSharedCorpus(
      final String id, final String input, final Map<String, Object> expected) {
    final MutableContext context = new MutableContext().add(ExperimentationOptOut.ATTRIBUTE, input);
    if (Boolean.TRUE.equals(expected.get("accepted"))) {
      assertEquals(
          Instant.ofEpochSecond(
              ((Number) expected.get("seconds")).longValue(),
              ((Number) expected.get("nanos")).longValue()),
          ExperimentationOptOut.cutoff(ExperimentationOptOutMode.IMMEDIATE, context));
    } else {
      assertThrows(
          IllegalArgumentException.class,
          () -> ExperimentationOptOut.cutoff(ExperimentationOptOutMode.IMMEDIATE, context));
    }
  }

It needs these imports in ExperimentationOptOutTest: com.datadog.openfeature.internal.JsonReading, java.io.ByteArrayOutputStream, java.io.IOException, java.io.InputStream, java.util.ArrayList, java.util.List, java.util.Map, and static java.nio.charset.StandardCharsets.UTF_8. The test exercises the cutoff path. Activation parsing calls the same OptOutTimestamps.parse, so the grammar is shared, but an activation-side test would still pin the wiring.

5. Verification
  • With all the fixes from my comments applied, ./gradlew :dd-openfeature:test passes all 1325 tests.
  • Swapping the parser back to ISO_OFFSET_DATE_TIME turns the 8 corpus rows above red.
  • The cutoff parse alone drops from about 400 ns and 1,512 B to about 37 ns and 24 B per call (JDK 21 microbenchmark). The PR parses once per evaluation for an opted-out subject.

} catch (final DateTimeException e) {
// Do not expose the supplied context value in the diagnostic.
}
}
throw new IllegalArgumentException(INVALID_TIMESTAMP);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

[P3] Return a sentinel for an invalid cutoff instead of throwing IllegalArgumentException, so the catch in FlagEvaluator cannot relabel an unrelated failure as an invalid opt-out timestamp.

FlagEvaluator catches every IllegalArgumentException thrown while cutoff() runs. That includes one thrown by the context itself, from keySet() or getValue(). In an enabled mode, such a failure returns INVALID_CONTEXT with the opt-out timestamp message, which points the operator at the wrong cause. In disabled mode, a flag with no targeting rules evaluates normally with the same context. The throw also builds an exception with a stack trace on every evaluation for an invalid subject. The comment on line 108 of FlagEvaluator sketches the same INVALID_CUTOFF sentinel for a different reason, so both changes fit together.

Evidence — a context subclass whose lookup throws IllegalArgumentException, against this PR
Mode Result
disabled evaluates normally
immediate, future_experiments_only INVALID_CONTEXT, message dd_experimentation_opt_out_at must be an RFC3339 timestamp string; …

With the fix, the exception reaches the evaluator's general catch and returns GENERAL. The test below fails on this PR and passes with the fix.

Suggested fix — passes all 1270 :dd-openfeature:test tests with the rest of the review patch

These hunks overlap with my comment on line 51: the first ExperimentationOptOut hunk also drops keySet(). The FlagEvaluator hunk also logs a one-time warning, which needs a private final AtomicBoolean warnedInvalidOptOut = new AtomicBoolean(); field and the java.util.concurrent.atomic.AtomicBoolean import; without them, delete the inner if block. The sentinel itself needs both ExperimentationOptOut hunks.

--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java
@@ -44,15 +43,32 @@ public final class ExperimentationOptOut {
     }
   }
 
-  /** Parses the cutoff once per evaluation; null means no exclusion. */
+  /**
+   * Returned by {@link #cutoff} for a present but invalid attribute. Compare by identity: {@code
+   * Instant.MIN} is earlier than any timestamp the formatter accepts, so a parsed cutoff is never
+   * this instance.
+   */
+  static final Instant INVALID_CUTOFF = Instant.MIN;
+
+  /**
+   * Parses the cutoff once per evaluation.
+   *
+   * @return {@code null} when no exclusion applies, {@link #INVALID_CUTOFF} when the attribute is
+   *     present but invalid, otherwise the cutoff.
+   */
   static Instant cutoff(final ExperimentationOptOutMode mode, final EvaluationContext context) {
-    if ((mode != ExperimentationOptOutMode.IMMEDIATE
-            && mode != ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY)
-        || !context.keySet().contains(ATTRIBUTE)) {
+    if (mode != ExperimentationOptOutMode.IMMEDIATE
+        && mode != ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY) {
       return null;
     }
+    // getValue is a lookup per context layer. keySet() on the LayeredEvaluationContext that the
+    // OpenFeature client passes copies every key of every layer on each evaluation. An OpenFeature
+    // null (new Value()) is non-null here and stays invalid below.
     final Value value = context.getValue(ATTRIBUTE);
-    if (value != null && value.isString()) {
+    if (value == null) {
+      return null;
+    }
+    if (value.isString()) {
       try {
         // Use the same timestamp parser as allocation startAt/endAt.
         return DateTimeFormatter.ISO_OFFSET_DATE_TIME.parse(value.asString(), Instant::from);
@@ -60,7 +76,7 @@ public final class ExperimentationOptOut {
         // Do not expose the supplied context value in the diagnostic.
       }
     }
-    throw new IllegalArgumentException(INVALID_TIMESTAMP);
+    return INVALID_CUTOFF;
   }
 
   /** Checks prepared activation states before selecting an allocation. */
--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java
@@ -103,10 +107,15 @@ public final class FlagEvaluator {
         return error(defaultValue, ErrorCode.INVALID_CONTEXT, null, observeFullEvaluationData);
       }
 
-      final Instant experimentationCutoff;
-      try {
-        experimentationCutoff = ExperimentationOptOut.cutoff(experimentationOptOutMode, context);
-      } catch (final IllegalArgumentException e) {
+      final Instant experimentationCutoff =
+          ExperimentationOptOut.cutoff(experimentationOptOutMode, context);
+      if (experimentationCutoff == ExperimentationOptOut.INVALID_CUTOFF) {
+        if (warnedInvalidOptOut.compareAndSet(false, true)) {
+          LOGGER.warn(
+              "{}. Evaluations with this context return the default with INVALID_CONTEXT."
+                  + " Later occurrences are not logged.",
+              ExperimentationOptOut.INVALID_TIMESTAMP);
+        }
         // This diagnostic is fixed SDK text. It contains no customer context values.
         return ProviderEvaluation.<T>builder()
             .value(defaultValue)

The production change alone fails exactly 19 existing rows, and all of them assert the old throw: 12 in invalidValuesAreRejectedOnlyWhenEnabled and 7 in timestampValidationMatchesAllocationTimestamps. Both change to assertSame(ExperimentationOptOut.INVALID_CUTOFF, …). The import hunks and the last hunk also carry the test from my comment on line 51, which needs ImmutableContext, HashMap and Map.

--- a/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOutTest.java
+++ b/dd-openfeature/src/test/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOutTest.java
@@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 import com.datadog.openfeature.ExperimentationOptOutMode;
@@ -11,12 +12,15 @@ import com.datadog.openfeature.Provider;
 import com.datadog.openfeature.internal.bridge.v1.AgentBridge;
 import com.datadog.openfeature.internal.config.Settings;
 import com.datadog.openfeature.internal.ufc.Allocation;
+import dev.openfeature.sdk.ImmutableContext;
 import dev.openfeature.sdk.MutableContext;
 import dev.openfeature.sdk.Value;
 import java.time.DateTimeException;
 import java.time.Instant;
 import java.time.format.DateTimeFormatter;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 import java.util.stream.Stream;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
@@ -120,10 +123,7 @@ class ExperimentationOptOutTest {
         new ExperimentationOptOutMode[] {
           ExperimentationOptOutMode.IMMEDIATE, ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY
         }) {
-      final IllegalArgumentException error =
-          assertThrows(
-              IllegalArgumentException.class, () -> ExperimentationOptOut.cutoff(mode, context));
-      assertEquals(ExperimentationOptOut.INVALID_TIMESTAMP, error.getMessage());
+      assertSame(ExperimentationOptOut.INVALID_CUTOFF, ExperimentationOptOut.cutoff(mode, context));
     }
     assertNull(ExperimentationOptOut.cutoff(ExperimentationOptOutMode.DISABLED, context));
   }
@@ -165,9 +165,18 @@ class ExperimentationOptOutTest {
     if (accepted) {
       assertNotNull(ExperimentationOptOut.cutoff(ExperimentationOptOutMode.IMMEDIATE, context));
     } else {
-      assertThrows(
-          IllegalArgumentException.class,
-          () -> ExperimentationOptOut.cutoff(ExperimentationOptOutMode.IMMEDIATE, context));
+      assertSame(
+          ExperimentationOptOut.INVALID_CUTOFF,
+          ExperimentationOptOut.cutoff(ExperimentationOptOutMode.IMMEDIATE, context));
+    }
+  }
+
+  @Test
+  void rawJavaNullEntryIsTreatedAsAbsent() {
+    final Map<String, Value> attributes = new HashMap<>();
+    attributes.put(ExperimentationOptOut.ATTRIBUTE, null);
+    for (final ExperimentationOptOutMode mode : ExperimentationOptOutMode.values()) {
+      assertNull(ExperimentationOptOut.cutoff(mode, new ImmutableContext("subject", attributes)));
     }
   }
 

The new test, for ExperimentationOptOutProviderTest (it needs import dev.openfeature.sdk.EvaluationContext;):

@Test
void unrelatedContextFailureIsNotReportedAsInvalidTimestamp() throws Exception {
  final DDEvaluator evaluator =
      new DDEvaluator(
          () -> {}, AgentBridge.NONE, TestSettings.of(), ExperimentationOptOutMode.IMMEDIATE);
  evaluator.accept(
      UniversalFlagConfigParser.parse(configuration(null, "true", false).getBytes(UTF_8)));
  final EvaluationContext broken =
      new MutableContext("subject") {
        @Override
        public Set<String> keySet() {
          throw new IllegalArgumentException("tenant not loaded");
        }

        @Override
        public dev.openfeature.sdk.Value getValue(final String key) {
          throw new IllegalArgumentException("tenant not loaded");
        }
      };
  final ProviderEvaluation<String> result =
      evaluator.evaluate(String.class, "target", "default", broken);
  // The failure is reported as GENERAL, not as an invalid opt-out timestamp.
  assertEquals(ErrorCode.GENERAL, result.getErrorCode());
}

}

/** Checks prepared activation states before selecting an allocation. */
static boolean hasInvalidActivation(final List<Allocation> allocations) {
if (allocations == null) {
return false;
}
for (final Allocation allocation : allocations) {
if (allocation != null && allocation.invalidRandomizationActivation) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working fully autonomously overnight:

Heads up: In future mode, a malformed randomizationActivatedAt on a doLog=false allocation fails the whole flag with PARSE_ERROR. The pinned contract table says that doLog=false allocations evaluate normally.

This loop ignores doLog. So for a subject with a valid cutoff, a bad activation on an ordinary allocation returns the default for that flag, although an ordinary allocation never uses its activation. The cases.json row future_experiments_only/valid/invalid/0 (doLog: false) expects PARSE_ERROR, so Java matches the draft shared corpus in DataDog/ffe-system-test-data#55. The draft is not settled, so I suggest no code change until it is.

The written texts do not state this rule clearly:

  • The ffe-system-test-data README at the pinned submodule commit, "Future experiments only" table, row doLog=false: "Evaluate normally." The table has no exception for malformed activation.
  • dd-openfeature/README.md, line 57: "Ordinary allocations remain available." Line 59: "Malformed activation metadata returns PARSE_ERROR for the affected flag only in future mode with a valid subject timestamp." A reader can take line 59 as an exception to line 57, but neither line says whether it covers doLog=false allocations.

Options:

  1. Keep the current behavior. Change the contract table and dd-openfeature/README.md line 57 to say that, in future mode with a valid subject cutoff, a malformed activation on any allocation fails the flag.

  2. Check only exposure-logging allocations, and flip that corpus row in the shared repository first:

    if (allocation != null
        && allocation.invalidRandomizationActivation
        && Boolean.TRUE.equals(allocation.doLog)) {

I would take option 2. It keeps ordinary delivery when the bad field cannot change the result, which matches the table. Because the corpus is meant to be a shared contract for all SDKs, decide it there first. The snippet is a sketch; I did not compile it.

return true;
}
}
return false;
}

/** Excludes before targeting; unknown activation does not qualify as older. */
static boolean excludes(
final ExperimentationOptOutMode mode, final Instant cutoff, final Allocation allocation) {
if (cutoff == null) {
return false;
}
if (!Boolean.TRUE.equals(allocation.doLog)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Comment from Claude working with Aaron Silverman:

Heads up: SDKs disagree on what an omitted doLog means. I suggest one shared rule for every SDK, client and server: omitted (or null) means false, for both exposure logging and opt-out.

Java already does this, here and in exposure logging (Boolean.TRUE.equals). libdatadog and Go treat an omitted doLog as true for both. The Node core and the open .NET opt-out PR log no exposure for it, but exclude it as an experiment. Draft decision D010 in DataDog/ffe-system-test-data#55 calls omission invalid input but sets no default, so the split stays open.

If exposure-logging allocations are the minority, a false default also lets the configuration omit doLog on ordinary allocations and save bytes.

What each SDK does today — explicit true/false agree everywhere
SDK Omitted: exposure? Omitted: opt-out experiment? Null
libdatadog (Ruby, Python, PHP) yes (default_do_log → true) yes that flag fails to parse
Go yes (doLog := true unless set) yes same as omitted
Java, this PR no no same as omitted
.NET, DataDog/dd-trace-dotnet#9470 no yes (DoLog != false) same as omitted
Node core, openfeature-js-client no (!!allocation.doLog) yes (doLog !== false) same as omitted
browser, iOS, Android server-computed; a missing doLog is rejected as invalid server-side rejected

No allocation in the ffe-system-test-data fixtures omits doLog or sets it to null today, so a shared default changes no fixture result. It would change libdatadog and Go behavior for any configuration that omits doLog, and libdatadog's for a null doLog (from a parse failure to ordinary evaluation), so the configuration producers need to confirm they always send it. The .NET and Node rows describe an open PR and current main.

Suggested contract rule
  • An omitted or null doLog means false: no exposure, and not an experiment allocation for opt-out.
  • Add policy rows with omitted and null doLog to the shared corpus, for both modes.
  • libdatadog changes default_do_log to false and accepts null. Go changes its nil default. .NET and the Node core change their opt-out checks to == true.
  • Java needs no change.

return false;
}
return mode == ExperimentationOptOutMode.IMMEDIATE
|| allocation.randomizationActivatedAt == null
|| !allocation.randomizationActivatedAt.isBefore(cutoff);
}
}
Loading
Loading