Fixed usage of isReady for usage collection of alerts and actions (e…

…lastic#83760) * Fixed usage of `isReady` for usage collection of alerts and actions * fixed index * fixed due to comments * fixed type check * fixed due to comments
watson · Nov 21, 2020 · 3ee6e47 · 3ee6e47
1 parent 658ecb6
commit 3ee6e47
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 54 deletions.
diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts
@@ -4,16 +4,16 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 import type { PublicMethodsOf } from '@kbn/utility-types';
-import { first, map } from 'rxjs/operators';
+import { first } from 'rxjs/operators';
 import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
+import { Observable } from 'rxjs';
 import {
   PluginInitializerContext,
   Plugin,
   CoreSetup,
   CoreStart,
   KibanaRequest,
   Logger,
-  SharedGlobalConfig,
   RequestHandler,
   IContextProvider,
   ElasticsearchServiceStart,
@@ -128,7 +128,6 @@ const includedHiddenTypes = [
 ];
 
 export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, PluginStartContract> {
-  private readonly kibanaIndex: Promise<string>;
   private readonly config: Promise<ActionsConfig>;
 
   private readonly logger: Logger;
@@ -143,20 +142,14 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
   private isESOUsingEphemeralEncryptionKey?: boolean;
   private readonly telemetryLogger: Logger;
   private readonly preconfiguredActions: PreConfiguredAction[];
+  private readonly kibanaIndexConfig: Observable<{ kibana: { index: string } }>;
 
   constructor(initContext: PluginInitializerContext) {
     this.config = initContext.config.create<ActionsConfig>().pipe(first()).toPromise();
-
-    this.kibanaIndex = initContext.config.legacy.globalConfig$
-      .pipe(
-        first(),
-        map((config: SharedGlobalConfig) => config.kibana.index)
-      )
-      .toPromise();
-
     this.logger = initContext.logger.get('actions');
     this.telemetryLogger = initContext.logger.get('usage');
     this.preconfiguredActions = [];
+    this.kibanaIndexConfig = initContext.config.legacy.globalConfig$;
   }
 
   public async setup(
@@ -220,22 +213,26 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
 
     const usageCollection = plugins.usageCollection;
     if (usageCollection) {
-      initializeActionsTelemetry(
-        this.telemetryLogger,
-        plugins.taskManager,
-        core,
-        await this.kibanaIndex
+      registerActionsUsageCollector(
+        usageCollection,
+        core.getStartServices().then(([_, { taskManager }]) => taskManager)
       );
-
-      core.getStartServices().then(async ([, startPlugins]) => {
-        registerActionsUsageCollector(usageCollection, startPlugins.taskManager);
-      });
     }
 
-    core.http.registerRouteHandlerContext(
-      'actions',
-      this.createRouteHandlerContext(core, await this.kibanaIndex)
-    );
+    this.kibanaIndexConfig.subscribe((config) => {
+      core.http.registerRouteHandlerContext(
+        'actions',
+        this.createRouteHandlerContext(core, config.kibana.index)
+      );
+      if (usageCollection) {
+        initializeActionsTelemetry(
+          this.telemetryLogger,
+          plugins.taskManager,
+          core,
+          config.kibana.index
+        );
+      }
+    });
 
     // Routes
     const router = core.http.createRouter();
@@ -269,7 +266,7 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
       actionExecutor,
       actionTypeRegistry,
       taskRunnerFactory,
-      kibanaIndex,
+      kibanaIndexConfig,
       isESOUsingEphemeralEncryptionKey,
       preconfiguredActions,
       instantiateAuthorization,
@@ -297,10 +294,12 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
         request
       );
 
+      const kibanaIndex = (await kibanaIndexConfig.pipe(first()).toPromise()).kibana.index;
+
       return new ActionsClient({
         unsecuredSavedObjectsClient,
         actionTypeRegistry: actionTypeRegistry!,
-        defaultKibanaIndex: await kibanaIndex,
+        defaultKibanaIndex: kibanaIndex,
         scopedClusterClient: core.elasticsearch.legacy.client.asScoped(request),
         preconfiguredActions,
         request,

diff --git a/x-pack/plugins/actions/server/usage/actions_usage_collector.test.ts b/x-pack/plugins/actions/server/usage/actions_usage_collector.test.ts
@@ -24,15 +24,15 @@ describe('registerActionsUsageCollector', () => {
   it('should call registerCollector', () => {
     registerActionsUsageCollector(
       usageCollectionMock as UsageCollectionSetup,
-      mockTaskManagerStart
+      new Promise(() => mockTaskManagerStart)
     );
     expect(usageCollectionMock.registerCollector).toHaveBeenCalledTimes(1);
   });
 
   it('should call makeUsageCollector with type = actions', () => {
     registerActionsUsageCollector(
       usageCollectionMock as UsageCollectionSetup,
-      mockTaskManagerStart
+      new Promise(() => mockTaskManagerStart)
     );
     expect(usageCollectionMock.makeUsageCollector).toHaveBeenCalledTimes(1);
     expect(usageCollectionMock.makeUsageCollector.mock.calls[0][0].type).toBe('actions');

diff --git a/x-pack/plugins/actions/server/usage/actions_usage_collector.ts b/x-pack/plugins/actions/server/usage/actions_usage_collector.ts
@@ -26,11 +26,14 @@ const byTypeSchema: MakeSchemaFrom<ActionsUsage>['count_by_type'] = {
 
 export function createActionsUsageCollector(
   usageCollection: UsageCollectionSetup,
-  taskManager: TaskManagerStartContract
+  taskManager: Promise<TaskManagerStartContract>
 ) {
   return usageCollection.makeUsageCollector<ActionsUsage>({
     type: 'actions',
-    isReady: () => true,
+    isReady: async () => {
+      await taskManager;
+      return true;
+    },
     schema: {
       count_total: { type: 'long' },
       count_active_total: { type: 'long' },
@@ -79,7 +82,7 @@ async function getLatestTaskState(taskManager: TaskManagerStartContract) {
 
 export function registerActionsUsageCollector(
   usageCollection: UsageCollectionSetup,
-  taskManager: TaskManagerStartContract
+  taskManager: Promise<TaskManagerStartContract>
 ) {
   const collector = createActionsUsageCollector(usageCollection, taskManager);
   usageCollection.registerCollector(collector);

diff --git a/x-pack/plugins/alerts/server/plugin.ts b/x-pack/plugins/alerts/server/plugin.ts
@@ -5,6 +5,7 @@
  */
 import type { PublicMethodsOf } from '@kbn/utility-types';
 import { first, map } from 'rxjs/operators';
+import { Observable } from 'rxjs';
 import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
 import { combineLatest } from 'rxjs';
 import { SecurityPluginSetup } from '../../security/server';
@@ -28,7 +29,6 @@ import {
   SavedObjectsServiceStart,
   IContextProvider,
   RequestHandler,
-  SharedGlobalConfig,
   ElasticsearchServiceStart,
   ILegacyClusterClient,
   StatusServiceSetup,
@@ -124,30 +124,25 @@ export class AlertingPlugin {
   private security?: SecurityPluginSetup;
   private readonly alertsClientFactory: AlertsClientFactory;
   private readonly telemetryLogger: Logger;
-  private readonly kibanaIndex: Promise<string>;
   private readonly kibanaVersion: PluginInitializerContext['env']['packageInfo']['version'];
   private eventLogService?: IEventLogService;
   private eventLogger?: IEventLogger;
+  private readonly kibanaIndexConfig: Observable<{ kibana: { index: string } }>;
 
   constructor(initializerContext: PluginInitializerContext) {
     this.config = initializerContext.config.create<AlertsConfig>().pipe(first()).toPromise();
     this.logger = initializerContext.logger.get('plugins', 'alerting');
     this.taskRunnerFactory = new TaskRunnerFactory();
     this.alertsClientFactory = new AlertsClientFactory();
     this.telemetryLogger = initializerContext.logger.get('usage');
-    this.kibanaIndex = initializerContext.config.legacy.globalConfig$
-      .pipe(
-        first(),
-        map((config: SharedGlobalConfig) => config.kibana.index)
-      )
-      .toPromise();
+    this.kibanaIndexConfig = initializerContext.config.legacy.globalConfig$;
     this.kibanaVersion = initializerContext.env.packageInfo.version;
   }
 
-  public async setup(
+  public setup(
     core: CoreSetup<AlertingPluginsStart, unknown>,
     plugins: AlertingPluginsSetup
-  ): Promise<PluginSetupContract> {
+  ): PluginSetupContract {
     this.licenseState = new LicenseState(plugins.licensing.license$);
     this.security = plugins.security;
 
@@ -187,15 +182,17 @@ export class AlertingPlugin {
 
     const usageCollection = plugins.usageCollection;
     if (usageCollection) {
-      initializeAlertingTelemetry(
-        this.telemetryLogger,
-        core,
-        plugins.taskManager,
-        await this.kibanaIndex
+      registerAlertsUsageCollector(
+        usageCollection,
+        core.getStartServices().then(([_, { taskManager }]) => taskManager)
       );
-
-      core.getStartServices().then(async ([, startPlugins]) => {
-        registerAlertsUsageCollector(usageCollection, startPlugins.taskManager);
+      this.kibanaIndexConfig.subscribe((config) => {
+        initializeAlertingTelemetry(
+          this.telemetryLogger,
+          core,
+          plugins.taskManager,
+          config.kibana.index
+        );
       });
     }
 

diff --git a/x-pack/plugins/alerts/server/usage/alerts_usage_collector.test.ts b/x-pack/plugins/alerts/server/usage/alerts_usage_collector.test.ts
@@ -22,12 +22,18 @@ describe('registerAlertsUsageCollector', () => {
   });
 
   it('should call registerCollector', () => {
-    registerAlertsUsageCollector(usageCollectionMock as UsageCollectionSetup, taskManagerStart);
+    registerAlertsUsageCollector(
+      usageCollectionMock as UsageCollectionSetup,
+      new Promise(() => taskManagerStart)
+    );
     expect(usageCollectionMock.registerCollector).toHaveBeenCalledTimes(1);
   });
 
   it('should call makeUsageCollector with type = alerts', () => {
-    registerAlertsUsageCollector(usageCollectionMock as UsageCollectionSetup, taskManagerStart);
+    registerAlertsUsageCollector(
+      usageCollectionMock as UsageCollectionSetup,
+      new Promise(() => taskManagerStart)
+    );
     expect(usageCollectionMock.makeUsageCollector).toHaveBeenCalledTimes(1);
     expect(usageCollectionMock.makeUsageCollector.mock.calls[0][0].type).toBe('alerts');
   });

diff --git a/x-pack/plugins/alerts/server/usage/alerts_usage_collector.ts b/x-pack/plugins/alerts/server/usage/alerts_usage_collector.ts
@@ -44,11 +44,14 @@ const byTypeSchema: MakeSchemaFrom<AlertsUsage>['count_by_type'] = {
 
 export function createAlertsUsageCollector(
   usageCollection: UsageCollectionSetup,
-  taskManager: TaskManagerStartContract
+  taskManager: Promise<TaskManagerStartContract>
 ) {
   return usageCollection.makeUsageCollector<AlertsUsage>({
     type: 'alerts',
-    isReady: () => true,
+    isReady: async () => {
+      await taskManager;
+      return true;
+    },
     fetch: async () => {
       try {
         const doc = await getLatestTaskState(await taskManager);
@@ -129,7 +132,7 @@ async function getLatestTaskState(taskManager: TaskManagerStartContract) {
 
 export function registerAlertsUsageCollector(
   usageCollection: UsageCollectionSetup,
-  taskManager: TaskManagerStartContract
+  taskManager: Promise<TaskManagerStartContract>
 ) {
   const collector = createAlertsUsageCollector(usageCollection, taskManager);
   usageCollection.registerCollector(collector);