From 3ee6e476eecd6a771ee34332bc2eb574dc517332 Mon Sep 17 00:00:00 2001 From: Yuliia Naumenko Date: Fri, 20 Nov 2020 20:27:03 -0800 Subject: [PATCH] Fixed usage of `isReady` for usage collection of alerts and actions (#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 --- x-pack/plugins/actions/server/plugin.ts | 51 +++++++++---------- .../usage/actions_usage_collector.test.ts | 4 +- .../server/usage/actions_usage_collector.ts | 9 ++-- x-pack/plugins/alerts/server/plugin.ts | 33 ++++++------ .../usage/alerts_usage_collector.test.ts | 10 +++- .../server/usage/alerts_usage_collector.ts | 9 ++-- 6 files changed, 62 insertions(+), 54 deletions(-) diff --git a/x-pack/plugins/actions/server/plugin.ts b/x-pack/plugins/actions/server/plugin.ts index a160735e89a935..e61936321b8e0a 100644 --- a/x-pack/plugins/actions/server/plugin.ts +++ b/x-pack/plugins/actions/server/plugin.ts @@ -4,8 +4,9 @@ * 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, @@ -13,7 +14,6 @@ import { CoreStart, KibanaRequest, Logger, - SharedGlobalConfig, RequestHandler, IContextProvider, ElasticsearchServiceStart, @@ -128,7 +128,6 @@ const includedHiddenTypes = [ ]; export class ActionsPlugin implements Plugin, PluginStartContract> { - private readonly kibanaIndex: Promise; private readonly config: Promise; private readonly logger: Logger; @@ -143,20 +142,14 @@ export class ActionsPlugin implements Plugin, 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().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, 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, Plugi actionExecutor, actionTypeRegistry, taskRunnerFactory, - kibanaIndex, + kibanaIndexConfig, isESOUsingEphemeralEncryptionKey, preconfiguredActions, instantiateAuthorization, @@ -297,10 +294,12 @@ export class ActionsPlugin implements Plugin, 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 index 0e6c2ff37eb029..39a61cebe92dcd 100644 --- 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,7 +24,7 @@ describe('registerActionsUsageCollector', () => { it('should call registerCollector', () => { registerActionsUsageCollector( usageCollectionMock as UsageCollectionSetup, - mockTaskManagerStart + new Promise(() => mockTaskManagerStart) ); expect(usageCollectionMock.registerCollector).toHaveBeenCalledTimes(1); }); @@ -32,7 +32,7 @@ describe('registerActionsUsageCollector', () => { 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 index fac57b6282c445..f86c6a40e05055 100644 --- 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['count_by_type'] = { export function createActionsUsageCollector( usageCollection: UsageCollectionSetup, - taskManager: TaskManagerStartContract + taskManager: Promise ) { return usageCollection.makeUsageCollector({ 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 ) { 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 index 99cb45130718ab..4bfb44425544a9 100644 --- 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,10 +124,10 @@ export class AlertingPlugin { private security?: SecurityPluginSetup; private readonly alertsClientFactory: AlertsClientFactory; private readonly telemetryLogger: Logger; - private readonly kibanaIndex: Promise; 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().pipe(first()).toPromise(); @@ -135,19 +135,14 @@ export class AlertingPlugin { 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, plugins: AlertingPluginsSetup - ): Promise { + ): 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 index a5f83bc393d4ec..e731e3f536261f 100644 --- 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 index de82dd31877afb..40a9983ae27861 100644 --- 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['count_by_type'] = { export function createAlertsUsageCollector( usageCollection: UsageCollectionSetup, - taskManager: TaskManagerStartContract + taskManager: Promise ) { return usageCollection.makeUsageCollector({ 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 ) { const collector = createAlertsUsageCollector(usageCollection, taskManager); usageCollection.registerCollector(collector);