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.

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

core(diagnostics): add diagnostic audits #7052

Merged
merged 9 commits into from
Feb 4, 2019
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,21 @@ Object {
Object {
"path": "font-display",
},
Object {
Copy link
Member

Choose a reason for hiding this comment

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

needs to be updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

"path": "diagnostics",
},
Object {
"path": "network-requests",
},
Object {
"path": "network-rtt",
},
Object {
"path": "network-server-latency",
},
Object {
"path": "main-thread-tasks",
},
Object {
"path": "metrics",
},
Expand Down Expand Up @@ -775,6 +787,24 @@ Object {
"id": "network-requests",
"weight": 0,
},
Object {
"group": "diagnostics",
"id": "network-rtt",
"weight": 0,
},
Object {
"group": "diagnostics",
"id": "network-server-latency",
"weight": 0,
},
Object {
"id": "main-thread-tasks",
"weight": 0,
},
Object {
"id": "diagnostics",
"weight": 0,
},
Object {
"id": "metrics",
"weight": 0,
Expand Down
75 changes: 75 additions & 0 deletions lighthouse-core/audits/diagnostics.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**
* @license Copyright 2019 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.js');
const MainThreadTasksComputed = require('../computed/main-thread-tasks.js');
const NetworkRecordsComputed = require('../computed/network-records.js');
const NetworkAnalysisComputed = require('../computed/network-analysis.js');
const NetworkAnalyzer = require('../lib/dependency-graph/simulator/network-analyzer.js');

class Diagnostics extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'diagnostics',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: 'Diagnostics',
description: 'Collection of useful page vitals.',
requiredArtifacts: ['traces', 'devtoolsLogs'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const tasks = await MainThreadTasksComputed.request(trace, context);
const records = await NetworkRecordsComputed.request(devtoolsLog, context);
const analysis = await NetworkAnalysisComputed.request(devtoolsLog, context);

const toplevelTasks = tasks.filter(t => !t.parent);
const mainDocumentTransferSize = NetworkAnalyzer.findMainDocument(records).transferSize;
const totalByteWeight = records.reduce((sum, r) => sum + (r.transferSize || 0), 0);
const totalTaskTime = toplevelTasks.reduce((sum, t) => sum + (t.duration || 0), 0);
const maxRtt = Math.max(...analysis.additionalRttByOrigin.values()) + analysis.rtt;
const maxServerLatency = Math.max(...analysis.serverResponseTimeByOrigin.values());

const diagnostics = {
numRequests: records.length,
numScripts: records.filter(r => r.resourceType === 'Script').length,
numStylesheets: records.filter(r => r.resourceType === 'Stylesheet').length,
numFonts: records.filter(r => r.resourceType === 'Font').length,
numTasks: toplevelTasks.length,
numTasksOver10ms: toplevelTasks.filter(t => t.duration > 10).length,
numTasksOver25ms: toplevelTasks.filter(t => t.duration > 25).length,
Copy link
Member

Choose a reason for hiding this comment

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

no numTasksOver50ms ? 😮

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

heh, I'll add it, for lantern I was treating numTasksOver10ms as the rough equivalent :)

numTasksOver50ms: toplevelTasks.filter(t => t.duration > 50).length,
numTasksOver100ms: toplevelTasks.filter(t => t.duration > 100).length,
numTasksOver500ms: toplevelTasks.filter(t => t.duration > 500).length,
rtt: analysis.rtt,
throughput: analysis.throughput,
maxRtt,
maxServerLatency,
totalByteWeight,
totalTaskTime,
mainDocumentTransferSize,
};

return {
score: 1,
rawValue: 1,
details: {items: [diagnostics]},
Copy link
Member

Choose a reason for hiding this comment

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

Just a note probably for another PR, but this format is weird for informational audits. Can we make a new details format for just info? Like, just put the fields directly in details? Or if ts won't like that:

details: {
   ...
   information: {
      key: value,
      ...
   },
   ...
}

The array makes it weird 😦

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah agreed, it's the same as the metrics audit for now

};
}
}

module.exports = Diagnostics;
60 changes: 60 additions & 0 deletions lighthouse-core/audits/main-thread-tasks.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/**
* @license Copyright 2019 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.js');
const MainThreadTasksComputed = require('../computed/main-thread-tasks.js');

class MainThreadTasks extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'main-thread-tasks',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: 'Tasks',
description: 'Lists the toplevel main thread tasks that executed during page load.',
requiredArtifacts: ['traces'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const trace = artifacts.traces[Audit.DEFAULT_PASS];
const tasks = await MainThreadTasksComputed.request(trace, context);

const results = tasks
.filter(task => task.duration > 5 && task.parent)
.map(task => {
return {
type: task.group.id,
duration: task.duration,
startTime: task.startTime,
};
});

const headings = [
{key: 'type', itemType: 'text', text: 'Task Type'},
{key: 'startTime', itemType: 'ms', granularity: 1, text: 'Start Time'},
{key: 'duration', itemType: 'ms', granularity: 1, text: 'End Time'},
];

const tableDetails = Audit.makeTableDetails(headings, results);

return {
score: 1,
rawValue: results.length,
details: tableDetails,
};
}
}

module.exports = MainThreadTasks;
81 changes: 81 additions & 0 deletions lighthouse-core/audits/network-rtt.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* @license Copyright 2019 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');
Copy link
Member

Choose a reason for hiding this comment

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

.js

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

const i18n = require('../lib/i18n/i18n.js');
const NetworkAnalysisComputed = require('../computed/network-analysis.js');

const UIStrings = {
/** Descriptive title of a Lighthouse audit that tells the user the round trip times to each origin the page connected to. This is displayed in a list of audit titles that Lighthouse generates. */
title: 'Network Round Trip Times',
/** Description of a Lighthouse audit that tells the user that a high network round trip time (RTT) can effect their website's performance because the server is physically far away from them thus making the RTT high. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
description: 'Network round trip times (RTT) have a large impact on performance. ' +
'If the RTT to an origin is high, it\'s an indication that servers closer to the user could ' +
'improve performance. [Learn more](https://hpbn.co/primer-on-latency-and-bandwidth/).',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class NetworkRTT extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'network-rtt',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: str_(UIStrings.title),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const analysis = await NetworkAnalysisComputed.request(devtoolsLog, context);

/** @type {number} */
let maxRtt = 0;
const baseRtt = analysis.rtt;
/** @type {Array<{origin: string, rtt: number}>} */
const results = [];
for (const [origin, additionalRtt] of analysis.additionalRttByOrigin.entries()) {
// TODO: https://github.com/GoogleChrome/lighthouse/issues/7041
if (!Number.isFinite(additionalRtt)) continue;
Copy link
Member

Choose a reason for hiding this comment

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

seems important to include these if using them for debugging/anomaly spotting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, done

// Ignore entries that don't look like real origins, like the __SUMMARY__ entry.
if (!origin.startsWith('http')) continue;

const rtt = additionalRtt + baseRtt;
results.push({origin, rtt});
maxRtt = Math.max(rtt, maxRtt);
}

results.sort((a, b) => b.rtt - a.rtt);

const headings = [
{key: 'origin', itemType: 'text', text: str_(i18n.UIStrings.columnURL)},
{key: 'rtt', itemType: 'ms', granularity: 1, text: str_(i18n.UIStrings.columnTimeSpent)},
];

const tableDetails = Audit.makeTableDetails(headings, results);

return {
score: 1,
rawValue: maxRtt,
displayValue: str_(i18n.UIStrings.ms, {timeInMs: maxRtt}),
details: tableDetails,
};
}
}

