CI Misc fixes (no libdatadog) - #4224
Merged
Merged
Conversation
The passing merge base resolved Twig 3.28.0. Later CI runs resolved Twig 3.29.0 because the fixtures run `composer update`. Twig 3.29 changed `TemplateWrapper::unwrap()` to require an `Environment`. Symfony Twig Bundle 6.2.7 still called `unwrap()` with no arguments.
Composer errors were swallowed after the final failing run. The Make target would therefore succeed after all retry attempts failed.
The test creates two Remote Config probe files and must wait until both
probes are installed before deleting those files. Its old ordering was:
```php
$p1 = put_span_probe_id1("a");
$p2 = put_span_probe_id1("b");
await_probe_installation(function () {}, 2);
```
`await_probe_installation()` detects installations indirectly:
1. It installs a dummy hook and records its numeric ID as the baseline.
2. It invokes the supplied callback.
3. It repeatedly installs more dummy hooks.
4. Gaps between hook IDs reveal how many probes were installed
asynchronously.
The race was that this test published both probes before step 1. If
Remote Config installed either probe quickly, its hook ID was already
included in the baseline. The helper could only see installations after
the baseline, so it waited for events that had already happened.
The consequences were:
- It could run all 15,000 polling iterations.
- On Windows PHP 8.5 ZTS, the test hit the runner's 60-second timeout.
- The helper silently returned success after exhausting the polls, so
platforms without the runner timeout could produce a false positive.
- The Windows runner retried the test, but the terminated shell
apparently left the PHP child holding the generated script. Recreating
it then failed with `Permission denied`.
The commit changes the ordering to:
```php
list($p1, $p2) = await_probe_installation(function () {
return [
put_span_probe_id1("a"),
put_span_probe_id1("b"),
];
}, 2);
```
The helper now takes its baseline before publishing both probes, so no
installation can be missed. It also replaces the silent timeout with:
```php
throw new RuntimeException(
"Probe installation timed out; $num probe(s) missing"
);
```
This commit fixes a false failure in the Swoole integration suite. The suite uses several PHP processes: the PHPUnit runner, a short process that reads the Swoole extension version, and the actual Swoole webservers. ddtrace was loaded in all of them. The runner and version probe therefore started Remote Config using the default agent address, `localhost:8126`, while the Swoole servers correctly used `test-agent:9126`. All processes shared one sidecar, whose log writer broadcasts messages to every active log destination. A failed Remote Config poll from the runner or version probe could consequently appear in the Swoole server log: ```text [ddtrace] [error] [sidecar] ... http://localhost:8126/v0.7/config ``` The framework checks that log after every test. Depending on the five-second polling timing, an otherwise successful Swoole test could be blamed for this unrelated error. The commit disables Remote Config in both noise-producing processes: - A target-specific `TEST_EXTRA_INI` option disables it in the Swoole PHPUnit runner. - The `phpversion()` subprocess explicitly starts with `datadog.remote_config_enabled=0`. Both changes are necessary because either process can independently generate the error. Remote Config remains enabled in the actual Swoole servers, which continue to use `test-agent:9126`, and genuine server errors are still detected.
This fixes a deterministic incompatibility between the abandoned Sensio Distribution Bundle and the modern Symfony Process version bundled with Composer. Before this commit, Symfony 3.4's Composer hooks called: ```text Sensio\...\ScriptHandler::buildBootstrap Sensio\...\ScriptHandler::installAssets ``` `buildBootstrap()` eventually constructs `new Process($commandString)`. Modern `Symfony\Component\Process\Process` requires an argument array, causing: ```text TypeError: Process::__construct(): Argument 1 must be array, string given ``` This broke all nine Symfony 3.4 jobs on PHP 7.2-7.4. Retrying Composer three times produced the same failure. The commit bypasses only those obsolete wrapper methods and directly runs the equivalent underlying operations: - Build `var/bootstrap.php.cache` using the bundle's own `build_bootstrap.php`. - Install assets into `web` using Symfony's console with the configured relative-symlink behavior. The remaining Sensio callbacks stay unchanged because they do not invoke the incompatible Process API. This does not alter the test's dependencies or application behavior; it changes how the fixture's installation steps are invoked. See `tests/Frameworks/Symfony/Version_3_4/composer.json:31`.
This makes the Laravel 11 and `Latest` fixtures compatible with CI's shared MySQL 5.6 service. Their default migration creates a unique `VARCHAR(255)` email column using `utf8mb4`: ```text 255 * 4 bytes = 1020 bytes ``` MySQL 5.6's InnoDB index limit is 767 bytes, so all 12 affected jobs failed deterministically with error 1071. Setting: ```php Builder::defaultStringLength(191); ``` reduces implicit string columns to at most 764 bytes, allowing the unique index. Existing Laravel 5.7-10 fixtures already use the same 191-character workaround.
This stabilizes a profiler correctness test that intermittently reported
the `[eval]` timeline event as missing.
The profiler had actually captured and submitted the event. In the
failing CI run, eval compilation took 4,156 ns. The analyzer expressed
each event's duration as an integer percentage of the total timeline
duration. If scheduling jitter made the `usleep(1)` idle event
sufficiently long, the eval event's share fell below 1% and was
truncated to 0%.
The test expected 100% +/- 99%, effectively requiring a value of at
least 1%. A real but very short eval event could therefore fail the
assertion:
```text
eval event exists
-> duration is less than 1% of total
-> integer percentage becomes 0%
-> assertion fails
```
The commit changes:
```php
eval('usleep(1);');
```
to:
```php
eval(str_repeat('$unused = 1;', 100) . 'usleep(1);');
```
The larger source string gives the eval compiler enough deterministic
work to keep its timeline duration above the analyzer's 1% floor.
This fixes a polling race in `client_side_stats_trace_filters.phpt`. The test creates seven independent traces: one should pass the configured filters and six should be rejected. With automatic flushing, closing each root span submitted it separately. The sidecar therefore produced one real trace payload and several empty payloads for the rejected traces. The old test stopped as soon as the request replayer returned any `/traces` request: ```text empty /traces request arrives -> polling loop stops -> no span names are extracted -> expected "in traces: op.pass" is missing ``` The passing trace had not been filtered incorrectly. CI's sidecar log showed `op.pass` serialized with sampling priority 1. It could simply arrive in a later batch. Asynchronous sidecar processing and reuse of the same request-replayer session token across suite passes aggravated the race by allowing a late empty request from an earlier pass to be consumed by the next one. The separate stats assertion still passed because it explicitly waited for stats containing `filter-test-service`. The commit makes the test deterministic in two ways: - It disables automatic flushing and explicitly flushes all seven cases together. - It uses `waitForRequest()` with a matcher that ignores empty `/traces` requests and waits for a payload containing spans. The assertion remains unchanged: only `op.pass` may appear in traces and stats.
This fixes a race in `agent_sampling_sidecar.phpt`, which verifies that sampling rates returned by the agent change trace priorities from 1 -> 0 -> 1. The test originally connected to the shared subprocess sidecar. At PHP startup, the tracer mapped the existing sampling configuration from `/dev/shm`. If that configuration remained unused for 50 seconds, the sidecar could delete it and later create a new file under the same name. This produced two different views: - The test's `file_get_contents()` saw the newly created file and its synchronization marker. - The tracer retained its mapping of the deleted, obsolete file. Consequently, the test believed the new rate had arrived, but the tracer still used the old or default rate. CI then reported: ```text Expected: Generic sampling: 0 Actual: Generic sampling: 1 ``` The commit sets: ```text DD_TRACE_SIDECAR_CONNECTION_MODE=thread ``` This gives each PHPT process its own sidecar and sampling state. A test invocation can no longer inherit a shared-memory segment left by an earlier process or have another process's sidecar activity replace it. It also changes logging to `warn` and removes informational flush messages from `EXPECTF`. Those messages are asynchronous sidecar lifecycle output whose timing is unrelated to the sampling behavior under test. Requiring their exact presence and ordering introduced another source of flakiness. The meaningful 1 -> 0 -> 1 assertions remain. The failure was reproduced locally using the sidecar's real 50-second eviction behavior. The old process mapping was visibly marked `(deleted)`. This is a test-isolation fix. It does not repair the underlying libdatadog behavior whereby an existing reader can retain a mapping after the sidecar deletes and recreates the corresponding shared-memory object.
This fixes intermittent FrankenPHP failures caused by the PHPUnit runner
using the wrong agent address.
The actual FrankenPHP server processes were already configured to use
the CI test agent at `test-agent:9126`. The PHP process running PHPUnit
itself was not. Because ddtrace was loaded in that parent process, it
created its own sidecar Remote Config target using the default endpoint:
```text
http://localhost:8126/v0.7/config
```
Nothing was listening there. The shared sidecar distributes its log
messages among registered clients, so the parent's periodic Remote
Config connection errors could appear in the FrankenPHP server's log.
`WebServer::checkErrors()` then interpreted them as server failures even
though the FrankenPHP requests and trace assertions were successful.
The failure was timing-dependent because Remote Config polls
periodically. A run failed only when one of those errors landed inside
the portion of the log examined by a test's teardown.
The commit supplies the valid test-agent address to the entire PHPUnit
invocation:
```make
$(eval TEST_EXTRA_ENV=DD_TRACE_AGENT_PORT=9126 \
DD_AGENT_HOST=test-agent)
```
`run_tests_debug` adds this environment to the parent PHP process, while
the spawned FrankenPHP processes continue using the same endpoint. No
errors are ignored and no test assertions are weakened; the source of
the errors is removed.
This fixes a timing race in the installed-package verification script. The verifier exercises PHP through CLI, PHP-FPM/nginx, and Apache, then asks request-replayer whether it received a trace. Previously it used fixed delays: - One second for CLI. - Two seconds for FPM and Apache. The CLI delay was exactly equal to `DD_TRACE_AGENT_FLUSH_INTERVAL=1000`. That left no allowance for sidecar scheduling or HTTP delivery after the timer expired. The package could work correctly while the verifier queried the replayer just before the trace arrived. This was reproduced naturally with the exact Alpine 3.21 CI package. Attempt 42 failed because the replay response contained telemetry but no trace. The matching trace arrived only 1.47 milliseconds after the failed query, proving that the trace was delayed rather than lost. The commit replaces all three fixed sleeps with `wait_for_trace()`. It polls once per second for up to ten seconds and succeeds only when the replay response contains `trace_id`. If no trace arrives within that bound, verification still fails and prints the final response. This checks the condition the test actually cares about instead of assuming a particular delivery time. It also applies the same behavior consistently to CLI, FPM/nginx, and Apache.
Append Remote Config requests without decoding and rewriting the full history. Drain the valid JSON array atomically so concurrent requests are recorded separately, and finish cleanup when replay clients disconnect.
Give the nginx master time to reap its worker before Symfony Process escalates to SIGKILL. Otherwise a stale worker can retain the listener and old document root, routing later FastCGI tests to the wrong app. Also close the inherited phpunit job-output descriptor so a stray server cannot keep the outer log pipeline open after the test process exits.
Increase the alternate signal stack so crash logging cannot exhaust it. Wait for complete telemetry helper shutdown before clearing its output, and detect legacy PHP CLI server readiness through TCP instead of its buffered startup banner.
Map GitHub 5xx and transport failures to exit 75 for GitLab. Preserve real build failures and missing archives without retrying.
cataphract
force-pushed
the
glopes/ci-misc-no-libdatadog
branch
from
September 23, 2026 15:07
56658dd to
7646f7f
Compare
Benchmarks [ tracer ]Benchmark execution time: 2026-09-23 20:49:00 Comparing candidate commit 499202d in PR branch Found 1 performance improvements and 2 performance regressions! Performance is the same for 191 metrics, 0 unstable metrics.
|
morrisonlevi
left a comment
morrisonlevi
left a comment
Collaborator
There was a problem hiding this comment.
Overall it looks good to me. The failures I see will be gone when we update libdatadog, which we purposefully moved to another PR. I've checked quite a bit of the changes as well.
Not quite ready to hit approve though, I have a few questions.
2 tasks
bwoebi
approved these changes
Sep 24, 2026
bwoebi
left a comment
bwoebi
left a comment
Collaborator
There was a problem hiding this comment.
Thanks Gustavo for aggregating all these fixes!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
See #4219
Removes changes involving libdatadog and a shutdown crash fix.
Reviewer checklist