[Telemetry] Use server's lastReported on the browser

src/plugins/telemetry/common/is_report_interval_expired.test.ts

@@ -0,0 +1,67 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { REPORT_INTERVAL_MS } from './constants';
+import { isReportIntervalExpired } from './is_report_interval_expired';
+
+describe('isReportIntervalExpired', () => {
+  test('true when undefined', () => {
+    expect(isReportIntervalExpired(undefined)).toBe(true);
+    expect(isReportIntervalExpired(void 0)).toBe(true);
+  });
+
+  describe('true when NaN', () => {
+    test('NaN', () => {
+      expect(isReportIntervalExpired(NaN)).toBe(true);
+    });
+
+    test('parseInt(undefined)', () => {
+      expect(isReportIntervalExpired(parseInt(undefined as unknown as string, 10))).toBe(true);
+    });
+
+    test('parseInt(null)', () => {
+      expect(isReportIntervalExpired(parseInt(null as unknown as string, 10))).toBe(true);
+    });
+
+    test('parseInt("")', () => {
+      expect(isReportIntervalExpired(parseInt('', 10))).toBe(true);
+    });
+
+    test('empty string', () => {
+      expect(isReportIntervalExpired('' as unknown as number)).toBe(true);
+    });
+
+    test('malformed string', () => {
+      expect(isReportIntervalExpired(`random_malformed_string` as unknown as number)).toBe(true);
+    });
+
+    test('other object', () => {
+      expect(isReportIntervalExpired({} as unknown as number)).toBe(true);
+    });
+  });
+
+  test('true when 0', () => {
+    expect(isReportIntervalExpired(0)).toBe(true);
+  });
+
+  test('true when actually expired', () => {
+    expect(isReportIntervalExpired(Date.now() - REPORT_INTERVAL_MS - 1000)).toBe(true);
+  });
+
+  test('false when close but not yet', () => {
+    expect(isReportIntervalExpired(Date.now() - REPORT_INTERVAL_MS + 1000)).toBe(false);
+  });
+
+  test('false when date in the future', () => {
+    expect(isReportIntervalExpired(Date.now() + 1000)).toBe(false);
+  });
+
+  test('false when date is now', () => {
+    expect(isReportIntervalExpired(Date.now())).toBe(false);
+  });
+});

src/plugins/telemetry/common/is_report_interval_expired.ts

@@ -0,0 +1,19 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { REPORT_INTERVAL_MS } from './constants';
+
+/**
+ * The report is considered expired if:
+ * - `lastReportAt` does not exist, is NaN or `REPORT_INTERVAL_MS` have passed ever since.
+ * @param lastReportAt
+ * @returns `true` if the report interval is considered expired
+ */
+export function isReportIntervalExpired(lastReportAt: number | undefined) {
+  return !lastReportAt || isNaN(lastReportAt) || Date.now() - lastReportAt > REPORT_INTERVAL_MS;
+}

src/plugins/telemetry/public/services/telemetry_sender.test.ts

@@ -42,87 +42,98 @@ describe('TelemetrySender', () => {
     });
 
     it('uses lastReport if set', () => {
-      const lastReport = `${Date.now()}`;
-      mockLocalStorage.getItem.mockReturnValueOnce(JSON.stringify({ lastReport }));
+      const lastReport = Date.now();
+      mockLocalStorage.getItem.mockReturnValueOnce(JSON.stringify({ lastReport: `${lastReport}` }));
       const telemetryService = mockTelemetryService();
       const telemetrySender = new TelemetrySender(telemetryService);
       expect(telemetrySender['lastReported']).toBe(lastReport);
     });
   });
 