module.exports = NetworkRTT;
module.exports.UIStrings = UIStrings;
78 changes: 78 additions & 0 deletions lighthouse-core/audits/network-server-latency.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/**
* @license Copyright 2019 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 i18n = require('../lib/i18n/i18n.js');
const NetworkAnalysisComputed = require('../computed/network-analysis.js');

const UIStrings = {
/** Descriptive title of a Lighthouse audit that tells the user the server latencies observed from each origin the page connected to. This is displayed in a list of audit titles that Lighthouse generates. */
title: 'Server Backend Latencies',
/** Description of a Lighthouse audit that tells the user that server latency can effect their website's performance negatively. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
description: 'Server latencies can impact web performance. ' +
'If the server latency of an origin is high, it\'s an indication the server is overloaded ' +
Copy link
Member

Choose a reason for hiding this comment

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

I think this description should also indicate how the RTT is factored in. Are these values JUST the backend latency or the BL + RTT?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it's our best guess for "just the backend latency"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure how to put this without unnecessarily confusing folks. I'm not confident that the number of people who will understand this subtlety is very high (or the people that will know what to do with this audit, honestly)

Copy link
Member

Choose a reason for hiding this comment

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

k. well given the updated title change i think it's good.

Copy link
Member

Choose a reason for hiding this comment

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

or the people that will know what to do with this audit, honestly

seems like the audit should always be hidden, then?

'or has poor backend performance. [Learn more](https://hpbn.co/primer-on-web-performance/#analyzing-the-resource-waterfall).',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class NetworkServerLatency extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'network-server-latency',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: str_(UIStrings.title),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const analysis = await NetworkAnalysisComputed.request(devtoolsLog, context);

/** @type {number} */
let maxLatency = 0;
/** @type {Array<{origin: string, serverReponseTime: number}>} */
const results = [];
for (const [origin, serverReponseTime] of analysis.serverResponseTimeByOrigin.entries()) {
// Ignore entries that don't look like real origins, like the __SUMMARY__ entry.
if (!origin.startsWith('http')) continue;

maxLatency = Math.max(serverReponseTime, maxLatency);
results.push({origin, serverReponseTime});
}

