Skip to content

Commit

Permalink
[Search] Fix telemetry collection to not send extra requests (#73382)
Browse files Browse the repository at this point in the history
* [Search] Fix telemetry collection to not send extra requests

* Update tracker to collect duration from ES response

* Fix type check

* Update docs and types

* Fix typescript

Co-authored-by: Liza K <liza.katz@elastic.co>
  • Loading branch information
lukasolson and Liza K committed Aug 3, 2020
1 parent 4d6b8ce commit 4758cb1
Show file tree
Hide file tree
Showing 24 changed files with 214 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,5 @@ Used internally for telemetry
<b>Signature:</b>

```typescript
usage: SearchUsage;
usage?: SearchUsage;
```
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
| [parseInterval(interval)](./kibana-plugin-plugins-data-server.parseinterval.md) | |
| [plugin(initializerContext)](./kibana-plugin-plugins-data-server.plugin.md) | Static code to be shared externally |
| [shouldReadFieldFromDocValues(aggregatable, esType)](./kibana-plugin-plugins-data-server.shouldreadfieldfromdocvalues.md) | |
| [usageProvider(core)](./kibana-plugin-plugins-data-server.usageprovider.md) | |

## Interfaces

Expand All @@ -49,6 +50,7 @@
| [PluginStart](./kibana-plugin-plugins-data-server.pluginstart.md) | |
| [Query](./kibana-plugin-plugins-data-server.query.md) | |
| [RefreshInterval](./kibana-plugin-plugins-data-server.refreshinterval.md) | |
| [SearchUsage](./kibana-plugin-plugins-data-server.searchusage.md) | |
| [TimeRange](./kibana-plugin-plugins-data-server.timerange.md) | |

## Variables
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) &gt; [SearchUsage](./kibana-plugin-plugins-data-server.searchusage.md)

## SearchUsage interface

<b>Signature:</b>

```typescript
export interface SearchUsage
```

## Methods

| Method | Description |
| --- | --- |
| [trackError()](./kibana-plugin-plugins-data-server.searchusage.trackerror.md) | |
| [trackSuccess(duration)](./kibana-plugin-plugins-data-server.searchusage.tracksuccess.md) | |

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) &gt; [SearchUsage](./kibana-plugin-plugins-data-server.searchusage.md) &gt; [trackError](./kibana-plugin-plugins-data-server.searchusage.trackerror.md)

## SearchUsage.trackError() method

<b>Signature:</b>

```typescript
trackError(): Promise<void>;
```
<b>Returns:</b>

`Promise<void>`

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) &gt; [SearchUsage](./kibana-plugin-plugins-data-server.searchusage.md) &gt; [trackSuccess](./kibana-plugin-plugins-data-server.searchusage.tracksuccess.md)

## SearchUsage.trackSuccess() method

<b>Signature:</b>

