FFL-2783: add tests for observeFullEvaluationData PII protection (gated as missing_feature) by vjfridge · Pull Request #7316 · DataDog/system-tests

vjfridge · 2026-07-16T03:00:16Z

Motivation

FFL-2783 — part of FFL-2780 (Protecting PII in flagevaluations track)

See informal RFC for more context

Summary

Adds three test classes to existing tests/ffe/test_flag_eval_evp.py covering the new per-env UFC observeFullEvaluationData field
Extends make_ufc_fixture with an observe_full_evaluation_data: bool | None = None kwarg. None omits the field entirely, so existing tests are unaffected
All three classes are deactivated as missing_feature (FFL-2783) on Go — the only SDK where the file-level gate would otherwise activate them. Other SDK manifests already deactivate the whole file via missing_feature (FFL-2446) or irrelevant.

Truth-table coverage

UFC `observeFullEvaluationData`	Expected `targeting_key`	Expected `context.evaluation`
absent	`sha256_`-prefixed (71 chars, lowercase hex suffix)	omitted
`false`	`sha256_`-prefixed	omitted
`true`	raw customer value, no prefix	full object with all attributes

Canonical PII vector (shared across SDKs)

Every SDK's unit tests should assert against the same input/output. Input: "jane.doe@datadoghq.com" → "sha256_b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b". Canonical attributes cover string, integer, dotted-key, and email PII shapes.

Forward-compat for existing SDKs

Introducing a new field inside the UFC environment object could theoretically break SDKs if their UFC parser rejects unknown fields. That risk is already covered by tests/ffe/test_dynamic_evaluation.py::Test_FFE_Unknown_Fields_Tolerance, which injects random unknown fields at every UFC level (including inside environment) and asserts each SDK continues to evaluate flags correctly. That test runs on every SDK where test_dynamic_evaluation.py is active, so no per-fixture retrofit is needed here.

Out of scope

Existing kill switch (DD_FLAGGING_EVALUATION_COUNTS_ENABLED) — per-SDK unit test territory, no system-tests scenario variant needed.

Test plan

./format.sh clean (mypy, ruff, node linters)
Once Go SDK ships FFL-2790, flip Go's three missing_feature (FFL-2783) rows to v2.X.Y-dev and confirm all three cells pass

🤖 Generated with Claude Code

Adds three non-blocking test classes covering the truth-table matrix for the new per-env UFC field: absent (backcompat), false (explicit disable), and true. Asserts hashed sha256_-prefixed targeting_key + omitted context.evaluation for the first two cells, and raw targeting_key + full context.evaluation for the third. Uses a canonical PII vector (jane.doe@datadoghq.com) so every SDK's L1 unit tests can assert against the same known hash output. All classes gated missing_feature (FFL-2783) on Go; other SDK manifests already skip the whole file. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>

github-actions · 2026-07-16T03:00:40Z

CODEOWNERS have been resolved as:

manifests/golang.yml                                                    @DataDog/dd-trace-go-guild
tests/ffe/test_flag_eval_evp.py                                         @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
tests/ffe/utils/fixtures.py                                             @DataDog/feature-flagging-and-experimentation-sdk @DataDog/system-tests-core
utils/__init__.py                                                       @DataDog/system-tests-core
utils/_decorators.py                                                    @DataDog/system-tests-core

datadog-datadog-us1-prod · 2026-07-16T03:05:47Z

Tests

⚠️ Warnings

🚦 4 Pipeline jobs failed

Testing the test | System Tests (java, prod) / End-to-end #1 / play 1
🧪 1 Test failed

All test failures are known flaky.
❄️ Known flaky: tests.integrations.test_inferred_proxy.Test_AWS_API_Gateway_Inferred_Span_Creation_v2.test_api_gateway_rest_inferred_span_creation_optional_tags[play] from system_tests_suite
ValueError: No span validates this test

self = &lt;tests.integrations.test_inferred_proxy.Test_AWS_API_Gateway_Inferred_Span_Creation_v2 object at 0x7f4a85bd77d0&gt;

    def test_api_gateway_rest_inferred_span_creation_optional_tags(self):
        assert self.r.text == &#34;ok&#34;
    