results.sort((a, b) => b.serverReponseTime - a.serverReponseTime);

const headings = [
{key: 'origin', itemType: 'text', text: str_(i18n.UIStrings.columnURL)},
{key: 'serverReponseTime', itemType: 'ms', granularity: 1,
text: str_(i18n.UIStrings.columnTimeSpent)},
];

const tableDetails = Audit.makeTableDetails(headings, results);

return {
score: Math.max(1 - (maxLatency / 500), 0),
Copy link
Member

Choose a reason for hiding this comment

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

fwiw, if it's informative this gets nulled out

Copy link
Collaborator Author

@patrickhulce patrickhulce Jan 31, 2019

Choose a reason for hiding this comment

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

removed

I played around with it being scored, but that felt like it put too much emphasis on it

rawValue: maxLatency,
displayValue: str_(i18n.UIStrings.ms, {timeInMs: maxLatency}),
details: tableDetails,
};
}
}

module.exports = NetworkServerLatency;
module.exports.UIStrings = UIStrings;
8 changes: 8 additions & 0 deletions lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,11 @@ const defaultConfig = {
'uses-rel-preload',
'uses-rel-preconnect',
'font-display',
'diagnostics',
'network-requests',
'network-rtt',
'network-server-latency',
'main-thread-tasks',
'metrics',
'offline-start-url',
'manual/pwa-cross-browser',
Expand Down Expand Up @@ -349,6 +353,10 @@ const defaultConfig = {
{id: 'dom-size', weight: 0, group: 'diagnostics'},
{id: 'critical-request-chains', weight: 0, group: 'diagnostics'},
{id: 'network-requests', weight: 0},
{id: 'network-rtt', weight: 0},
{id: 'network-server-latency', weight: 0},
{id: 'main-thread-tasks', weight: 0},
{id: 'diagnostics', weight: 0},
Copy link
Member

Choose a reason for hiding this comment

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

can we move these audits down to the bottom and add a comment? Kind of subtle that no group in the performance category means they're not visible :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

{id: 'metrics', weight: 0},
{id: 'user-timings', weight: 0, group: 'diagnostics'},
{id: 'bootup-time', weight: 0, group: 'diagnostics'},
Expand Down
16 changes: 16 additions & 0 deletions lighthouse-core/lib/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -715,6 +715,22 @@
"message": "Speed Index",
"description": "The name of the metric that summarizes how quickly the page looked visually complete. The name of this metric is largely abstract and can be loosely translated. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit."
},
"lighthouse-core/audits/network-rtt.js | description": {
"message": "Network round trip times (RTT) have a large impact on performance. If the RTT to an origin is high, it's an indication that servers closer to the user could improve performance. [Learn more](https://hpbn.co/primer-on-latency-and-bandwidth/).",
"description": "Description of a Lighthouse audit that tells the user that a high network round trip time (RTT) can effect their website's performance because the server is physically far away from them thus making the RTT high. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation."
},
"lighthouse-core/audits/network-rtt.js | title": {
"message": "Network Round Trip Times",
"description": "Descriptive title of a Lighthouse audit that tells the user the round trip times to each origin the page connected to. This is displayed in a list of audit titles that Lighthouse generates."
},
"lighthouse-core/audits/network-server-latency.js | description": {
"message": "Server latencies can impact web performance. If the server latency of an origin is high, it's an indication the server is overloaded or has poor backend performance. [Learn more](https://hpbn.co/primer-on-web-performance/#analyzing-the-resource-waterfall).",
"description": "Description of a Lighthouse audit that tells the user that server latency can effect their website's performance negatively. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation."
},
"lighthouse-core/audits/network-server-latency.js | title": {
"message": "Server Backend Latencies",
"description": "Descriptive title of a Lighthouse audit that tells the user the server latencies observed from each origin the page connected to. This is displayed in a list of audit titles that Lighthouse generates."
},
"lighthouse-core/audits/redirects.js | description": {
"message": "Redirects introduce additional delays before the page can be loaded. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/redirects).",
"description": "Description of a Lighthouse audit that tells users why they should reduce the number of server-side redirects on their page. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation."
Expand Down
Loading