From 4ed1cb3e3881272e90e744c4aa5496d983d64d77 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alejandro=20Fern=C3=A1ndez=20Haro?=
 <alejandro.haro@elastic.co>
Date: Tue, 6 Oct 2020 12:45:58 +0100
Subject: [PATCH] [Telemetry] Synchronous `setup` and `start` methods (#79457)

---
 src/plugins/telemetry/server/fetcher.ts | 13 +++---
 src/plugins/telemetry/server/plugin.ts  | 56 +++++++++++++++++--------
 2 files changed, 44 insertions(+), 25 deletions(-)

diff --git a/src/plugins/telemetry/server/fetcher.ts b/src/plugins/telemetry/server/fetcher.ts
index e6d909965f5f..fadfc01f628f 100644
--- a/src/plugins/telemetry/server/fetcher.ts
+++ b/src/plugins/telemetry/server/fetcher.ts
@@ -18,7 +18,7 @@
  */
 
 import moment from 'moment';
-import { Observable } from 'rxjs';
+import { Observable, Subscription, timer } from 'rxjs';
 import { take } from 'rxjs/operators';
 // @ts-ignore
 import fetch from 'node-fetch';
@@ -61,7 +61,7 @@ export class FetcherTask {
   private readonly config$: Observable<TelemetryConfigType>;
   private readonly currentKibanaVersion: string;
   private readonly logger: Logger;
-  private intervalId?: NodeJS.Timeout;
+  private intervalId?: Subscription;
   private lastReported?: number;
   private isSending = false;
   private internalRepository?: SavedObjectsClientContract;
@@ -82,15 +82,14 @@ export class FetcherTask {
     this.telemetryCollectionManager = telemetryCollectionManager;
     this.elasticsearchClient = elasticsearch.legacy.createClient('telemetry-fetcher');
 
-    setTimeout(() => {
-      this.sendIfDue();
-      this.intervalId = setInterval(() => this.sendIfDue(), this.checkIntervalMs);
-    }, this.initialCheckDelayMs);
+    this.intervalId = timer(this.initialCheckDelayMs, this.checkIntervalMs).subscribe(() =>
+      this.sendIfDue()
+    );
   }
 
   public stop() {
     if (this.intervalId) {
-      clearInterval(this.intervalId);
+      this.intervalId.unsubscribe();
     }
     if (this.elasticsearchClient) {
       this.elasticsearchClient.close();
diff --git a/src/plugins/telemetry/server/plugin.ts b/src/plugins/telemetry/server/plugin.ts
index dfbbe3355e69..b423cbb07ba3 100644
--- a/src/plugins/telemetry/server/plugin.ts
+++ b/src/plugins/telemetry/server/plugin.ts
@@ -18,7 +18,7 @@
  */
 
 import { URL } from 'url';
-import { Observable } from 'rxjs';
+import { AsyncSubject, Observable } from 'rxjs';
 import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
 import {
   TelemetryCollectionManagerPluginSetup,
@@ -30,11 +30,11 @@ import {
   PluginInitializerContext,
   ISavedObjectsRepository,
   CoreStart,
-  IUiSettingsClient,
   SavedObjectsClient,
   Plugin,
   Logger,
   IClusterClient,
+  UiSettingsServiceStart,
 } from '../../../core/server';
 import { registerRoutes } from './routes';
 import { registerCollection } from './telemetry_collection';
@@ -82,8 +82,11 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
   private readonly config$: Observable<TelemetryConfigType>;
   private readonly isDev: boolean;
   private readonly fetcherTask: FetcherTask;
+  /**
+   * @private Used to mark the completion of the old UI Settings migration
+   */
+  private readonly oldUiSettingsHandled$ = new AsyncSubject();
   private savedObjectsClient?: ISavedObjectsRepository;
-  private uiSettingsClient?: IUiSettingsClient;
   private elasticsearchClient?: IClusterClient;
 
   constructor(initializerContext: PluginInitializerContext<TelemetryConfigType>) {
@@ -97,10 +100,10 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
     });
   }
 
-  public async setup(
+  public setup(
     { elasticsearch, http, savedObjects }: CoreSetup,
     { usageCollection, telemetryCollectionManager }: TelemetryPluginsDepsSetup
-  ): Promise<TelemetryPluginSetup> {
+  ): TelemetryPluginSetup {
     const currentKibanaVersion = this.currentKibanaVersion;
     const config$ = this.config$;
     const isDev = this.isDev;
@@ -131,25 +134,21 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
     };
   }
 
-  public async start(core: CoreStart, { telemetryCollectionManager }: TelemetryPluginsDepsStart) {
+  public start(core: CoreStart, { telemetryCollectionManager }: TelemetryPluginsDepsStart) {
     const { savedObjects, uiSettings, elasticsearch } = core;
-    this.savedObjectsClient = savedObjects.createInternalRepository();
-    const savedObjectsClient = new SavedObjectsClient(this.savedObjectsClient);
-    this.uiSettingsClient = uiSettings.asScopedToClient(savedObjectsClient);
+    const savedObjectsInternalRepository = savedObjects.createInternalRepository();
+    this.savedObjectsClient = savedObjectsInternalRepository;
     this.elasticsearchClient = elasticsearch.client;
 
-    try {
-      await handleOldSettings(savedObjectsClient, this.uiSettingsClient);
-    } catch (error) {
-      this.logger.warn('Unable to update legacy telemetry configs.');
-    }
-
-    this.fetcherTask.start(core, { telemetryCollectionManager });
+    // Not catching nor awaiting these promises because they should never reject
+    this.handleOldUiSettings(uiSettings);
+    this.startFetcherWhenOldSettingsAreHandled(core, telemetryCollectionManager);
 
     return {
       getIsOptedIn: async () => {
-        const internalRepository = new SavedObjectsClient(savedObjects.createInternalRepository());
-        const telemetrySavedObject = await getTelemetrySavedObject(internalRepository!);
+        await this.oldUiSettingsHandled$.pipe(take(1)).toPromise(); // Wait for the old settings to be handled
+        const internalRepository = new SavedObjectsClient(savedObjectsInternalRepository);
+        const telemetrySavedObject = await getTelemetrySavedObject(internalRepository);
         const config = await this.config$.pipe(take(1)).toPromise();
         const allowChangingOptInStatus = config.allowChangingOptInStatus;
         const configTelemetryOptIn = typeof config.optIn === 'undefined' ? null : config.optIn;
@@ -166,6 +165,27 @@ export class TelemetryPlugin implements Plugin<TelemetryPluginSetup, TelemetryPl
     };
   }
 
+  private async handleOldUiSettings(uiSettings: UiSettingsServiceStart) {
+    const savedObjectsClient = new SavedObjectsClient(this.savedObjectsClient!);
+    const uiSettingsClient = uiSettings.asScopedToClient(savedObjectsClient);
+
+    try {
+      await handleOldSettings(savedObjectsClient, uiSettingsClient);
+    } catch (error) {
+      this.logger.warn('Unable to update legacy telemetry configs.');
+    }
+    // Set the mark in the AsyncSubject as complete so all the methods that require this method to be completed before working, can move on
+    this.oldUiSettingsHandled$.complete();
+  }
+
+  private async startFetcherWhenOldSettingsAreHandled(
+    core: CoreStart,
+    telemetryCollectionManager: TelemetryCollectionManagerPluginStart
+  ) {
+    await this.oldUiSettingsHandled$.pipe(take(1)).toPromise(); // Wait for the old settings to be handled
+    this.fetcherTask.start(core, { telemetryCollectionManager });
+  }
+
   private registerMappings(registerType: SavedObjectsRegisterType) {
     registerType({
       name: 'telemetry',