Skip to content

Commit

Permalink
[ResponseOps][Cases] Fix a bug with cases telemetry where data from o…
Browse files Browse the repository at this point in the history
…ther spaces are not included (elastic#193166)

## Summary

The Find SO API supports the `namespaces` parameter where you can define
the spaces that the SO client should search for. If you omit the
`namespaces` parameter, the SO client will use the active space. This PR
creates a wrapper around the SO client to add the `namespaces: ['*']` to
all Find SO usages to count telemetry on all spaces.

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

### For maintainers

- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: kibanamachine <[email protected]>
  • Loading branch information
cnasikas and kibanamachine authored Sep 19, 2024
1 parent 1c39fa2 commit 6b372b7
Show file tree
Hide file tree
Showing 28 changed files with 379 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { getCasesSystemActionData } from './queries/case_system_action';
import { getUserCommentsTelemetryData } from './queries/comments';
import { getConfigurationTelemetryData } from './queries/configuration';
import { getConnectorsTelemetryData } from './queries/connectors';
import { getPushedTelemetryData } from './queries/pushes';
import { getPushedTelemetryData } from './queries/push';
import { getUserActionsTelemetryData } from './queries/user_actions';
import type { CasesTelemetry, CollectTelemetryDataParams } from './types';

Expand Down
17 changes: 9 additions & 8 deletions x-pack/plugins/cases/server/telemetry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,7 @@
* 2.0.
*/

import type {
CoreSetup,
ISavedObjectsRepository,
Logger,
PluginInitializerContext,
} from '@kbn/core/server';
import type { CoreSetup, Logger, PluginInitializerContext } from '@kbn/core/server';
import { SavedObjectsErrorHelpers } from '@kbn/core/server';
import type { TaskManagerSetupContract } from '@kbn/task-manager-plugin/server';
import type { UsageCollectionSetup } from '@kbn/usage-collection-plugin/server';
Expand All @@ -25,6 +20,7 @@ import {
} from '../../common/constants';
import type { CasesTelemetry } from './types';
import { casesSchema } from './schema';
import { TelemetrySavedObjectsClient } from './telemetry_saved_objects_client';

export { scheduleCasesTelemetryTask } from './schedule_telemetry_task';

Expand All @@ -42,13 +38,18 @@ export const createCasesTelemetry = ({
usageCollection,
logger,
}: CreateCasesTelemetryArgs) => {
const getInternalSavedObjectClient = async (): Promise<ISavedObjectsRepository> => {
const getInternalSavedObjectClient = async (): Promise<TelemetrySavedObjectsClient> => {
const [coreStart] = await core.getStartServices();
return coreStart.savedObjects.createInternalRepository([
const soClient = coreStart.savedObjects.createInternalRepository([
...SAVED_OBJECT_TYPES,
FILE_SO_TYPE,
CASE_RULES_SAVED_OBJECT,
]);

// Wrapping the internalRepository with the `TelemetrySavedObjectsClient`
// to ensure some best practices when collecting "all the telemetry"
// (i.e.: `.find` requests should query all spaces)
return new TelemetrySavedObjectsClient(soClient);
};

taskManager.registerTaskDefinitions({
Expand Down
11 changes: 9 additions & 2 deletions x-pack/plugins/cases/server/telemetry/queries/alerts.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,15 @@

import { loggingSystemMock, savedObjectsRepositoryMock } from '@kbn/core/server/mocks';
import { getAlertsTelemetryData } from './alerts';
import { TelemetrySavedObjectsClient } from '../telemetry_saved_objects_client';

describe('alerts', () => {
const logger = loggingSystemMock.createLogger();

describe('getAlertsTelemetryData', () => {
const savedObjectsClient = savedObjectsRepositoryMock.create();
const telemetrySavedObjectsClient = new TelemetrySavedObjectsClient(savedObjectsClient);

savedObjectsClient.find.mockResolvedValue({
total: 5,
saved_objects: [],
Expand All @@ -35,7 +38,10 @@ describe('alerts', () => {
});

it('it returns the correct res', async () => {
const res = await getAlertsTelemetryData({ savedObjectsClient, logger });
const res = await getAlertsTelemetryData({
savedObjectsClient: telemetrySavedObjectsClient,
logger,
});
expect(res).toEqual({
all: {
total: 5,
Expand All @@ -48,7 +54,7 @@ describe('alerts', () => {
});

it('should call find with correct arguments', async () => {
await getAlertsTelemetryData({ savedObjectsClient, logger });
await getAlertsTelemetryData({ savedObjectsClient: telemetrySavedObjectsClient, logger });
expect(savedObjectsClient.find).toBeCalledWith({
aggs: {
counts: {
Expand Down Expand Up @@ -117,6 +123,7 @@ describe('alerts', () => {
page: 0,
perPage: 0,
type: 'cases-comments',
namespaces: ['*'],
});
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@

import { loggingSystemMock, savedObjectsRepositoryMock } from '@kbn/core/server/mocks';
import { getCasesSystemActionData } from './case_system_action';
import { TelemetrySavedObjectsClient } from '../telemetry_saved_objects_client';

describe('casesSystemAction', () => {
const logger = loggingSystemMock.createLogger();

describe('getCasesSystemActionData', () => {
const savedObjectsClient = savedObjectsRepositoryMock.create();
const telemetrySavedObjectsClient = new TelemetrySavedObjectsClient(savedObjectsClient);

beforeEach(() => {
jest.clearAllMocks();
Expand All @@ -26,7 +28,10 @@ describe('casesSystemAction', () => {
});

it('calculates the metrics correctly', async () => {
const res = await getCasesSystemActionData({ savedObjectsClient, logger });
const res = await getCasesSystemActionData({
savedObjectsClient: telemetrySavedObjectsClient,
logger,
});
expect(res).toEqual({ totalCasesCreated: 4, totalRules: 2 });
});

Expand All @@ -38,8 +43,49 @@ describe('casesSystemAction', () => {
page: 1,
});

const res = await getCasesSystemActionData({ savedObjectsClient, logger });
const res = await getCasesSystemActionData({
savedObjectsClient: telemetrySavedObjectsClient,
logger,
});

expect(res).toEqual({ totalCasesCreated: 0, totalRules: 0 });
});

it('should call find with correct arguments', async () => {
savedObjectsClient.find.mockResolvedValue({
total: 1,
saved_objects: [],
per_page: 1,
page: 1,
});

await getCasesSystemActionData({
savedObjectsClient: telemetrySavedObjectsClient,
logger,
});

expect(savedObjectsClient.find.mock.calls[0][0]).toMatchInlineSnapshot(`
Object {
"aggs": Object {
"counterSum": Object {
"sum": Object {
"field": "cases-rules.attributes.counter",
},
},
"totalRules": Object {
"cardinality": Object {
"field": "cases-rules.attributes.rules.id",
},
},
},
"namespaces": Array [
"*",
],
"page": 1,
"perPage": 1,
"type": "cases-rules",
}
`);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export const getCasesSystemActionData = async ({
cardinality: { field: `${CASE_RULES_SAVED_OBJECT}.attributes.rules.id` },
},
},
namespaces: ['*'],
});

return {
Expand Down
21 changes: 19 additions & 2 deletions x-pack/plugins/cases/server/telemetry/queries/cases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import type {
FileAttachmentAggregationResults,
} from '../types';
import { getCasesTelemetryData } from './cases';
import { TelemetrySavedObjectsClient } from '../telemetry_saved_objects_client';

const MOCK_FIND_TOTAL = 5;
const SOLUTION_TOTAL = 1;
Expand All @@ -23,6 +24,7 @@ describe('getCasesTelemetryData', () => {
describe('getCasesTelemetryData', () => {
const logger = loggingSystemMock.createLogger();
const savedObjectsClient = savedObjectsRepositoryMock.create();
const telemetrySavedObjectsClient = new TelemetrySavedObjectsClient(savedObjectsClient);

const mockFind = (aggs: object, so: SavedObjectsFindResponse['saved_objects'] = []) => {
savedObjectsClient.find.mockResolvedValueOnce({
Expand Down Expand Up @@ -322,7 +324,10 @@ describe('getCasesTelemetryData', () => {
};
};

const res = await getCasesTelemetryData({ savedObjectsClient, logger });
const res = await getCasesTelemetryData({
savedObjectsClient: telemetrySavedObjectsClient,
logger,
});

const allAttachmentsTotal = 5;
const allAttachmentsAverage = allAttachmentsTotal / MOCK_FIND_TOTAL;
Expand Down Expand Up @@ -406,7 +411,7 @@ describe('getCasesTelemetryData', () => {
it('should call find with correct arguments', async () => {
mockResponse();

await getCasesTelemetryData({ savedObjectsClient, logger });
await getCasesTelemetryData({ savedObjectsClient: telemetrySavedObjectsClient, logger });

expect(savedObjectsClient.find.mock.calls[0][0]).toMatchInlineSnapshot(`
Object {
Expand Down Expand Up @@ -660,6 +665,9 @@ describe('getCasesTelemetryData', () => {
},
},
},
"namespaces": Array [
"*",
],
"page": 0,
"perPage": 0,
"type": "cases",
Expand Down Expand Up @@ -974,6 +982,9 @@ describe('getCasesTelemetryData', () => {
},
},
},
"namespaces": Array [
"*",
],
"page": 0,
"perPage": 0,
"type": "cases-comments",
Expand Down Expand Up @@ -1023,6 +1034,7 @@ describe('getCasesTelemetryData', () => {
page: 0,
perPage: 0,
type: 'cases-comments',
namespaces: ['*'],
});

expect(savedObjectsClient.find.mock.calls[3][0]).toEqual({
Expand Down Expand Up @@ -1068,6 +1080,7 @@ describe('getCasesTelemetryData', () => {
page: 0,
perPage: 0,
type: 'cases-user-actions',
namespaces: ['*'],
});

for (const [index, sortField] of ['created_at', 'updated_at', 'closed_at'].entries()) {
Expand All @@ -1079,6 +1092,7 @@ describe('getCasesTelemetryData', () => {
sortField,
sortOrder: 'desc',
type: 'cases',
namespaces: ['*'],
});
}

Expand Down Expand Up @@ -1172,6 +1186,9 @@ describe('getCasesTelemetryData', () => {
"function": "is",
"type": "function",
},
"namespaces": Array [
"*",
],
"page": 0,
"perPage": 0,
"type": "file",
Expand Down
19 changes: 13 additions & 6 deletions x-pack/plugins/cases/server/telemetry/queries/cases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* 2.0.
*/

import type { ISavedObjectsRepository, SavedObjectsFindResponse } from '@kbn/core/server';
import type { SavedObjectsFindResponse } from '@kbn/core/server';
import { FILE_SO_TYPE } from '@kbn/files-plugin/common';
import { fromKueryExpression } from '@kbn/es-query';
import {
Expand Down Expand Up @@ -37,6 +37,7 @@ import {
} from './utils';
import type { CasePersistedAttributes } from '../../common/types/case';
import { CasePersistedStatus } from '../../common/types/case';
import type { TelemetrySavedObjectsClient } from '../telemetry_saved_objects_client';

export const getLatestCasesDates = async ({
savedObjectsClient,
Expand All @@ -48,6 +49,7 @@ export const getLatestCasesDates = async ({
sortField,
sortOrder: 'desc',
type: CASE_SAVED_OBJECT,
namespaces: ['*'],
});

const savedObjects = await Promise.all([
Expand Down Expand Up @@ -145,7 +147,7 @@ export const getCasesTelemetryData = async ({
};

const getCasesSavedObjectTelemetry = async (
savedObjectsClient: ISavedObjectsRepository
savedObjectsClient: TelemetrySavedObjectsClient
): Promise<SavedObjectsFindResponse<unknown, CaseAggregationResult>> => {
const caseByOwnerAggregationQuery = OWNERS.reduce(
(aggQuery, owner) => ({
Expand All @@ -169,6 +171,7 @@ const getCasesSavedObjectTelemetry = async (
page: 0,
perPage: 0,
type: CASE_SAVED_OBJECT,
namespaces: ['*'],
aggs: {
...caseByOwnerAggregationQuery,
...getCountsAggregationQuery(CASE_SAVED_OBJECT),
Expand Down Expand Up @@ -231,7 +234,7 @@ const getAssigneesAggregations = () => ({
});

const getCommentsSavedObjectTelemetry = async (
savedObjectsClient: ISavedObjectsRepository
savedObjectsClient: TelemetrySavedObjectsClient
): Promise<SavedObjectsFindResponse<unknown, AttachmentAggregationResult>> => {
const attachmentRegistries = () => ({
externalReferenceTypes: {
Expand Down Expand Up @@ -275,6 +278,7 @@ const getCommentsSavedObjectTelemetry = async (
page: 0,
perPage: 0,
type: CASE_COMMENT_SAVED_OBJECT,
namespaces: ['*'],
aggs: {
...attachmentsByOwnerAggregationQuery,
...attachmentRegistries(),
Expand All @@ -288,7 +292,7 @@ const getCommentsSavedObjectTelemetry = async (
};

const getFilesTelemetry = async (
savedObjectsClient: ISavedObjectsRepository
savedObjectsClient: TelemetrySavedObjectsClient
): Promise<SavedObjectsFindResponse<unknown, FileAttachmentAggregationResults>> => {
const averageSize = () => ({
averageSize: {
Expand Down Expand Up @@ -332,17 +336,19 @@ const getFilesTelemetry = async (
perPage: 0,
type: FILE_SO_TYPE,
filter: filterCaseIdExists,
namespaces: ['*'],
aggs: { ...filesByOwnerAggregationQuery, ...averageSize(), ...top20MimeTypes() },
});
};

const getAlertsTelemetry = async (
savedObjectsClient: ISavedObjectsRepository
savedObjectsClient: TelemetrySavedObjectsClient
): Promise<SavedObjectsFindResponse<unknown, ReferencesAggregation>> => {
return savedObjectsClient.find<unknown, ReferencesAggregation>({
page: 0,
perPage: 0,
type: CASE_COMMENT_SAVED_OBJECT,
namespaces: ['*'],
filter: getOnlyAlertsCommentsFilter(),
aggs: {
...getReferencesAggregationQuery({
Expand All @@ -355,12 +361,13 @@ const getAlertsTelemetry = async (
};

const getConnectorsTelemetry = async (
savedObjectsClient: ISavedObjectsRepository
savedObjectsClient: TelemetrySavedObjectsClient
): Promise<SavedObjectsFindResponse<unknown, ReferencesAggregation>> => {
return savedObjectsClient.find<unknown, ReferencesAggregation>({
page: 0,
perPage: 0,
type: CASE_USER_ACTION_SAVED_OBJECT,
namespaces: ['*'],
filter: getOnlyConnectorsFilter(),
aggs: {
...getReferencesAggregationQuery({
Expand Down
Loading

0 comments on commit 6b372b7

Please sign in to comment.