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.

Sign up for GitHub

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

Jump to bottom

Added server timing to REST API response #1206

Merged
merged 13 commits into from May 14, 2024
Merged

Added server timing to REST API response #1206

merged 13 commits into from May 14, 2024

Conversation

ashwinparthasarathi
Copy link
Contributor

@ashwinparthasarathi ashwinparthasarathi commented May 7, 2024 •
edited

Summary

Fixes #1149

Relevant technical choices

Updated the core plugin to add ServerTiming to WP REST API end-points as well

How to test

  1. On the site, visit the REST endpoint http://localhost:8888/wp-json/ with the Dev Tools open on the Network Tab
  2. Record the data in the Timing tab as seen here before and after the change.
  3. The difference is documented as follows,

  • Before the change is introduced,
    before-change-rest-api

  • After the change,
    after-change-rest-api

props: @swissspidy for the code.

@ashwinparthasarathi ashwinparthasarathi marked this pull request as ready for review May 7, 2024 18:29
@github-actions GitHub Actions
Copy link

github-actions bot commented May 7, 2024 •
edited

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: ashwinparthasarathi <ashwinparthasarathi@git.wordpress.org>
Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: mukeshpanchal27 <mukesh27@git.wordpress.org>
Co-authored-by: joemcgill <joemcgill@git.wordpress.org>
Co-authored-by: adamsilverstein <adamsilverstein@git.wordpress.org>
Co-authored-by: swissspidy <swissspidy@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

westonruter
westonruter requested changes May 7, 2024
View reviewed changes
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
ashwinparthasarathi and others added 5 commits May 8, 2024 01:35
@ashwinparthasarathi @westonruter
Update includes/server-timing/hooks.php
d806f93 
Co-authored-by: Weston Ruter <westonruter@google.com>
@ashwinparthasarathi @westonruter
Update includes/server-timing/hooks.php
288cc08 
Co-authored-by: Weston Ruter <westonruter@google.com>
@ashwinparthasarathi
Updated code as per review comments
f695170
@ashwinparthasarathi
Merge branch 'update/add-server-timing-in-rest-api' of github.com:ash…
d45bd02 
…winparthasarathi/performance into update/add-server-timing-in-rest-api
@ashwinparthasarathi @westonruter
Update includes/server-timing/hooks.php
6e3a3a7 
Co-authored-by: Weston Ruter <westonruter@google.com>
@mukeshpanchal27 mukeshpanchal27 added [Type] Enhancement A suggestion for improvement of an existing feature [Focus] Server Response Time Issues related to the Server Response Time priority focus area Performance Lab Plugin Issue relates to work in the Performance Lab Plugin only no milestone PRs that do not have a defined milestone for release labels May 8, 2024
ashwinparthasarathi and others added 2 commits May 8, 2024 11:42
@ashwinparthasarathi @mukeshpanchal27
Update includes/server-timing/hooks.php
c370810 
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
@ashwinparthasarathi @mukeshpanchal27
Update includes/server-timing/hooks.php
fe88513 
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
westonruter
westonruter requested changes May 8, 2024
View reviewed changes
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

Almost there.

includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
joemcgill
joemcgill requested changes May 9, 2024
View reviewed changes
Copy link
Member

@joemcgill joemcgill left a comment

Choose a reason for hiding this comment

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

Thanks! I've left a lot of feedback, but the only real blocking issue I see is the function name being introduced.

includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
includes/server-timing/hooks.php Outdated Show resolved Hide resolved
ashwinparthasarathi
ashwinparthasarathi commented May 12, 2024
View reviewed changes
Copy link
Contributor Author

@ashwinparthasarathi ashwinparthasarathi left a comment

Choose a reason for hiding this comment

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

Changed code based on review comments.

@ashwinparthasarathi @westonruter
Update includes/server-timing/hooks.php
82ffc1a 
Updated header value for Server-Timing

Co-authored-by: Weston Ruter <westonruter@google.com>
adamsilverstein
adamsilverstein approved these changes May 14, 2024
View reviewed changes
Copy link
Member

@adamsilverstein adamsilverstein left a comment

Choose a reason for hiding this comment

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

Nice work!

@westonruter westonruter added this to the performance-lab 3.1.0 milestone May 14, 2024
@westonruter westonruter merged commit 27d81f3 into WordPress:trunk May 14, 2024
15 checks passed
@ashwinparthasarathi ashwinparthasarathi deleted the update/add-server-timing-in-rest-api branch May 15, 2024 12:22
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

@adamsilverstein adamsilverstein adamsilverstein approved these changes

@swissspidy swissspidy Awaiting requested review from swissspidy

@mukeshpanchal27 mukeshpanchal27 Awaiting requested review from mukeshpanchal27

@joemcgill joemcgill Awaiting requested review from joemcgill

Assignees
No one assigned
Labels
[Focus] Server Response Time Issues related to the Server Response Time priority focus area no milestone PRs that do not have a defined milestone for release Performance Lab Plugin Issue relates to work in the Performance Lab Plugin only [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Milestone
performance-lab 3.1.0
Development

Successfully merging this pull request may close these issues.

Server-Timing: Add in REST API
5 participants
@ashwinparthasarathi @westonruter @joemcgill @adamsilverstein @mukeshpanchal27