Skip to content

Commit 1f87b08

Browse files
lukasolsonLiza Klukeelmerselasticmachine
authored
[Search] Remove long-running query pop-up (#75385) (#77294)
* [Search] Remove long-running query pop-up * Don't timeout if requestTimeout isn't configured * Remove unused kibanaUtils * Remove unused kibanaReact * Re-add reference to kibanaUtils * Remove unused translations and update documentation * Add new x-pack advanced setting searchTimeout and use it in the EnhancedSearchInterceptor * docs * Re-add toast when queries time out * Fix types * Update error message with capabilities * Update docs * Update docs * Move search server routes into a directory. * Add internal/_msearch route. * Remove legacy search API, rewrite default search strategy to use internal route. * Remove legacy es_client code. * Handle msearch options on server. * Remove elasticsearch-browser dependency. * Update generated docs. * Rely on server timeout in OSS (?) Use UI setting in xpack. * Rename function * Add features to dependencies * Undefined check * doc * Code review fixes * code review * doc * loading count * simplify code review and fix jest tets * type check * Remove esShard from client * cleanup request parameters from FE * doc * doc * Align request parameters on server, Remove leftover parameters from client Shim responses for search and msearch routes * docs Stop using toSnakeCase Updates jest tests * add management docs * docs * Remove import * Break circular dep + fix msearch test * Remove deleted type * Fix jest * Bring toSnakeCase back * docs * fix jest * Add new x-pack advanced setting searchTimeout and use it in the EnhancedSearchInterceptor * docs * Rely on server timeout in OSS (?) Use UI setting in xpack. * Rename function * doc * Remove esShard from client * cleanup request parameters from FE * doc * doc * Align request parameters on server, Remove leftover parameters from client Shim responses for search and msearch routes * docs Stop using toSnakeCase Updates jest tests * add management docs * docs * Remove import * Break circular dep + fix msearch test * Remove deleted type * Fix jest * Bring toSnakeCase back * docs * fix jest * Fix merge * Fix types * Allow timeout to be undefined * Fix jest test * Upldate docs * Fix msearch jest * Merge correction * docs * Fix rollup search merge * Fix merge * Use i18n Co-authored-by: Liza K <liza.katz@elastic.co> Co-authored-by: Luke Elmers <luke.elmers@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com> Co-authored-by: Liza K <liza.katz@elastic.co> Co-authored-by: Luke Elmers <luke.elmers@elastic.co> Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
1 parent e96eff6 commit 1f87b08

18 files changed

+110
-418
lines changed

‎docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.getpendingcount_.md‎

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,11 @@ export declare class SearchInterceptor
2121
| Property | Modifiers | Type | Description |
2222
| --- | --- | --- | --- |
2323
| [deps](./kibana-plugin-plugins-data-public.searchinterceptor.deps.md) | | <code>SearchInterceptorDeps</code> | |
24+
| [showTimeoutError](./kibana-plugin-plugins-data-public.searchinterceptor.showtimeouterror.md) | | <code>((e: Error) =&gt; void) &amp; import(&quot;lodash&quot;).Cancelable</code> | |
2425

2526
## Methods
2627

2728
| Method | Modifiers | Description |
2829
| --- | --- | --- |
29-
| [getPendingCount$()](./kibana-plugin-plugins-data-public.searchinterceptor.getpendingcount_.md) | | Returns an <code>Observable</code> over the current number of pending searches. This could mean that one of the search requests is still in flight, or that it has only received partial responses. |
3030
| [search(request, options)](./kibana-plugin-plugins-data-public.searchinterceptor.search.md) | | Searches using the given <code>search</code> method. Overrides the <code>AbortSignal</code> with one that will abort either when <code>cancelPending</code> is called, when the request times out, or when the original <code>AbortSignal</code> is aborted. Updates <code>pendingCount$</code> when the request is started/finalized. |
3131

‎docs/development/plugins/data/public/kibana-plugin-plugins-data-public.searchinterceptor.showtimeouterror.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
2+
3+
[Home](./index.md) &gt; [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) &gt; [SearchInterceptor](./kibana-plugin-plugins-data-public.searchinterceptor.md) &gt; [showTimeoutError](./kibana-plugin-plugins-data-public.searchinterceptor.showtimeouterror.md)
4+
5+
## SearchInterceptor.showTimeoutError property
6+
7+
<b>Signature:</b>
8+
9+
```typescript
10+
protected showTimeoutError: ((e: Error) => void) & import("lodash").Cancelable;
11+
```

‎src/plugins/data/public/public.api.md‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,6 @@ import { Search } from '@elastic/elasticsearch/api/requestParams';
6565
import { SearchResponse } from 'elasticsearch';
6666
import { SerializedFieldFormat as SerializedFieldFormat_2 } from 'src/plugins/expressions/common';
6767
import { Subscription } from 'rxjs';
68-
import { Toast } from 'kibana/public';
6968
import { ToastInputFields } from 'src/core/public/notifications';
7069
import { ToastsSetup } from 'kibana/public';
7170
import { TransportRequestOptions } from '@elastic/elasticsearch/lib/Transport';
@@ -1740,11 +1739,6 @@ export class SearchInterceptor {
17401739
protected application: CoreStart['application'];
17411740
// (undocumented)
17421741
protected readonly deps: SearchInterceptorDeps;
1743-
getPendingCount$(): Observable<number>;
1744-
// @internal (undocumented)
1745-
protected hideToast: () => void;
1746-
// @internal
1747-
protected longRunningToast?: Toast;
17481742
// @internal
17491743
protected pendingCount$: BehaviorSubject<number>;
17501744
// @internal (undocumented)
@@ -1758,8 +1752,8 @@ export class SearchInterceptor {
17581752
combinedSignal: AbortSignal;
17591753
cleanup: () => void;
17601754
};
1761-
// @internal (undocumented)
1762-
protected showToast: () => void;
1755+
// (undocumented)
1756+
protected showTimeoutError: ((e: Error) => void) & import("lodash").Cancelable;
17631757
// @internal
17641758
protected timeoutSubscriptions: Subscription;
17651759
}

‎src/plugins/data/public/search/collectors/create_usage_collector.test.ts‎

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -63,31 +63,4 @@ describe('Search Usage Collector', () => {
6363
SEARCH_EVENT_TYPE.QUERIES_CANCELLED
6464
);
6565
});
66-
67-
test('tracks long popups', async () => {
68-
await usageCollector.trackLongQueryPopupShown();
69-
expect(mockUsageCollectionSetup.reportUiStats).toHaveBeenCalled();
70-
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][1]).toBe(METRIC_TYPE.LOADED);
71-
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][2]).toBe(
72-
SEARCH_EVENT_TYPE.LONG_QUERY_POPUP_SHOWN
73-
);
74-
});
75-
76-
test('tracks long popups dismissed', async () => {
77-
await usageCollector.trackLongQueryDialogDismissed();
78-
expect(mockUsageCollectionSetup.reportUiStats).toHaveBeenCalled();
79-
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][1]).toBe(METRIC_TYPE.CLICK);
80-
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][2]).toBe(
81-
SEARCH_EVENT_TYPE.LONG_QUERY_DIALOG_DISMISSED
82-
);
83-
});
84-
85-
test('tracks run query beyond timeout', async () => {
86-
await usageCollector.trackLongQueryRunBeyondTimeout();
87-
expect(mockUsageCollectionSetup.reportUiStats).toHaveBeenCalled();
88-
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][1]).toBe(METRIC_TYPE.CLICK);
89-
expect(mockUsageCollectionSetup.reportUiStats.mock.calls[0][2]).toBe(
90-
SEARCH_EVENT_TYPE.LONG_QUERY_RUN_BEYOND_TIMEOUT
91-
);
92-
});
9366
});

