feat(metrics_server): add support for tunnel time (Jigsaw-Code#1527)

* feat(metrics_server): add support for tunnel time

* Add tests.

* Update `nodejs` runtime to 18.

* Update integration test.

* Use seconds instead of ms.

* Inline the minimun tunnel time and remove the unnecessary comment.

* Add field types to test case names.

* Make tunnel time optional.

* Add TODO to drop `userId`.

* Undo use of gdate.

* Make the test reports more realistic with 1 country.

src/metrics_server/README.md

@@ -23,6 +23,7 @@ The metrics server supports two URL paths:
         userId: string,
         countries: string[],
         bytesTransferred: number,
+        tunnelTimeSec: number,
     }]
   }
   ```

src/metrics_server/app_dev.yaml

@@ -1,4 +1,4 @@
-runtime: nodejs16
+runtime: nodejs18
 service: dev
 handlers:
 - url: /.*

src/metrics_server/app_prod.yaml

@@ -1,4 +1,4 @@
-runtime: nodejs16
+runtime: nodejs18
 service: prod
 handlers:
 - url: /.*

src/metrics_server/connection_metrics.spec.ts

@@ -18,6 +18,21 @@ import {
   postConnectionMetrics,
 } from './connection_metrics';
 import {InsertableTable} from './infrastructure/table';
+import {HourlyConnectionMetricsReport, HourlyUserConnectionMetricsReport} from './model';
+
+const VALID_USER_REPORT = {
+  userId: 'uid0',
+  countries: ['US'],
+  bytesTransferred: 123,
+  tunnelTimeSec: 789,
+};
+
+const VALID_REPORT: HourlyConnectionMetricsReport = {
+  serverId: 'id',
+  startUtcMs: 1,
+  endUtcMs: 2,
+  userReports: [structuredClone(VALID_USER_REPORT)],
+};
 
 class FakeConnectionsTable implements InsertableTable<ConnectionRow> {
   public rows: ConnectionRow[] | undefined;
@@ -33,13 +48,15 @@ describe('postConnectionMetrics', () => {
     const userReports = [
       {
         userId: 'uid0',
-        countries: ['US', 'UK'],
+        countries: ['UK'],
         bytesTransferred: 123,
+        tunnelTimeSec: 987,
       },
       {
         userId: 'uid1',
         countries: ['EC'],
         bytesTransferred: 456,
+        tunnelTimeSec: 654,
       },
       {
         userId: '',
@@ -61,6 +78,7 @@ describe('postConnectionMetrics', () => {
         endTimestamp: new Date(report.endUtcMs).toISOString(),
         userId: userReports[0].userId,
         bytesTransferred: userReports[0].bytesTransferred,
+        tunnelTimeSec: userReports[0].tunnelTimeSec,
         countries: userReports[0].countries,
       },
       {
@@ -69,6 +87,7 @@ describe('postConnectionMetrics', () => {
         endTimestamp: new Date(report.endUtcMs).toISOString(),
         userId: userReports[1].userId,
         bytesTransferred: userReports[1].bytesTransferred,
+        tunnelTimeSec: userReports[1].tunnelTimeSec,
         countries: userReports[1].countries,
       },
       {
@@ -77,6 +96,7 @@ describe('postConnectionMetrics', () => {
         endTimestamp: new Date(report.endUtcMs).toISOString(),
         userId: undefined,
         bytesTransferred: userReports[2].bytesTransferred,
+        tunnelTimeSec: undefined,
         countries: userReports[2].countries,
       },
       {
@@ -85,6 +105,7 @@ describe('postConnectionMetrics', () => {
         endTimestamp: new Date(report.endUtcMs).toISOString(),
         userId: userReports[3].userId,
         bytesTransferred: userReports[3].bytesTransferred,
+        tunnelTimeSec: undefined,
         countries: userReports[3].countries,
       },
     ];
@@ -95,183 +116,119 @@ describe('postConnectionMetrics', () => {
 describe('isValidConnectionMetricsReport', () => {
   it('returns true for valid report', () => {
     const userReports = [
-      {userId: 'uid0', countries: ['AA'], bytesTransferred: 111},
-      {userId: 'uid1', bytesTransferred: 222},
+      {userId: 'uid0', countries: ['AA'], bytesTransferred: 111, tunnelTimeSec: 1},
+      {userId: 'uid1', bytesTransferred: 222, tunnelTimeSec: 2},
       {userId: 'uid2', countries: [], bytesTransferred: 333},
       {countries: ['BB'], bytesTransferred: 444},
       {userId: '', countries: ['CC'], bytesTransferred: 555},
     ];
     const report = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports};
-    expect(isValidConnectionMetricsReport(report)).toBeTruthy();
+    expect(isValidConnectionMetricsReport(report)).toBeTrue();
   });
   it('returns false for missing report', () => {
-    expect(isValidConnectionMetricsReport(undefined)).toBeFalsy();
+    expect(isValidConnectionMetricsReport(undefined)).toBeFalse();
   });
   it('returns false for inconsistent timestamp values', () => {
-    const userReports = [
-      {userId: 'uid0', countries: ['US', 'UK'], bytesTransferred: 123},
-      {userId: 'uid1', countries: ['EC'], bytesTransferred: 456},
-    ];
-    const invalidReport = {
-      serverId: 'id',
-      startUtcMs: 999, // startUtcMs > endUtcMs
-      endUtcMs: 1,
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport)).toBeFalsy();
+    const report = structuredClone(VALID_REPORT);
+    // startUtcMs > endUtcMs
+    report.startUtcMs = 999;
+    report.endUtcMs = 1;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
   it('returns false for out-of-bounds transferred bytes', () => {
-    const userReports = [
-      {
-        userId: 'uid0',
-        countries: ['US', 'UK'],
-        bytesTransferred: -123, // Should not be negative
-      },
-      {userId: 'uid1', countries: ['EC'], bytesTransferred: 456},
-    ];
-    const invalidReport = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports};
-    expect(isValidConnectionMetricsReport(invalidReport)).toBeFalsy();
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].bytesTransferred = -123;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
 
-    const userReports2 = [
-      {userId: 'uid0', countries: ['US', 'UK'], bytesTransferred: 123},
-      {
-        userId: 'uid1',
-        countries: ['EC'],
-        bytesTransferred: 2 * Math.pow(2, 40), // 2TB is above the server capacity
-      },
-    ];
-    const invalidReport2 = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports: userReports2};
-    expect(isValidConnectionMetricsReport(invalidReport2)).toBeFalsy();
+    // 2TB is above the server capacity
+    report.userReports[0].bytesTransferred = 2 * Math.pow(2, 40);
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
-  it('returns false for missing report fields', () => {
-    const invalidReport = {
-      // Missing `userReports`
-      serverId: 'id',
-      startUtcMs: 1,
-      endUtcMs: 2,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport)).toBeFalsy();
-
-    const invalidReport2 = {
-      serverId: 'id',
-      startUtcMs: 1,
-      endUtcMs: 2,
-      userReports: [], // Should not be empty
-    };
-    expect(isValidConnectionMetricsReport(invalidReport2)).toBeFalsy();
-
-    const userReports = [
-      {userId: 'uid0', countries: ['US', 'UK'], bytesTransferred: 123},
-      {userId: 'uid1', countries: ['EC'], bytesTransferred: 456},
-    ];
-    const invalidReport3 = {
-      // Missing `serverId`
-      startUtcMs: 1,
-      endUtcMs: 2,
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport3)).toBeFalsy();
-
-    const invalidReport4 = {
-      // Missing `startUtcMs`
-      serverId: 'id',
-      endUtcMs: 2,
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport4)).toBeFalsy();
-
-    const invalidReport5 = {
-      // Missing `endUtcMs`
-      serverId: 'id',
-      startUtcMs: 2,
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport5)).toBeFalsy();
+  it('returns false for out-of-bounds tunnel time', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].tunnelTimeSec = -123;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
-  it('returns false for missing user report fields', () => {
-    const invalidReport1 = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports: [
-      {
-        // Missing `userId` and `countries`
-        bytesTransferred: 123,
-      },
-    ]};
-    expect(isValidConnectionMetricsReport(invalidReport1)).toBeFalsy();
-
-    const invalidReport2 = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports: {
-      // Missing `bytesTransferred`
-      userId: 'uid0',
-      countries: ['US', 'UK'],
-    }};
-    expect(isValidConnectionMetricsReport(invalidReport2)).toBeFalsy();
+  it('returns false for missing user reports', () => {
+    const report: Partial<HourlyConnectionMetricsReport> = structuredClone(VALID_REPORT);
+    delete report['userReports'];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
-  it('returns false for incorrect report field types', () => {
-    const invalidReport = {
-      serverId: 'id',
-      startUtcMs: 1,
-      endUtcMs: 2,
-      userReports: [1, 2, 3], // Should be `HourlyUserConnectionMetricsReport[]`
-    };
-    expect(isValidConnectionMetricsReport(invalidReport)).toBeFalsy();
-
-    const userReports = [
-      {userId: 'uid0', countries: ['US', 'UK'], bytesTransferred: 123},
-      {userId: 'uid1', countries: ['EC'], bytesTransferred: 456},
-    ];
-    const invalidReport2 = {
-      serverId: 987, // Should be a string
-      startUtcMs: 1,
-      endUtcMs: 2,
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport2)).toBeFalsy();
-
-    const invalidReport3 = {
-      serverId: 'id',
-      startUtcMs: '100', // Should be a number
-      endUtcMs: 200,
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport3)).toBeFalsy();
-
-    const invalidReport4 = {
-      // Missing `startUtcMs`
-      serverId: 'id',
-      startUtcMs: 1,
-      endUtcMs: '200', // Should be a number
-      userReports,
-    };
-    expect(isValidConnectionMetricsReport(invalidReport4)).toBeFalsy();
+  it('returns false for empty user reports', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports = [];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
-  it('returns false for incorrect user report field types ', () => {
-    const userReports = [
-      {
-        userId: 1234, // Should be a string
-        countries: ['US', 'UK'],
-        bytesTransferred: 123,
-      },
-      {userId: 'uid1', countries: ['EC'], bytesTransferred: 456},
-    ];
-    const invalidReport = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports};
-    expect(isValidConnectionMetricsReport(invalidReport)).toBeFalsy();
-
-    const userReports2 = [
-      {
-        userId: 'uid0',
-        countries: [1, 2, 3], // Should be string[]
-        bytesTransferred: 123,
-      },
-    ];
-    const invalidReport2 = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports: userReports2};
-    expect(isValidConnectionMetricsReport(invalidReport2)).toBeFalsy();
-
-    const userReports3 = [
-      {
-        userId: 'uid0',
-        countries: ['US', 'UK'],
-        bytesTransferred: '1234', // Should be a number
-      },
-    ];
-    const invalidReport3 = {serverId: 'id', startUtcMs: 1, endUtcMs: 2, userReports: userReports3};
-    expect(isValidConnectionMetricsReport(invalidReport3)).toBeFalsy();
+  it('returns false for missing `serverId`', () => {
+    const report: Partial<HourlyConnectionMetricsReport> = structuredClone(VALID_REPORT);
+    delete report['serverId'];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for missing `startUtcMs`', () => {
+    const report: Partial<HourlyConnectionMetricsReport> = structuredClone(VALID_REPORT);
+    delete report['startUtcMs'];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for missing `endUtcMs`', () => {
+    const report: Partial<HourlyConnectionMetricsReport> = structuredClone(VALID_REPORT);
+    delete report['endUtcMs'];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for user report missing both `userId` and `countries`', () => {
+    const userReport: Partial<HourlyUserConnectionMetricsReport> =
+      structuredClone(VALID_USER_REPORT);
+    delete userReport['userId'];
+    delete userReport['countries'];
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0] = userReport as HourlyUserConnectionMetricsReport;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for missing user report field `bytesTransferred`', () => {
+    const report = structuredClone(VALID_REPORT);
+    const userReport: Partial<HourlyUserConnectionMetricsReport> =
+      structuredClone(VALID_USER_REPORT);
+    delete userReport['bytesTransferred'];
+    report.userReports[0] = userReport as HourlyUserConnectionMetricsReport;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for user report field types that is not `HourlyUserConnectionMetricsReport`', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports = [1, 2, 3] as unknown as HourlyUserConnectionMetricsReport[];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `serverId` field type that is not a string', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.serverId = 987 as unknown as string;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `startUtcMs` field type that is not a number', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.startUtcMs = '100' as unknown as number;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `endUtcMs` field type that is not a number', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.endUtcMs = '100' as unknown as number;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `userId` field type that is not a string', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].userId = 1234 as unknown as string;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `countries` field type that is not a string', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].countries = [1, 2, 3] as unknown as string[];
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `bytesTransferred` field type that is not a number', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].bytesTransferred = '1234' as unknown as number;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
+  });
+  it('returns false for `tunnelTimeSec` field type that is not a number', () => {
+    const report = structuredClone(VALID_REPORT);
+    report.userReports[0].tunnelTimeSec = '789' as unknown as number;
+    expect(isValidConnectionMetricsReport(report)).toBeFalse();
   });
 });