test(php): cover OpenFeature in all weblog environments - #7393
test(php): cover OpenFeature in all weblog environments#7393leoromanovsky wants to merge 8 commits into
Conversation
|
|
🎉 All green!🧪 All tests passed 🔗 Commit SHA: 54a205d | Docs | Datadog PR Page | Give us feedback! |
|
Validated against the current PHP fix build.
Local results: The current GitHub Actions run does not use this PHP build:
Therefore, the PHP fix will unblock the FFE failures. It will not make every PHP job green because the AppSec failures are separate. |
…enfeature-all-environments
…enfeature-all-environments # Conflicts: # manifests/php.yml
![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: 54a205d2c7
ℹ️ 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".
| assert self.response.status_code == 200, f"Flag evaluation failed: {self.response.text}" | ||
| result = json.loads(self.response.text) | ||
|
|
||
| if result.get("errorCode") == "PROVIDER_NOT_READY": |
There was a problem hiding this comment.
On PHP 8 weblogs, where this commit installs the OpenFeature SDK and activates the test, this branch lets the test pass when OpenFeature is completely broken or unwired. The PHP handler catches any Throwable—including missing classes, provider construction failures, or incompatible SDK calls—and maps it to PROVIDER_NOT_READY, so every supported weblog could return the default without this test detecting the regression. Reserve this fallback behavior for PHP 7 through a separate manifest-scoped contract test, and require the configured true value here.
Useful? React with 👍 / 👎.
| refs: | ||
| - &php7 "apache-mod-7.0, apache-mod-7.0-zts, apache-mod-7.1, apache-mod-7.1-zts, apache-mod-7.2, apache-mod-7.2-zts, apache-mod-7.3, apache-mod-7.3-zts, apache-mod-7.4, apache-mod-7.4-zts, php-fpm-7.0, php-fpm-7.1, php-fpm-7.2, php-fpm-7.3, php-fpm-7.4" |
There was a problem hiding this comment.
php.yml had no refs section before this change, so this adds its first reusable anchor. .cursor/rules/test-activation.mdc permits new references only in manifests that already contain references and with explicit approval; expand this weblog list at the affected conditions rather than introducing refs: here.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
| def setup_openfeature_evaluation(self) -> None: | ||
| self.flag_key = "openfeature-evaluation" | ||
| config = make_ufc_fixture(self.flag_key, variation_type="BOOLEAN") | ||
| rc.tracer_rc_state.reset().set_config(f"{RC_PATH}/openfeature-evaluation/config", config).apply() |
There was a problem hiding this comment.
apply() calls send_state() with acknowledgement waiting enabled and can raise when RC delivery or acknowledgement times out. Because an exception from setup_openfeature_evaluation aborts the entire scenario rather than producing an isolated test failure, make this setup operation non-throwing and perform its validation in the matching test, as required by .cursor/rules/pr-review.mdc.
AGENTS.md reference: AGENTS.md:L33-L35
Useful? React with 👍 / 👎.
| if [[ "$(printf '%s\n' "$PHP_VERSION" "8.0" | sort -V | head -n1)" = "8.0" ]]; then | ||
| COMPOSER_DISCARD_CHANGES=true composer require "open-feature/sdk:^2.2" \ | ||
| --prefer-dist --no-interaction --ignore-platform-req=ext-mbstring |
There was a problem hiding this comment.
This shared installer is also called by utils/build/docker/php/parametric/Dockerfile, where the locked application dependencies are installed under /binaries and no FFE endpoint uses OpenFeature. On that PHP 8.2 build, this unconditional command instead runs from /var/www/html, creates or updates a separate Composer project, and adds an unnecessary unpinned network resolution that can fail an otherwise reproducible parametric build. Guard the requirement to weblog images that actually expose /ffe, or install it in the weblog-specific build path.
Useful? React with 👍 / 👎.
1 participant
Motivation
A customer found that an application error handler can convert a provider warning into an exception. OpenFeature then returns the code default with a
GENERALerror.The existing PHP FFE tests called the native Datadog client. They did not cross the OpenFeature provider and logger boundary where this failure occurred.
flowchart LR App["Application"] --> Client["OpenFeature client"] Client --> Provider["Datadog provider"] Provider --> Evaluator["Datadog evaluator"] Provider -. "warning" .-> Handler["Application error handler"] Handler --> Exception["Exception"] Exception --> Error["Code default plus GENERAL error"] Test["Previous FFE test"] --> Endpoint["POST /ffe"] Endpoint --> Native["Native Datadog client"] Native --> Evaluator Test -. "missed this boundary" .-> ProviderThe OpenFeature PHP SDK requires PHP 8 or later. System tests still support PHP 7. PHP 7 must keep the
/ffecontract when the SDK is unavailable.Changes and Decisions
The
/ffeendpoint now uses OpenFeature by default. Tests that need the native client can setevaluationApitonative.The generic test omits
evaluationApi. An available provider must return the configured value. An unavailable provider must return the supplied default withERRORandPROVIDER_NOT_READY.flowchart TD Test["Generic FFE test"] --> Endpoint["POST /ffe"] Endpoint --> Default["Default to OpenFeature"] Default --> Available{"Provider available?"} Available -->|"Yes"| Success["Configured value and no error"] Available -->|"No"| Fallback["Supplied default<br/>ERROR<br/>PROVIDER_NOT_READY"] Success --> Contract["Contract passes"] Fallback --> ContractThe shared PHP installer adds the OpenFeature SDK on PHP 8 and later. Laravel and Symfony use the shared
/ffehandler. PHP 7 marks provider-dependent tests as unavailable but keeps the generic contract test enabled.The PHP 8 success path depends on DataDog/dd-trace-php#4071.
Validation
These local runs used this PR at
0dad5d48bec63cffdd206b724013d26f497c0c8c. They used the CI-built ARM64 package fromdd-trace-phpbf360ef80839f483179e37215536d1713116167c.The PHP-FPM 8.2 image used PHP
8.2.32. All four images reported tracer version1.24.0+dev.bf360ef80839f483179e37215536d1713116167c.