‎src/plugins/data/public/search/collectors/create_usage_collector.ts‎

Lines changed: 0 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -48,29 +48,5 @@ export const createUsageCollector = (
4848
SEARCH_EVENT_TYPE.QUERIES_CANCELLED
4949
);
5050
},
51-
trackLongQueryPopupShown: async () => {
52-
const currentApp = await getCurrentApp();
53-
return usageCollection?.reportUiStats(
54-
currentApp!,
55-
METRIC_TYPE.LOADED,
56-
SEARCH_EVENT_TYPE.LONG_QUERY_POPUP_SHOWN
57-
);
58-
},
59-
trackLongQueryDialogDismissed: async () => {
60-
const currentApp = await getCurrentApp();
61-
return usageCollection?.reportUiStats(
62-
currentApp!,
63-
METRIC_TYPE.CLICK,
64-
SEARCH_EVENT_TYPE.LONG_QUERY_DIALOG_DISMISSED
65-
);
66-
},
67-
trackLongQueryRunBeyondTimeout: async () => {
68-
const currentApp = await getCurrentApp();
69-
return usageCollection?.reportUiStats(
70-
currentApp!,
71-
METRIC_TYPE.CLICK,
72-
SEARCH_EVENT_TYPE.LONG_QUERY_RUN_BEYOND_TIMEOUT
73-
);
74-
},
7551
};
7652
};

