Make Server-Timing header output more robust#736
Conversation
|
@swissspidy Can you share a bit more context? What leads to it being "too late"? I have never encountered the problem, so would like to understand better. |
|
When I run this in WP core‘s Docker environment, the callback in the shutdown hook is never fired, so it‘s not sending the header. I assume it‘s because the shutdown handler is called long after the page has been served. |
|
@swissspidy That's odd. I have been using this in the WP core Docker env several times without problems 🤔 Maybe it's something more specific than that? |
|
IDK 🤷 Maybe I can debug at contributor day or so |
| // phpcs:ignore PHPCompatibility.Constants.NewConstants | ||
| defined( 'PHP_INT_MIN' ) ? PHP_INT_MIN : -1000 |
There was a problem hiding this comment.
Curious why this action was configured to run early. Wouldn't it be better to run as late as possible?
There was a problem hiding this comment.
It is run early based on the idea that the template output was already completed when shutdown runs. If shutdown did a bunch of other things (extremely unlikely but possible), then it would inflate the time in the wp-template metric.
|
Will the removal of the |
|
Any open buffers are automatically flushed when the page response is finished, per docs:
|
felixarntz
left a comment
There was a problem hiding this comment.
Tested this in my core dev environment, and it works as expected, also no change in outcomes. LGTM, thanks for the fix!
Summary
I activated the plugin on my local WP core Docker environment and filtered
perflab_server_timing_use_output_bufferto enable output buffering, yet I did not see theServer-Timingheader.The
shutdownhook is too late to send the header, but using theob_start()callback for this works.Relevant technical choices
Checklist
[Focus]orInfrastructurelabel.[Type]label.no milestonelabel.