Repository navigation
Conversation
Evaluate prerequisites with ordered memoized native evaluation and preserve eligible exposures across agentless and Agent Remote Configuration delivery. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Codeowners resolved asResolved from the full PR diff against |
Dependency direction analysis
|
❌ ErrorsYour PR has failed checks. Please review the issues below and take necessary action before merging. 🚦 4 Pipeline jobs failed
ℹ️ InfoNo 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: 06a741f | Docs | View more details | Give us feedback! |
Preserve the independent-flag evaluation path while retaining prerequisite evaluation and exposure records. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
BenchmarksBenchmark execution time: 2026-11-06 23:01:08 Comparing candidate commit 06a741f in PR branch Found 12 performance improvements and 0 performance regressions! Performance is the same for 604 metrics, 10 unstable metrics, 1 flaky benchmarks without significant changes.
|
…ndent-flags-support
Pin the shared harness revision that registers RC capability bit 50, fixing parametric capability validation failures in GitHub and GitLab CI. System-tests fix: DataDog/system-tests#8293 Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Keep configuration delivery enabled when native capability discovery raises, and include the effective capability header in agentless startup diagnostics. Cover reporter failures in both delivery paths and supported/unsupported startup diagnostics. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Codex Review Summary: ✅ CompletedThis 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: 1007b76502
ℹ️ 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".
![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
The pinned libdatadog evaluator internals were unavailable for inspection, so prerequisite evaluation semantics and runtime delivery behavior remain unverified.
🤖 Bits Code Review · Commit 1007b76
FFE reviewer selectionReviewers: @sameerank Why: Dependent Flags — The PR turns on prerequisite (dependent) flag evaluation in the Python SDK. Reviewers come from the matched pod. 🤖 Automated selection. Selection detailsInitiative: project / dependent-flags Why this match: The PR turns on prerequisite (dependent) flag evaluation in the Python SDK. The native binding now advertises dependent-flag-variant-key-v1 and returns per-prerequisite evaluation records. The provider records each prerequisite evaluation it reaches and reports exposures only for prerequisites that contributed to the assignment. Agentless HTTP and Remote Configuration delivery send the capability. Package-manager pin changes only support this work. Why these owners: Eligible reviewers have already approved the current head; no additional reviewers are needed for the responsibilities those approvals cover.
Additional review: openfeature-architecture — The PR changes shared provider and native contracts beyond dependent-flag logic. Every flag resolution now goes through eval_flag_with_trace instead of get_assignment, with a measured 9–10% overhead. The ResolutionDetails native result type gains a new field. The tracer/native capability handshake is now fail-safe, so a failing reporter advertises no capabilities for both agentless and RC sources. The provider's evaluation, metrics and exposure paths gain extra per-prerequisite event emission. Coverage: @sameerank. Evidence: src/native/ffe.rs, ddtrace/internal/openfeature/_native.py, ddtrace/internal/openfeature/_provider.py The PR author was excluded. Existing reviewers and teams were preserved. Debug or fix a problemWorkflow run · Bits Code session
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: Evaluated head: Registry revision: Pinned skill source: Instructions: workflow-injected snapshot (no plugin loaded) Instruction snapshot SHA-256: |
Keep independent evaluations compatible with older native results, cover both consent transitions, use the merged system-tests pin, and register measured native-resolution benchmark gates. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Set native-resolution limits about 10% above measured CI candidate means and record the calibration pipeline, preserving regression gates for both configurations. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
|
Calibrated the new native-resolution gates in 5554077 from CI release-wheel measurements, rather than workspace A/A timings. The raw artifacts from benchmark job 2209594150 compare stacked baseline
Each configuration has 60 samples per wheel; sample-size and autocorrelation warnings still apply. The platform's unchanged summary should not obscure the measured increase in the raw samples. This accepts the already documented independent-flag cost of the dependent-evaluator upgrade and prerequisite trace support. The native independent-flag fast path remains enabled. The old 800/850 ns limits were at or below the CI baseline; the new limits retain about 10.6% headroom above the CI candidate and continue to gate both configurations. This follow-up changes only the two thresholds and calibration comments. Full repository lint checks and generated CI validation pass. A fresh pipeline will rerun the benchmark jobs before the gate; the gate is not being bypassed. |
|
Fresh verification on The gate reports static resolution 851.017–856.481 ns < 950 ns and targeting 914.175–920.739 ns < 1,010 ns. Functional test child pipeline This resolves the benchmark-limit failure by CI calibration. It does not eliminate the native performance cost: the fresh raw means compare baseline/candidate 793.50 → 853.75 ns for static (+7.59%) and 848.97 → 917.46 ns for targeting (+8.07%). |
Build allocation metadata only when requested and restore the original independent-resolution benchmark limits. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
aarsilv
added
the
crucible: reviewing
| # Keep libdatadog crates on one mainline revision to share native types. | ||
| # This adds prerequisite evaluation and its Remote Configuration capability: | ||
| # https://github.com/DataDog/libdatadog/pull/2748 | ||
| libdd-ffe = { git = "https://github.com/DataDog/libdatadog", rev = "1cf7834903025181d51480bc6a5a6a174e4e7b47", optional = true, features = ["pyo3"] } |
There was a problem hiding this comment.
🤖 Comment from Claude working with Aaron Silverman:
Heads up: This pin would regress main and does not match the current DataDog/libdatadog#2748 head. Please keep this PR unmerged until DataDog/libdatadog#2748 is merged and released.
1cf7834is a merge commit on the open feat(ffe): evaluate dependent flags with prerequisite telemetry libdatadog#2748 branch. Its workspace version is 47.0.0, which is 72 commits (14 of them marked breaking) after v46.1.0. That native upgrade covers trace export, obfuscation, Remote Config, TLS, IPC, crashtracker and profiling.mainhere moved to libdatadog v46.1.1 in chore(deps): bump libdatadog to v46.1.1 #21480 for "preserve session tokens on trace stats requests" (fix: preserve session tokens on trace stats requests libdatadog#2794,f42b3ab). Neither1cf7834nor983b4b0contains it, so a rebase that keeps this pin removes that fix.- feat(ffe): evaluate dependent flags with prerequisite telemetry libdatadog#2748's head (
983b4b0) changed behavior after this pin. WithmaxDependencyDepth: 256, this pin rejects the whole configuration, while983b4b0fails only evaluations that reach a dependency. Explicitnulldefaults to 10 here but is invalid at983b4b0. An empty value list is accepted at983b4b0;test_empty_prerequisite_variant_list_rejects_only_its_flagencodes the old behavior.
Suggested order: rebase DataDog/libdatadog#2748 on libdatadog main (which brings in f42b3ab), merge and release it. Then pin all crates here to that release and update the tests that encode 1cf7834 behavior.
## Description Move direct UFC dependent-flags negotiation from the capabilities header to the `cap_dependent-flags-variant-key=v1` query parameter. Existing endpoint query parameters are preserved, and the end-to-end local delivery fixture now selects dependent configurations from that parameter while Remote Config negotiation remains unchanged. ## Testing - Python AST parsing and `git diff --check` pass. - The mandated OpenFeature test runner could not execute: its image pull filled the workspace filesystem, and environment setup failed with `ENOSPC` before tests started. ## Risks Scoped to agentless UFC request construction and its local delivery fixture. Reverting this PR restores the header transport. ## Additional Notes No release note: this corrects the transport contract within the unmerged dependent-flags base PR and does not independently expose a released customer feature.
Stage graph exposures per evaluation and publish/cache them only after successful finalization. Preserve prerequisite evaluations on failures, consume the corrected native depth telemetry, and activate shared dependency telemetry system tests in CI. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Verify asynchronous OpenFeature clients preserve prerequisite evaluation events while discarding failed graph exposures and cache admission on both supported SDK versions. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Run feature tests before the shared after-script exchanges credentials and tags the job. Preserve framework tagging on both jobs without requiring an API key before the dependent-flag test starts. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
Use the updated prerequisite evaluator and current main fixes, align PyO3 with its native dependencies, and preserve extraction behavior under PyO3 0.30. Environment: Datadog workspace Co-Authored-By: Codex GPT-6 <noreply@localhost>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06a741f0c7
ℹ️ 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".
| parameter = _dependent_flags_variant_key_parameter() | ||
| if parameter is None: | ||
| return urlunsplit(("", "", self._request_path, self._request_query, "")) or "/" |
There was a problem hiding this comment.
When an older native extension is used with a custom endpoint that already contains cap_dependent-flags-variant-key=v1, the parameter is None branch returns the original query unchanged, so the request still advertises dependent-flag support even though capability discovery explicitly reported none. The server may then return a dependent configuration that the loaded evaluator cannot process; parse and remove this parameter regardless of support, and append v1 only when the evaluator advertises it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
A fork during exposure publication can permanently stall flag evaluation in the child. The same provider-wide lock also lets slow exposure delivery block concurrent cached evaluations.
🤖 Bits Code Review · Commit 06a741f
| self._exposure_cache: LRUCache[ | ||
| tuple[str, str], tuple[typing.Optional[str], typing.Optional[str], typing.Optional[int]] | ||
| ] = LRUCache(maxsize=65536) | ||
| self._exposure_lock = RLock() |
There was a problem hiding this comment.
If another application thread holds this lock when the process forks, the child inherits a locked primitive whose owner no longer exists. Subsequent exposure-producing evaluations, including cache hits, can hang indefinitely. The imported RLock is a plain _thread.RLock, and this provider never resets it after fork. Use forksafe.RLock and update its import.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
| with self._exposure_lock: | ||
| if self._exposure_cache.get(key, None) == value: | ||
| return | ||
| get_exposure_writer().enqueue(event) |
There was a problem hiding this comment.
ExposureWriter.enqueue() synchronously flushes over HTTP when buffering another event would exceed the 5 MiB payload limit. Holding the provider-wide lock across enqueue makes concurrent exposure-producing evaluations wait for delivery timeouts and retries, including cached flag/subject pairs that previously bypassed the writer. Keep blocking delivery outside the cache lock while preserving concurrent deduplication and admission only after successful enqueue.
React: 👍 useful · 👎 wrong · 😕 unclear · 👀 right, but not acting on it
What each reaction means
- 👍 Useful: correct, and you're fixing it or will.
- 👎 Wrong: the finding is incorrect, for example the bug can't happen or the code was misread.
- 😕 Unclear: you can't tell what it means or why it matters.
- 👀 Right, not acting: correct, but intended, out of scope for this PR, a known risk, or too minor.
🤖 Bits Code Review · Open Bits AI session
Description
Enables variant-key prerequisite flag evaluation for Python server applications using agentless HTTP or Agent Remote Configuration. Agentless delivery advertises
cap_dependent-flags-variant-key=v1; Agent RC advertises capability bit 50.Each root call records reached prerequisites once, including errors and prerequisites on rejected paths. Exposure events require the selected allocation's
doLogand membership in the successful dependency path. Root and prerequisite exposures are staged until all OpenFeature application hooks succeed; failed calls neither enqueue exposures nor populate the deduplication cache. Re-entrant and overlapping calls retain independent batches, and direct provider calls preserve their existing reporting behavior.Pins libdatadog to
cb3bfba0d51adba06f9afca32841573fa7bad0b3from #2748, including error records for depth-rejected prerequisites, the public trace constructor/diagnostics updates, and current main's trace-stats session-token fix. PyO3 and its build dependencies are aligned to 0.30, which that native baseline requires; deprecated extraction opt-out annotations are removed because opt-out is now the default. Preserves per-root memoization, caller-default behavior on dependency failures, entry-time consent, and independent-flag compatibility. Native allocation metadata remains lazily constructed.Validation
-Fso the unreleased-feature declaration cannot hide failures. The job pins harness commitd3cec0314166889cf31555252c94d94b9140c4d8; existing system-test jobs retain their pins. Release-wide activation remains in system-tests #8233.878c1b75c72d7b524236e374f25ed530d0bee694completed with 60 samples per configuration: static 602 ± 21 ns → 606 ± 40 ns; targeting 653 ± 44 ns → 652 ± 47 ns (mean ± standard deviation). Neither configuration reports a significant regression. Samples overlap and show shared-host jitter; these measure the independent native binding, not the provider hook lifecycle. Artifacts: run5cd6eaa3-2ab5-4d48-9f86-dd8dac28d146.Compatibility and remaining validation
Stacked on #21336 and consumes libdatadog #2748. The native revision is unreleased and the source-build Rust minimum is 1.91.1. Replace the temporary pin with an approved merged revision before release. Local suite coverage is Linux x86_64 / Python 3.11, plus the optimized benchmark wheel on Python 3.12; release-wheel and supported-platform validation remains with CI. The system-test run used the documented local-source mount, while the added CI job consumes release wheels.