&gt;       interfaces.library.validate_one_span(
            self.r,
            validator=mandatory_tags_validator_factory(
...
Not introduced in this PR.
Testing the test | System Tests (java, dev) / End-to-end #1 / jersey-grizzly2 1

Testing the test | System Tests (php, dev) / End-to-end #2 / apache-mod-7.0 2
View all 4 failed jobs.

ℹ️ Info

No other issues found (see more)

❄️ No new flaky tests detected

Useful? React with 👍 / 👎

_{This comment will be updated automatically if new data arrives.

🔗 Commit SHA: 63b0c15 | Docs | Datadog PR Page | Give us feedback!}

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1ad980b2f

ℹ️ 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".

chatgpt-codex-connector · 2026-07-16T03:08:15Z

+        rc.tracer_rc_state.reset().set_config(
+            f"{RC_PATH}/{config_id}/config",
+            make_ufc_fixture(self.flag_key),
+        ).apply()


Move fallible setup work into the test

These new setup_* methods perform remote-config application and the weblog request before the test body; if either path raises or times out in CI, pytest reports a setup error and the scenario can stop instead of recording a normal test failure. .cursor/rules/pr-review.mdc says setup methods must only trigger observability and failures/assertions belong in test_*, so keep setup minimal and move the fallible validation path into the matching test method.

Useful? React with 👍 / 👎.

Not addressing this one. The other eight test classes in this file (Test_FFE_EVP_Flagevaluation_Basic, _Count, _Context_Bounds, _Runtime_Default, _Load_Aggregation, _Burst_Aggregation, _High_Cardinality_Aggregation, _Degradation) all place rc...apply() and evaluate_flag(...) in setup_* and only run assert self.r.status_code == 200 in test_*. Diverging in only the three new classes would create an inconsistency reviewers would flag; if the setup-vs-test split needs enforcement here, it belongs in a dedicated refactor across the whole file.

…sts-nonblocking

The three new missing_feature rows should reference FFL-2784 (Go SDK implementation) rather than FFL-2783 (this L3 test ticket) so an SDK dev lands directly on the work that unblocks activation. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>

- Rename CANONICAL_* to PII_* and move constants + helpers into their existing sections (top-of-file constants, mid-file helpers) for consistency with the rest of the file. - Iterate every matching flagevaluation event in all three PII test classes so a clean first event cannot mask a leaky later event. - Add assert_no_raw_pii_in_event helper that walks the serialized event and asserts none of the raw PII values (targeting key or attributes) appear anywhere — guards against SDKs that route raw PII into unexpected fields (e.g., context.user_email) even when context.evaluation is correctly omitted. Applied to both hashed cells. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>

cbeauchesne

From framework usage, just a small request change.

Cold you also get a review from someone familiar with the feature for the test logic ?

cbeauchesne · 2026-07-17T09:41:12Z

+
+@scenarios.feature_flagging_and_experimentation
+@features.feature_flags_evp_flagevaluation
+@pytest.mark.skip_if_xfail


There are semantic decorators in utils that allow to achieve the same, and also declare why we want to skip them : @slow and @scenario_crash, could you use them ?

Thanks for raising this! You're right that using @pytest.mark.skip_if_xfail directly is too raw. We looked at @slow and @scenario_crash but neither quite fits — these tests aren't slow, and they don't crash the scenario. They're gated on SDK implementation status.

So we added a new alias @not_yet_implemented in utils/_decorators.py (same underlying skip_if_xfail mark, just with a clearer name) and used it consistently across all 11 classes in the file.

…ly in test_flag_eval_evp.py @slow and @scenario_crash both alias skip_if_xfail but carry wrong semantics for tests gated on SDK implementation status. Add @not_yet_implemented as a clearer alias and use it throughout the FFE EVP test file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

leoromanovsky

logic is good, before merging please:

add a env var=false case
adjust labels on existing test cases

leoromanovsky · 2026-07-17T16:13:03Z

-@pytest.mark.skip_if_xfail
+@not_yet_implemented


hehe @cbeauchesne skipped these because they are not set up correctly; I needed to break system test convention to get them to work with PHP, which has its own sidecar that needs an additional await to count received EVP events in the backend. It's on my backlog to address, but a more correct skip decorator would be flaky or bug

leoromanovsky · 2026-07-17T16:17:05Z

 EVP_FULL_TIER_PER_FLAG_CAP = 10_000
 EVP_DEGRADATION_OVERFLOW_EVALS = 2_000

+# Fixed input/output vector for the PII-protection tests. Every SDK's L1 unit tests


Suggested change

# Fixed input/output vector for the PII-protection tests. Every SDK's L1 unit tests

# Fixed input/output vector for the PII-protection tests. Every SDK's unit tests

codegen tool leaked 😄

leoromanovsky · 2026-07-17T16:18:18Z

+# Fixed input/output vector for the PII-protection tests. Every SDK's L1 unit tests
+# should assert against the same input to prove byte-identical hashing across SDKs.
+PII_TARGETING_KEY = "jane.doe@datadoghq.com"
+PII_TARGETING_KEY_HASHED = "sha256_b4698f9b6d186781fa8dc59e533578fa2d8379a46b1cf6db85cda6aa9c99e51b"


I heard you mention this, good idea adding the prefix that will make it look less surprising when customers see it and I can see how this will help the autocomplete design to ignore it.

leoromanovsky · 2026-07-17T16:32:36Z

            assert "targeting_key" not in event, f"degraded event must omit targeting_key: {event}"
            assert object_key(event.get("variant"), "variant") == "on"
            assert object_key(event.get("allocation"), "allocation") == "default-allocation"
+


Please add a test for DD_FLAGGING_EVALUATION_COUNTS_ENABLED=false + observeFullEvaluationData=true - sorry I missed adding coverage for the false case.

vjfridge requested review from a team as code owners July 16, 2026 03:00

vjfridge requested review from sameerank and typotter and removed request for a team July 16, 2026 03:00

chatgpt-codex-connector Bot reviewed Jul 16, 2026

View reviewed changes

vjfridge changed the title ~~FFL-2783: add L3 tests for observeFullEvaluationData PII protection (non-blocking)~~ FFL-2783: add L3 tests for observeFullEvaluationData PII protection (gated missing_feature) Jul 16, 2026

vjfridge changed the title ~~FFL-2783: add L3 tests for observeFullEvaluationData PII protection (gated missing_feature)~~ FFL-2783: add tests for observeFullEvaluationData PII protection (gated as missing_feature) Jul 16, 2026

vjfridge and others added 3 commits July 16, 2026 14:01

Merge branch 'main' into vickie/FFL-2783-observeFullEvaluationData-te…

4320460

…sts-nonblocking

cbeauchesne requested changes Jul 17, 2026

View reviewed changes

vjfridge and others added 2 commits July 17, 2026 11:37

FFL-2783: fix ruff formatting issues

63b0c15

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

leoromanovsky approved these changes Jul 17, 2026

View reviewed changes

	# Fixed input/output vector for the PII-protection tests. Every SDK's L1 unit tests
	# Fixed input/output vector for the PII-protection tests. Every SDK's unit tests

		@pytest.mark.skip_if_xfail
		@not_yet_implemented

Uh oh!

Conversation

vjfridge commented Jul 16, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Summary

Truth-table coverage

Canonical PII vector (shared across SDKs)

Forward-compat for existing SDKs

Out of scope

Test plan

Uh oh!

github-actions Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

datadog-datadog-us1-prod Bot commented Jul 16, 2026 • edited by datadog-datadog-prod-us1-2 Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ Warnings

ℹ️ Info

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

vjfridge Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

cbeauchesne left a comment

Choose a reason for hiding this comment

Uh oh!

cbeauchesne Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

vjfridge Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

leoromanovsky left a comment

Choose a reason for hiding this comment

Uh oh!

leoromanovsky Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

leoromanovsky Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

leoromanovsky Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

leoromanovsky Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vjfridge commented Jul 16, 2026 •

edited by atlassian Bot

Loading

github-actions Bot commented Jul 16, 2026 •

edited

Loading

datadog-datadog-us1-prod Bot commented Jul 16, 2026 •

edited by datadog-datadog-prod-us1-2 Bot

Loading