Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PHP to live debugger system tests #2792

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add PHP to live debugger system tests #2792

wants to merge 3 commits into from

Conversation

bwoebi
Copy link
Contributor

@bwoebi bwoebi commented Jul 23, 2024

Prepare system tests for live debugger in dd-trace-php version 1.4.

I've verified locally that these system tests pass with the artifact from https://output.circle-artifacts.com/output/job/59686ec3-ad57-47e9-8202-09fb9a298447/artifacts/0/dd-library-php-1.4.0+457f890a449fc458b3b2955d3952e56a129c3eae-aarch64-linux-gnu.tar.gz.

This PR fixes a couple mistakes in existing tests, adds a bit more debug logging to the PHP tracer outputs and makes the probes conform to the current debugging schema (PHP looks at the type rather than the name of the probe).

Note that, by design of the RC client in PHP, no probes are received if nothing is running. Hence there are calls to await_probes.php, which actively waits for the probes to be installed, and will also emit diagnostics, which the system-tests can interpret as signal of all probes having been successfully installed.

Further, it's not trivial as in PHP to define routes with wildcards (either you say "all args after first slash are function args" or "route is always static". I've opted to replace a couple slashes in paths by underscores. Not just as nice, but works perfectly fine and is easier.

Reviewer checklist

  • If PR title starts with [<language>], double-check that only <language> is impacted by the change
  • No system-tests internal is modified. Otherwise, I have the approval from R&P team
  • CI is green, or failing jobs are not related to this change (and you are 100% sure about this statement)
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added (or removed)?

@bwoebi bwoebi force-pushed the bob/debugger-php branch 4 times, most recently from 3fadb7b to 063b1a9 Compare July 23, 2024 14:20
@bwoebi bwoebi force-pushed the bob/debugger-php branch 2 times, most recently from 3656217 to ad01acc Compare September 28, 2024 03:48
@bwoebi bwoebi marked this pull request as ready for review September 30, 2024 20:07
@bwoebi bwoebi requested review from a team as code owners September 30, 2024 20:07
@bwoebi bwoebi requested review from taegyunkim, tabgok, nayeem-kamal, smola and ValentinZakharov and removed request for a team September 30, 2024 20:07
@bwoebi
Copy link
Contributor Author

bwoebi commented Sep 30, 2024

This PR is ready for review. It's working against the bob/live-debugger branch in dd-trace-php (which is to be merged tomorrow).

I'd like to highlight in particular the changes to utils/proxy/core.py ... Not having these changes makes PHP fail due to delays of more than 5 seconds (refetch interval) between applying and evaluating in some live debugger system tests.
If these changes are not there the RC client will respond 404, which PHP takes as signal that the RC has been turned off on the agent and will un-apply all configurations...

@robertomonteromiguel
Copy link
Collaborator

I move this PR to draft until we can execute the CI build without failures (until the PHP PR is merged )

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants