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

[data.search] Use incrementCounter for search telemetry #91230

Merged
merged 10 commits into from
Feb 18, 2021
17 changes: 17 additions & 0 deletions src/plugins/data/server/saved_objects/migrations/to_v7_12_0.ts
Original file line number Diff line number Diff line change
@@ -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: {} };
};
4 changes: 4 additions & 0 deletions src/plugins/data/server/saved_objects/search_telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/

import { SavedObjectsType } from 'kibana/server';
import { migrate712 } from './migrations/to_v7_12_0';

export const searchTelemetry: SavedObjectsType = {
name: 'search-telemetry',
Expand All @@ -16,4 +17,7 @@ export const searchTelemetry: SavedObjectsType = {
dynamic: false,
properties: {},
},
migrations: {
'7.12.0': migrate712,
},
};
12 changes: 8 additions & 4 deletions src/plugins/data/server/search/collectors/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<SearchTelemetry>;

export function fetchProvider(config$: Observable<SharedGlobalConfig>) {
return async ({ esClient }: CollectorFetchContext): Promise<Usage> => {
return async ({ esClient }: CollectorFetchContext): Promise<ReportedUsage> => {
const config = await config$.pipe(first()).toPromise();
const { body: esResponse } = await esClient.search<ESResponse>(
{
Expand All @@ -37,6 +37,10 @@ export function fetchProvider(config$: Observable<SharedGlobalConfig>) {
averageDuration: null,
};
}
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 };
};
}
10 changes: 8 additions & 2 deletions src/plugins/data/server/search/collectors/register.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ 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 interface ReportedUsage {
successCount: number;
errorCount: number;
averageDuration: number | null;
Expand All @@ -21,7 +27,7 @@ export async function registerUsageCollector(
context: PluginInitializerContext
) {
try {
const collector = usageCollection.makeUsageCollector<Usage>({
const collector = usageCollection.makeUsageCollector<ReportedUsage>({
lukasolson marked this conversation as resolved.
Show resolved Hide resolved
type: 'search',
isReady: () => true,
fetch: fetchProvider(context.config.legacy.globalConfig$),
Expand Down
73 changes: 33 additions & 40 deletions src/plugins/data/server/search/collectors/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,63 +6,56 @@
* Side Public License, v 1.
*/

import { once } from 'lodash';
import type { CoreSetup, Logger } from 'kibana/server';
import { SavedObjectsErrorHelpers } from '../../../../../core/server';
import type { IEsSearchResponse } from '../../../common';
import type { Usage } from './register';

const SAVED_OBJECT_ID = 'search-telemetry';
const MAX_RETRY_COUNT = 3;

export interface SearchUsage {
trackError(): Promise<void>;
trackSuccess(duration: number): Promise<void>;
}

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());
const getRepository = once(async () => {
const [coreStart] = await core.getStartServices();
return coreStart.savedObjects.createInternalRepository();
});

let attributes: Usage;
let doesSavedObjectExist: boolean = true;

try {
const response = await repository.get<Usage>(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);
const trackSuccess = async (duration: number, retryCount = 0) => {
const repository = await getRepository();
try {
await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [
{ fieldName: 'successCount' },
Copy link
Contributor

Choose a reason for hiding this comment

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

IIRC, we only migrate SO between versions id the data from one version to the next needs to be persisted (i.e. end-user facing SO). Since telemetry-related SO is fetched once every 24 hours and indexed and that hasn't changed, it doesn't really matter if we blow the SO object out the water and reinitialize a new one when the cluster upgrades. If you did want to persist the data, then I guess you'd need to add a migration. @Bamieh WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

I agree with tina. if a transformation is not possible of the old data simply using a new saved object id and dropping this one is fine. We can drop the existing data in the same savedobject type via migrations.

const migrate713 = (doc) => {
  return {
    ...doc,
    attributes: {},
  };
};

Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated this PR with a migration as suggested, @Bamieh could you take one more look? Thanks!

{
fieldName: 'totalDuration',
incrementBy: duration,
},
]);
} catch (e) {
if (SavedObjectsErrorHelpers.isConflictError(e) && retryCount < MAX_RETRY_COUNT) {
setTimeout(() => trackSuccess(duration, retryCount + 1), 1000);
}
}
};

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 trackError = async (retryCount = 0) => {
const repository = await getRepository();
try {
await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [
{ fieldName: 'errorCount' },
]);
} catch (e) {
if (SavedObjectsErrorHelpers.isConflictError(e) && retryCount < MAX_RETRY_COUNT) {
setTimeout(() => trackError(retryCount + 1), 1000);
}
};
}
};

return {
trackError: () => getTracker('errorCount')(),
trackSuccess: getTracker('successCount'),
};
return { trackSuccess, trackError };
}

/**
Expand Down