Skip to content

Commit

Permalink
Adding telemetry usage counter for create rule API when predefined ID…
Browse files Browse the repository at this point in the history
… is specified
  • Loading branch information
ymao1 committed Aug 13, 2021
1 parent fe3b7d6 commit cd5c5d2
Show file tree
Hide file tree
Showing 10 changed files with 329 additions and 44 deletions.
33 changes: 33 additions & 0 deletions x-pack/plugins/alerting/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { AlertingPlugin, AlertingPluginsSetup, PluginSetupContract } from './plugin';
import { createUsageCollectionSetupMock } from 'src/plugins/usage_collection/server/mocks';
import { coreMock, statusServiceMock } from '../../../../src/core/server/mocks';
import { licensingMock } from '../../licensing/server/mocks';
import { encryptedSavedObjectsMock } from '../../encrypted_saved_objects/server/mocks';
Expand Down Expand Up @@ -60,6 +61,38 @@ describe('Alerting Plugin', () => {
);
});

it('should create usage counter if usageCollection plugin is defined', async () => {
const context = coreMock.createPluginInitializerContext<AlertsConfig>({
healthCheck: {
interval: '5m',
},
invalidateApiKeysTask: {
interval: '5m',
removalDelay: '1h',
},
maxEphemeralActionsPerAlert: 10,
});
plugin = new AlertingPlugin(context);

const encryptedSavedObjectsSetup = encryptedSavedObjectsMock.createSetup();
const usageCollectionSetup = createUsageCollectionSetupMock();

const setupMocks = coreMock.createSetup();
// need await to test number of calls of setupMocks.status.set, because it is under async function which awaiting core.getStartServices()
await plugin.setup(setupMocks, {
licensing: licensingMock.createSetup(),
encryptedSavedObjects: encryptedSavedObjectsSetup,
taskManager: taskManagerMock.createSetup(),
eventLog: eventLogServiceMock.create(),
actions: actionsMock.createSetup(),
statusService: statusServiceMock.createSetupContract(),
usageCollection: usageCollectionSetup,
});

expect(usageCollectionSetup.createUsageCounter).toHaveBeenCalled();
expect(usageCollectionSetup.registerCollector).toHaveBeenCalled();
});

