From 8114137b44f35a36cb74d7748fb0bf3493f99d56 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 11 Feb 2021 14:58:08 -0700 Subject: [PATCH 1/7] [data.search] Use incrementCounter for search telemetry --- .../data/server/search/collectors/fetch.ts | 2 +- .../data/server/search/collectors/register.ts | 4 +- .../data/server/search/collectors/usage.ts | 64 ++++++------------- src/plugins/telemetry/schema/oss_plugins.json | 4 +- 4 files changed, 26 insertions(+), 48 deletions(-) diff --git a/src/plugins/data/server/search/collectors/fetch.ts b/src/plugins/data/server/search/collectors/fetch.ts index 05e5558157b3ce..bcec9f2895a4a0 100644 --- a/src/plugins/data/server/search/collectors/fetch.ts +++ b/src/plugins/data/server/search/collectors/fetch.ts @@ -34,7 +34,7 @@ export function fetchProvider(config$: Observable) { return { successCount: 0, errorCount: 0, - averageDuration: null, + totalDuration: 0, }; } return esResponse.hits.hits[0]._source['search-telemetry']; diff --git a/src/plugins/data/server/search/collectors/register.ts b/src/plugins/data/server/search/collectors/register.ts index 2a5637d86e1bf9..d7fd06fe2592d2 100644 --- a/src/plugins/data/server/search/collectors/register.ts +++ b/src/plugins/data/server/search/collectors/register.ts @@ -13,7 +13,7 @@ import { fetchProvider } from './fetch'; export interface Usage { successCount: number; errorCount: number; - averageDuration: number | null; + totalDuration: number; } export async function registerUsageCollector( @@ -28,7 +28,7 @@ export async function registerUsageCollector( schema: { successCount: { type: 'long' }, errorCount: { type: 'long' }, - averageDuration: { type: 'float' }, + totalDuration: { type: 'long' }, }, }); usageCollection.registerCollector(collector); diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index c5dc2414c0e808..11b8fbbc4cf40f 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -6,9 +6,9 @@ * Side Public License, v 1. */ +import { once } from 'lodash'; import type { CoreSetup, Logger } from 'kibana/server'; import type { IEsSearchResponse } from '../../../common'; -import type { Usage } from './register'; const SAVED_OBJECT_ID = 'search-telemetry'; @@ -18,50 +18,28 @@ export interface SearchUsage { } export function usageProvider(core: CoreSetup): SearchUsage { - const getTracker = (eventType: keyof Usage) => { - return async (duration?: number) => { - const repository = await core - .getStartServices() - .then(([coreStart]) => coreStart.savedObjects.createInternalRepository()); - - let attributes: Usage; - let doesSavedObjectExist: boolean = true; - - try { - const response = await repository.get(SAVED_OBJECT_ID, SAVED_OBJECT_ID); - attributes = response.attributes; - } catch (e) { - doesSavedObjectExist = false; - attributes = { - successCount: 0, - errorCount: 0, - averageDuration: 0, - }; - } - - attributes[eventType]++; - - // Only track the average duration for successful requests - if (eventType === 'successCount') { - attributes.averageDuration = - ((duration ?? 0) + (attributes.averageDuration ?? 0)) / (attributes.successCount ?? 1); - } - - try { - if (doesSavedObjectExist) { - await repository.update(SAVED_OBJECT_ID, SAVED_OBJECT_ID, attributes); - } else { - await repository.create(SAVED_OBJECT_ID, attributes, { id: SAVED_OBJECT_ID }); - } - } catch (e) { - // Version conflict error, swallow - } - }; - }; + const getRepository = once(async () => { + const [coreStart] = await core.getStartServices(); + return coreStart.savedObjects.createInternalRepository(); + }); return { - trackError: () => getTracker('errorCount')(), - trackSuccess: getTracker('successCount'), + trackSuccess: async (duration: number) => { + const repository = await getRepository(); + await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [ + { fieldName: 'successCount' }, + { + fieldName: 'totalDuration', + incrementBy: duration, + }, + ]); + }, + trackError: async () => { + const repository = await getRepository(); + await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [ + { fieldName: 'errorCount' }, + ]); + }, }; } diff --git a/src/plugins/telemetry/schema/oss_plugins.json b/src/plugins/telemetry/schema/oss_plugins.json index c129fd006ae156..c3796adc85a46f 100644 --- a/src/plugins/telemetry/schema/oss_plugins.json +++ b/src/plugins/telemetry/schema/oss_plugins.json @@ -45,8 +45,8 @@ "errorCount": { "type": "long" }, - "averageDuration": { - "type": "float" + "totalDuration": { + "type": "long" } } }, From 29d165e0c5f6ae895168373a427cbf452ec67520 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 11 Feb 2021 15:25:36 -0700 Subject: [PATCH 2/7] Update reported type --- src/plugins/data/server/search/collectors/fetch.ts | 14 +++++++++----- .../data/server/search/collectors/register.ts | 10 +++++++--- src/plugins/telemetry/schema/oss_plugins.json | 4 ++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/plugins/data/server/search/collectors/fetch.ts b/src/plugins/data/server/search/collectors/fetch.ts index bcec9f2895a4a0..17823f37c3b4d7 100644 --- a/src/plugins/data/server/search/collectors/fetch.ts +++ b/src/plugins/data/server/search/collectors/fetch.ts @@ -11,14 +11,14 @@ import { first } from 'rxjs/operators'; import { SharedGlobalConfig } from 'kibana/server'; import { SearchResponse } from 'elasticsearch'; import { CollectorFetchContext } from 'src/plugins/usage_collection/server'; -import { Usage } from './register'; +import { CollectedUsage, ReportedUsage } from './register'; interface SearchTelemetry { - 'search-telemetry': Usage; + 'search-telemetry': CollectedUsage; } type ESResponse = SearchResponse; export function fetchProvider(config$: Observable) { - return async ({ esClient }: CollectorFetchContext): Promise => { + return async ({ esClient }: CollectorFetchContext): Promise => { const config = await config$.pipe(first()).toPromise(); const { body: esResponse } = await esClient.search( { @@ -34,9 +34,13 @@ export function fetchProvider(config$: Observable) { return { successCount: 0, errorCount: 0, - totalDuration: 0, + averageDuration: 0, }; } - return esResponse.hits.hits[0]._source['search-telemetry']; + const { successCount, errorCount, totalDuration } = esResponse.hits.hits[0]._source[ + 'search-telemetry' + ]; + const averageDuration = totalDuration / successCount; + return { successCount, errorCount, averageDuration }; }; } diff --git a/src/plugins/data/server/search/collectors/register.ts b/src/plugins/data/server/search/collectors/register.ts index d7fd06fe2592d2..dcc59bff2867c9 100644 --- a/src/plugins/data/server/search/collectors/register.ts +++ b/src/plugins/data/server/search/collectors/register.ts @@ -10,25 +10,29 @@ import { PluginInitializerContext } from 'kibana/server'; import { UsageCollectionSetup } from '../../../../usage_collection/server'; import { fetchProvider } from './fetch'; -export interface Usage { +export interface CollectedUsage { successCount: number; errorCount: number; totalDuration: number; } +export type ReportedUsage = Omit & { + averageDuration: number; +}; + export async function registerUsageCollector( usageCollection: UsageCollectionSetup, context: PluginInitializerContext ) { try { - const collector = usageCollection.makeUsageCollector({ + const collector = usageCollection.makeUsageCollector({ type: 'search', isReady: () => true, fetch: fetchProvider(context.config.legacy.globalConfig$), schema: { successCount: { type: 'long' }, errorCount: { type: 'long' }, - totalDuration: { type: 'long' }, + averageDuration: { type: 'float' }, }, }); usageCollection.registerCollector(collector); diff --git a/src/plugins/telemetry/schema/oss_plugins.json b/src/plugins/telemetry/schema/oss_plugins.json index c3796adc85a46f..c129fd006ae156 100644 --- a/src/plugins/telemetry/schema/oss_plugins.json +++ b/src/plugins/telemetry/schema/oss_plugins.json @@ -45,8 +45,8 @@ "errorCount": { "type": "long" }, - "totalDuration": { - "type": "long" + "averageDuration": { + "type": "float" } } }, From 85d882e0e417c7d032cf0a0f7e3faa2a6ee68034 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 11 Feb 2021 16:55:23 -0700 Subject: [PATCH 3/7] Retry conflicts --- .../data/server/search/collectors/usage.ts | 28 ++++++++++++++----- 1 file changed, 21 insertions(+), 7 deletions(-) diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index 11b8fbbc4cf40f..37c81b554b5912 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -11,6 +11,7 @@ import type { CoreSetup, Logger } from 'kibana/server'; import type { IEsSearchResponse } from '../../../common'; const SAVED_OBJECT_ID = 'search-telemetry'; +const MAX_RETRY_COUNT = 3; export interface SearchUsage { trackError(): Promise; @@ -23,9 +24,9 @@ export function usageProvider(core: CoreSetup): SearchUsage { return coreStart.savedObjects.createInternalRepository(); }); - return { - trackSuccess: async (duration: number) => { - const repository = await getRepository(); + const trackSuccess = async (duration: number, retryCount = 0) => { + const repository = await getRepository(); + try { await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [ { fieldName: 'successCount' }, { @@ -33,14 +34,27 @@ export function usageProvider(core: CoreSetup): SearchUsage { incrementBy: duration, }, ]); - }, - trackError: async () => { - const repository = await getRepository(); + } catch (e) { + if (e.statusCode === 409 && retryCount < MAX_RETRY_COUNT) { + setTimeout(() => trackSuccess(duration, retryCount + 1), 1000); + } + } + }; + + const trackError = async (retryCount = 0) => { + const repository = await getRepository(); + try { await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [ { fieldName: 'errorCount' }, ]); - }, + } catch (e) { + if (e.statusCode === 409 && retryCount < MAX_RETRY_COUNT) { + setTimeout(() => trackError(retryCount + 1), 1000); + } + } }; + + return { trackSuccess, trackError }; } /** From cb263e197fe63909a02479658d42add313de25f8 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 11 Feb 2021 18:53:14 -0700 Subject: [PATCH 4/7] Fix telemetry check --- src/plugins/data/server/search/collectors/fetch.ts | 2 +- src/plugins/data/server/search/collectors/register.ts | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/plugins/data/server/search/collectors/fetch.ts b/src/plugins/data/server/search/collectors/fetch.ts index 17823f37c3b4d7..6dfc29e2cf2a66 100644 --- a/src/plugins/data/server/search/collectors/fetch.ts +++ b/src/plugins/data/server/search/collectors/fetch.ts @@ -34,7 +34,7 @@ export function fetchProvider(config$: Observable) { return { successCount: 0, errorCount: 0, - averageDuration: 0, + averageDuration: null, }; } const { successCount, errorCount, totalDuration } = esResponse.hits.hits[0]._source[ diff --git a/src/plugins/data/server/search/collectors/register.ts b/src/plugins/data/server/search/collectors/register.ts index dcc59bff2867c9..a370377c30eeaf 100644 --- a/src/plugins/data/server/search/collectors/register.ts +++ b/src/plugins/data/server/search/collectors/register.ts @@ -16,9 +16,11 @@ export interface CollectedUsage { totalDuration: number; } -export type ReportedUsage = Omit & { - averageDuration: number; -}; +export interface ReportedUsage { + successCount: number; + errorCount: number; + averageDuration: number | null; +} export async function registerUsageCollector( usageCollection: UsageCollectionSetup, From 1ced937e30ac26850f718e37eb604650605afaea Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Tue, 16 Feb 2021 11:34:56 -0700 Subject: [PATCH 5/7] Use saved object migration to drop previous document --- .../saved_objects/migrations/to_v7_12_0.ts | 17 +++++++++++++++++ .../server/saved_objects/search_telemetry.ts | 4 ++++ 2 files changed, 21 insertions(+) create mode 100644 src/plugins/data/server/saved_objects/migrations/to_v7_12_0.ts diff --git a/src/plugins/data/server/saved_objects/migrations/to_v7_12_0.ts b/src/plugins/data/server/saved_objects/migrations/to_v7_12_0.ts new file mode 100644 index 00000000000000..955028c0f9bf20 --- /dev/null +++ b/src/plugins/data/server/saved_objects/migrations/to_v7_12_0.ts @@ -0,0 +1,17 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import type { SavedObjectMigrationFn } from 'kibana/server'; + +/** + * Drop the previous document's attributes, which report `averageDuration` incorrectly. + * @param doc + */ +export const migrate712: SavedObjectMigrationFn = (doc) => { + return { ...doc, attributes: {} }; +}; diff --git a/src/plugins/data/server/saved_objects/search_telemetry.ts b/src/plugins/data/server/saved_objects/search_telemetry.ts index 24f884c85b7c53..33ad4b74f31697 100644 --- a/src/plugins/data/server/saved_objects/search_telemetry.ts +++ b/src/plugins/data/server/saved_objects/search_telemetry.ts @@ -7,6 +7,7 @@ */ import { SavedObjectsType } from 'kibana/server'; +import { migrate712 } from './migrations/to_v7_12_0'; export const searchTelemetry: SavedObjectsType = { name: 'search-telemetry', @@ -16,4 +17,7 @@ export const searchTelemetry: SavedObjectsType = { dynamic: false, properties: {}, }, + migrations: { + '7.12.0': migrate712, + }, }; From 1487a91673e78e734c106d1417ddbe134e715644 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 18 Feb 2021 10:12:08 -0700 Subject: [PATCH 6/7] Review feedback --- src/plugins/data/server/search/collectors/usage.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index 37c81b554b5912..6f9eac5614b4fe 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -8,6 +8,7 @@ import { once } from 'lodash'; import type { CoreSetup, Logger } from 'kibana/server'; +import { SavedObjectsErrorHelpers } from 'kibana/server'; import type { IEsSearchResponse } from '../../../common'; const SAVED_OBJECT_ID = 'search-telemetry'; @@ -35,7 +36,7 @@ export function usageProvider(core: CoreSetup): SearchUsage { }, ]); } catch (e) { - if (e.statusCode === 409 && retryCount < MAX_RETRY_COUNT) { + if (SavedObjectsErrorHelpers.isConflictError(e) && retryCount < MAX_RETRY_COUNT) { setTimeout(() => trackSuccess(duration, retryCount + 1), 1000); } } @@ -48,7 +49,7 @@ export function usageProvider(core: CoreSetup): SearchUsage { { fieldName: 'errorCount' }, ]); } catch (e) { - if (e.statusCode === 409 && retryCount < MAX_RETRY_COUNT) { + if (SavedObjectsErrorHelpers.isConflictError(e) && retryCount < MAX_RETRY_COUNT) { setTimeout(() => trackError(retryCount + 1), 1000); } } From 4868c01d644e44c095e1c84dfca6c59f1347bce2 Mon Sep 17 00:00:00 2001 From: Lukas Olson Date: Thu, 18 Feb 2021 10:37:53 -0700 Subject: [PATCH 7/7] Fix import --- src/plugins/data/server/search/collectors/usage.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plugins/data/server/search/collectors/usage.ts b/src/plugins/data/server/search/collectors/usage.ts index 6f9eac5614b4fe..c9f0a5bf249442 100644 --- a/src/plugins/data/server/search/collectors/usage.ts +++ b/src/plugins/data/server/search/collectors/usage.ts @@ -8,7 +8,7 @@ import { once } from 'lodash'; import type { CoreSetup, Logger } from 'kibana/server'; -import { SavedObjectsErrorHelpers } from 'kibana/server'; +import { SavedObjectsErrorHelpers } from '../../../../../core/server'; import type { IEsSearchResponse } from '../../../common'; const SAVED_OBJECT_ID = 'search-telemetry';