PHP 8.0: Update inward singing job

[joshuabenuck]

Summary

Our inward singing jenkins job needs to be updated to include a PHP 8.0 build of the agent. This ticket is part of issue #32.

Desired Behavior

• Running the top-level inward singing job results in a build and test of a PHP 8.0 version of the agent.
• A separate child job exists for PHP 8.0 (following the pattern of the existing jobs).
• The building and testing occurs on 32-bit libc, 64-bit libc, 64-bit musl, and 64-bit FreeBSD targets.

Pre-reqs and dependencies

• 32-bit and 64-bit libc PHP 8.0 agent builds
• 64-bit musl AMI updates
• 64-bit FreeBSD AMI updates

Additional context

See the MMF doc for more details.

[joshuabenuck]

The inward singing jobs pull from the testing download site. To fully test this, we will need to push a test build to the download site. This, in turn, requires that we can build and publish a PHP 8 capable agent.

Pulling the agent release from the downloads site is the last mile of the prerequisites for this ticket. The majority of the work is in getting an agent build that can be uploaded to the testing site.

The actual changes to create a PHP 8 version of the inward singing job is fairly minimal. There's an environment variable set before the test app is run. This needs to be updated to point to a PHP 8 based docker image name.

Note: Not all of the frameworks tested by the inward singing job may support PHP 8. We may need to adjust which ones are run. Since the 7.4 job runs a framework not run on the other versions, look at it to see how this is accomplished.

[joshuabenuck]

I've started the process of running the tests locally as it allows us to avoid having to wait for the official build to be ready. Here's what I have found so far.

• Running the test apps with PHP_VERSION=8.0-fpm-buster ./run-app <app name>.
• If building from source locally, be sure to modify the app's docker-compose.yml to set PHP_AGENT_MODE, PHP_AGENT_REPO_DIR, and PHP_AGENT_BRANCH, as described in the readme. You will need to place the agent's source in the subdirectory that contains the sample app (such as shared for the laravel app).
• dummy app: seems to work without modification. I get a result parsing error at the end, but that is fairly common on my laptop.
• laravel app: requires upgrading to laravel 6... No version of laravel 5.8.* has support for PHP 8.

[joshuabenuck]

• When running under PHP 8.0 and Laravel 6.., the laravel test app results contain messages such as 8 bytes in 1 blocks are possibly lost in loss record 45 of 7,398 (expected in PHP 7.4+), but they also contains messages like Conditional jump or move depends on uninitialised value(s). These latter messages should be investigated further.
• The slim test app uses slim 3.12. PHP 8.0 support was added in Slim 4.7.
• The slim test app uses Guzzle 6.0. Guzzle didn't add support for PHP 8.0 until Guzzle 7.2.0. This may be what is causing:

Slim Application
Error The application could not run because of the following error:

Details
Type: Error
Message: Undefined constant "GuzzleHttp\IDNA_DEFAULT"
File: /slim/app/vendor/guzzlehttp/guzzle/src/Client.php Line: 218
Trace #0 /slim/app/vendor/guzzlehttp/guzzle/src/Client.php(154): GuzzleHttp\Client->buildUri(Object(GuzzleHttp\Psr7�ri), Array)

• Running the slim test app with Guzzle 7.2.0 and Slim 4.7.0 works, but there's a warning about not being able to preload an unlinked class. Since the primary purpose of this test is to validate preloading, this needs to be investigated further.

[joshuabenuck]

When running a test-app, it is not uncommon on my system for the results parsing to fail with an error on line 41 of XMLHelpers.php with the message Unexpected number of kind elements.

This looks to be caused by a race condition that occurs when locust exits. The shutdown of the locust container doesn't give the php container enough time to finish processing the running transactions. This results in partial xml files in the .valgrind-* directory.

My proposal to fix this is to add a small delay after locust exits. This should give the running transactions enough time to finish writing their results to disk.

Alternatively (or additionally), we can investigate improving the error handling of the valgrind results processor so that it more gracefully handles these errors. We may want to consider this if the delay proves insufficient or we find ourselves wanting to regularly abort a run while still being able to view partial results.

[joshuabenuck]

Closing out this ticket.

• A PR against the internal test-apps repo has been created which makes the necessary updates to the tests so they run against PHP 8.0.
• A PHP 8.0 inward singing job has been created. It is currently configured to pull from the PR with the PHP 8.0 changes and to build against the php8 branch of the agent.
• The uninitialized variables error has been researched. It looks to be one we first encountered with PHP 7.3 (based on the description of a pre-existing valgrind-suppression rule).
• Adding a delay at the end of the test run does not appear to completely eliminate the unexpected number of kind elements error. This delay should be removed as part of the removal of the valgrind-processor.

Work to be done as part of the next MMF:

• Investigate the unlinked class warning in the slim test-app.
• Merge the test-apps PR once the test changes have been finalized.
• Remove the valgrind-processor from the test-apps and switch to a script based approach utilizing raw valgrind output.
• Time permitting, add additional valgrind suppression rules to clean up the test output on PHP 7.4+.
• Update the PHP 8.0 inward singing loop to pull from the main branch of test-apps and to build the agent from the dev branch.