Repository navigation
Support per-subject experimentation opt-out #12950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
7732d04
71c521a
c3846c9
beebb7b
bcbaa28
736ffef
00ad1fd
85fade1
f483c71
660b5b9
a4d3dfb
578ffa7
a7d8e4e
55ba0bd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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`. | ||
| 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. | ||
| 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 |
|---|---|---|
| @@ -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, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Comment from Claude working with Aaron Silverman: [P2] Consider removing the public
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;
}
|
||
| /** 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 |
|---|---|---|
|
|
@@ -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"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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.
|
||
| return mode == null ? "disabled" : mode; | ||
| } | ||
|
|
||
| /** | ||
| * @return the Datadog API key, or {@code null} if not set. | ||
| */ | ||
|
|
||
| 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 "": | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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"); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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 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
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;
}
}
// 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 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** 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)) { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🤖 Comment from Claude working with Aaron Silverman: [P1] Replace
1. The 8 corpus rows where Java disagrees — JDK 21, this PR's
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 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:testpasses all 1325 tests. - Swapping the parser back to
ISO_OFFSET_DATE_TIMEturns 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.
There was a problem hiding this comment.
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
doLogmeansfalse: no exposure, and not an experiment allocation for opt-out. - Add policy rows with omitted and null
doLogto the shared corpus, for both modes. - libdatadog changes
default_do_logtofalseand accepts null. Go changes its nil default. .NET and the Node core change their opt-out checks to== true. - Java needs no change.
There was a problem hiding this comment.
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.startAtandendAt" describeISO_OFFSET_DATE_TIME, which is not the shared grammar (see my parser comment). State the grammar instead of naming the parser.INVALID_CONTEXT" is true. Once configuration has loaded, it applies to every flag evaluated with that context, including flags without experiments. Say so explicitly.IllegalArgumentException" changes if the warn-and-disable suggestion is adopted.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.randomizationActivatedAtis absent, future mode skips everydoLog=trueallocation for an opted-out subject, the same as immediate mode.Suggested text — assumes the parser and warn-and-disable suggestions are adopted