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

Add a Performance guide for Server Timing #25575

Merged
merged 7 commits into from
May 17, 2023
Merged

Add a Performance guide for Server Timing #25575

merged 7 commits into from
May 17, 2023

Conversation

Elchi3
Copy link
Member

@Elchi3 Elchi3 commented Mar 23, 2023

Description

Adds a guide page for Server Timing.

Ideally, #25508 lands first because then I can:

  • add this guide to group data
  • update the main performance page to link to this guide

(If it doesn't land first then I will update #25508 to include this page.)

Motivation

Part of openwebdocs/project#62

Additional details

None.

@Elchi3 Elchi3 requested a review from a team as a code owner March 23, 2023 16:14
@Elchi3 Elchi3 requested review from wbamberg and removed request for a team March 23, 2023 16:14
@github-actions github-actions bot added the Content:WebAPI Web API docs label Mar 23, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 23, 2023 •
edited
Loading

Preview URLs

External URLs (1)

URL: /en-US/docs/Web/API/Performance_API/Server_timing
Title: Server timing

(comment last updated: 2023-05-17 10:25:53)

dawei-wang
dawei-wang reviewed Mar 23, 2023
View reviewed changes
Copy link
Contributor

@dawei-wang dawei-wang left a comment

Choose a reason for hiding this comment

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

A couple of grammar suggestions

files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
dawei-wang
dawei-wang reviewed Mar 23, 2023
View reviewed changes
Copy link
Contributor

@dawei-wang dawei-wang left a comment

Choose a reason for hiding this comment

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

linting

files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
@Elchi3 Elchi3 requested a review from a team as a code owner April 10, 2023 09:24
@Elchi3 Elchi3 requested review from dipikabh and removed request for a team April 10, 2023 09:24
wbamberg
wbamberg reviewed Apr 27, 2023
View reviewed changes
Copy link
Collaborator

@wbamberg wbamberg 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.

This is generally great. One optional comment.

It is would be good if https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Server-Timing described the syntax of this header in a helpful way.

I mean it could say something like:

This header contains one or more metrics, separated by commas. Each metric has a name, an optional duration, and an optional description. These components are separated by semi-colons.

The duration component consists of the string "dur", followed by "=", followed by the value, like "dur=23.2".

The description component consists of the string "desc", followed by "=", followed by the value, like "desc=DB lookup".

(say, between the current para 1 and para 2)

Somewhere it might be worth spelling out that these components map to PerformanceServerTiming values like:

  • "name" -> PerformanceServerTiming.name
  • "dur" -> PerformanceServerTiming.duration
  • "desc" -> PerformanceServerTiming.description

files/en-us/web/api/performance_api/server_timing/index.md
Comment on lines +55 to +57
// Logs "cache (Cache Read) duration: 23.2"
// Logs "db () duration: 53"
// Logs "app () duration: 47.2"
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be helpful for this to refer to a particular value for the header. I mean like:

Given a Server-Timing header with the following value:

"Server-Timing": "cache;desc="Cache Read";dur=23.2,db;dur=53,app;dur=47.2

...this code will log:

cache (Cache Read) duration: 23.2
db () duration: 53
app () duration: 47.2

(or something like that).

dipikabh
dipikabh requested changes May 16, 2023
View reviewed changes
Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

Thanks for adding this guide, @Elchi3!

I've added a few edit suggestions
(for some reason, the preview links are not working)

files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/performance_api/server_timing/index.md Outdated Show resolved Hide resolved
@Elchi3 Elchi3 requested a review from a team as a code owner May 17, 2023 10:23
@Elchi3 Elchi3 requested review from teoli2003 and removed request for a team May 17, 2023 10:23
@github-actions github-actions bot added the Content:HTTP HTTP docs label May 17, 2023
@Elchi3
Copy link
Member Author

Elchi3 commented May 17, 2023

Thanks so much for your review @dipikabh and @wbamberg! I applied the fantastic suggestions and also updated the Server-Timing page to be more useful.

dipikabh
dipikabh approved these changes May 17, 2023
View reviewed changes
Copy link
Contributor

@dipikabh dipikabh left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the updates!

wbamberg
wbamberg approved these changes May 17, 2023
View reviewed changes
Copy link
Collaborator

@wbamberg wbamberg left a comment

Choose a reason for hiding this comment

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

👍 thank you @Elchi3 !

@wbamberg wbamberg merged commit 3210ffb into mdn:main May 17, 2023
@Elchi3 Elchi3 deleted the guide-server-timing branch May 18, 2023 10:44
wbamberg added a commit to wbamberg/content that referenced this pull request Jun 1, 2023
@wbamberg
Merge remote-tracking branch 'origin/pwa-main-page' into pwa-main-page
c953f68 
* origin/pwa-main-page: (86 commits)
  Add Prettier to lint command (mdn#20674)
  Remove SVG <discard> docs (mdn#26856)
  Remove docs for <applet> (mdn#26850)
  refactor: migrate media.prod.mdn.mozit.cloud URLs (mdn#26809)
  chore: Remove embedlivesample macro calls with a fourth parameter (mdn#26816)
  Bump @mdn/yari from 2.21.0 to 2.22.0 (mdn#26858)
  WebTransport API (mdn#26529)
  Fix issue 26739: correct beforeunload situation wrt bfcache (mdn#26819)
  add missing colon (mdn#26846)
  Replace reference to time domain with amplitude domain in `AnalyserNode .fftSize` (mdn#26840)
  Fix issue 26048: document security requirements for opening and focus… (mdn#26838)
  Change all CSS spec URLs back to drafts.csswg.org (mdn#26833)
  Synchronize with BCD v5.2.58 (mdn#26829)
  Rename nonexistent globalScope to globalThis in service worker event examples (mdn#26808)
  Small typo fix (mdn#26822)
  removing scrollbar (mdn#26813)
  Fix typo (mdn#26826)
  docs(:not()): Update the syntax and example titles (mdn#26814)
  Add a Performance guide for Server Timing (mdn#25575)
  Fix a summary (mdn#26815)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dawei-wang dawei-wang dawei-wang left review comments

@dipikabh dipikabh dipikabh approved these changes

@wbamberg wbamberg wbamberg approved these changes

@teoli2003 teoli2003 Awaiting requested review from teoli2003 teoli2003 is a code owner automatically assigned from mdn/yari-content-http

Assignees
No one assigned
Labels
Content:HTTP HTTP docs Content:WebAPI Web API docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Elchi3 @wbamberg @dawei-wang @dipikabh