-  describe('saveToBrowser', () => {
-    it('uses lastReport', () => {
-      const lastReport = `${Date.now()}`;
+  describe('updateLastReported', () => {
+    it('stores the new lastReported value in the storage', () => {
+      const lastReport = Date.now();
       const telemetryService = mockTelemetryService();
       const telemetrySender = new TelemetrySender(telemetryService);
-      telemetrySender['lastReported'] = lastReport;
-      telemetrySender['saveToBrowser']();
+      telemetrySender['updateLastReported'](lastReport);
 
       expect(mockLocalStorage.setItem).toHaveBeenCalledTimes(1);
       expect(mockLocalStorage.setItem).toHaveBeenCalledWith(
         LOCALSTORAGE_KEY,
-        JSON.stringify({ lastReport })
+        JSON.stringify({ lastReport: `${lastReport}` })
       );
     });
   });
 
   describe('shouldSendReport', () => {
-    it('returns false whenever optIn is false', () => {
+    it('returns false whenever optIn is false', async () => {
       const telemetryService = mockTelemetryService();
       telemetryService.getIsOptedIn = jest.fn().mockReturnValue(false);
       const telemetrySender = new TelemetrySender(telemetryService);
-      const shouldSendReport = telemetrySender['shouldSendReport']();
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
 
       expect(telemetryService.getIsOptedIn).toBeCalledTimes(1);
       expect(shouldSendReport).toBe(false);
     });
 
-    it('returns true if lastReported is undefined', () => {
+    it('returns true if lastReported is undefined (both local and global)', async () => {
       const telemetryService = mockTelemetryService();
       telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
+      telemetryService.fetchLastReported = jest.fn().mockResolvedValue(undefined);
       const telemetrySender = new TelemetrySender(telemetryService);
-      const shouldSendReport = telemetrySender['shouldSendReport']();
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
 
       expect(telemetrySender['lastReported']).toBeUndefined();
       expect(shouldSendReport).toBe(true);
+      expect(telemetryService.fetchLastReported).toHaveBeenCalledTimes(1);
     });
 
-    it('returns true if lastReported passed REPORT_INTERVAL_MS', () => {
+    it('returns true if lastReported passed REPORT_INTERVAL_MS', async () => {
       const lastReported = Date.now() - (REPORT_INTERVAL_MS + 1000);
 
       const telemetryService = mockTelemetryService();
       telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
       const telemetrySender = new TelemetrySender(telemetryService);
-      telemetrySender['lastReported'] = `${lastReported}`;
-      const shouldSendReport = telemetrySender['shouldSendReport']();
+      telemetrySender['lastReported'] = lastReported;
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
       expect(shouldSendReport).toBe(true);
     });
 
-    it('returns false if lastReported is within REPORT_INTERVAL_MS', () => {
+    it('returns false if local lastReported is within REPORT_INTERVAL_MS', async () => {
       const lastReported = Date.now() + 1000;
 
       const telemetryService = mockTelemetryService();
       telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
       const telemetrySender = new TelemetrySender(telemetryService);
-      telemetrySender['lastReported'] = `${lastReported}`;
-      const shouldSendReport = telemetrySender['shouldSendReport']();
+      telemetrySender['lastReported'] = lastReported;
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
       expect(shouldSendReport).toBe(false);
     });
 
-    it('returns true if lastReported is malformed', () => {
+    it('returns false if local lastReported is expired but the remote is within REPORT_INTERVAL_MS', async () => {
       const telemetryService = mockTelemetryService();
       telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
+      telemetryService.fetchLastReported = jest.fn().mockResolvedValue(Date.now() + 1000);
       const telemetrySender = new TelemetrySender(telemetryService);
-      telemetrySender['lastReported'] = `random_malformed_string`;
-      const shouldSendReport = telemetrySender['shouldSendReport']();
+      telemetrySender['lastReported'] = Date.now() - (REPORT_INTERVAL_MS + 1000);
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
+      expect(shouldSendReport).toBe(false);
+    });
+
+    it('returns true if lastReported is malformed', async () => {
+      const telemetryService = mockTelemetryService();
+      telemetryService.getIsOptedIn = jest.fn().mockReturnValue(true);
+      const telemetrySender = new TelemetrySender(telemetryService);
+      telemetrySender['lastReported'] = `random_malformed_string` as unknown as number;
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
       expect(shouldSendReport).toBe(true);
     });
 
-    it('returns false if we are in screenshot mode', () => {
+    it('returns false if we are in screenshot mode', async () => {
       const telemetryService = mockTelemetryService({ isScreenshotMode: true });
       telemetryService.getIsOptedIn = jest.fn().mockReturnValue(false);
       const telemetrySender = new TelemetrySender(telemetryService);
-      const shouldSendReport = telemetrySender['shouldSendReport']();
+      const shouldSendReport = await telemetrySender['shouldSendReport']();
 
       expect(telemetryService.getIsOptedIn).toBeCalledTimes(0);
       expect(shouldSendReport).toBe(false);
@@ -165,13 +176,14 @@ describe('TelemetrySender', () => {
       const telemetrySender = new TelemetrySender(telemetryService);
       telemetrySender['shouldSendReport'] = jest.fn().mockReturnValue(true);
       telemetrySender['sendUsageData'] = jest.fn().mockReturnValue(true);
-      telemetrySender['saveToBrowser'] = jest.fn();
-      telemetrySender['lastReported'] = `${lastReported}`;
+      telemetrySender['updateLastReported'] = jest.fn().mockImplementation((value) => {
+        expect(value).not.toBe(lastReported);
+      });
+      telemetrySender['lastReported'] = lastReported;
 
       await telemetrySender['sendIfDue']();
 
-      expect(telemetrySender['lastReported']).not.toBe(lastReported);
-      expect(telemetrySender['saveToBrowser']).toBeCalledTimes(1);
+      expect(telemetrySender['updateLastReported']).toBeCalledTimes(1);
       expect(telemetrySender['retryCount']).toEqual(0);
       expect(telemetrySender['sendUsageData']).toHaveBeenCalledTimes(1);
     });
@@ -181,7 +193,7 @@ describe('TelemetrySender', () => {
       const telemetrySender = new TelemetrySender(telemetryService);
       telemetrySender['shouldSendReport'] = jest.fn().mockReturnValue(true);
       telemetrySender['sendUsageData'] = jest.fn();
-      telemetrySender['saveToBrowser'] = jest.fn();
+      telemetrySender['updateLastReported'] = jest.fn();
       telemetrySender['retryCount'] = 9;
 
       await telemetrySender['sendIfDue']();
@@ -272,7 +284,7 @@ describe('TelemetrySender', () => {
       telemetryService.getTelemetryUrl = jest.fn().mockReturnValue(mockTelemetryUrl);
       telemetryService.fetchTelemetry = jest.fn().mockReturnValue(mockTelemetryPayload);
       telemetrySender['shouldSendReport'] = jest.fn().mockReturnValue(true);
-      telemetrySender['saveToBrowser'] = jest.fn();
+      telemetrySender['updateLastReported'] = jest.fn();
 
       await telemetrySender['sendUsageData']();
 

src/plugins/telemetry/public/services/telemetry_sender.ts

@@ -6,18 +6,15 @@
  * Side Public License, v 1.
  */
 
-import {
-  REPORT_INTERVAL_MS,
-  LOCALSTORAGE_KEY,
-  PAYLOAD_CONTENT_ENCODING,
-} from '../../common/constants';
+import { LOCALSTORAGE_KEY, PAYLOAD_CONTENT_ENCODING } from '../../common/constants';
 import { TelemetryService } from './telemetry_service';
 import { Storage } from '../../../kibana_utils/public';
 import type { EncryptedTelemetryPayload } from '../../common/types';
+import { isReportIntervalExpired } from '../../common/is_report_interval_expired';
 
 export class TelemetrySender {
   private readonly telemetryService: TelemetryService;
-  private lastReported?: string;
+  private lastReported?: number;
   private readonly storage: Storage;
   private intervalId: number = 0; // setInterval returns a positive integer, 0 means no interval is set
   private retryCount: number = 0;
@@ -32,38 +29,56 @@ export class TelemetrySender {
 
     const attributes = this.storage.get(LOCALSTORAGE_KEY);
     if (attributes) {
-      this.lastReported = attributes.lastReport;
+      this.lastReported = parseInt(attributes.lastReport, 10);
     }
   }
 
-  private saveToBrowser = () => {
+  private updateLastReported = (lastReported: number) => {
+    this.lastReported = lastReported;
     // we are the only code that manipulates this key, so it's safe to blindly overwrite the whole object
-    this.storage.set(LOCALSTORAGE_KEY, { lastReport: this.lastReported });
+    this.storage.set(LOCALSTORAGE_KEY, { lastReport: `${this.lastReported}` });
   };
 
-  private shouldSendReport = (): boolean => {
+  /**
+   * Using the local and SO's `lastReported` values, it decides whether the last report should be considered as expired
+   * @returns `true` if a new report should be generated. `false` otherwise.
+   */
+  private isReportDue = async (): Promise<boolean> => {
+    // Try to decide with the local `lastReported` to avoid querying the server
+    if (!isReportIntervalExpired(this.lastReported)) {
+      // If it is not expired locally, there's no need to send it again yet.
+      return false;
+    }
+
+    // Double-check with the server's value
+    const globalLastReported = await this.telemetryService.fetchLastReported();
+
+    if (globalLastReported) {
+      // Update the local value to avoid repetitions of this request (it was already expired, so it doesn't really matter if the server's value is older)
+      this.updateLastReported(globalLastReported);
+    }
+
+    return isReportIntervalExpired(globalLastReported);
+  };
+
+  /**
+   * Using configuration and the lastReported dates, it decides whether a new telemetry report should be sent.
+   * @returns `true` if a new report should be sent. `false` otherwise.
+   */
+  private shouldSendReport = async (): Promise<boolean> => {
     if (this.telemetryService.canSendTelemetry()) {
-      if (!this.lastReported) {
-        return true;
-      }
-      // returns NaN for any malformed or unset (null/undefined) value
-      const lastReported = parseInt(this.lastReported, 10);
-      // If it's been a day since we last sent telemetry
-      if (isNaN(lastReported) || Date.now() - lastReported > REPORT_INTERVAL_MS) {
-        return true;
-      }
+      return await this.isReportDue();
     }
 
     return false;
   };
 
   private sendIfDue = async (): Promise<void> => {
-    if (!this.shouldSendReport()) {
+    if (!(await this.shouldSendReport())) {
       return;
     }
     // optimistically update the report date and reset the retry counter for a new time report interval window
-    this.lastReported = `${Date.now()}`;
-    this.saveToBrowser();
+    this.updateLastReported(Date.now());
     this.retryCount = 0;
     await this.sendUsageData();
   };
@@ -89,6 +104,8 @@ export class TelemetrySender {
             })
         )
       );
+
+      await this.telemetryService.updateLastReported().catch(() => {}); // Let's catch the error. Worst-case scenario another Telemetry report will be generated somewhere else.
     } catch (err) {
       // ignore err and try again but after a longer wait period.
       this.retryCount = this.retryCount + 1;