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(fr): collect OOPIF network data #12992

Merged
merged 13 commits into from Sep 8, 2021
27 changes: 15 additions & 12 deletions lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js
Expand Up @@ -43,18 +43,21 @@ async function runLighthouse(url, configJson, testRunnerOptions = {}) {
const port = launchedChrome.port;
const connection = new ChromeProtocol(port);

// Run Lighthouse.
const logLevel = testRunnerOptions.isDebug ? 'info' : undefined;
const runnerResult = await lighthouse(url, {port, logLevel}, configJson, connection);
if (!runnerResult) throw new Error('No runnerResult');

// Clean up and return results.
await launchedChrome.kill();
return {
lhr: runnerResult.lhr,
artifacts: runnerResult.artifacts,
log: '', // TODO: if want to run in parallel, need to capture lighthouse-logger output.
};
try {
// Run Lighthouse.
const logLevel = testRunnerOptions.isDebug ? 'info' : undefined;
const runnerResult = await lighthouse(url, {port, logLevel}, configJson, connection);
if (!runnerResult) throw new Error('No runnerResult');

return {
lhr: runnerResult.lhr,
artifacts: runnerResult.artifacts,
log: '', // TODO: if want to run in parallel, need to capture lighthouse-logger output.
};
} finally {
// Clean up and return results.
await launchedChrome.kill();
}
}

