-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Add time to first byte audit #2126
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
/** | ||
* @license | ||
* Copyright 2017 Google Inc. All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
|
||
'use strict'; | ||
|
||
const Audit = require('./audit'); | ||
const Formatter = require('../report/formatter'); | ||
const URL = require('../lib/url-shim'); | ||
|
||
const TTFB_THRESHOLD = 200; | ||
const TTFB_THRESHOLD_BUFFER = 15; | ||
|
||
class TTFBMetric extends Audit { | ||
/** | ||
* @return {!AuditMeta} | ||
*/ | ||
static get meta() { | ||
return { | ||
category: 'Performance', | ||
name: 'time-to-firstbyte', | ||
description: 'Time To First Byte (TTFB)', | ||
informative: true, | ||
helpText: 'Time To First Byte identifies the time at which your server sends a response.' + | ||
'[Learn more](https://developers.google.com/web/tools/chrome-devtools/network-performance/issues).', | ||
requiredArtifacts: ['networkRecords'] | ||
}; | ||
} | ||
|
||
static caclulateTTFB(record) { | ||
const timing = record._timing; | ||
|
||
return timing.receiveHeadersEnd - timing.sendEnd; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious...do we want sendEnd - sendStart? Seeing Souders and other articles calculate TTFB that way. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've copied these headers from devtools. Not sure which to follow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah this is measuring more like response time than TTFB There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be sweet if DT had the same names as resource timing. Alas. Do we have access to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe we don't have true TTFB in devtools. |
||
} | ||
|
||
/** | ||
* @param {!Artifacts} artifacts | ||
* @return {!AuditResult} | ||
*/ | ||
static audit(artifacts) { | ||
const networkRecords = artifacts.networkRecords[Audit.DEFAULT_PASS]; | ||
const results = []; | ||
const walk = (node) => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. unfortunate that all consumers have to do the walk, I wonder if we should offer a flattened list as a computed artifact too? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. would be easier though as we only need the tree for the critical request chain. should I create an artifact named There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was thinking just expose as a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good to me |
||
const children = Object.keys(node); | ||
|
||
children.forEach(id => { | ||
const child = node[id]; | ||
|
||
const networkRecord = networkRecords.find(record => record._requestId === id); | ||
|
||
if (networkRecord) { | ||
const ttfb = TTFBMetric.caclulateTTFB(networkRecord); | ||
results.push({ | ||
url: URL.getDisplayName(networkRecord._url), | ||
ttfb: `${Math.round(ttfb).toLocaleString()} ms`, | ||
rawTTFB: ttfb | ||
}); | ||
} | ||
|
||
if (child.children) { | ||
walk(child.children); | ||
} | ||
}); | ||
}; | ||
|
||
return artifacts.requestCriticalRequestChains(networkRecords).then(tree => { | ||
walk(tree); | ||
|
||
const recordsOverBudget = results.filter(row => | ||
row.rawTTFB > TTFB_THRESHOLD + TTFB_THRESHOLD_BUFFER); | ||
let displayValue; | ||
|
||
if (recordsOverBudget.length) { | ||
displayValue = recordsOverBudget.length + | ||
` critical request(s) went over the ${TTFB_THRESHOLD} ms threshold`; | ||
} | ||
|
||
return { | ||
rawValue: recordsOverBudget.length === 0, | ||
displayValue, | ||
extendedInfo: { | ||
formatter: Formatter.SUPPORTED_FORMATS.TABLE, | ||
value: { | ||
results, | ||
tableHeadings: { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For the v2 report, you'll want to put this in a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh i'll have a look |
||
url: 'Request URL', | ||
ttfb: 'Time To First Byte', | ||
}, | ||
}, | ||
}, | ||
}; | ||
}); | ||
} | ||
} | ||
|
||
module.exports = TTFBMetric; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,89 @@ | ||
/** | ||
* Copyright 2017 Google Inc. All rights reserved. | ||
* | ||
* Licensed under the Apache License, Version 2.0 (the "License"); | ||
* you may not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, software | ||
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. | ||
*/ | ||
'use strict'; | ||
|
||
const TimeToFirstByte = require('../../audits/time-to-firstbyte.js'); | ||
const assert = require('assert'); | ||
|
||
/* eslint-env mocha */ | ||
describe('Performance: time-to-firstbyte audit', () => { | ||
it('fails when ttfb is higher than 215ms', () => { | ||
const networkRecords = [ | ||
{_url: 'https://google.com/', _requestId: '0', _timing: {receiveHeadersEnd: 500, sendEnd: 200}}, | ||
{_url: 'https://google.com/styles.css', _requestId: '1', _timing: {receiveHeadersEnd: 414, sendEnd: 200}}, | ||
{_url: 'https://google.com/image.jpg', _requestId: '2', _timing: {receiveHeadersEnd: 600, sendEnd: 400}}, | ||
]; | ||
const artifacts = {networkRecords: {defaultPass: networkRecords}}; | ||
|
||
artifacts.requestCriticalRequestChains = () => { | ||
return Promise.resolve( | ||
{ | ||
'0': { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. offering a flattened version would make the tests prettier too :) |
||
request: {url: networkRecords[0]._url}, | ||
children: { | ||
'1': { | ||
request: {url: networkRecords[1]._url}, | ||
children: {}, | ||
} | ||
}, | ||
}, | ||
'1': { | ||
request: {url: networkRecords[2]._url}, | ||
children: {}, | ||
} | ||
} | ||
); | ||
}; | ||
|
||
TimeToFirstByte.audit(artifacts).then(result => { | ||
assert.strictEqual(result.rawValue, false); | ||
assert.ok(result.displayValue.includes('1 request(s)')); | ||
}); | ||
}); | ||
|
||
it('succeeds when no request is under 215ms', () => { | ||
const networkRecords = [ | ||
{_url: 'https://google.com/', _requestId: '0', _timing: {receiveHeadersEnd: 300, sendEnd: 200}}, | ||
{_url: 'https://google.com/styles.css', _requestId: '1', _timing: {receiveHeadersEnd: 414, sendEnd: 200}}, | ||
{_url: 'https://google.com/image.jpg', _requestId: '2', _timing: {receiveHeadersEnd: 600, sendEnd: 400}}, | ||
]; | ||
const artifacts = {networkRecords: {defaultPass: networkRecords}}; | ||
|
||
artifacts.requestCriticalRequestChains = () => { | ||
return Promise.resolve( | ||
{ | ||
'0': { | ||
request: {url: networkRecords[0]._url}, | ||
children: { | ||
'1': { | ||
request: {url: networkRecords[1]._url}, | ||
children: {}, | ||
} | ||
}, | ||
}, | ||
'1': { | ||
request: {url: networkRecords[2]._url}, | ||
children: {}, | ||
} | ||
} | ||
); | ||
}; | ||
|
||
TimeToFirstByte.audit(artifacts).then(result => { | ||
assert.strictEqual(result.rawValue, true); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should probably use a variable threshold since throttling could easily make every request look like a "failure", maybe we determine a threshold based on the minimum TTFB?
either way we should probably just mark as
informative: true
in themeta
and give the top X requests sorted by slownessThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean to go through all requests? Or just sort the critical ones?
🤔 throttling shouldn't mess up the ttfb as it's a server side metric? So what variable are we thinking of? 10%?
TIL:
informative: true
, didn't know we had these options 😛There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well kinda
receiveHeadersEnd - sendEnd
will be at leastRTT/2 + server response
time, right? if we're emulating RTT to 400ms for example everything would start to be flaggedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems right indeed, I believe we're not able to calculate real TTFB. I could do an estimate of a roundtrip as you calculate download time