-
Notifications
You must be signed in to change notification settings - Fork 29
Add wp profile requests command to monitor HTTP requests
#206
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: swissspidy <[email protected]>
wp profile requests command to monitor HTTP requests
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: swissspidy <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: swissspidy <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
|
@copilot but those parameters are all unused, that can't be it. please check again |
Co-authored-by: swissspidy <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: swissspidy <[email protected]>
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces a new wp profile requests command to monitor HTTP requests during the WordPress load process. The implementation is well-executed, including comprehensive Behat tests, updates to command registration, and the core profiling logic in Profiler.php. The changes correctly utilize WordPress hooks to track requests and thoughtfully handle preempted/mocked scenarios. A small but important fix in Formatter.php prevents PHP warnings with non-numeric data. My main feedback is to address some code duplication in Profiler.php to enhance maintainability. Overall, this is a valuable addition to the profiling command suite.
| if ( 'request' === $this->type && ! is_null( $this->request_start_time ) ) { | ||
| $request_time = microtime( true ) - $this->request_start_time; | ||
| $status = ''; | ||
|
|
||
| // Extract status code from response | ||
| if ( is_wp_error( $response ) ) { | ||
| $status = 'Error'; | ||
| } elseif ( is_array( $response ) && isset( $response['response']['code'] ) ) { | ||
| $status = $response['response']['code']; | ||
| } | ||
|
|
||
| $logger = new Logger( | ||
| array( | ||
| 'method' => $this->request_args['method'], | ||
| 'url' => $this->request_args['url'], | ||
| 'status' => $status, | ||
| ) | ||
| ); | ||
| $logger->time = $request_time; | ||
|
|
||
| $this->loggers[] = $logger; | ||
|
|
||
| // Reset for next request | ||
| $this->request_start_time = null; | ||
| $this->request_args = null; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic inside this if block is very similar to the logic for handling preempted requests in wp_request_begin. To improve maintainability, consider extracting this duplicated code into a private helper method for logging HTTP requests. This would centralize the logic for creating a Logger instance, assigning properties, and adding it to the loggers array.
requests()method in Command.php to display HTTP request metricswp profile requestscommandBug Fix
Fixed PHP warning "A non-numeric value encountered" by checking if values are numeric before attempting to sum them in Formatter.php. String fields like 'method', 'url', and 'status' are now properly skipped when calculating totals.
Original prompt
wp profile requests#129💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.