Skip to content

Commit

Permalink
[ML] New Platform server shim: update usage collector to use core sav…
Browse files Browse the repository at this point in the history
…edObjects (elastic#58058) (elastic#58367)

*  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
  • Loading branch information
alvarezmelissa87 authored Feb 24, 2020
1 parent 8c5498c commit 712f280
Show file tree
Hide file tree
Showing 8 changed files with 55 additions and 130 deletions.
3 changes: 2 additions & 1 deletion x-pack/legacy/plugins/ml/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
1 change: 0 additions & 1 deletion x-pack/legacy/plugins/ml/server/lib/ml_telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

export {
createMlTelemetry,
getSavedObjectsClient,
incrementFileDataVisualizerIndexCreationCount,
storeMlTelemetry,
MlTelemetry,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -28,11 +26,10 @@ export function makeMlUsageCollector(
isReady: () => true,
fetch: async (): Promise<MlTelemetry> => {
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();
Expand Down
102 changes: 25 additions & 77 deletions x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import {
createMlTelemetry,
getSavedObjectsClient,
incrementFileDataVisualizerIndexCreationCount,
ML_TELEMETRY_DOC_ID,
MlTelemetry,
Expand All @@ -26,82 +25,40 @@ 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,
},
};
});

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
) {
Expand Down Expand Up @@ -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 },
});
});
Expand Down
39 changes: 14 additions & 25 deletions x-pack/legacy/plugins/ml/server/lib/ml_telemetry/ml_telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
Expand All @@ -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<void> {
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;
}
Expand All @@ -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;
Expand All @@ -80,5 +69,5 @@ export async function incrementFileDataVisualizerIndexCreationCount(
}

const mlTelemetry = createMlTelemetry(indicesCount);
storeMlTelemetry(elasticsearchPlugin, savedObjects, mlTelemetry);
storeMlTelemetry(internalRepository, mlTelemetry);
}
21 changes: 6 additions & 15 deletions x-pack/legacy/plugins/ml/server/new_platform/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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';
Expand All @@ -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';
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -240,7 +231,7 @@ export class Plugin {
fileDataVisualizerRoutes(extendedRouteInitializationDeps);

initMlServerLog(logInitializationDeps);
makeMlUsageCollector(plugins.usageCollection, usageInitializationDeps);
makeMlUsageCollector(plugins.usageCollection, coreSavedObjects);
}

public stop() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading

0 comments on commit 712f280

Please sign in to comment.