describe('registerType()', () => {
let setup: PluginSetupContract;
const sampleAlertType: AlertType<never, never, never, never, never, 'default'> = {
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/alerting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ import {
SavedObjectsBulkGetObject,
ServiceStatusLevels,
} from '../../../../src/core/server';
import type { AlertingRequestHandlerContext } from './types';
import { AlertingRequestHandlerContext, ALERTS_FEATURE_ID } from './types';
import { defineRoutes } from './routes';
import { LICENSE_TYPE, LicensingPluginSetup, LicensingPluginStart } from '../../licensing/server';
import {
Expand Down Expand Up @@ -221,6 +221,9 @@ export class AlertingPlugin {
});
}

// Usage counter for telemetry
const usageCounter = plugins.usageCollection?.createUsageCounter(ALERTS_FEATURE_ID);

setupSavedObjects(
core.savedObjects,
plugins.encryptedSavedObjects,
Expand Down Expand Up @@ -274,7 +277,12 @@ export class AlertingPlugin {
// Routes
const router = core.http.createRouter<AlertingRequestHandlerContext>();
// Register routes
defineRoutes(router, this.licenseState, plugins.encryptedSavedObjects);
defineRoutes({
router,
licenseState: this.licenseState,
usageCounter,
encryptedSavedObjects: plugins.encryptedSavedObjects,
});

return {
registerType<
Expand Down
125 changes: 118 additions & 7 deletions x-pack/plugins/alerting/server/routes/create_rule.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ import { rulesClientMock } from '../rules_client.mock';
import { AlertTypeDisabledError } from '../lib';
import { AsApiContract } from './lib';
import { SanitizedAlert } from '../types';
import { encryptedSavedObjectsMock } from '../../../encrypted_saved_objects/server/mocks';
import { usageCountersServiceMock } from 'src/plugins/usage_collection/server/usage_counters/usage_counters_service.mock';

const rulesClient = rulesClientMock.create();

Expand Down Expand Up @@ -105,8 +107,16 @@ describe('createRuleRoute', () => {
it('creates a rule with proper parameters', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();

createRuleRoute(router, licenseState);
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract();
const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test');

createRuleRoute({
router,
licenseState,
encryptedSavedObjects,
usageCounter: mockUsageCounter,
});

const [config, handler] = router.post.mock.calls[0];

Expand All @@ -124,6 +134,7 @@ describe('createRuleRoute', () => {

expect(await handler(context, req, res)).toEqual({ body: createResult });

expect(mockUsageCounter.incrementCounter).not.toHaveBeenCalled();
expect(rulesClient.create).toHaveBeenCalledTimes(1);
expect(rulesClient.create.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Expand Down Expand Up @@ -166,15 +177,107 @@ describe('createRuleRoute', () => {
});
});

it('allows providing a custom id', async () => {
it('allows providing a custom id in default space', async () => {
const expectedResult = {
...createResult,
id: 'custom-id',
};
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract();
const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test');

createRuleRoute({
router,
licenseState,
encryptedSavedObjects,
usageCounter: mockUsageCounter,
});

const [config, handler] = router.post.mock.calls[0];

expect(config.path).toMatchInlineSnapshot(`"/api/alerting/rule/{id?}"`);

rulesClient.create.mockResolvedValueOnce({
...mockedAlert,
id: 'custom-id',
});

const [context, req, res] = mockHandlerArguments(
{ rulesClient },
{
params: { id: 'custom-id' },
body: ruleToCreate,
},
['ok']
);

expect(await handler(context, req, res)).toEqual({ body: expectedResult });

expect(mockUsageCounter.incrementCounter).toHaveBeenCalledWith({
counterName: 'ruleCreatedWithPredefinedIdInDefaultSpace',
incrementBy: 1,
});
expect(rulesClient.create).toHaveBeenCalledTimes(1);
expect(rulesClient.create.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Object {
"data": Object {
"actions": Array [
Object {
"group": "default",
"id": "2",
"params": Object {
"foo": true,
},
},
],
"alertTypeId": "1",
"consumer": "bar",
"enabled": true,
"name": "abc",
"notifyWhen": "onActionGroupChange",
"params": Object {
"bar": true,
},
"schedule": Object {
"interval": "10s",
},
"tags": Array [
"foo",
],
"throttle": "30s",
},
"options": Object {
"id": "custom-id",
},
},
]
`);

expect(res.ok).toHaveBeenCalledWith({
body: expectedResult,
});
});

createRuleRoute(router, licenseState);
it('allows providing a custom id in non-default space', async () => {
const expectedResult = {
...createResult,
id: 'custom-id',
};
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });
const mockUsageCountersSetup = usageCountersServiceMock.createSetupContract();
const mockUsageCounter = mockUsageCountersSetup.createUsageCounter('test');

createRuleRoute({
router,
licenseState,
encryptedSavedObjects,
usageCounter: mockUsageCounter,
});

const [config, handler] = router.post.mock.calls[0];

Expand All @@ -184,6 +287,7 @@ describe('createRuleRoute', () => {
...mockedAlert,
id: 'custom-id',
});
rulesClient.getSpaceId.mockReturnValueOnce('another-space');

const [context, req, res] = mockHandlerArguments(
{ rulesClient },
Expand All @@ -196,6 +300,10 @@ describe('createRuleRoute', () => {

expect(await handler(context, req, res)).toEqual({ body: expectedResult });

expect(mockUsageCounter.incrementCounter).toHaveBeenCalledWith({
counterName: 'ruleCreatedWithPredefinedIdInCustomSpace',
incrementBy: 1,
});
expect(rulesClient.create).toHaveBeenCalledTimes(1);
expect(rulesClient.create.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Expand Down Expand Up @@ -241,8 +349,9 @@ describe('createRuleRoute', () => {
it('ensures the license allows creating rules', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });

createRuleRoute(router, licenseState);
createRuleRoute({ router, licenseState, encryptedSavedObjects });

const [, handler] = router.post.mock.calls[0];

Expand All @@ -258,12 +367,13 @@ describe('createRuleRoute', () => {
it('ensures the license check prevents creating rules', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });

(verifyApiAccess as jest.Mock).mockImplementation(() => {
throw new Error('OMG');
});

createRuleRoute(router, licenseState);
createRuleRoute({ router, licenseState, encryptedSavedObjects });

const [, handler] = router.post.mock.calls[0];

Expand All @@ -279,8 +389,9 @@ describe('createRuleRoute', () => {
it('ensures the rule type gets validated for the license', async () => {
const licenseState = licenseStateMock.create();
const router = httpServiceMock.createRouter();
const encryptedSavedObjects = encryptedSavedObjectsMock.createSetup({ canEncrypt: true });

createRuleRoute(router, licenseState);
createRuleRoute({ router, licenseState, encryptedSavedObjects });

const [, handler] = router.post.mock.calls[0];

Expand Down
23 changes: 16 additions & 7 deletions x-pack/plugins/alerting/server/routes/create_rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,8 @@
* 2.0.
*/

import { IRouter } from 'kibana/server';
import { schema } from '@kbn/config-schema';
import { validateDurationSchema, ILicenseState, AlertTypeDisabledError } from '../lib';
import { validateDurationSchema, AlertTypeDisabledError } from '../lib';
import { CreateOptions } from '../rules_client';
import {
RewriteRequestCase,
Expand All @@ -19,10 +18,10 @@ import {
SanitizedAlert,
validateNotifyWhenType,
AlertTypeParams,
AlertingRequestHandlerContext,
BASE_ALERTING_API_PATH,
AlertNotifyWhenType,
} from '../types';
import { RouteOptions } from '.';

export const bodySchema = schema.object({
name: schema.string(),
Expand Down Expand Up @@ -93,10 +92,7 @@ const rewriteBodyRes: RewriteResponseCase<SanitizedAlert<AlertTypeParams>> = ({
})),
});

export const createRuleRoute = (
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState
) => {
export const createRuleRoute = ({ router, licenseState, usageCounter }: RouteOptions) => {
router.post(
{
path: `${BASE_ALERTING_API_PATH}/rule/{id?}`,
Expand All @@ -115,6 +111,19 @@ export const createRuleRoute = (
const rulesClient = context.alerting.getRulesClient();
const rule = req.body;
const params = req.params;

if (params?.id) {
if (usageCounter) {
const usageCounterName = rulesClient.getSpaceId()
? 'ruleCreatedWithPredefinedIdInCustomSpace'
: 'ruleCreatedWithPredefinedIdInDefaultSpace';
usageCounter?.incrementCounter({
counterName: usageCounterName,
incrementBy: 1,
});
}
}

try {
const createdRule: SanitizedAlert<AlertTypeParams> = await rulesClient.create<AlertTypeParams>(
{
Expand Down
20 changes: 13 additions & 7 deletions x-pack/plugins/alerting/server/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { IRouter } from 'kibana/server';
import { UsageCounter } from 'src/plugins/usage_collection/server';
import { ILicenseState } from '../lib';
import { defineLegacyRoutes } from './legacy';
import { AlertingRequestHandlerContext } from '../types';
Expand All @@ -28,13 +29,18 @@ import { unmuteAllRuleRoute } from './unmute_all_rule';
import { unmuteAlertRoute } from './unmute_alert';
import { updateRuleApiKeyRoute } from './update_rule_api_key';

export function defineRoutes(
router: IRouter<AlertingRequestHandlerContext>,
licenseState: ILicenseState,
encryptedSavedObjects: EncryptedSavedObjectsPluginSetup
) {
defineLegacyRoutes(router, licenseState, encryptedSavedObjects);
createRuleRoute(router, licenseState);
export interface RouteOptions {
router: IRouter<AlertingRequestHandlerContext>;
licenseState: ILicenseState;
encryptedSavedObjects: EncryptedSavedObjectsPluginSetup;
usageCounter?: UsageCounter;
}

export function defineRoutes(opts: RouteOptions) {
const { router, licenseState, encryptedSavedObjects } = opts;

defineLegacyRoutes(opts);
createRuleRoute(opts);
getRuleRoute(router, licenseState);
updateRuleRoute(router, licenseState);
deleteRuleRoute(router, licenseState);
Expand Down
Loading

0 comments on commit cd5c5d2

Please sign in to comment.