Skip to content

Allow disabling Server-Timing entirely using PERFLAB_DISABLE_SERVER_TIMING constant#795

Merged
mukeshpanchal27 merged 5 commits intotrunkfrom
enhance/794-allow-disabling-server-timing
Aug 10, 2023
Merged

Allow disabling Server-Timing entirely using PERFLAB_DISABLE_SERVER_TIMING constant#795
mukeshpanchal27 merged 5 commits intotrunkfrom
enhance/794-allow-disabling-server-timing

Conversation

@felixarntz
Copy link
Member

@felixarntz felixarntz commented Aug 8, 2023

Summary

Fixes #794

Note that the branch is based on the branch from #784 since it touches a lot of related code. Once that PR is merged, this one can have its base updated to trunk.

Testing

  1. Set constant PERFLAB_DISABLE_SERVER_TIMING to true in your wp-config.php.
  2. Ensure that Server-Timing header is not present in response.
  3. Ensure that Tools > Server-Timing WP Admin screen is not available.
  4. Ensure that, on a fresh install, the PL plugin's version of the object-cache.php drop-in is not placed.

Checklist

  • PR has either [Focus] or Infrastructure label.
  • PR has a [Type] label.
  • PR has a milestone or the no milestone label.

@felixarntz felixarntz added [Type] Enhancement A suggestion for improvement of an existing feature Infrastructure Issues for the overall performance plugin infrastructure Needs Review labels Aug 8, 2023
@felixarntz felixarntz added this to the PL Plugin 2.6.0 milestone Aug 8, 2023
@felixarntz felixarntz requested a review from JustinyAhin as a code owner August 8, 2023 18:29
westonruter
westonruter reviewed Aug 8, 2023
View reviewed changes
mukeshpanchal27
mukeshpanchal27 requested changes Aug 9, 2023
View reviewed changes
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the PR @felixarntz. The Server-Timing API remains accessible through the handy mini plugin you've crafted: https://gist.github.com/felixarntz/63c05392dbf7d51cc7f8f4a424b1ff39.

For a comprehensive review, here's a rundown of the steps to reproduce the issue:

  1. Introduce define( 'PERFLAB_DISABLE_SERVER_TIMING', true ); in your config.php file.
  2. Verify the deactivation of Server-Timing API.
  3. Implement the Server Timing Extra mini plugin from https://gist.github.com/felixarntz/63c05392dbf7d51cc7f8f4a424b1ff39.
  4. Confirm the successful reactivation of Server-Timing API.

Correct me if i miss anything here. Thanks!

Base automatically changed from add/server-timing-hook-ui to trunk August 9, 2023 18:04
@felixarntz
Copy link
Member Author

felixarntz commented Aug 9, 2023

Thanks @mukeshpanchal27, good catch! In 93afb03 I implemented a fix that ensures that, even when another plugin uses the Server-Timing API, at least the header output is still being suppressed if it is generally disabled.

This way it is ensured there are no fatal errors, but the header won't be sent.

@felixarntz
Copy link
Member Author

felixarntz commented Aug 9, 2023

Noting that there are some unit test failures, however these are also present in trunk and stem from changes in the WordPress 6.3 release. See #796 for a PR to address them.

mukeshpanchal27
mukeshpanchal27 approved these changes Aug 10, 2023
View reviewed changes
Copy link
Member

@mukeshpanchal27 mukeshpanchal27 left a comment

Choose a reason for hiding this comment

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

Thanks @felixarntz for the changes, LGTM!

@mukeshpanchal27 mukeshpanchal27 merged commit 0c84c02 into trunk Aug 10, 2023
@mukeshpanchal27 mukeshpanchal27 deleted the enhance/794-allow-disabling-server-timing branch August 10, 2023 04:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonruter westonruter westonruter approved these changes

@swissspidy swissspidy swissspidy approved these changes

@mukeshpanchal27 mukeshpanchal27 mukeshpanchal27 approved these changes

@JustinyAhin JustinyAhin Awaiting requested review from JustinyAhin

+1 more reviewer

@szepeviktor szepeviktor szepeviktor approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Infrastructure Issues for the overall performance plugin infrastructure [Type] Enhancement A suggestion for improvement of an existing feature

Projects

None yet

Milestone

PL Plugin 2.6.0

Development

Successfully merging this pull request may close these issues.

Server-Timing with wp-before-template is always sent

5 participants

@felixarntz @westonruter @swissspidy @szepeviktor @mukeshpanchal27

Comments