```typescript
trackSuccess(duration: number): Promise<void>;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| duration | <code>number</code> | |

<b>Returns:</b>

`Promise<void>`

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->

[Home](./index.md) &gt; [kibana-plugin-plugins-data-server](./kibana-plugin-plugins-data-server.md) &gt; [usageProvider](./kibana-plugin-plugins-data-server.usageprovider.md)

## usageProvider() function

<b>Signature:</b>

```typescript
export declare function usageProvider(core: CoreSetup): SearchUsage;
```

## Parameters

| Parameter | Type | Description |
| --- | --- | --- |
| core | <code>CoreSetup</code> | |

<b>Returns:</b>

`SearchUsage`

Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,4 @@ describe('Search Usage Collector', () => {
SEARCH_EVENT_TYPE.LONG_QUERY_RUN_BEYOND_TIMEOUT
);
});

test('tracks response errors', async () => {
const duration = 10;
await usageCollector.trackError(duration);
expect(mockCoreSetup.http.post).toBeCalled();
expect(mockCoreSetup.http.post.mock.calls[0][0]).toBe('/api/search/usage');
});

test('tracks response duration', async () => {
const duration = 5;
await usageCollector.trackSuccess(duration);
expect(mockCoreSetup.http.post).toBeCalled();
expect(mockCoreSetup.http.post.mock.calls[0][0]).toBe('/api/search/usage');
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,5 @@ export const createUsageCollector = (
SEARCH_EVENT_TYPE.LONG_QUERY_RUN_BEYOND_TIMEOUT
);
},
trackError: async (duration: number) => {
return core.http.post('/api/search/usage', {
body: JSON.stringify({
eventType: 'error',
duration,
}),
});
},
trackSuccess: async (duration: number) => {
return core.http.post('/api/search/usage', {
body: JSON.stringify({
eventType: 'success',
duration,
}),
});
},
};
};
2 changes: 0 additions & 2 deletions src/plugins/data/public/search/collectors/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,4 @@ export interface SearchUsageCollector {
trackLongQueryPopupShown: () => Promise<void>;
trackLongQueryDialogDismissed: () => Promise<void>;
trackLongQueryRunBeyondTimeout: () => Promise<void>;
trackError: (duration: number) => Promise<void>;
trackSuccess: (duration: number) => Promise<void>;
}
9 changes: 1 addition & 8 deletions src/plugins/data/public/search/search_interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
*/

import { BehaviorSubject, throwError, timer, Subscription, defer, from, Observable } from 'rxjs';
import { finalize, filter, tap } from 'rxjs/operators';
import { finalize, filter } from 'rxjs/operators';
import { ApplicationStart, Toast, ToastsStart, CoreStart } from 'kibana/public';
import { getCombinedSignal, AbortError } from '../../common/utils';
import { IEsSearchRequest, IEsSearchResponse } from '../../common/search';
Expand Down Expand Up @@ -123,13 +123,6 @@ export class SearchInterceptor {
this.pendingCount$.next(++this.pendingCount);

return this.runSearch(request, combinedSignal).pipe(
tap({
next: (e) => {
if (this.deps.usageCollector) {
this.deps.usageCollector.trackSuccess(e.rawResponse.took);
}
},
}),
finalize(() => {
this.pendingCount$.next(--this.pendingCount);
cleanup();
Expand Down
2 changes: 2 additions & 0 deletions src/plugins/data/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,8 @@ export {
ISearchStart,
getDefaultSearchParams,
getTotalLoaded,
usageProvider,
SearchUsage,
} from './search';

// Search namespace
Expand Down
20 changes: 20 additions & 0 deletions src/plugins/data/server/search/collectors/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

export { usageProvider, SearchUsage } from './usage';
50 changes: 0 additions & 50 deletions src/plugins/data/server/search/collectors/routes.ts

This file was deleted.

23 changes: 11 additions & 12 deletions src/plugins/data/server/search/collectors/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,18 @@
*/

import { CoreSetup } from 'kibana/server';
import { DataPluginStart } from '../../plugin';
import { Usage } from './register';

const SAVED_OBJECT_ID = 'search-telemetry';

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

export function usageProvider(core: CoreSetup<object, DataPluginStart>): SearchUsage {
export function usageProvider(core: CoreSetup): SearchUsage {
const getTracker = (eventType: keyof Usage) => {
return async (duration: number) => {
return async (duration?: number) => {
const repository = await core
.getStartServices()
.then(([coreStart]) => coreStart.savedObjects.createInternalRepository());
Expand All @@ -52,17 +51,17 @@ export function usageProvider(core: CoreSetup<object, DataPluginStart>): SearchU

attributes[eventType]++;

const averageDuration =
(duration + (attributes.averageDuration ?? 0)) /
((attributes.errorCount ?? 0) + (attributes.successCount ?? 0));

const newAttributes = { ...attributes, averageDuration };
// 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, newAttributes);
await repository.update(SAVED_OBJECT_ID, SAVED_OBJECT_ID, attributes);
} else {
await repository.create(SAVED_OBJECT_ID, newAttributes, { id: SAVED_OBJECT_ID });
await repository.create(SAVED_OBJECT_ID, attributes, { id: SAVED_OBJECT_ID });
}
} catch (e) {
// Version conflict error, swallow
Expand All @@ -71,7 +70,7 @@ export function usageProvider(core: CoreSetup<object, DataPluginStart>): SearchU
};

return {
trackError: getTracker('errorCount'),
trackError: () => getTracker('errorCount')(),
trackSuccess: getTracker('successCount'),
};
}
27 changes: 18 additions & 9 deletions src/plugins/data/server/search/es_search/es_search_strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ import { first } from 'rxjs/operators';
import { SharedGlobalConfig, Logger } from 'kibana/server';
import { SearchResponse } from 'elasticsearch';
import { Observable } from 'rxjs';
import { SearchUsage } from '../collectors/usage';
import { ISearchStrategy, getDefaultSearchParams, getTotalLoaded } from '..';

export const esSearchStrategyProvider = (
config$: Observable<SharedGlobalConfig>,
logger: Logger
logger: Logger,
usage?: SearchUsage
): ISearchStrategy => {
return {
search: async (context, request, options) => {
Expand All @@ -43,15 +45,22 @@ export const esSearchStrategyProvider = (
...request.params,
};

const rawResponse = (await context.core.elasticsearch.legacy.client.callAsCurrentUser(
'search',
params,
options
)) as SearchResponse<any>;
try {
const rawResponse = (await context.core.elasticsearch.legacy.client.callAsCurrentUser(
'search',
params,
options
)) as SearchResponse<any>;

// The above query will either complete or timeout and throw an error.
// There is no progress indication on this api.
return { rawResponse, ...getTotalLoaded(rawResponse._shards) };
if (usage) usage.trackSuccess(rawResponse.took);

// The above query will either complete or timeout and throw an error.
// There is no progress indication on this api.
return { rawResponse, ...getTotalLoaded(rawResponse._shards) };
} catch (e) {
if (usage) usage.trackError();
throw e;
}
},
};
};
2 changes: 2 additions & 0 deletions src/plugins/data/server/search/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,5 @@
export { ISearchStrategy, ISearchOptions, ISearchSetup, ISearchStart } from './types';

export { getDefaultSearchParams, getTotalLoaded } from './es_search';

export { usageProvider, SearchUsage } from './collectors';
Loading

0 comments on commit 4758cb1

Please sign in to comment.