core(network-analyzer): estimate from lrStatistics (#15158)

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;
 

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<LH.Crdp.Network.ResourceType>} */
@@ -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;

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', () => {