module.exports = {
Expand Down
15 changes: 12 additions & 3 deletions lighthouse-cli/test/smokehouse/report-assert.js
Expand Up @@ -157,9 +157,11 @@ function makeComparison(name, actualResult, expectedResult) {
* @param {LocalConsole} localConsole
* @param {LH.Result} lhr
* @param {Smokehouse.ExpectedRunnerResult} expected
* @param {{isBundled?: boolean}=} reportOptions
*/
function pruneExpectations(localConsole, lhr, expected) {
function pruneExpectations(localConsole, lhr, expected, reportOptions) {
const isFraggleRock = lhr.configSettings.channel === 'fraggle-rock-cli';
const isBundled = reportOptions && reportOptions.isBundled;

/**
* Lazily compute the Chrome version because some reports are explicitly asserting error conditions.
Expand Down Expand Up @@ -211,13 +213,20 @@ function pruneExpectations(localConsole, lhr, expected) {
`Actual channel: ${lhr.configSettings.channel}`,
].join(' '));
delete obj[key];
} else if (value._skipInBundled && !isBundled) {
localConsole.log([
`[${key}] marked as skip in bundled and runner is bundled, pruning expectation:`,
JSON.stringify(value, null, 2),
].join(' '));
delete obj[key];
} else {
pruneRecursively(value);
}
}

delete obj._legacyOnly;
delete obj._fraggleRockOnly;
delete obj._skipInBundled;
delete obj._minChromiumMilestone;
delete obj._maxChromiumMilestone;
}
Expand Down Expand Up @@ -358,13 +367,13 @@ function reportAssertion(localConsole, assertion) {
* summary. Returns count of passed and failed tests.
* @param {{lhr: LH.Result, artifacts: LH.Artifacts, networkRequests?: string[]}} actual
* @param {Smokehouse.ExpectedRunnerResult} expected
* @param {{isDebug?: boolean}=} reportOptions
* @param {{isDebug?: boolean, isBundled?: boolean}=} reportOptions
* @return {{passed: number, failed: number, log: string}}
*/
function report(actual, expected, reportOptions = {}) {
const localConsole = new LocalConsole();

expected = pruneExpectations(localConsole, actual.lhr, expected);
expected = pruneExpectations(localConsole, actual.lhr, expected, reportOptions);
const comparisons = collateResults(localConsole, actual, expected);

let correctCount = 0;
Expand Down
Expand Up @@ -11,6 +11,16 @@
*/
module.exports = {
extends: 'lighthouse:default',
categories: {
performance: {
title: 'Performance',
auditRefs: [{id: 'iframe-elements', weight: 0}],
},
},
audits: [
// Include an audit that *forces* the IFrameElements artifact to be used for our test.
{path: 'oopif-iframe-audit'},
],
settings: {
// This test runs in CI and hits the outside network of a live site.
// Be a little more forgiving on how long it takes all network requests of several nested iframes
Expand Down
29 changes: 29 additions & 0 deletions lighthouse-core/audits/oopif-iframe-audit.js
@@ -0,0 +1,29 @@
/**
* @license Copyright 2021 The Lighthouse Authors. 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';

/**
* @fileoverview This is a fake audit used exclusively in smoke tests to force inclusion of IFrameElements artifact.
* It is included here for complex reasons in the way the bundled smoketests work.
*
* The smokehouse configs are evaluated first in the node CLI side (which requires an absolute path using LH_ROOT).
* The smokehouse configs are then *re-evaluated* in the bundled context for execution by Lighthouse (which *cannot* use an absolute path using LH_ROOT).
*
* This mismatch in environment demands that the audit path in the config must be context-aware,
* yet the require-graph for the config is included before even the CLI knows that it will be using
* a bundled runner. Rather than force a massive smoketest architecture change, we include a harmless,
* test-only audit in our core list instead.
*/

module.exports = {
meta: {
id: 'iframe-elements',
title: 'IFrame Elements',
description: 'Audit to force the inclusion of IFrameElements artifact',
requiredArtifacts: ['IFrameElements'],
},
audit: () => ({score: 1}),
};
3 changes: 3 additions & 0 deletions lighthouse-core/fraggle-rock/gather/driver.js
Expand Up @@ -16,6 +16,7 @@ const throwNotConnectedFn = () => {

/** @type {LH.Gatherer.FRProtocolSession} */
const defaultSession = {
setTargetInfo: throwNotConnectedFn,
hasNextProtocolTimeout: throwNotConnectedFn,
getNextProtocolTimeout: throwNotConnectedFn,
setNextProtocolTimeout: throwNotConnectedFn,
Expand All @@ -24,6 +25,8 @@ const defaultSession = {
off: throwNotConnectedFn,
addProtocolMessageListener: throwNotConnectedFn,
removeProtocolMessageListener: throwNotConnectedFn,
addSessionAttachedListener: throwNotConnectedFn,
removeSessionAttachedListener: throwNotConnectedFn,
sendCommand: throwNotConnectedFn,
};

Expand Down
36 changes: 35 additions & 1 deletion lighthouse-core/fraggle-rock/gather/session.js
Expand Up @@ -18,22 +18,35 @@ class ProtocolSession {
*/
constructor(session) {
this._session = session;
/** @type {LH.Crdp.Target.TargetInfo|undefined} */
this._targetInfo = undefined;
/** @type {number|undefined} */
this._nextProtocolTimeout = undefined;
/** @type {WeakMap<any, any>} */
this._callbackMap = new WeakMap();

// FIXME: Monkeypatch puppeteer to be able to listen to *all* protocol events.
// This patched method will now emit a copy of every event on `*`.
const originalEmit = session.emit;
// @ts-expect-error - Test for the monkeypatch.
if (originalEmit[SessionEmitMonkeypatch]) return;
session.emit = (method, ...args) => {
originalEmit.call(session, '*', {method, params: args[0]});
// OOPIF sessions need to emit their sessionId so downstream processors can recognize
// the target the event came from.
const sessionId = this._targetInfo && this._targetInfo.type === 'iframe' ?
this._targetInfo.targetId : undefined;
originalEmit.call(session, '*', {method, params: args[0], sessionId});
return originalEmit.call(session, method, ...args);
};
// @ts-expect-error - It's monkeypatching 🤷‍♂️.
session.emit[SessionEmitMonkeypatch] = true;
}

/** @param {LH.Crdp.Target.TargetInfo} targetInfo */
setTargetInfo(targetInfo) {
this._targetInfo = targetInfo;
}

/**
* @return {boolean}
*/
Expand Down Expand Up @@ -75,6 +88,27 @@ class ProtocolSession {
this._session.once(eventName, /** @type {*} */ (callback));
}

/**
* Bind to the puppeteer `sessionattached` listener and return an LH ProtocolSession.
* @param {(session: ProtocolSession) => void} callback
*/
addSessionAttachedListener(callback) {
/** @param {import('puppeteer').CDPSession} session */
const listener = session => callback(new ProtocolSession(session));
this._callbackMap.set(callback, listener);
this._session.connection().on('sessionattached', listener);
}

/**
* Unbind to the puppeteer `sessionattached` listener.
* @param {(session: ProtocolSession) => void} callback
*/
removeSessionAttachedListener(callback) {
const listener = this._callbackMap.get(callback);
if (!listener) return;
this._session.connection().off('sessionattached', listener);
}

/**
* Bind to our custom event that fires for *any* protocol event.
* @param {(payload: LH.Protocol.RawEventMessage) => void} callback
Expand Down
12 changes: 9 additions & 3 deletions lighthouse-core/gather/devtools-log.js
Expand Up @@ -46,9 +46,15 @@ class DevtoolsLog {
* @param {LH.Protocol.RawEventMessage} message
*/
record(message) {
if (this._isRecording && (!this._filter || this._filter.test(message.method))) {
this._messages.push(message);
}
// We're not recording, skip the rest of the checks.
if (!this._isRecording) return;
// The event was likely an internal puppeteer method that uses Symbols.
if (typeof message.method !== 'string') return;
// The event didn't pass our filter, do not record it.
if (this._filter && !this._filter.test(message.method)) return;

// We passed all the checks, record the message.
this._messages.push(message);
}
}

Expand Down
15 changes: 15 additions & 0 deletions lighthouse-core/gather/driver.js
Expand Up @@ -198,6 +198,21 @@ class Driver {
this._connection.off('protocolevent', callback);
}

/** @param {LH.Crdp.Target.TargetInfo} targetInfo */
setTargetInfo(targetInfo) { // eslint-disable-line no-unused-vars
// OOPIF handling in legacy driver is implicit.
}

/** @param {(session: LH.Gatherer.FRProtocolSession) => void} callback */
addSessionAttachedListener(callback) { // eslint-disable-line no-unused-vars
// OOPIF handling in legacy driver is implicit.
}

/** @param {(session: LH.Gatherer.FRProtocolSession) => void} callback */
removeSessionAttachedListener(callback) { // eslint-disable-line no-unused-vars
// OOPIF handling in legacy driver is implicit.
}

/**
* Debounce enabling or disabling domains to prevent driver users from
* stomping on each other. Maintains an internal count of the times a domain
Expand Down
57 changes: 53 additions & 4 deletions lighthouse-core/gather/driver/network-monitor.js
Expand Up @@ -12,9 +12,11 @@

const log = require('lighthouse-logger');
const {EventEmitter} = require('events');
const MessageLog = require('../devtools-log.js');
const NetworkRecorder = require('../../lib/network-recorder.js');
const NetworkRequest = require('../../lib/network-request.js');
const URL = require('../../lib/url-shim.js');
const TargetManager = require('./target-manager.js');

/** @typedef {import('../../lib/network-recorder.js').NetworkRecorderEvent} NetworkRecorderEvent */
/** @typedef {'network-2-idle'|'network-critical-idle'|'networkidle'|'networkbusy'|'network-critical-busy'|'network-2-busy'} NetworkMonitorEvent_ */
Expand All @@ -23,6 +25,8 @@ const URL = require('../../lib/url-shim.js');
class NetworkMonitor extends EventEmitter {
/** @type {NetworkRecorder|undefined} */
_networkRecorder = undefined;
/** @type {TargetManager|undefined} */
_targetManager = undefined;
/** @type {Array<LH.Crdp.Page.Frame>} */
_frameNavigations = [];

Expand All @@ -31,11 +35,19 @@ class NetworkMonitor extends EventEmitter {
super();
this._session = session;

this._messageLog = new MessageLog(/^(Page|Network)\./);
Copy link
Member

Choose a reason for hiding this comment

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

file naming may be off now. network-monitor now doing devtools-log duties (recording Page events too). devtools-monitor?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the page events are largely network related (lifecycle networkidle) and aren't even used, so I wasn't very concerned about the extra bit. if folks feel it's a candidate for confusion, I can emit the messages and have the gatherer merge with page events instead, but that extra level of indirection felt unnecessary

Copy link
Member

Choose a reason for hiding this comment

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

The restructuring seems totally fine, I was just talking about naming.

the page events are largely network related (lifecycle networkidle) and aren't even used

but now DevtoolsLog the Gatherer gets the devtools log from NetworkMonitor. A rename might be in order?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

gotcha 👍 protocol-monitor SGTM

Copy link
Member

Choose a reason for hiding this comment

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

gotcha 👍 protocol-monitor SGTM

lol I take it you changed your mind? :)

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 I got in there and it just didn't feel right, everything is so network-focused.

just reemitting the messages and letting the log continue to be captured by the gatherer :) (also bonus we get strict event listener types for the monitor now)


this._onTargetAttached = this._onTargetAttached.bind(this);

/** @type {Map<string, LH.Gatherer.FRProtocolSession>} */
this._sessions = new Map();

/** @param {LH.Crdp.Page.FrameNavigatedEvent} event */
this._onFrameNavigated = event => this._frameNavigations.push(event.frame);

/** @param {LH.Protocol.RawEventMessage} event */
this._onProtocolMessage = event => {
this._messageLog.record(event);
if (!this._networkRecorder) return;
this._networkRecorder.dispatch(event);
};
Expand All @@ -49,14 +61,28 @@ class NetworkMonitor extends EventEmitter {
this.off = (event, listener) => super.off(event, listener);
}

/**
* @param {{target: {targetId: string}, session: LH.Gatherer.FRProtocolSession}} session
*/
async _onTargetAttached({session, target}) {
const targetId = target.targetId;

this._sessions.set(targetId, session);
session.addProtocolMessageListener(this._onProtocolMessage);

await session.sendCommand('Network.enable');
}

/**
* @return {Promise<void>}
*/
async enable() {
if (this._networkRecorder) return;
if (this._targetManager) return;

this._frameNavigations = [];
this._sessions = new Map();
this._networkRecorder = new NetworkRecorder();
this._targetManager = new TargetManager(this._session);

/**
* Reemit the same network recorder events.
Expand All @@ -71,22 +97,38 @@ class NetworkMonitor extends EventEmitter {
this._networkRecorder.on('requeststarted', reEmit('requeststarted'));
this._networkRecorder.on('requestloaded', reEmit('requestloaded'));

this._messageLog.reset();
this._messageLog.beginRecording();

this._session.on('Page.frameNavigated', this._onFrameNavigated);
this._session.addProtocolMessageListener(this._onProtocolMessage);
this._targetManager.addTargetAttachedListener(this._onTargetAttached);

await this._session.sendCommand('Page.enable');
await this._session.sendCommand('Network.enable');
await this._targetManager.enable();
}

/**
* @return {Promise<void>}
*/
async disable() {
if (!this._targetManager) return;

this._messageLog.reset();
this._messageLog.endRecording();

this._session.off('Page.frameNavigated', this._onFrameNavigated);
this._session.removeProtocolMessageListener(this._onProtocolMessage);
this._targetManager.removeTargetAttachedListener(this._onTargetAttached);

for (const session of this._sessions.values()) {
session.removeProtocolMessageListener(this._onProtocolMessage);
}

await this._targetManager.disable();

this._frameNavigations = [];
this._networkRecorder = undefined;
this._targetManager = undefined;
this._sessions = new Map();
}

/** @return {Promise<string | undefined>} */
Expand All @@ -103,6 +145,13 @@ class NetworkMonitor extends EventEmitter {
return finalNavigation && finalNavigation.url;
}

/**
* @return {LH.DevtoolsLog}
*/
getMessageLog() {
return this._messageLog.messages;
}

/**
* @return {Array<NetworkRequest>}
*/
Expand Down