Skip to content

[HttpClient] Add total_time to the response log #58555

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

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

jwage
Copy link
Contributor

@jwage jwage commented Oct 13, 2024

Q A
Branch? 7.2
Bug fix? no
New feature? yes
Deprecations? no
Issues
License MIT

This pull request adds the response total_time to the info log.

  • Thoughts on the formatting? Usually in my app code I pass variables to the 2nd argument of info() to pass context instead of using sprintf, that way I can parse out variables the logs more easily.

@carsonbot carsonbot added this to the 7.2 milestone Oct 13, 2024
@OskarStark OskarStark changed the title [HttpClient] Add total_time to the response log. [HttpClient] Add total_time to the response log Oct 14, 2024
Copy link
Member

@chalasr chalasr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing variables through the log context looks good unless there are serious performance considerations

@nicolas-grekas
Copy link
Member

Thank you @jwage.

@nicolas-grekas nicolas-grekas force-pushed the log-response-total-time branch from f8ab2fc to c8bfb79 Compare October 22, 2024 19:26
@nicolas-grekas nicolas-grekas merged commit b916ed9 into symfony:7.2 Oct 22, 2024
1 check was pending
nicolas-grekas added a commit that referenced this pull request Nov 20, 2024
…s-grekas)

This PR was merged into the 7.2 branch.

Discussion
----------

[HttpClient] Fix computing stats for PUSH with Amp

| Q             | A
| ------------- | ---
| Branch?       | 7.2
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

Tests with PUSH/vulcain fail after #58555.

Computing stats for PUSH could be backported to lower versions, but let's keep things simple and consider this as an improvement, so not for lower branches.

Commits
-------

792b970 [HttpClient] Fix computing stats for PUSH with Amp
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.

7 participants