Repository navigation
Support per-subject experimentation opt-out - #12950
leoromanovsky wants to merge 14 commits into
Conversation
leoromanovsky
added
tag: ai generated
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
…imentation-opt-out-immediate # Conflicts: # metadata/supported-configurations.json # products/feature-flagging/feature-flagging-api/src/main/java/datadog/trace/api/openfeature/Provider.java
…imentation-opt-out-immediate
…imentation-opt-out-immediate
leoromanovsky
changed the title
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
More details
Both providers preserve ordinary fallback delivery when experiment allocations are excluded, and malformed activation metadata is isolated to the affected flag. Validation was limited to static review.
🤖 Bits Code Review · Commit 660b5b9
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 660b5b9611
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| final Instant experimentationCutoff; | ||
| try { | ||
| experimentationCutoff = ExperimentationOptOut.cutoff(experimentationOptOutMode, context); |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P1] Should an invalid dd_experimentation_opt_out_at skip only the doLog=true allocations, instead of returning the default for every flag?
With an enabled mode, one malformed or non-string value returns INVALID_CONTEXT here, before the flag lookup. Every flag for that subject then returns its code default. That includes kill switches, plain rollouts, and flags with no experiment allocation. This follows draft decisions D002 and D016 in DataDog/ffe-system-test-data#55, and libdatadog does the same. But it removes ordinary feature delivery, which opt-out is meant to keep.
Both designs keep experiment allocations closed. They differ for ordinary allocations and for flags that do not exist.
Inputs that reach this path — each one returns the default for every flag
Executed against this PR's FlagEvaluator, with an enabled mode:
| Context value | Result on a flag with no doLog=true allocation |
|---|---|
java.time.Instant, through MutableContext.add(key, Instant) |
INVALID_CONTEXT, default |
"2026-10-19" (date only) |
INVALID_CONTEXT, default |
a JSON-null, structure, or list Value |
INVALID_CONTEXT, default |
| any of the above, on a flag that does not exist | INVALID_CONTEXT, not FLAG_NOT_FOUND |
Instant is a first-class OpenFeature Java value, so a Java integration can pass it without noticing the problem.
Options — I prefer 1
- Change the contract. An invalid cutoff skips every
doLog=trueallocation and evaluates the rest as usual. Experiments still fail closed, and a bad integration keeps ordinary delivery. libdatadog and the other SDKs would need the same change. - Keep D016, and make the scope explicit. The README already says invalid values return
INVALID_CONTEXT. Add: "An invalid value makes every flag evaluation for that context return the application default withINVALID_CONTEXT. This includes flags without experiments and flags that do not exist. Pass the timestamp as an RFC 3339String.java.time.Instantvalues are rejected."
A sketch of option 1. I did not compile it, because it depends on the contract decision:
// ExperimentationOptOut: cutoff() returns this sentinel where it throws today.
static final Instant INVALID_CUTOFF = Instant.MIN;
// ExperimentationOptOut.excludes: an invalid cutoff excludes every exposure-logging allocation.
if (cutoff == INVALID_CUTOFF) {
return Boolean.TRUE.equals(allocation.doLog);
}The future-mode activation check in FlagEvaluator would also need to skip an invalid cutoff, so a malformed randomizationActivatedAt does not fail the flag for that subject.
| 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.
🤖 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: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.
| 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.
🤖 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.
|
|
||
| if (experimentationCutoff != null | ||
| && experimentationOptOutMode == ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY | ||
| && ExperimentationOptOut.hasInvalidActivation(flag.allocations)) { |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] Move this activation check below the !flag.enabled return, so a disabled flag reports DISABLED, as it does in libdatadog.
Today a disabled flag with a malformed randomizationActivatedAt returns ERROR/PARSE_ERROR for an opted-out subject in future mode. libdatadog rejects a disabled flag at compile time (FlagDisabled), before it checks activation, so Ruby reports DISABLED. The value is the default either way. The reason and error code differ between the two SDKs, and so do the evaluation metrics and flag-evaluation events where those are enabled. This only affects opted-out subjects.
No test pins this order: moving the block below the disabled return leaves all 1264 module tests green.
Suggested fix and test — the test is red on this PR and green with the fix
--- 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
@@ -129,6 +129,14 @@ public final class FlagEvaluator {
return error(defaultValue, ErrorCode.FLAG_NOT_FOUND, null, observeFullEvaluationData);
}
+ if (!flag.enabled) {
+ return ProviderEvaluation.<T>builder()
+ .value(defaultValue)
+ .reason(Reason.DISABLED.name())
+ .flagMetadata(consentMetadata(observeFullEvaluationData))
+ .build();
+ }
+
if (experimentationCutoff != null
&& experimentationOptOutMode == ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY
&& ExperimentationOptOut.hasInvalidActivation(flag.allocations)) {
@@ -139,14 +147,6 @@ public final class FlagEvaluator {
observeFullEvaluationData);
}
- if (!flag.enabled) {
- return ProviderEvaluation.<T>builder()
- .value(defaultValue)
- .reason(Reason.DISABLED.name())
- .flagMetadata(consentMetadata(observeFullEvaluationData))
- .build();
- }
-
if (flag.allocations == null) {
return error(
defaultValue, // A disabled flag reports DISABLED before activation timestamps are checked, as in libdatadog.
@Test
void disabledFlagReportsDisabledBeforeInvalidActivation() throws Exception {
final ProviderEvaluation<String> result =
evaluate(
configuration(false, "\"bad\""),
ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY,
new MutableContext("subject").add(ATTRIBUTE, CUTOFF));
assertNull(result.getErrorCode());
assertEquals(Reason.DISABLED.name(), result.getReason());
assertEquals("default", result.getValue());
}This test is also part of the test class in my comment on the exclusion loop, which supplies the configuration and evaluate helpers. Take it from either place.
| for (final Allocation allocation : flag.allocations) { | ||
| if (experimentationCutoff != null | ||
| && ExperimentationOptOut.excludes( | ||
| experimentationOptOutMode, experimentationCutoff, allocation)) { |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P2] Add a test that an excluded allocation's rules are never evaluated.
The order here is correct: exclusion comes before isAllocationActive, rules, and shards. But no test pins the order relative to rules. Moving the exclusion below evaluateRules leaves all 1264 module tests green. If that order regressed, an excluded allocation with a numeric rule on a non-numeric attribute would turn an opted-out subject's ordinary result into TYPE_MISMATCH.
Suggested test class — excludedAllocationRulesAreNotEvaluated is red on that reordering and green on this PR
The whole file, including the tests from my other comments. It compiles in dd-openfeature/src/test/java/com/datadog/openfeature/.
package com.datadog.openfeature;
import static java.nio.charset.StandardCharsets.UTF_8;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import com.datadog.openfeature.internal.bridge.v1.AgentBridge;
import com.datadog.openfeature.internal.config.TestSettings;
import com.datadog.openfeature.internal.ufc.UniversalFlagConfigParser;
import dev.openfeature.sdk.ErrorCode;
import dev.openfeature.sdk.MutableContext;
import dev.openfeature.sdk.MutableStructure;
import dev.openfeature.sdk.ProviderEvaluation;
import dev.openfeature.sdk.Reason;
import dev.openfeature.sdk.Value;
import java.time.Instant;
import java.util.Arrays;
import java.util.Collections;
import java.util.stream.Stream;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.EnumSource;
import org.junit.jupiter.params.provider.MethodSource;
/** Pins opt-out behavior the policy corpus does not reach. */
class ExperimentationOptOutGapTest {
private static final String ATTRIBUTE = "dd_experimentation_opt_out_at";
private static final String CUTOFF = "2026-10-19T12:00:00Z";
/** The experiment allocation is sharded and has a numeric rule; the ordinary one is neither. */
private static String configuration(final boolean enabled, final String activation) {
final String experiment =
"{\"key\":\"experiment\",\"doLog\":true"
+ (activation == null ? "" : ",\"randomizationActivatedAt\":" + activation)
+ ",\"rules\":[{\"conditions\":[{\"attribute\":\"age\",\"operator\":\"GT\",\"value\":18}]}]"
+ ",\"splits\":[{\"variationKey\":\"experiment\",\"shards\":[{\"salt\":\"s\",\"totalShards\":10000,"
+ "\"ranges\":[{\"start\":0,\"end\":10000}]}]}]}";
final String ordinary =
"{\"key\":\"ordinary\",\"doLog\":false,\"splits\":[{\"variationKey\":\"ordinary\",\"shards\":[]}]}";
return "{\"flags\":{\"target\":{\"key\":\"target\",\"enabled\":"
+ enabled
+ ",\"variationType\":\"STRING\",\"variations\":{"
+ "\"experiment\":{\"key\":\"experiment\",\"value\":\"experiment\"},"
+ "\"ordinary\":{\"key\":\"ordinary\",\"value\":\"ordinary\"}},"
+ "\"allocations\":["
+ experiment
+ ","
+ ordinary
+ "]}}}";
}
private static ProviderEvaluation<String> evaluate(
final String json, final ExperimentationOptOutMode mode, final MutableContext context)
throws Exception {
final DDEvaluator evaluator =
new DDEvaluator(() -> {}, AgentBridge.NONE, TestSettings.of(), mode);
evaluator.accept(UniversalFlagConfigParser.parse(json.getBytes(UTF_8)));
return evaluator.evaluate(String.class, "target", "default", context);
}
// Exclusion happens before randomization: a skipped allocation needs no targeting key.
@ParameterizedTest
@EnumSource(
value = ExperimentationOptOutMode.class,
names = {"IMMEDIATE", "FUTURE_EXPERIMENTS_ONLY"})
void excludedAllocationSkipsShardingWithoutTargetingKey(final ExperimentationOptOutMode mode)
throws Exception {
final MutableContext context = new MutableContext().add(ATTRIBUTE, CUTOFF).add("age", 30);
final ProviderEvaluation<String> result = evaluate(configuration(true, null), mode, context);
assertNull(result.getErrorCode());
assertEquals("ordinary", result.getValue());
// Control: without opt-out the experiment's shards need the targeting key.
final ProviderEvaluation<String> control =
evaluate(configuration(true, null), ExperimentationOptOutMode.DISABLED, context);
assertEquals(ErrorCode.TARGETING_KEY_MISSING, control.getErrorCode());
}
// Exclusion happens before targeting: a skipped allocation's rules are never evaluated.
@ParameterizedTest
@EnumSource(
value = ExperimentationOptOutMode.class,
names = {"IMMEDIATE", "FUTURE_EXPERIMENTS_ONLY"})
void excludedAllocationRulesAreNotEvaluated(final ExperimentationOptOutMode mode)
throws Exception {
final MutableContext context =
new MutableContext("subject").add(ATTRIBUTE, CUTOFF).add("age", "not-a-number");
final ProviderEvaluation<String> result = evaluate(configuration(true, null), mode, context);
assertNull(result.getErrorCode());
assertEquals("ordinary", result.getValue());
final ProviderEvaluation<String> control =
evaluate(configuration(true, null), ExperimentationOptOutMode.DISABLED, context);
assertEquals(ErrorCode.TYPE_MISMATCH, control.getErrorCode());
}
// A disabled flag reports DISABLED before activation timestamps are checked, as in libdatadog.
@Test
void disabledFlagReportsDisabledBeforeInvalidActivation() throws Exception {
final ProviderEvaluation<String> result =
evaluate(
configuration(false, "\"bad\""),
ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY,
new MutableContext("subject").add(ATTRIBUTE, CUTOFF));
assertNull(result.getErrorCode());
assertEquals(Reason.DISABLED.name(), result.getReason());
assertEquals("default", result.getValue());
}
static Stream<Arguments> nonStringAttributes() {
return Stream.of(
Arguments.of("Instant", new Value(Instant.parse(CUTOFF))),
Arguments.of("double", new Value(1.5)),
Arguments.of("structure", new Value(new MutableStructure().add("at", CUTOFF))),
Arguments.of("list", new Value(Collections.singletonList(new Value(CUTOFF)))),
Arguments.of("empty list", new Value(Arrays.asList())),
Arguments.of("null value", new Value()));
}
// Any present non-string value is INVALID_CONTEXT, including a java.time.Instant.
@ParameterizedTest(name = "{0}")
@MethodSource("nonStringAttributes")
void nonStringAttributeIsInvalidContextOnlyWhenEnabled(final String name, final Value value)
throws Exception {
for (final ExperimentationOptOutMode mode : ExperimentationOptOutMode.values()) {
final MutableContext context =
new MutableContext("subject", Collections.singletonMap(ATTRIBUTE, value));
final ProviderEvaluation<String> result =
evaluate(configuration(true, "\"2026-01-01T00:00:00Z\""), mode, context);
if (mode == ExperimentationOptOutMode.IMMEDIATE
|| mode == ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY) {
assertEquals(ErrorCode.INVALID_CONTEXT, result.getErrorCode(), name + " " + mode);
assertEquals("default", result.getValue(), name + " " + mode);
} else {
// The age rule does not match, so the subject falls through to ordinary.
assertNull(result.getErrorCode(), name + " " + mode);
assertEquals("ordinary", result.getValue(), name + " " + mode);
}
}
}
// 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]);
}
}
}
}
}excludedAllocationSkipsShardingWithoutTargetingKey is a direct check only: moving the targeting-key check before the exclusion already turns the shared special-characters fixture cases red. disabledFlagReportsDisabledBeforeInvalidActivation and disabledFeatureFlagsDoNotValidateMode are red on this PR until the fixes in my other comments are applied.
| /** 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.
🤖 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.
|
|
||
| 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`. |
There was a problem hiding this comment.
🤖 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.
- "a valid RFC3339 timestamp" and "Timestamp validation uses the same parser as allocation
startAtandendAt" describeISO_OFFSET_DATE_TIME, which is not the shared grammar (see my parser comment). State the grammar instead of naming the parser. - "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. - "Unsupported values throw
IllegalArgumentException" changes if the warn-and-disable suggestion is adopted. - 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
randomizationActivatedAtis absent, future mode skips everydoLog=trueallocation for an opted-out subject, the same as immediate mode.
- If a completed winner rollout still carries
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.|
|
||
| /** 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.
🤖 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_ENABLEDand others). Addstatic final String EXPERIMENTATION_OPT_OUT_MODE = "feature.flags.experimentation.opt.out.mode";and reuse it inSettingsTest. requireNonNull.Provider.Optionswritesjava.util.Objects.requireNonNullinline. The module imports it statically elsewhere (ExposureHook).- Activation validity.
AllocationcarriesinvalidRandomizationActivationper allocation and has three constructors, andhasInvalidActivationwalks 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 onFlag. 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 whenrandomizationActivatedAtwas present but unparseable. Future mode with a valid subject cutoff then fails the flag withPARSE_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 withPARSE_ERRORwhen 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.
| @@ -0,0 +1,5 @@ | |||
| # Experimentation opt-out test cases | |||
|
|
|||
| `cases.json` covers 90 combinations of opt-out mode, subject timestamp, allocation activation, | |||
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
[P3] Read these 90 cases from the shared corpus instead of a separate copy, or at least record where they come from.
cases.json matches the 90 "kind": "policy" cases of contracts/experimentation-opt-out/corpus/browser-proposal.jsonl in ffe-system-test-data on args and expected. Only the id prefix, decision_ids, and kind were dropped. Ruby carries a copy of the same lines, and libdatadog carries another. No check compares this copy with its source. The test checks the count, unique ids, and each case's result, so an upstream change would not be noticed. It also lives under dd-smoke-tests, but only the dd-openfeature unit test reads it.
Suggested approach
- Once Add a Lean proof prototype and shared SDK decision history ffe-system-test-data#55 merges, bump the existing
ffe-system-test-datasubmodule. Readbrowser-proposal.jsonlfrom it, keep the"kind": "policy"lines, and deletecases.json. The contract corpus also carries the timestamp rows from my parser comment. - Until then, add to this README the source path, the ffe-system-test-data commit, and the extraction rule. Ruby's fixture README already records a source SHA-256.
| if (cutoff == null) { | ||
| return false; | ||
| } | ||
| if (!Boolean.TRUE.equals(allocation.doLog)) { |
There was a problem hiding this comment.
🤖 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.
| "dd.feature.flags.configuration.source", "agentless", | ||
| "dd.feature.flags.configuration.source.agentless.base.url", | ||
| server.uri("/").toString(), | ||
| "dd.feature.flags.experimentation.opt.out.mode", "invalid-overridden-mode", |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P2] Add rows that leave the option at DEFAULT and set the mode only through configuration.
The README makes configuration the main way to turn this on (DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODE, or -Ddd.feature.flags.experimentation.opt.out.mode). No test evaluates a flag through a Provider in that setup. Every row of this table sets an explicit mode, so the property on this line is always overridden. providerConfigurationPrecedence checks ExperimentationOptOut.resolve(...) directly, and then only constructs and shuts down a Provider. So if Provider resolved the mode correctly but gave its evaluator DISABLED for DEFAULT, all tests would still pass.
The evaluation-telemetry opt-out has this coverage: systemPropertyEnablesPerSubjectOptOut, systemPropertyDefaultsToDisabled, and systemPropertyIsCapturedAtProviderConstruction evaluate through a provider.
Why the other tests do not cover it
publicProvidersApplyOptOutAfterHttpConfiguration(this test) uses explicit modes only.CanonicalFixtureTest,EvaluationTelemetryOptOutTest, and the JMH benchmark inject aDDEvaluatorthat they build with an explicit mode. With an injected evaluator,Provider.buildEvaluator()returns that evaluator, so the provider's resolved mode is not used.ExperimentationOptOutTest.providerConfigurationPrecedenceassertsresolve(...)and never evaluates a flag.SettingsTestchecks only the setting string.- The smoke tests do not set the mode, so they run in disabled mode only.
I found this by reading the tests. I did not run a mutation.
Suggested change (sketch, not compiled)
Add a configured column, use it for the system property, and add DEFAULT rows:
@TableTest({
"Scenario | deprecatedAlias | mode | configured | expectedValue",
"SDK disabled | false | DISABLED | invalid-overridden-mode | experiment ",
"SDK immediate | false | IMMEDIATE | invalid-overridden-mode | ordinary ",
"SDK future | false | FUTURE_EXPERIMENTS_ONLY | invalid-overridden-mode | ordinary ",
"alias disabled | true | DISABLED | invalid-overridden-mode | experiment ",
"alias immediate | true | IMMEDIATE | invalid-overridden-mode | ordinary ",
"alias future | true | FUTURE_EXPERIMENTS_ONLY | invalid-overridden-mode | ordinary ",
"SDK config disabled | false | DEFAULT | disabled | experiment ",
"SDK config immediate | false | DEFAULT | immediate | ordinary ",
"SDK config future | false | DEFAULT | future_experiments_only | ordinary ",
"alias config immediate | true | DEFAULT | immediate | ordinary ",
"alias config future | true | DEFAULT | future_experiments_only | ordinary "
})
@SuppressWarnings("deprecation")
void publicProvidersApplyOptOutAfterHttpConfiguration(
final boolean deprecatedAlias,
final ExperimentationOptOutMode mode,
final String configured,
final String expectedValue)
throws Exception {On this line:
"dd.feature.flags.experimentation.opt.out.mode", configured,The equal activation makes both enabled modes skip the experiment allocation and return ordinary. Disabled mode returns experiment. Run spotlessApply after the edit, because spotless also formats @TableTest tables.
Optional: to pin that the provider reads configuration only at construction, add System.setProperty("dd.feature.flags.experimentation.opt.out.mode", "disabled"); next to the existing options.experimentationOptOutMode(ExperimentationOptOutMode.DEFAULT); line. The finally block already restores the property.
| } | ||
|
|
||
| @Test | ||
| void unreachableInvalidActivationFailsOnlyOptedOutFutureMode() throws Exception { |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P3] Add the configuration-refresh and exposure-deduplication tests that the shared fixture contract asks each SDK to keep.
The ffe-system-test-data README at the pinned submodule commit says: "Malformed activation parsing, configuration refresh, exposure deduplication, and telemetry independence also need SDK-local tests." This PR covers malformed activation (this test and RandomizationActivationTest) and telemetry independence (experimentationAndEvaluationTelemetryControlsRemainIndependent). No experimentation opt-out test calls accept(...) twice on one evaluator, or evaluates the same flag twice on one provider. The evaluation-telemetry opt-out already has tests of this kind (telemetryAttributeRemainsAvailableForTargetingAndExposure and eachEvaluationUsesItsOwnDecisionWithoutChangingExistingEvents), but they do not set an experimentation mode.
By reading, the behavior is correct. Activation state belongs to each parsed configuration. ExposureLoggingHook reads the deduplication cache only when __dd_do_log is true, so a skipped experiment allocation does not touch it. These tests would pin that behavior.
Refresh test for ExperimentationOptOutProviderTest (sketch, not compiled)
It uses only helpers and imports that this file already has. Each step loads a new configuration into the same evaluator.
@Test
void futureModeFollowsEachConfigurationSnapshot() throws Exception {
final DDEvaluator evaluator =
new DDEvaluator(
() -> {},
AgentBridge.NONE,
TestSettings.of(),
ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY);
final MutableContext context = new MutableContext("subject").add(ATTRIBUTE, CUTOFF);
final String[][] steps = {
{"before", "experiment"}, {"equal", "ordinary"}, {"invalid", null}, {"before", "experiment"}
};
for (final String[] step : steps) {
evaluator.accept(
UniversalFlagConfigParser.parse(
configuration(activation(step[0]), "true", false).getBytes(UTF_8)));
final ProviderEvaluation<String> result =
evaluator.evaluate(String.class, "target", "default", context);
if (step[1] == null) {
assertEquals(ErrorCode.PARSE_ERROR, result.getErrorCode(), step[0]);
assertEquals("default", result.getValue(), step[0]);
} else {
assertNull(result.getErrorCode(), step[0]);
assertEquals(step[1], result.getValue(), step[0]);
}
}
}Deduplication test for EvaluationTelemetryOptOutTest (sketch, not compiled)
The subject alternates between opted out and participating. Only the first participating evaluation logs an exposure. The allocation setup copies experimentationAndEvaluationTelemetryControlsRemainIndependent, with the activation equal to the cutoff. All identifiers are already imported in this file.
@ParameterizedTest
@ValueSource(strings = {"IMMEDIATE", "FUTURE_EXPERIMENTS_ONLY"})
void optOutFlappingDoesNotRepeatTheExperimentExposure(final String modeName) throws Exception {
final ExperimentationOptOutMode mode = ExperimentationOptOutMode.valueOf(modeName);
final String cutoff = "2026-10-19T12:00:00Z";
final Flag flag = defaultConfiguration().flags.get("flag");
final Allocation original = flag.allocations.get(0);
// Activation equal to the cutoff: both enabled modes skip it for an opted-out subject.
final Allocation experiment =
new Allocation(
original.key,
original.rules,
original.startAt,
original.endAt,
original.splits,
original.doLog,
java.time.Instant.parse(cutoff));
final Allocation ordinary =
new Allocation(
"ordinary",
null,
null,
null,
singletonList(new Split(emptyList(), "off", emptyMap(), 8)),
false);
final Map<String, Variant> variants = new HashMap<>(flag.variations);
variants.put("off", new Variant("off", 2));
this.evaluator =
new CapturingEvaluator(
new ServerConfiguration(
"",
"",
false,
null,
singletonMap(
"flag",
new Flag("flag", true, ValueType.INTEGER, variants, asList(experiment, ordinary)))),
mode);
this.api.setProviderAndWait(
provider(new Provider.Options().experimentationOptOutMode(mode), this.evaluator));
this.client = this.api.getClient();
final int[] expected = {2, 1, 2, 1}; // opted out, participating, opted out, participating
for (int i = 0; i < expected.length; i++) {
final MutableContext context = new MutableContext("subject-1");
if (i % 2 == 0) {
context.add("dd_experimentation_opt_out_at", cutoff);
}
final FlagEvaluationDetails<Integer> details =
this.client.getIntegerDetails("flag", 0, context);
assertNull(details.getErrorCode());
assertEquals(expected[i], details.getValue());
}
assertEquals(1, this.exposures.size());
}
| this.bridge = AgentBridges.detect(); | ||
| this.settings = Settings.load(this.bridge); | ||
| this.experimentationOptOutMode = | ||
| ExperimentationOptOut.resolve(options.experimentationOptOutMode, this.settings); |
There was a problem hiding this comment.
🤖 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.
|
🤖 Comment from Claude working fully autonomously overnight: Reminder: Before release, link the Java system-tests work from the PR body, and update the draft parametric controller to the mode API.
Suggested PR-body addition: a short "Dependencies" list with the two PRs above, and one run receipt for #8135 against this branch. The receipt should have the dd-trace-java SHA, the system-tests SHA, the scenarios, and the pass counts. The missing receipt is a gap in the PR record. It does not mean that no one ran the tests. |
| return false; | ||
| } | ||
| for (final Allocation allocation : allocations) { | ||
| if (allocation != null && allocation.invalidRandomizationActivation) { |
There was a problem hiding this comment.
🤖 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 returnsPARSE_ERRORfor 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 coversdoLog=falseallocations.
Options:
-
Keep the current behavior. Change the contract table and
dd-openfeature/README.mdline 57 to say that, in future mode with a valid subject cutoff, a malformed activation on any allocation fails the flag. -
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.
|
|
||
| @SuppressWarnings("unchecked") | ||
| FixtureCase(final String fileName, final int index, final Map<String, Object> json) { | ||
| this.json = json; |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
Heads up: The submodule bump in this PR adds fixture cases with serialId and doLog expectations, but CanonicalFixtureTest does not check them. The gap is older than this PR, so a follow-up is fine.
The bump from 0c46747 to 8c4d241 adds test-case-serial-id-metadata.json, test-case-split-features.json, and test-case-span-enrichment-inputs.json. The fixture README says: "Consumers exposing selected metadata must also check the supplied serialId and doLog expectations against the actual selection." Java exposes both, as the __dd_split_serial_id and __dd_do_log flag metadata keys. FixtureCase reads only the value, reason, error code, variant, and allocation key.
How to add the check (sketch, not compiled)
-
Build the evaluator in
evaluateCanonicalFixturewith span enrichment on. Without it,FlagEvaluatoradds the two keys only fordoLog=trueselections:new DDEvaluator( mock(Runnable.class), AgentBridge.NONE, TestSettings.of("experimental.flagging.provider.span.enrichment.enabled", "true"), mode);
-
In
FixtureCase, keep an absentserialIdapart from"serialId": null. Null means the split has no serial id, so the key must be absent:this.checksSerialId = result.containsKey("serialId"); this.resultSerialId = (Number) result.get("serialId"); this.resultDoLog = (Boolean) result.get("doLog");
Declare the three fields (
final boolean checksSerialId; final Number resultSerialId; final Boolean resultDoLog;). -
After the existing assertions:
final Map<String, Object> metadata = details.getFlagMetadata() == null ? emptyMap() : details.getFlagMetadata().asUnmodifiableMap(); if (testCase.checksSerialId) { assertEquals( testCase.resultSerialId == null ? null : testCase.resultSerialId.intValue(), metadata.get("__dd_split_serial_id")); } if (testCase.resultDoLog != null) { assertEquals(testCase.resultDoLog, metadata.get("__dd_do_log")); }
| final String targetingKey = context.getTargetingKey(); | ||
|
|
||
| for (final Allocation allocation : flag.allocations) { | ||
| if (experimentationCutoff != null |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P2] Validate MATCHES and NOT_MATCHES patterns when the configuration is parsed, so an opted-out subject also gets PARSE_ERROR for a flag with an invalid regex.
In immediate and future modes, this continue skips a doLog=true allocation before evaluateRules runs. The pattern is compiled only inside matchesRegex. So when the invalid regex is in an excluded allocation, the opted-out subject gets a later allocation or the default, with no error. Draft decision D019 in DataDog/ffe-system-test-data#55 says: "Do not silently treat the invalid condition as false and select a later allocation. This configuration-validity rule applies independently of experimentation opt-out." The shared fixture's invalid-regex-flag says: "SDKs must remove this flag rather than falling through to a later allocation". The shared case only uses a subject without the opt-out attribute, so the corpus does not catch this.
Evidence — executed against this PR's evaluator
The flag has a doLog=true allocation with an invalid MATCHES rule (*@datadoghq.com) on email, then a doLog=false allocation that serves ordinary. The subject has an email, so evaluation reaches the condition.
| Mode | Opted out | Result on this PR | Result with the fix below |
|---|---|---|---|
| default, disabled | no or yes | PARSE_ERROR |
PARSE_ERROR |
| immediate, future_experiments_only | no | PARSE_ERROR |
PARSE_ERROR |
| immediate, future_experiments_only | yes | ordinary, no error |
PARSE_ERROR |
excludedAllocationWithInvalidRegexStillFailsTheFlag (below) fails on this PR in both enabled modes and passes with the fix.
Suggested fix — compile the pattern in the parser; the whole review patch passes all 1270 :dd-openfeature:test tests
The parser already drops a flag with an invalid operand into invalidFlags (one flag only; the rest of the configuration stays usable). This adds regexes to that check and keeps the compiled Pattern, as semverComparand is kept today.
--- 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
@@ -15,6 +15,8 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
import javax.annotation.Nullable;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -562,6 +564,18 @@ public final class UniversalFlagConfigParser {
"flag \"" + flagKey + "\" has a non-boolean condition");
}
break;
+ case MATCHES:
+ case NOT_MATCHES:
+ // Validate at parse time so an invalid regex fails the flag for every subject,
+ // including subjects whose allocations are skipped before rule evaluation.
+ try {
+ condition.regex =
+ Pattern.compile(
+ ConditionConfiguration.normalizeRegex(String.valueOf(condition.value)));
+ } catch (final PatternSyntaxException e) {
+ throw new IllegalArgumentException("flag \"" + flagKey + "\" has an invalid regex");
+ }
+ break;
default:
break;
}--- a/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ConditionConfiguration.java
+++ b/dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/ConditionConfiguration.java
@@ -1,5 +1,7 @@
package com.datadog.openfeature.internal.ufc;
+import java.util.regex.Pattern;
+
public final class ConditionConfiguration {
public final ConditionOperator operator;
public final String attribute;
@@ -9,10 +11,24 @@ public final class ConditionConfiguration {
// (not from JSON) when the operator is a SEMVER_* operator.
public transient ParsedSemver semverComparand;
+ // The compiled MATCHES/NOT_MATCHES pattern. Set during configuration preprocessing.
+ public transient Pattern regex;
+
public ConditionConfiguration(
final ConditionOperator operator, final String attribute, final Object value) {
this.operator = operator;
this.attribute = attribute;
this.value = value;
}
+
+ /** Translates POSIX character classes to their Java equivalents. */
+ public static String normalizeRegex(final String regex) {
+ return regex
+ .replace("[:alnum:]", "\\p{Alnum}")
+ .replace("[:alpha:]", "\\p{Alpha}")
+ .replace("[:digit:]", "\\p{Digit}")
+ .replace("[:lower:]", "\\p{Lower}")
+ .replace("[:upper:]", "\\p{Upper}")
+ .replace("[:space:]", "\\p{Space}");
+ }
}--- 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
@@ -314,9 +330,9 @@ public final class FlagEvaluator {
switch (condition.operator) {
case MATCHES:
- return matchesRegex(attributeValue, condition.value);
+ return matchesRegex(attributeValue, condition);
case NOT_MATCHES:
- return !matchesRegex(attributeValue, condition.value);
+ return !matchesRegex(attributeValue, condition);
case ONE_OF:
return isOneOf(attributeValue, condition.value);
case NOT_ONE_OF:
@@ -346,23 +362,18 @@ public final class FlagEvaluator {
}
}
- private static boolean matchesRegex(final Object attributeValue, final Object conditionValue) {
- // PatternSyntaxException is intentionally not caught here so it propagates to evaluate(),
- // which maps it to ErrorCode.PARSE_ERROR.
- final Pattern pattern = Pattern.compile(normalizeRegex(String.valueOf(conditionValue)));
+ private static boolean matchesRegex(
+ final Object attributeValue, final ConditionConfiguration condition) {
+ // The parser compiles and validates the pattern. Hand-built configurations compile here;
+ // PatternSyntaxException then propagates to evaluate(), which maps it to PARSE_ERROR.
+ final Pattern pattern =
+ condition.regex != null
+ ? condition.regex
+ : Pattern.compile(
+ ConditionConfiguration.normalizeRegex(String.valueOf(condition.value)));
return pattern.matcher(String.valueOf(attributeValue)).find();
}
- private static String normalizeRegex(final String regex) {
- return regex
- .replace("[:alnum:]", "\\p{Alnum}")
- .replace("[:alpha:]", "\\p{Alpha}")
- .replace("[:digit:]", "\\p{Digit}")
- .replace("[:lower:]", "\\p{Lower}")
- .replace("[:upper:]", "\\p{Upper}")
- .replace("[:space:]", "\\p{Space}");
- }
-
private static boolean isOneOf(final Object attributeValue, final Object conditionValue) {
if (!(conditionValue instanceof Iterable)) {
return false;The test, for ExperimentationOptOutProviderTest:
@ParameterizedTest
@ValueSource(strings = {"immediate", "future_experiments_only"})
void excludedAllocationWithInvalidRegexStillFailsTheFlag(final String mode) throws Exception {
final String json =
"{\"flags\":{\"target\":{\"key\":\"target\",\"enabled\":true,\"variationType\":\"STRING\","
+ "\"variations\":{\"experiment\":{\"key\":\"experiment\",\"value\":\"experiment\"},"
+ "\"ordinary\":{\"key\":\"ordinary\",\"value\":\"ordinary\"}},\"allocations\":["
+ "{\"key\":\"experiment\",\"doLog\":true,\"rules\":[{\"conditions\":[{\"attribute\":\"email\","
+ "\"operator\":\"MATCHES\",\"value\":\"*@datadoghq.com\"}]}],"
+ "\"splits\":[{\"variationKey\":\"experiment\",\"shards\":[]}]},"
+ "{\"key\":\"ordinary\",\"doLog\":false,"
+ "\"splits\":[{\"variationKey\":\"ordinary\",\"shards\":[]}]}]}}}";
final DDEvaluator evaluator =
new DDEvaluator(
() -> {},
AgentBridge.NONE,
TestSettings.of(),
ExperimentationOptOutMode.valueOf(mode.toUpperCase(Locale.ROOT)));
evaluator.accept(UniversalFlagConfigParser.parse(json.getBytes(UTF_8)));
final ProviderEvaluation<String> result =
evaluator.evaluate(
String.class,
"target",
"default",
new MutableContext("subject").add("email", "user@example.com").add(ATTRIBUTE, CUTOFF));
// An invalid regex fails the flag whether or not the subject opted out.
assertEquals(ErrorCode.PARSE_ERROR, result.getErrorCode());
assertEquals("default", result.getValue());
}What else the fix changes — all consistent with D019; worth one line in the PR description
- A subject without the regex's attribute also gets
PARSE_ERRORnow. Before,resolveAttributereturned null and the condition was false before the compile. - A disabled flag, or an expired or unreachable allocation, with an invalid regex now returns
PARSE_ERROR. The parser's other operand checks already fail such flags the same way. - The parser logs one
Dropping malformed FFE flagwarning per configuration for the flag. - A parsed pattern is compiled once per configuration, instead of on each evaluation that reaches the condition.
- A pattern that
java.util.regexrejects but that was rarely reached now fails the flag for every subject whose context is otherwise valid.
The same shape: a variant value that violates the flag's type — not fixed above
The shared variant-type-mismatch-flag has one doLog=true allocation whose variant is a string on an INTEGER flag. Its fixture comment says "SDKs must remove this flag". It is detected only when the variant is mapped, so the exclusion hides it too. Executed against the evaluator:
| Mode | Opted out | Result |
|---|---|---|
| any | no | PARSE_ERROR |
| default, disabled | yes | PARSE_ERROR |
| immediate, future_experiments_only | yes | default value, reason DEFAULT, no error |
A parse-time check of variant values against variationType would close it the same way. I did not write that check, and D019 names only regexes.
| static Instant cutoff(final ExperimentationOptOutMode mode, final EvaluationContext context) { | ||
| if ((mode != ExperimentationOptOutMode.IMMEDIATE | ||
| && mode != ExperimentationOptOutMode.FUTURE_EXPERIMENTS_ONLY) | ||
| || !context.keySet().contains(ATTRIBUTE)) { |
There was a problem hiding this comment.
🤖 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);
}
| + "]}}}"; | ||
| } | ||
|
|
||
| private static Path fixture(final String name) { |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P3] Declare the fixture directories as inputs of the test task, so that a change to cases.json or to the ffe-system-test-data submodule is an input change of the task.
policyCases() reads cases.json through this helper, and CanonicalFixtureTest reads the shared fixtures, from dd-smoke-tests by path. Gradle does not know about these files. After a fixture-only change, :dd-openfeature:test reports UP-TO-DATE, and the build succeeds without running the tests. CanonicalFixtureTest already read its fixtures this way before this PR; this PR adds cases.json. CI is less exposed, because its build cache key includes the pipeline ID. I saw the stale result in a local build.
What I ran — one expectation in cases.json changed to a wrong value
| Build | Task outcome | Result |
|---|---|---|
| This PR, after the edit | :dd-openfeature:test UP-TO-DATE |
passes |
This PR, after the edit, --rerun |
runs | 1 failure, policy case disabled/absent/missing/0 |
| With the fix, after the edit | runs | 1 failure, the same case |
| With the fix, after reverting the edit | FROM-CACHE |
passes |
Suggested fix — dd-openfeature/build.gradle.kts
--- a/dd-openfeature/build.gradle.kts
+++ b/dd-openfeature/build.gradle.kts
@@ -46,6 +46,18 @@ jmh {
jmhVersion = libs.versions.jmh.get()
}
+// Tests read fixtures from dd-smoke-tests by path. Declare them as inputs, so that a fixture change
+// is an input change: the task is no longer UP-TO-DATE, and its cache key changes.
+tasks.named<Test>("test") {
+ val fixtures = rootProject.layout.projectDirectory.dir("dd-smoke-tests/openfeature/src/test/resources")
+ inputs.dir(fixtures.dir("experimentation-opt-out"))
+ .withPropertyName("experimentationOptOutCases")
+ .withPathSensitivity(PathSensitivity.RELATIVE)
+ inputs.dir(fixtures.dir("ffe-system-test-data"))
+ .withPropertyName("ffeSystemTestData")
+ .withPathSensitivity(PathSensitivity.RELATIVE)
+}
+
tasks.withType<JavaCompile>().configureEach {
configureCompiler(JavaVersion.VERSION_11)
}
| // Do not expose the supplied context value in the diagnostic. | ||
| } | ||
| } | ||
| throw new IllegalArgumentException(INVALID_TIMESTAMP); |
There was a problem hiding this comment.
🤖 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());
}
| .reason(Reason.ERROR.name()) | ||
| .errorCode(ErrorCode.INVALID_CONTEXT) | ||
| .errorMessage(ExperimentationOptOut.INVALID_TIMESTAMP) | ||
| .flagMetadata(consentMetadata(observeFullEvaluationData)) |
There was a problem hiding this comment.
🤖 Comment from Claude working fully autonomously overnight:
[P3] Two new branches have no test: the consent metadata on this INVALID_CONTEXT result, and the allocations == null guard in ExperimentationOptOut.hasInvalidActivation. Two small tests pin them.
FlagEvalLoggingHookreadsobserve_full_evaluation_datafrom this metadata. If this line is dropped, a configuration with full evaluation data on would log this error as if it were off. The whole module suite stays green with the line removed.- A flag without
allocationsparses as valid. In future mode with a valid cutoff, it must still returnGENERAL("Missing allocations"). If the guard returnstrue, it returnsPARSE_ERRORinstead, and the opt-out and canonical fixture tests stay green.
Mutation results
| Mutant | Tests run | Result on this PR | With the new test |
|---|---|---|---|
Remove .flagMetadata(consentMetadata(observeFullEvaluationData)) from this result |
the whole module suite, 1264 tests | all pass | invalidSubjectKeepsConsentMetadata fails |
hasInvalidActivation: allocations == null returns true |
ExperimentationOptOut* and CanonicalFixtureTest, 592 tests |
all pass | futureModeKeepsMissingAllocationsError fails |
Suggested tests — for ExperimentationOptOutProviderTest; they use its existing evaluate and configuration helpers and pass on this PR
@Test
void invalidSubjectKeepsConsentMetadata() throws Exception {
final String json =
configuration(null, "true", false)
.replaceFirst("^\\{", "{\"observeFullEvaluationData\":true,");
final ProviderEvaluation<String> result = evaluate(json, "immediate", "invalid", "target");
assertEquals(ErrorCode.INVALID_CONTEXT, result.getErrorCode());
assertEquals(Boolean.TRUE, result.getFlagMetadata().getBoolean("observe_full_evaluation_data"));
}
@Test
void futureModeKeepsMissingAllocationsError() throws Exception {
final ProviderEvaluation<String> result =
evaluate(
"{\"flags\":{\"target\":{\"enabled\":true,\"variationType\":\"STRING\",\"variations\":{}}}}",
"future_experiments_only",
"valid",
"target");
assertEquals(ErrorCode.GENERAL, result.getErrorCode());
}
| private ExperimentationOptOutMode experimentationOptOutMode = ExperimentationOptOutMode.DEFAULT; | ||
|
|
||
| /** | ||
| * Sets per-subject experimentation opt-out. Overrides configuration when not DEFAULT. |
There was a problem hiding this comment.
🤖 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,
/**
| } | ||
| final String configured = settings.experimentationOptOutMode(); | ||
| switch (configured.trim().toLowerCase(Locale.ROOT)) { | ||
| case "": |
There was a problem hiding this comment.
🤖 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) {
|
|
||
| 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. |
There was a problem hiding this comment.
🤖 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.
- "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.exposureEventcallsflattenContext(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. - 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 returnsINVALID_CONTEXT. "Omit the attribute for subjects who can participate" does not say this.
Evidence
- Executed against this PR:
DDEvaluator.exposureEventwith 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:00Zon the API context of aLayeredEvaluationContext, and.add(ATTRIBUTE, (String) null)on the invocation context. The evaluation returnsINVALID_CONTEXT. The mergedkeySet()contains the attribute, andgetValuereturns aValuewhoseisNull()is true. This PR'scutoff()gives the same result, because the key is present and the value is not a string. - In SDK 1.23.0,
LayeredEvaluationContext.getValueasks 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.
What Does This Do
Subjects can opt out of experimentation while continuing to receive ordinary feature allocations. Configure
DD_FEATURE_FLAGS_EXPERIMENTATION_OPT_OUT_MODEbefore constructing the provider and supplydd_experimentation_opt_out_atin the evaluation context.disabled(default)immediatefuture_experiments_onlyExplicit provider options override configuration. The deprecated provider alias uses the same evaluator and supports the same options.
Motivation
Customers need per-subject experimentation controls without losing ordinary feature delivery or independently configured evaluation telemetry.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: FFLSDK-258
FFE reviewer selection
Reviewers: @greghuels, @sameerank
Why: Privacy opt-out and event logging rules — The PR adds a per-subject (per-evaluation-context) experimentation opt-out to the Java OpenFeature provider. Global reviewers cover a shortage in the matched pod.
🤖 Automated selection.
Selection details
Initiative: project / ufc-compliance
Why this match: The PR adds a per-subject (per-evaluation-context) experimentation opt-out to the Java OpenFeature provider. When the context contains dd_experimentation_opt_out_at and the mode is immediate or future_experiments_only, the evaluator skips allocations that have doLog=true, so no experiment exposure events are logged for that subject. Ordinary feature delivery keeps working. This matches the project's purpose: per-context suppression of exposure event egress to honor a user opt-out. The new configuration setting, the provider options, the allocation activation-time parsing and the tests all support that opt-out behavior. It is not layers, holdouts, bandits or EVP evaluation-telemetry work.
Why these owners: The matched initiative has fewer than two approved, non-author candidates. The approved global pool supplies the remaining review owners; proposed memberships were not activated.
The PR author was excluded. Existing reviewers and teams were preserved.
Debug or fix a problem
Workflow run · Bits Code session
dry_run=false. Unchanged completed runs are skipped. If the receipt saysneeds_reconciliationor a write outcome is uncertain, ask the workflow owner to reconcile GitHub and the receipt before retrying; do not delete the lock blindly.Evidence and run details
✅ Complete — reviewer coverage verified. The workflow checked pending requests or submitted reviews on the evaluated head.
Generated asynchronously by the FFE reviewer-routing workflow.
Changed paths:
dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/ExperimentationOptOut.java,dd-openfeature/src/main/java/com/datadog/openfeature/internal/evaluation/FlagEvaluator.java,dd-openfeature/src/main/java/com/datadog/openfeature/ExperimentationOptOutMode.java,dd-openfeature/src/main/java/com/datadog/openfeature/Provider.java,dd-openfeature/src/main/java/com/datadog/openfeature/internal/config/Settings.java,dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/Allocation.java,dd-openfeature/src/main/java/com/datadog/openfeature/internal/ufc/UniversalFlagConfigParser.java,dd-openfeature/README.md,metadata/supported-configurations.jsonEvaluated head:
55ba0bdad77e02a3398975429a66f80aac1a05a3Registry revision:
revision:1946aa2a-530e-487a-84a7-83aff44a1089Pinned skill source:
d214cfc65ae154b97d39af5cf32caf48aaec9bf0Instructions: workflow-injected snapshot (no plugin loaded)
Instruction snapshot SHA-256:
f23b2a88c13c0da90e2dd58246a14f257af91e2195c555039bd555b52e6f3516