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.

Sign up for GitHub

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

[Alerting] Adding telemetry usage counter for create rule API when predefined ID is specified #108572

Merged
merged 9 commits into from
Aug 17, 2021
Merged

[Alerting] Adding telemetry usage counter for create rule API when predefined ID is specified #108572

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cd5c5d2
Adding telemetry usage counter for create rule API when predefined ID…
ymao1 Aug 13, 2021
5f96d6f
Merge branch 'master' of https://github.com/elastic/kibana into alert…
ymao1 Aug 16, 2021
f9deb90
Checking explicitly for default space id
ymao1 Aug 16, 2021
1e43aec
Handling undefined space id when spaces is disabled
ymao1 Aug 16, 2021
ba8891e
Merge branch 'master' of https://github.com/elastic/kibana into alert…
ymao1 Aug 16, 2021
38c2c4b
Consolidating logic in single function
ymao1 Aug 16, 2021
507e4ef
Merge branch 'master' of https://github.com/elastic/kibana into alert…
ymao1 Aug 16, 2021
6859712
Tracking total usage and custom usage
ymao1 Aug 16, 2021
70e5b9a
Merge branch 'master' of https://github.com/elastic/kibana into alert…
ymao1 Aug 16, 2021
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
33 changes: 33 additions & 0 deletions x-pack/plugins/alerting/server/plugin.test.ts
View file
Open in desktop
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
View file
Open in desktop
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({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed this interface to options object instead of list of params bc the list is getting long and I'm planning to add logger as well in another PR

router,
licenseState: this.licenseState,
usageCounter,
encryptedSavedObjects: plugins.encryptedSavedObjects,
});

return {
registerType<
Expand Down
210 changes: 203 additions & 7 deletions x-pack/plugins/alerting/server/routes/create_rule.test.ts
View file
Open in desktop
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 when space is undefined', 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];

createRuleRoute(router, licenseState);
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,
});
});

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];

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

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

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,
});
});

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];

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

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

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: 'ruleCreatedWithPredefinedIdInCustomSpace',
incrementBy: 1,
});
expect(rulesClient.create).toHaveBeenCalledTimes(1);
expect(rulesClient.create.mock.calls[0]).toMatchInlineSnapshot(`
Array [
Expand Down Expand Up @@ -241,8 +434,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 +452,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 +474,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
Loading