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

Jump to bottom

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

Merged
merged 10 commits into from
Feb 18, 2021
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8114137
[data.search] Use incrementCounter for search telemetry
lukasolson Feb 11, 2021
29d165e
Update reported type
lukasolson Feb 11, 2021
85d882e
Retry conflicts
lukasolson Feb 11, 2021
cb263e1
Fix telemetry check
lukasolson Feb 12, 2021
d41dcb1
Merge branch 'master' into telemetry-increment
lukasolson Feb 16, 2021
1ced937
Use saved object migration to drop previous document
lukasolson Feb 16, 2021
ecdd770
Merge branch 'master' into telemetry-increment
kibanamachine Feb 18, 2021
1487a91
Review feedback
lukasolson Feb 18, 2021
33cf2ca
Merge branch 'master' into telemetry-increment
lukasolson Feb 18, 2021
4868c01
Fix import
lukasolson Feb 18, 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
14 changes: 9 additions & 5 deletions src/plugins/data/server/search/collectors/fetch.ts
View file
Open in desktop
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 @@ -34,9 +34,13 @@ export function fetchProvider(config$: Observable<SharedGlobalConfig>) {
return {
successCount: 0,
errorCount: 0,
averageDuration: null,
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 };
};
}
10 changes: 7 additions & 3 deletions src/plugins/data/server/search/collectors/register.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,22 @@ 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;
averageDuration: number | null;
totalDuration: number;
}

export type ReportedUsage = Omit<CollectedUsage, 'totalDuration'> & {
averageDuration: number;
};

export async function registerUsageCollector(
usageCollection: UsageCollectionSetup,
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
64 changes: 21 additions & 43 deletions src/plugins/data/server/search/collectors/usage.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -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<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);
}

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' },
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,
},
]);
},
trackError: async () => {
const repository = await getRepository();
await repository.incrementCounter(SAVED_OBJECT_ID, SAVED_OBJECT_ID, [
{ fieldName: 'errorCount' },
]);
},
};
}

Expand Down