Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adds telemetry support to alerting and actions plugins #49832 #54214

Closed
Closed
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions x-pack/legacy/plugins/actions/mappings.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,16 @@
"type": "binary"
}
}
},
"actions-telemetry": {
"properties": {
"executions_total": {
"type": "integer"
},
"excutions_count_by_type": {
"enabled": false,
"type": "object"
}
}
}
}
3 changes: 3 additions & 0 deletions x-pack/legacy/plugins/actions/server/lib/action_executor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
GetServicesFunction,
RawAction,
} from '../types';
import { incrementActionExecutionsCount } from '../usage/actions_telemetry';

export interface ActionExecutorContext {
logger: Logger;
Expand Down Expand Up @@ -119,6 +120,8 @@ export class ActionExecutor {

logger.debug(`action executed successfully: ${actionLabel}`);

incrementActionExecutionsCount(services.savedObjectsClient, actionType.name);

// return basic response if none provided
if (result == null) return { status: 'ok', actionId };

Expand Down
10 changes: 10 additions & 0 deletions x-pack/legacy/plugins/actions/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
} from './routes';
import { extendRouteWithLicenseCheck } from './extend_route_with_license_check';
import { LicenseState } from './lib/license_state';
import { registerActionsUsageCollector } from './usage';

export interface PluginSetupContract {
registerType: ActionTypeRegistry['register'];
Expand Down Expand Up @@ -107,6 +108,15 @@ export class Plugin {
actionsConfigUtils,
});

registerActionsUsageCollector(
plugins.usageCollection,
plugins.savedObjects,
actionTypeRegistry,
{
isActionsEnabled: config.enabled,
}
);

// Routes
core.http.route(extendRouteWithLicenseCheck(createActionRoute, this.licenseState));
core.http.route(extendRouteWithLicenseCheck(deleteActionRoute, this.licenseState));
Expand Down
5 changes: 5 additions & 0 deletions x-pack/legacy/plugins/actions/server/shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import Hapi from 'hapi';
import { Legacy } from 'kibana';
import * as Rx from 'rxjs';
import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { ActionsConfigType } from './types';
import { TaskManager } from '../../task_manager/server';
import { XPackMainPlugin } from '../../xpack_main/server/xpack_main';
Expand Down Expand Up @@ -77,7 +78,9 @@ export interface ActionsPluginsSetup {
task_manager: TaskManagerSetupContract;
xpack_main: XPackMainPluginSetupContract;
encryptedSavedObjects: EncryptedSavedObjectsSetupContract;
savedObjects: SavedObjectsLegacyService;
licensing: LicensingPluginSetup;
usageCollection: UsageCollectionSetup;
}
export interface ActionsPluginsStart {
security?: SecurityPluginStartContract;
Expand Down Expand Up @@ -137,6 +140,8 @@ export function shim(
encryptedSavedObjects: newPlatform.setup.plugins
.encryptedSavedObjects as EncryptedSavedObjectsSetupContract,
licensing: newPlatform.setup.plugins.licensing as LicensingPluginSetup,
usageCollection: newPlatform.setup.plugins.usageCollection,
savedObjects: server.savedObjects,
};

const pluginsStart: ActionsPluginsStart = {
Expand Down
136 changes: 136 additions & 0 deletions x-pack/legacy/plugins/actions/server/usage/actions_telemetry.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import {
createActionsTelemetry,
incrementActionExecutionsCount,
ACTIONS_TELEMETRY_DOC_ID,
storeActionsTelemetry,
} from './actions_telemetry';
import { ActionsTelemetry } from './types';

describe('actions_telemetry', () => {
describe('createActionsTelemetry', () => {
it('should create a ActionsTelemetry object', () => {
const actionsTelemetry = createActionsTelemetry(1);
expect(actionsTelemetry.executions_total).toBe(1);
});
it('should ignore undefined or unknown values', () => {
const actionsTelemetry = createActionsTelemetry(undefined);
expect(actionsTelemetry.executions_total).toBe(0);
});
});

describe('storeActionsTelemetry', () => {
let actionsTelemetry: ActionsTelemetry;
let savedObjectsClientInstance: any;

beforeEach(() => {
savedObjectsClientInstance = { create: jest.fn() };
actionsTelemetry = {
executions_total: 1,
excutions_count_by_type: {},
};
});

it('should call savedObjectsClient create with the given ActionsTelemetry object', () => {
storeActionsTelemetry(savedObjectsClientInstance, actionsTelemetry);
expect(savedObjectsClientInstance.create.mock.calls[0][1]).toBe(actionsTelemetry);
});

it('should call savedObjectsClient create with the actions-telemetry document type and ID', () => {
storeActionsTelemetry(savedObjectsClientInstance, actionsTelemetry);
expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe('actions-telemetry');
expect(savedObjectsClientInstance.create.mock.calls[0][2].id).toBe(ACTIONS_TELEMETRY_DOC_ID);
});

it('should call savedObjectsClient create with overwrite: true', () => {
storeActionsTelemetry(savedObjectsClientInstance, actionsTelemetry);
expect(savedObjectsClientInstance.create.mock.calls[0][2].overwrite).toBe(true);
});
});

describe('incrementActionExecutionsCount', () => {
let savedObjectsClientInstance: any;

function createSavedObjectsClientInstance(
telemetryEnabled?: boolean,
executionsTotal?: number
) {
return {
create: jest.fn(),
get: jest.fn(obj => {
switch (obj) {
case 'telemetry':
if (telemetryEnabled === undefined) {
throw Error;
}
return {
attributes: {
enabled: telemetryEnabled,
},
};
case 'actions-telemetry':
// emulate that a non-existing saved object will throw an error
if (executionsTotal === undefined) {
throw Error;
}
return {
attributes: {
executions_total: executionsTotal,
excutions_count_by_type: { test: executionsTotal },
},
};
}
}),
};
}

function mockInit(telemetryEnabled?: boolean, executionsTotal?: number): void {
savedObjectsClientInstance = createSavedObjectsClientInstance(
telemetryEnabled,
executionsTotal
);
}

it('should not increment if telemetry status cannot be determined', async () => {
mockInit();
await incrementActionExecutionsCount(savedObjectsClientInstance, 'test');

expect(savedObjectsClientInstance.create.mock.calls).toHaveLength(0);
});

it('should not increment if telemetry status is disabled', async () => {
mockInit(false);
await incrementActionExecutionsCount(savedObjectsClientInstance, 'test');

expect(savedObjectsClientInstance.create.mock.calls).toHaveLength(0);
});

it('should initialize executions_total with 1 and excutions_count_by_type with proper key value pair', async () => {
mockInit(true, 0);
await incrementActionExecutionsCount(savedObjectsClientInstance, 'test');

expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe('actions-telemetry');
expect(savedObjectsClientInstance.create.mock.calls[0][1]).toEqual({
executions_total: 1,
excutions_count_by_type: { test: 1 },
});
});

it('should increment index_creation_count to 2', async () => {
mockInit(true, 1);
await incrementActionExecutionsCount(savedObjectsClientInstance, 'some');
await incrementActionExecutionsCount(savedObjectsClientInstance, 'test');

expect(savedObjectsClientInstance.create.mock.calls[0][0]).toBe('actions-telemetry');
expect(savedObjectsClientInstance.create.mock.calls[0][1]).toEqual({
executions_total: 2,
excutions_count_by_type: { some: 1, test: 1 },
});
});
});
});
68 changes: 68 additions & 0 deletions x-pack/legacy/plugins/actions/server/usage/actions_telemetry.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { ActionsTelemetry, ActionsTelemetrySavedObject } from './types';

export const ACTIONS_TELEMETRY_DOC_ID = 'actions-telemetry';

export function createActionsTelemetry(
executionsTotal: number = 0,
executionsByActionType: Record<string, number> = {}
): ActionsTelemetry {
return {
executions_total: executionsTotal,
excutions_count_by_type: executionsByActionType,
};
}
// savedObjects
export function storeActionsTelemetry(
savedObjectsClient: any,
actionsTelemetry: ActionsTelemetry
): void {
savedObjectsClient.create('actions-telemetry', actionsTelemetry, {
id: ACTIONS_TELEMETRY_DOC_ID,
overwrite: true,
});
}

export async function incrementActionExecutionsCount(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems very expensive to make this call on every action execution, as it's making 3 SO calls in the function. Is there a pattern where incrementActionExecutionsCount() would just update a local object in the Kibana server, and then every 30 minutes or so update the SO? Not sure how other plugins do this.

savedObjectsClient: any,
actionTypeId: string
): Promise<void> {
try {
const { attributes } = await savedObjectsClient.get('telemetry', 'telemetry');
if (attributes.enabled === false) {
return;
}
} catch (error) {
// if we aren't allowed to get the telemetry document,
// we assume we couldn't opt in to telemetry and won't increment the index count.
return;
}

let executionsTotal = 1;
let executionsByActionTypes = {};

try {
const { attributes } = (await savedObjectsClient.get(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like there's a race condition here - shouldn't storeActionsTelemetry() use optimistic concurrency to update the SO? It doesn't seem like it does. Maybe for telemetry we don't care that much if it's off by a little?

'actions-telemetry',
ACTIONS_TELEMETRY_DOC_ID
)) as ActionsTelemetrySavedObject;
executionsTotal = attributes.executions_total + 1;
const executionsByActionType = attributes.excutions_count_by_type[actionTypeId]
? attributes.excutions_count_by_type[actionTypeId]
: 0;
executionsByActionTypes = {
...attributes.excutions_count_by_type,
[actionTypeId]: executionsByActionType + 1,
};
} catch (e) {
/* silently fail, this will happen if the saved object doesn't exist yet. */
}

const actionsTelemetry = createActionsTelemetry(executionsTotal, executionsByActionTypes);
storeActionsTelemetry(savedObjectsClient, actionsTelemetry);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { UsageCollectionSetup } from 'src/plugins/usage_collection/server';
import { registerActionsUsageCollector } from './actions_usage_collector';
import { ActionTypeRegistry } from '../action_type_registry';
import { taskManagerMock } from '../../../task_manager/server/task_manager.mock';
import { TaskRunnerFactory, ActionExecutor } from '../lib';
import { configUtilsMock } from '../actions_config.mock';

const mockTaskManager = taskManagerMock.create();
const actionTypeRegistryParams = {
taskManager: mockTaskManager,
taskRunnerFactory: new TaskRunnerFactory(new ActionExecutor()),
actionsConfigUtils: configUtilsMock,
};

beforeEach(() => jest.resetAllMocks());

describe('registerActionsUsageCollector', () => {
let usageCollectionMock: jest.Mocked<UsageCollectionSetup>;
let actionTypeRegistry: ActionTypeRegistry;
let savedObjects: any;
let savedObjectsClientInstance: any;

beforeEach(() => {
savedObjectsClientInstance = { create: jest.fn() };
const internalRepository = jest.fn();
actionTypeRegistry = new ActionTypeRegistry(actionTypeRegistryParams);
savedObjects = {
SavedObjectsClient: jest.fn(() => savedObjectsClientInstance),
getSavedObjectsRepository: jest.fn(() => internalRepository),
};
usageCollectionMock = ({
makeUsageCollector: jest.fn(),
registerCollector: jest.fn(),
} as unknown) as jest.Mocked<UsageCollectionSetup>;
});

it('should call registerCollector', () => {
registerActionsUsageCollector(usageCollectionMock, savedObjects, actionTypeRegistry, {
isActionsEnabled: true,
});
expect(usageCollectionMock.registerCollector).toHaveBeenCalledTimes(1);
});

it('should call makeUsageCollector with type = actions', () => {
registerActionsUsageCollector(usageCollectionMock, savedObjects, actionTypeRegistry, {
isActionsEnabled: true,
});
expect(usageCollectionMock.makeUsageCollector).toHaveBeenCalledTimes(1);
expect(usageCollectionMock.makeUsageCollector.mock.calls[0][0].type).toBe('actions');
});
});
Loading