From 5926b330b182f631ff218706defbe17316f21e8e Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Mon, 24 Feb 2020 11:17:29 -0500 Subject: [PATCH] [ML] New Platform server shim: update usage collector to use core savedObjects (#58058) * use NP savedObjects and createInternalRepository for usage collection * fix route doc typo * update MlTelemetrySavedObject type * remove fileDataVisualizer routes dependency on legacy es plugin * update mlTelemetry tests * remove deprecated use of getSavedObjectsClient --- x-pack/legacy/plugins/ml/index.ts | 3 +- .../ml/server/lib/ml_telemetry/index.ts | 1 - .../ml_telemetry/make_ml_usage_collector.ts | 15 ++- .../lib/ml_telemetry/ml_telemetry.test.ts | 102 +++++------------- .../server/lib/ml_telemetry/ml_telemetry.ts | 39 +++---- .../plugins/ml/server/new_platform/plugin.ts | 21 ++-- .../ml/server/routes/file_data_visualizer.ts | 2 +- .../ml/server/routes/job_audit_messages.ts | 2 +- 8 files changed, 55 insertions(+), 130 deletions(-) diff --git a/x-pack/legacy/plugins/ml/index.ts b/x-pack/legacy/plugins/ml/index.ts index 0ef5e14e44f718..09f1b9ccedce4f 100755 --- a/x-pack/legacy/plugins/ml/index.ts +++ b/x-pack/legacy/plugins/ml/index.ts @@ -81,7 +81,8 @@ export const ml = (kibana: any) => { injectUiAppVars: server.injectUiAppVars, http: mlHttpService, savedObjects: server.savedObjects, - elasticsearch: kbnServer.newPlatform.setup.core.elasticsearch, // NP + coreSavedObjects: kbnServer.newPlatform.start.core.savedObjects, + elasticsearch: kbnServer.newPlatform.setup.core.elasticsearch, }; const { usageCollection, cloud, home } = kbnServer.newPlatform.setup.plugins; const plugins = { diff --git a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/index.ts b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/index.ts index 5da4f6b62bcec9..dffd95f50e0d95 100644 --- a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/index.ts +++ b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/index.ts @@ -6,7 +6,6 @@ export { createMlTelemetry, - getSavedObjectsClient, incrementFileDataVisualizerIndexCreationCount, storeMlTelemetry, MlTelemetry, diff --git a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/make_ml_usage_collector.ts b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/make_ml_usage_collector.ts index 293480b2aa5dc8..a120450bbb2b0a 100644 --- a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/make_ml_usage_collector.ts +++ b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/make_ml_usage_collector.ts @@ -5,19 +5,17 @@ */ import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; +import { SavedObjectsServiceStart } from 'src/core/server'; import { createMlTelemetry, - getSavedObjectsClient, ML_TELEMETRY_DOC_ID, MlTelemetry, MlTelemetrySavedObject, } from './ml_telemetry'; -import { UsageInitialization } from '../../new_platform/plugin'; - export function makeMlUsageCollector( usageCollection: UsageCollectionSetup | undefined, - { elasticsearchPlugin, savedObjects }: UsageInitialization + savedObjects: SavedObjectsServiceStart ): void { if (!usageCollection) { return; @@ -28,11 +26,10 @@ export function makeMlUsageCollector( isReady: () => true, fetch: async (): Promise => { try { - const savedObjectsClient = getSavedObjectsClient(elasticsearchPlugin, savedObjects); - const mlTelemetrySavedObject = (await savedObjectsClient.get( - 'ml-telemetry', - ML_TELEMETRY_DOC_ID - )) as MlTelemetrySavedObject; + const mlTelemetrySavedObject: MlTelemetrySavedObject = await savedObjects + .createInternalRepository() + .get('ml-telemetry', ML_TELEMETRY_DOC_ID); + return mlTelemetrySavedObject.attributes; } catch (err) { return createMlTelemetry(); diff --git a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.test.ts b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.test.ts index fcf3763626b6f6..9d14ffb31be631 100644 --- a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.test.ts +++ b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.test.ts @@ -6,7 +6,6 @@ import { createMlTelemetry, - getSavedObjectsClient, incrementFileDataVisualizerIndexCreationCount, ML_TELEMETRY_DOC_ID, MlTelemetry, @@ -26,22 +25,11 @@ describe('ml_telemetry', () => { }); describe('storeMlTelemetry', () => { - let elasticsearchPlugin: any; - let savedObjects: any; let mlTelemetry: MlTelemetry; - let savedObjectsClientInstance: any; + let internalRepository: any; beforeEach(() => { - savedObjectsClientInstance = { create: jest.fn() }; - const callWithInternalUser = jest.fn(); - const internalRepository = jest.fn(); - elasticsearchPlugin = { - getCluster: jest.fn(() => ({ callWithInternalUser })), - }; - savedObjects = { - SavedObjectsClient: jest.fn(() => savedObjectsClientInstance), - getSavedObjectsRepository: jest.fn(() => internalRepository), - }; + internalRepository = { create: jest.fn(), get: jest.fn() }; mlTelemetry = { file_data_visualizer: { index_creation_count: 1, @@ -49,59 +37,28 @@ describe('ml_telemetry', () => { }; }); - it('should call savedObjectsClient create with the given MlTelemetry object', () => { - storeMlTelemetry(elasticsearchPlugin, savedObjects, mlTelemetry); - expect(savedObjectsClientInstance.create.mock.calls[0][1]).toBe(mlTelemetry); + it('should call internalRepository create with the given MlTelemetry object', () => { + storeMlTelemetry(internalRepository, mlTelemetry); + expect(internalRepository.create.mock.calls[0][1]).toBe(mlTelemetry); }); - it('should call savedObjectsClient create with the ml-telemetry document type and ID', () => { - storeMlTelemetry(elasticsearchPlugin, savedObjects, mlTelemetry); - expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe('ml-telemetry'); - expect(savedObjectsClientInstance.create.mock.calls[0][2].id).toBe(ML_TELEMETRY_DOC_ID); + it('should call internalRepository create with the ml-telemetry document type and ID', () => { + storeMlTelemetry(internalRepository, mlTelemetry); + expect(internalRepository.create.mock.calls[0][0]).toBe('ml-telemetry'); + expect(internalRepository.create.mock.calls[0][2].id).toBe(ML_TELEMETRY_DOC_ID); }); - it('should call savedObjectsClient create with overwrite: true', () => { - storeMlTelemetry(elasticsearchPlugin, savedObjects, mlTelemetry); - expect(savedObjectsClientInstance.create.mock.calls[0][2].overwrite).toBe(true); - }); - }); - - describe('getSavedObjectsClient', () => { - let elasticsearchPlugin: any; - let savedObjects: any; - let savedObjectsClientInstance: any; - let callWithInternalUser: any; - let internalRepository: any; - - beforeEach(() => { - savedObjectsClientInstance = { create: jest.fn() }; - callWithInternalUser = jest.fn(); - internalRepository = jest.fn(); - elasticsearchPlugin = { - getCluster: jest.fn(() => ({ callWithInternalUser })), - }; - savedObjects = { - SavedObjectsClient: jest.fn(() => savedObjectsClientInstance), - getSavedObjectsRepository: jest.fn(() => internalRepository), - }; - }); - - it('should return a SavedObjectsClient initialized with the saved objects internal repository', () => { - const result = getSavedObjectsClient(elasticsearchPlugin, savedObjects); - - expect(result).toBe(savedObjectsClientInstance); - expect(savedObjects.SavedObjectsClient).toHaveBeenCalledWith(internalRepository); + it('should call internalRepository create with overwrite: true', () => { + storeMlTelemetry(internalRepository, mlTelemetry); + expect(internalRepository.create.mock.calls[0][2].overwrite).toBe(true); }); }); describe('incrementFileDataVisualizerIndexCreationCount', () => { - let elasticsearchPlugin: any; let savedObjects: any; - let savedObjectsClientInstance: any; - let callWithInternalUser: any; let internalRepository: any; - function createSavedObjectsClientInstance( + function createInternalRepositoryInstance( telemetryEnabled?: boolean, indexCreationCount?: number ) { @@ -136,51 +93,42 @@ describe('ml_telemetry', () => { } function mockInit(telemetryEnabled?: boolean, indexCreationCount?: number): void { - savedObjectsClientInstance = createSavedObjectsClientInstance( - telemetryEnabled, - indexCreationCount - ); - callWithInternalUser = jest.fn(); - internalRepository = jest.fn(); + internalRepository = createInternalRepositoryInstance(telemetryEnabled, indexCreationCount); savedObjects = { - SavedObjectsClient: jest.fn(() => savedObjectsClientInstance), - getSavedObjectsRepository: jest.fn(() => internalRepository), - }; - elasticsearchPlugin = { - getCluster: jest.fn(() => ({ callWithInternalUser })), + createInternalRepository: jest.fn(() => internalRepository), }; } it('should not increment if telemetry status cannot be determined', async () => { mockInit(); - await incrementFileDataVisualizerIndexCreationCount(elasticsearchPlugin, savedObjects); + await incrementFileDataVisualizerIndexCreationCount(savedObjects); - expect(savedObjectsClientInstance.create.mock.calls).toHaveLength(0); + expect(internalRepository.create.mock.calls).toHaveLength(0); }); it('should not increment if telemetry status is disabled', async () => { mockInit(false); - await incrementFileDataVisualizerIndexCreationCount(elasticsearchPlugin, savedObjects); + await incrementFileDataVisualizerIndexCreationCount(savedObjects); - expect(savedObjectsClientInstance.create.mock.calls).toHaveLength(0); + expect(internalRepository.create.mock.calls).toHaveLength(0); }); it('should initialize index_creation_count with 1', async () => { mockInit(true); - await incrementFileDataVisualizerIndexCreationCount(elasticsearchPlugin, savedObjects); + await incrementFileDataVisualizerIndexCreationCount(savedObjects); - expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe('ml-telemetry'); - expect(savedObjectsClientInstance.create.mock.calls[0][1]).toEqual({ + expect(internalRepository.create.mock.calls[0][0]).toBe('ml-telemetry'); + expect(internalRepository.create.mock.calls[0][1]).toEqual({ file_data_visualizer: { index_creation_count: 1 }, }); }); it('should increment index_creation_count to 2', async () => { mockInit(true, 1); - await incrementFileDataVisualizerIndexCreationCount(elasticsearchPlugin, savedObjects); + await incrementFileDataVisualizerIndexCreationCount(savedObjects); - expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe('ml-telemetry'); - expect(savedObjectsClientInstance.create.mock.calls[0][1]).toEqual({ + expect(internalRepository.create.mock.calls[0][0]).toBe('ml-telemetry'); + expect(internalRepository.create.mock.calls[0][1]).toEqual({ file_data_visualizer: { index_creation_count: 2 }, }); }); diff --git a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.ts b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.ts index 1bac3f17806449..d76b1ee94e21e9 100644 --- a/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.ts +++ b/x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.ts @@ -4,11 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { ElasticsearchPlugin } from 'src/legacy/core_plugins/elasticsearch'; -import { SavedObjectsLegacyService } from 'src/legacy/server/kbn_server'; -import { callWithInternalUserFactory } from '../../client/call_with_internal_user_factory'; +import { + SavedObjectAttributes, + SavedObjectsServiceStart, + ISavedObjectsRepository, +} from 'src/core/server'; -export interface MlTelemetry { +export interface MlTelemetry extends SavedObjectAttributes { file_data_visualizer: { index_creation_count: number; }; @@ -29,35 +31,22 @@ export function createMlTelemetry(count: number = 0): MlTelemetry { } // savedObjects export function storeMlTelemetry( - elasticsearchPlugin: ElasticsearchPlugin, - savedObjects: SavedObjectsLegacyService, + internalRepository: ISavedObjectsRepository, mlTelemetry: MlTelemetry ): void { - const savedObjectsClient = getSavedObjectsClient(elasticsearchPlugin, savedObjects); - savedObjectsClient.create('ml-telemetry', mlTelemetry, { + internalRepository.create('ml-telemetry', mlTelemetry, { id: ML_TELEMETRY_DOC_ID, overwrite: true, }); } -// needs savedObjects and elasticsearchPlugin -export function getSavedObjectsClient( - elasticsearchPlugin: ElasticsearchPlugin, - savedObjects: SavedObjectsLegacyService -): any { - const { SavedObjectsClient, getSavedObjectsRepository } = savedObjects; - const callWithInternalUser = callWithInternalUserFactory(elasticsearchPlugin); - const internalRepository = getSavedObjectsRepository(callWithInternalUser); - return new SavedObjectsClient(internalRepository); -} export async function incrementFileDataVisualizerIndexCreationCount( - elasticsearchPlugin: ElasticsearchPlugin, - savedObjects: SavedObjectsLegacyService + savedObjects: SavedObjectsServiceStart ): Promise { - const savedObjectsClient = getSavedObjectsClient(elasticsearchPlugin, savedObjects); - + const internalRepository = await savedObjects.createInternalRepository(); try { - const { attributes } = await savedObjectsClient.get('telemetry', 'telemetry'); + const { attributes } = await internalRepository.get('telemetry', 'telemetry'); + if (attributes.enabled === false) { return; } @@ -70,7 +59,7 @@ export async function incrementFileDataVisualizerIndexCreationCount( let indicesCount = 1; try { - const { attributes } = (await savedObjectsClient.get( + const { attributes } = (await internalRepository.get( 'ml-telemetry', ML_TELEMETRY_DOC_ID )) as MlTelemetrySavedObject; @@ -80,5 +69,5 @@ export async function incrementFileDataVisualizerIndexCreationCount( } const mlTelemetry = createMlTelemetry(indicesCount); - storeMlTelemetry(elasticsearchPlugin, savedObjects, mlTelemetry); + storeMlTelemetry(internalRepository, mlTelemetry); } diff --git a/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts b/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts index 10961182be841a..43c276ac63a13d 100644 --- a/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts +++ b/x-pack/legacy/plugins/ml/server/new_platform/plugin.ts @@ -14,6 +14,7 @@ import { CoreSetup, IRouter, IScopedClusterClient, + SavedObjectsServiceStart, } from 'src/core/server'; import { ElasticsearchPlugin } from 'src/legacy/core_plugins/elasticsearch'; import { UsageCollectionSetup } from 'src/plugins/usage_collection/server'; @@ -28,12 +29,10 @@ import { LICENSE_TYPE } from '../../common/constants/license'; import { annotationRoutes } from '../routes/annotations'; import { jobRoutes } from '../routes/anomaly_detectors'; import { dataFeedRoutes } from '../routes/datafeeds'; -// @ts-ignore: could not find declaration file for module import { indicesRoutes } from '../routes/indices'; import { jobValidationRoutes } from '../routes/job_validation'; import { makeMlUsageCollector } from '../lib/ml_telemetry'; import { notificationRoutes } from '../routes/notification_settings'; -// @ts-ignore: could not find declaration file for module import { systemRoutes } from '../routes/system'; import { dataFrameAnalyticsRoutes } from '../routes/data_frame_analytics'; import { dataRecognizer } from '../routes/modules'; @@ -45,7 +44,6 @@ import { filtersRoutes } from '../routes/filters'; import { resultsServiceRoutes } from '../routes/results_service'; import { jobServiceRoutes } from '../routes/job_service'; import { jobAuditMessagesRoutes } from '../routes/job_audit_messages'; -// @ts-ignore: could not find declaration file for module import { fileDataVisualizerRoutes } from '../routes/file_data_visualizer'; import { initMlServerLog, LogInitialization } from '../client/log'; import { HomeServerPluginSetup } from '../../../../../../src/plugins/home/server'; @@ -67,6 +65,7 @@ export interface MlCoreSetup { injectUiAppVars: (id: string, callback: () => {}) => any; http: MlHttpServiceSetup; savedObjects: SavedObjectsLegacyService; + coreSavedObjects: SavedObjectsServiceStart; elasticsearch: ElasticsearchServiceSetup; } export interface MlInitializerContext extends PluginInitializerContext { @@ -93,15 +92,11 @@ export interface RouteInitialization { route(route: ServerRoute | ServerRoute[]): void; router: IRouter; xpackMainPlugin: MlXpackMainPlugin; - savedObjects?: SavedObjectsLegacyService; + savedObjects?: SavedObjectsServiceStart; spacesPlugin: any; securityPlugin: any; cloud?: CloudSetup; } -export interface UsageInitialization { - elasticsearchPlugin: ElasticsearchPlugin; - savedObjects: SavedObjectsLegacyService; -} declare module 'kibana/server' { interface RequestHandlerContext { @@ -123,7 +118,7 @@ export class Plugin { public setup(core: MlCoreSetup, plugins: PluginsSetup) { const xpackMainPlugin: MlXpackMainPlugin = plugins.xpackMain; - const { http } = core; + const { http, coreSavedObjects } = core; const pluginId = this.pluginId; mirrorPluginStatus(xpackMainPlugin, plugins.ml); @@ -208,14 +203,10 @@ export class Plugin { const extendedRouteInitializationDeps: RouteInitialization = { ...routeInitializationDeps, config: this.config, - savedObjects: core.savedObjects, + savedObjects: coreSavedObjects, spacesPlugin: plugins.spaces, cloud: plugins.cloud, }; - const usageInitializationDeps: UsageInitialization = { - elasticsearchPlugin: plugins.elasticsearch, - savedObjects: core.savedObjects, - }; const logInitializationDeps: LogInitialization = { log: this.log, @@ -240,7 +231,7 @@ export class Plugin { fileDataVisualizerRoutes(extendedRouteInitializationDeps); initMlServerLog(logInitializationDeps); - makeMlUsageCollector(plugins.usageCollection, usageInitializationDeps); + makeMlUsageCollector(plugins.usageCollection, coreSavedObjects); } public stop() {} diff --git a/x-pack/legacy/plugins/ml/server/routes/file_data_visualizer.ts b/x-pack/legacy/plugins/ml/server/routes/file_data_visualizer.ts index 95f2a9fe7298f0..d5a992c9332930 100644 --- a/x-pack/legacy/plugins/ml/server/routes/file_data_visualizer.ts +++ b/x-pack/legacy/plugins/ml/server/routes/file_data_visualizer.ts @@ -138,7 +138,7 @@ export function fileDataVisualizerRoutes({ // follow-up import calls to just add additional data will include the `id` of the created // index, we'll ignore those and don't increment the counter. if (id === undefined) { - await incrementFileDataVisualizerIndexCreationCount(elasticsearchPlugin, savedObjects!); + await incrementFileDataVisualizerIndexCreationCount(savedObjects!); } const result = await importData( diff --git a/x-pack/legacy/plugins/ml/server/routes/job_audit_messages.ts b/x-pack/legacy/plugins/ml/server/routes/job_audit_messages.ts index 72983129900052..76986b935b993a 100644 --- a/x-pack/legacy/plugins/ml/server/routes/job_audit_messages.ts +++ b/x-pack/legacy/plugins/ml/server/routes/job_audit_messages.ts @@ -50,7 +50,7 @@ export function jobAuditMessagesRoutes({ xpackMainPlugin, router }: RouteInitial /** * @apiGroup JobAuditMessages * - * @api {get} /api/ml/results/anomalies_table_data Get all audit messages + * @api {get} /api/ml/job_audit_messages/messages Get all audit messages * @apiName GetAllJobAuditMessages * @apiDescription Returns all audit messages */