diff --git a/core/lib/dependency-graph/simulator/network-analyzer.js b/core/lib/dependency-graph/simulator/network-analyzer.js index a1634187776d..27116d658916 100644 --- a/core/lib/dependency-graph/simulator/network-analyzer.js +++ b/core/lib/dependency-graph/simulator/network-analyzer.js @@ -139,6 +139,15 @@ class NetworkAnalyzer { return NetworkAnalyzer._estimateValueByOrigin(records, ({timing, connectionReused, record}) => { if (connectionReused) return; + // In LR, network records are missing connection timing, but we've smuggled it in via headers. + if (global.isLightrider && record.lrStatistics) { + if (record.protocol.startsWith('h3')) { + return record.lrStatistics.TCPMs; + } else { + return [record.lrStatistics.TCPMs / 2, record.lrStatistics.TCPMs / 2]; + } + } + const {connectStart, sslStart, sslEnd, connectEnd} = timing; if (connectEnd >= 0 && connectStart >= 0 && record.protocol.startsWith('h3')) { // These values are equal to sslStart and sslEnd for h3. @@ -247,6 +256,10 @@ class NetworkAnalyzer { */ static _estimateResponseTimeByOrigin(records, rttByOrigin) { return NetworkAnalyzer._estimateValueByOrigin(records, ({record, timing}) => { + // Lightrider does not have timings for sendEnd, but we do have this timing which should be + // close to the response time. + if (global.isLightrider && record.lrStatistics) return record.lrStatistics.requestMs; + if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return; if (!Number.isFinite(timing.sendEnd) || timing.sendEnd < 0) return; diff --git a/core/lib/network-request.js b/core/lib/network-request.js index f7fa5345c0a5..f2acc0d0e0da 100644 --- a/core/lib/network-request.js +++ b/core/lib/network-request.js @@ -57,7 +57,7 @@ import UrlUtils from './url-utils.js'; // Lightrider X-Header names for timing information. // See: _updateTransferSizeForLightrider and _updateTimingsForLightrider. -const HEADER_TCP = 'X-TCPMs'; +const HEADER_TCP = 'X-TCPMs'; // Note: this should have been called something like ConnectMs, as it includes SSL. const HEADER_SSL = 'X-SSLMs'; const HEADER_REQ = 'X-RequestMs'; const HEADER_RES = 'X-ResponseMs'; @@ -80,14 +80,10 @@ const HEADER_PROTOCOL_IS_H2 = 'X-ProtocolIsH2'; /** * @typedef LightriderStatistics - * The difference in networkEndTime between the observed Lighthouse networkEndTime and Lightrider's derived networkEndTime. - * @property {number} endTimeDeltaMs - * The time spent making a TCP connection (connect + SSL). - * @property {number} TCPMs - * The time spent requesting a resource from a remote server, we use this to approx RTT. - * @property {number} requestMs - * The time spent transferring a resource from a remote server. - * @property {number} responseMs + * @property {number} endTimeDeltaMs The difference in networkEndTime between the observed Lighthouse networkEndTime and Lightrider's derived networkEndTime. + * @property {number} TCPMs The time spent making a TCP connection (connect + SSL). Note: this is poorly named. + * @property {number} requestMs The time spent requesting a resource from a remote server, we use this to approx RTT. Note: this is poorly names, it really should be "server response time". + * @property {number} responseMs Time to receive the entire response payload starting the clock on receiving the first fragment (first non-header byte). */ /** @type {LH.Util.SelfMap} */ @@ -501,6 +497,7 @@ class NetworkRequest { // Make sure all times are initialized and are non-negative. const TCPMs = TCPMsHeader ? Math.max(0, parseInt(TCPMsHeader.value)) : 0; + // This is missing for h2 requests, but present for h1. See b/283843975 const SSLMs = SSLMsHeader ? Math.max(0, parseInt(SSLMsHeader.value)) : 0; const requestMs = requestMsHeader ? Math.max(0, parseInt(requestMsHeader.value)) : 0; const responseMs = responseMsHeader ? Math.max(0, parseInt(responseMsHeader.value)) : 0; diff --git a/core/test/lib/dependency-graph/simulator/network-analyzer-test.js b/core/test/lib/dependency-graph/simulator/network-analyzer-test.js index de85462ad8e7..1cdee1fb5825 100644 --- a/core/test/lib/dependency-graph/simulator/network-analyzer-test.js +++ b/core/test/lib/dependency-graph/simulator/network-analyzer-test.js @@ -14,6 +14,10 @@ const devtoolsLog = readJson('../../../fixtures/traces/progressive-app-m60.devto const devtoolsLogWithRedirect = readJson('../../../fixtures/artifacts/redirect/devtoolslog.json', import.meta); describe('DependencyGraph/Simulator/NetworkAnalyzer', () => { + afterEach(() => { + global.isLightrider = undefined; + }); + let recordId; function createRecord(opts) { @@ -248,6 +252,22 @@ describe('DependencyGraph/Simulator/NetworkAnalyzer', () => { assertCloseEnough(result.median, resultApprox.median, 30); }); }); + + it('should use lrStatistics when needed', () => { + global.isLightrider = true; + const record = createRecord({timing: {}, lrStatistics: {TCPMs: 100}}); + const result = NetworkAnalyzer.estimateRTTByOrigin([record]); + const expected = {min: 50, max: 50, avg: 50, median: 50}; + assert.deepStrictEqual(result.get('https://example.com'), expected); + }); + + it('should use lrStatistics when needed (h3)', () => { + global.isLightrider = true; + const record = createRecord({protocol: 'h3', timing: {}, lrStatistics: {TCPMs: 100}}); + const result = NetworkAnalyzer.estimateRTTByOrigin([record]); + const expected = {min: 100, max: 100, avg: 100, median: 100}; + assert.deepStrictEqual(result.get('https://example.com'), expected); + }); }); describe('#estimateServerResponseTimeByOrigin', () => { @@ -299,6 +319,14 @@ describe('DependencyGraph/Simulator/NetworkAnalyzer', () => { assertCloseEnough(result.median, resultApprox.median, 30); }); }); + + it('should use lrStatistics when needed', () => { + global.isLightrider = true; + const record = createRecord({timing: {}, lrStatistics: {requestMs: 100}}); + const result = NetworkAnalyzer.estimateServerResponseTimeByOrigin([record]); + const expected = {min: 100, max: 100, avg: 100, median: 100}; + assert.deepStrictEqual(result.get('https://example.com'), expected); + }); }); describe('#estimateThroughput', () => {