Repository navigation
Add provisional per-subject evaluation telemetry opt-out tests - #8080
leoromanovsky wants to merge 3 commits into
Conversation
|
|
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🔗 Commit SHA: 20ce91e | Docs | View more details | Give us feedback! |
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. |
![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: 20ce91eb06
ℹ️ 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".
| ] | ||
| self.cases.extend(shared_cases) | ||
|
|
||
| rc.tracer_rc_state.reset().set_config(f"datadog/2/FFE_FLAGS/opt-out-{signal}/config", fixture).apply() |
There was a problem hiding this comment.
When Remote Config application or a worker request raises, this call propagates from each setup_* method and aborts the entire end-to-end scenario before the remaining setups and assertions run. Store setup errors/results and assert them from the matching test_* methods instead, so a failed candidate implementation is reported as a test failure rather than a scenario crash.
AGENTS.md reference: AGENTS.md:L34-L36
Useful? React with 👍 / 👎.
| scenario_groups=[scenario_groups.ffe], | ||
| ) | ||
|
|
||
| feature_flagging_evaluation_telemetry_opt_out = DdTraceEndToEndScenario( |
There was a problem hiding this comment.
DdTraceEndToEndScenario automatically adds the all, end_to_end, and tracer_release groups, so these two provisional scenarios are selected by ordinary official runs for every tracer. The new manifest declarations do not prevent that work: missing_feature is implemented as xfail, meaning the four setup methods still run, including RC waits and 84 evaluations per scenario. This contradicts the documented separate activation step and adds two full container scenarios even before any SDK is supported; define these scenarios without the default groups or otherwise keep them out of normal CI until activation.
Useful? React with 👍 / 👎.
| for point in metric.get("points", []): | ||
| total += float(point["value"] if isinstance(point, dict) else point[1]) | ||
| assert_count(total, expected_count, flag) |
There was a problem hiding this comment.
For SDKs that export the OpenTelemetry counter with cumulative temporality, the Agent capture can contain the same accumulated value in multiple periodic exports; this scenario explicitly exports every second and spends several more seconds running later setups before shutdown. Summing every captured point and requiring exact equality therefore reports more than three evaluations even when the implementation is correct. The existing metric-count test in tests/ffe/test_flag_eval_metrics.py deliberately accepts total_count >= self.eval_count; this assertion should instead select the final cumulative value or otherwise account for temporality while retaining the zero-count opt-out check.
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Motivation
Applications need flag results while suppressing evaluation telemetry for selected subjects. Add shared regression tests for that contract.
Candidate SDK implementations must pass the applicable local checks before delivery. Released-version CI activation is a separate step.
Changes and Decisions
Companion implementations: Go, Java, Node.js, Python, Ruby, .NET.
Validation
Reran the updated scenarios locally on October 18 against six prepared candidate SDK builds, using Linux ARM64 containers and Agent 7.74.0.
All 46 applicable checks passed. .NET excludes the EVP evaluation-event check because it has no producer for that signal.
These runs exercise real provider evaluation, controlled Remote Configuration, and captured Agent traffic. They do not establish released-version compatibility.
Negative control: forced Go activation off in the enabled scenario. Event, metric, and trace checks failed on leaked telemetry; exposures passed.
Restoring activation returned all four checks to green.