Handle async requests using Guzzle

[PROFeNoM]

Description

Fixes #2444

TL;DR: Set the curl information as soon as possible, and don't overwrite it.

Shortened Explanation

The current implementation fundamentally assumes that curl_multi_info_read is called after curl_multi_exec's completion, which is not the case in Guzzle, resulting in either:

1. Overwriting the information that was already set on a curl_exec span by dummy information;
2. Setting dummy information on the curl_exec span.

Explanation

The existing implementation fundamentally assumes a specific execution flow, particularly that curl_multi_info_read is invoked after the completion of curl_multi_exec. However, in the context of Guzzle Promises and its tick method (which repeatedly checks only once for completed requests), this assumption is not valid, leading to unintended consequences.

The sequence of operations during a typical execution involves the following flow:

• Repeat
  • Call CurlMultiHandler::tick
  • Call curl_multi_exec from CurlMultiHandler::tick
  • Call CurlMultiHandler::processMessages from CurlMultiHandler::tick
  • Call curl_multi_info_read from CurlMultiHandler::processMessages
    • curl_multi_info_read can initiate the following course of action:
      • Trigger the hook set on curl_multi_info_read from CurlIntegration
      • Remove the handle

Completion and curl_getinfo

• As the completion of curl_multi_exec occurs in CurlMultiHandler::tick under the CurlMultiHandler::execute loop, a final call to CurlMultiHandler::tick is triggered, as the queue might not be empty (array_shift left to do).
  • Therefore, an additional call to the already completed and processed handle may be done again. This handle may or may not lead to a furnished curl_getinfo, depending on whether it was previously read.
• Alternatively, the curl_multi_exec call may complete, but Guzzle can retry handles, hence inducing the creation of an additional curl_multi_exec span. This retry scenario is the root cause of the flakiness observed in the added test.

To address these issues, the proposed changes in this pull request aim to set the curl information as soon as possible, avoiding overwriting or putting dummy information during the execution flow outlined above, i.e.:

• Set the information either from the curl_multi_info_read or curl_multi_exec hook, whichever comes first with the curl information
• Only overwrite the information if no network_destination is set (or the currently set information resulted in unparsable-host), which is theoretically always set when setting the curl attributes.

Notes:

• This PR also adds snapshot testing for non-web requests :)
  • A Memcache test was changed as a result, but I guess it shouldn't matter, considering the test is still valid; it's just another environment?

Reviewer checklist

• Test coverage seems ok.
• Appropriate labels assigned.

[pr-commenter]

Benchmarks

Benchmark execution time: 2024-01-12 09:15:02

Comparing candidate commit b8db1b0 in PR branch alex/tmp/debug2444 with baseline commit b032b1e in branch master.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 40 metrics, 49 unstable metrics.

scenario:PDOBench/benchPDOOverheadWithDBM

• 🟩 execution_time [-15.860µs; -8.615µs] or [-5.088%; -2.764%]

[bwoebi]

Thanks, hopefully we now cover everything :-)