‎src/plugins/data/public/search/collectors/types.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,9 @@
2020
export enum SEARCH_EVENT_TYPE {
2121
QUERY_TIMED_OUT = 'queryTimedOut',
2222
QUERIES_CANCELLED = 'queriesCancelled',
23-
LONG_QUERY_POPUP_SHOWN = 'longQueryPopupShown',
24-
LONG_QUERY_DIALOG_DISMISSED = 'longQueryDialogDismissed',
25-
LONG_QUERY_RUN_BEYOND_TIMEOUT = 'longQueryRunBeyondTimeout',
2623
}
2724

2825
export interface SearchUsageCollector {
2926
trackQueryTimedOut: () => Promise<void>;
3027
trackQueriesCancelled: () => Promise<void>;
31-
trackLongQueryPopupShown: () => Promise<void>;
32-
trackLongQueryDialogDismissed: () => Promise<void>;
33-
trackLongQueryRunBeyondTimeout: () => Promise<void>;
3428
}

‎src/plugins/data/public/search/long_query_notification.tsx‎

Lines changed: 0 additions & 59 deletions
This file was deleted.

‎src/plugins/data/public/search/search_interceptor.test.ts‎

Lines changed: 34 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,39 @@ describe('SearchInterceptor', () => {
9595
await flushPromises();
9696
});
9797

98+
test('Should not timeout if requestTimeout is undefined', async () => {
99+
searchInterceptor = new SearchInterceptor({
100+
startServices: mockCoreSetup.getStartServices(),
101+
uiSettings: mockCoreSetup.uiSettings,
102+
http: mockCoreSetup.http,
103+
toasts: mockCoreSetup.notifications.toasts,
104+
});
105+
mockCoreSetup.http.fetch.mockImplementationOnce((options: any) => {
106+
return new Promise((resolve, reject) => {
107+
options.signal.addEventListener('abort', () => {
108+
reject(new AbortError());
109+
});
110+
111+
setTimeout(resolve, 5000);
112+
});
113+
});
114+
const mockRequest: IEsSearchRequest = {
115+
params: {},
116+
};
117+
const response = searchInterceptor.search(mockRequest);
118+
119+
expect.assertions(1);
120+
const next = jest.fn();
121+
const complete = () => {
122+
expect(next).toBeCalled();
123+
};
124+
response.subscribe({ next, complete });
125+
126+
jest.advanceTimersByTime(5000);
127+
128+
await flushPromises();
129+
});
130+
98131
test('Observable should fail if user aborts (test merged signal)', async () => {
99132
const abortController = new AbortController();
100133
mockCoreSetup.http.fetch.mockImplementationOnce((options: any) => {
@@ -125,7 +158,7 @@ describe('SearchInterceptor', () => {
125158
await flushPromises();
126159
});
127160

128-
test('Immediatelly aborts if passed an aborted abort signal', async (done) => {
161+
test('Immediately aborts if passed an aborted abort signal', async (done) => {
129162
const abort = new AbortController();
130163
const mockRequest: IEsSearchRequest = {
131164
params: {},
@@ -141,44 +174,4 @@ describe('SearchInterceptor', () => {
141174
response.subscribe({ error });
142175
});
143176
});
144-
145-
describe('getPendingCount$', () => {
146-
test('should observe the number of pending requests', () => {
147-
const pendingCount$ = searchInterceptor.getPendingCount$();
148-
const pendingNext = jest.fn();
149-
pendingCount$.subscribe(pendingNext);
150-
151-
const mockResponse: any = { result: 200 };
152-
mockCoreSetup.http.fetch.mockResolvedValue(mockResponse);
153-
const mockRequest: IEsSearchRequest = {
154-
params: {},
155-
};
156-
const response = searchInterceptor.search(mockRequest);
157-
158-
response.subscribe({
159-
complete: () => {
160-
expect(pendingNext.mock.calls).toEqual([[0], [1], [0]]);
161-
},
162-
});
163-
});
164-
165-
test('should observe the number of pending requests on error', () => {
166-
const pendingCount$ = searchInterceptor.getPendingCount$();
167-
const pendingNext = jest.fn();
168-
pendingCount$.subscribe(pendingNext);
169-
170-
const mockResponse: any = { result: 500 };
171-
mockCoreSetup.http.fetch.mockRejectedValue(mockResponse);
172-
const mockRequest: IEsSearchRequest = {
173-
params: {},
174-
};
175-
const response = searchInterceptor.search(mockRequest);
176-
177-
response.subscribe({
178-
complete: () => {
179-
expect(pendingNext.mock.calls).toEqual([[0], [1], [0]]);
180-
},
181-
});
182-
});
183-
});
184177
});

0 commit comments

Comments
 (0)