Skip to content

Commit 3bf86b9

Browse files
committed
[Search service] Add timeout parameter from config to requests (#52352)
* Add timeout parameter to requests * export SharedGlobalConfig from `core/server`
1 parent 9bf0bd3 commit 3bf86b9

File tree

10 files changed

+60
-11
lines changed

10 files changed

+60
-11
lines changed

docs/development/core/server/kibana-plugin-server.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,5 +197,6 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
197197
| [SavedObjectsClientContract](./kibana-plugin-server.savedobjectsclientcontract.md) | Saved Objects is Kibana's data persisentence mechanism allowing plugins to use Elasticsearch for storing plugin state.<!-- -->\#\# SavedObjectsClient errors<!-- -->Since the SavedObjectsClient has its hands in everything we are a little paranoid about the way we present errors back to to application code. Ideally, all errors will be either:<!-- -->1. Caused by bad implementation (ie. undefined is not a function) and as such unpredictable 2. An error that has been classified and decorated appropriately by the decorators in [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md)<!-- -->Type 1 errors are inevitable, but since all expected/handle-able errors should be Type 2 the <code>isXYZError()</code> helpers exposed at <code>SavedObjectsErrorHelpers</code> should be used to understand and manage error responses from the <code>SavedObjectsClient</code>.<!-- -->Type 2 errors are decorated versions of the source error, so if the elasticsearch client threw an error it will be decorated based on its type. That means that rather than looking for <code>error.body.error.type</code> or doing substring checks on <code>error.body.error.reason</code>, just use the helpers to understand the meaning of the error:<!-- -->\`\`\`<!-- -->js if (SavedObjectsErrorHelpers.isNotFoundError(error)) { // handle 404 }<!-- -->if (SavedObjectsErrorHelpers.isNotAuthorizedError(error)) { // 401 handling should be automatic, but in case you wanted to know }<!-- -->// always rethrow the error unless you handle it throw error; \`\`\`<!-- -->\#\#\# 404s from missing index<!-- -->From the perspective of application code and APIs the SavedObjectsClient is a black box that persists objects. One of the internal details that users have no control over is that we use an elasticsearch index for persistance and that index might be missing.<!-- -->At the time of writing we are in the process of transitioning away from the operating assumption that the SavedObjects index is always available. Part of this transition is handling errors resulting from an index missing. These used to trigger a 500 error in most cases, and in others cause 404s with different error messages.<!-- -->From my (Spencer) perspective, a 404 from the SavedObjectsApi is a 404; The object the request/call was targeting could not be found. This is why \#14141 takes special care to ensure that 404 errors are generic and don't distinguish between index missing or document missing.<!-- -->\#\#\# 503s from missing index<!-- -->Unlike all other methods, create requests are supposed to succeed even when the Kibana index does not exist because it will be automatically created by elasticsearch. When that is not the case it is because Elasticsearch's <code>action.auto_create_index</code> setting prevents it from being created automatically so we throw a special 503 with the intention of informing the user that their Elasticsearch settings need to be updated.<!-- -->See [SavedObjectsClient](./kibana-plugin-server.savedobjectsclient.md) See [SavedObjectsErrorHelpers](./kibana-plugin-server.savedobjectserrorhelpers.md) |
198198
| [SavedObjectsClientFactory](./kibana-plugin-server.savedobjectsclientfactory.md) | Describes the factory used to create instances of the Saved Objects Client. |
199199
| [SavedObjectsClientWrapperFactory](./kibana-plugin-server.savedobjectsclientwrapperfactory.md) | Describes the factory used to create instances of Saved Objects Client Wrappers. |
200+
| [SharedGlobalConfig](./kibana-plugin-server.sharedglobalconfig.md) | |
200201
| [UiSettingsType](./kibana-plugin-server.uisettingstype.md) | UI element type to represent the settings. |
201202

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
2+
3+
[Home](./index.md) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [SharedGlobalConfig](./kibana-plugin-server.sharedglobalconfig.md)
4+
5+
## SharedGlobalConfig type
6+
7+
8+
<b>Signature:</b>
9+
10+
```typescript
11+
export declare type SharedGlobalConfig = RecursiveReadonly<{
12+
kibana: Pick<KibanaConfigType, typeof SharedGlobalConfigKeys.kibana[number]>;
13+
elasticsearch: Pick<ElasticsearchConfigType, typeof SharedGlobalConfigKeys.elasticsearch[number]>;
14+
path: Pick<PathConfigType, typeof SharedGlobalConfigKeys.path[number]>;
15+
}>;
16+
```

src/core/server/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ export {
143143
PluginInitializerContext,
144144
PluginManifest,
145145
PluginName,
146+
SharedGlobalConfig,
146147
} from './plugins';
147148

148149
export {

src/core/server/plugins/types.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ export const SharedGlobalConfigKeys = {
206206
path: ['data'] as const,
207207
};
208208

209+
/**
210+
* @public
211+
*/
209212
export type SharedGlobalConfig = RecursiveReadonly<{
210213
kibana: Pick<KibanaConfigType, typeof SharedGlobalConfigKeys.kibana[number]>;
211214
elasticsearch: Pick<ElasticsearchConfigType, typeof SharedGlobalConfigKeys.elasticsearch[number]>;

src/core/server/server.api.md

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1748,6 +1748,13 @@ export interface SessionStorageFactory<T> {
17481748
asScoped: (request: KibanaRequest) => SessionStorage<T>;
17491749
}
17501750

1751+
// @public (undocumented)
1752+
export type SharedGlobalConfig = RecursiveReadonly_2<{
1753+
kibana: Pick<KibanaConfigType_2, typeof SharedGlobalConfigKeys.kibana[number]>;
1754+
elasticsearch: Pick<ElasticsearchConfigType, typeof SharedGlobalConfigKeys.elasticsearch[number]>;
1755+
path: Pick<PathConfigType, typeof SharedGlobalConfigKeys.path[number]>;
1756+
}>;
1757+
17511758
// @public
17521759
export interface UiSettingsParams {
17531760
category?: string[];
@@ -1785,6 +1792,9 @@ export const validBodyOutput: readonly ["data", "stream"];
17851792
//
17861793
// src/core/server/http/router/response.ts:316:3 - (ae-forgotten-export) The symbol "KibanaResponse" needs to be exported by the entry point index.d.ts
17871794
// src/core/server/plugins/plugins_service.ts:43:5 - (ae-forgotten-export) The symbol "InternalPluginInfo" needs to be exported by the entry point index.d.ts
1788-
// src/core/server/plugins/types.ts:228:15 - (ae-forgotten-export) The symbol "SharedGlobalConfig" needs to be exported by the entry point index.d.ts
1795+
// src/core/server/plugins/types.ts:213:3 - (ae-forgotten-export) The symbol "KibanaConfigType" needs to be exported by the entry point index.d.ts
1796+
// src/core/server/plugins/types.ts:213:3 - (ae-forgotten-export) The symbol "SharedGlobalConfigKeys" needs to be exported by the entry point index.d.ts
1797+
// src/core/server/plugins/types.ts:214:3 - (ae-forgotten-export) The symbol "ElasticsearchConfigType" needs to be exported by the entry point index.d.ts
1798+
// src/core/server/plugins/types.ts:215:3 - (ae-forgotten-export) The symbol "PathConfigType" needs to be exported by the entry point index.d.ts
17891799

17901800
```

src/plugins/data/server/search/create_api.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ export function createApi({
3838
}
3939
// Give providers access to other search strategies by injecting this function
4040
const strategy = await strategyProvider(caller, api.search);
41-
return strategy.search(request);
41+
return strategy.search(request, options);
4242
},
4343
};
4444
return api;

src/plugins/data/server/search/es_search/es_search_strategy.test.ts

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* under the License.
1818
*/
1919

20-
import { coreMock } from '../../../../../core/server/mocks';
20+
import { coreMock, pluginInitializerContextConfigMock } from '../../../../../core/server/mocks';
2121
import { esSearchStrategyProvider } from './es_search_strategy';
2222

2323
describe('ES search strategy', () => {
@@ -31,6 +31,7 @@ describe('ES search strategy', () => {
3131
},
3232
});
3333
const mockSearch = jest.fn();
34+
const mockConfig$ = pluginInitializerContextConfigMock<any>({}).legacy.globalConfig$;
3435

3536
beforeEach(() => {
3637
mockApiCaller.mockClear();
@@ -41,6 +42,7 @@ describe('ES search strategy', () => {
4142
const esSearch = esSearchStrategyProvider(
4243
{
4344
core: mockCoreSetup,
45+
config$: mockConfig$,
4446
},
4547
mockApiCaller,
4648
mockSearch
@@ -49,55 +51,59 @@ describe('ES search strategy', () => {
4951
expect(typeof esSearch.search).toBe('function');
5052
});
5153

52-
it('logs the response if `debug` is set to `true`', () => {
54+
it('logs the response if `debug` is set to `true`', async () => {
5355
const spy = jest.spyOn(console, 'log');
5456
const esSearch = esSearchStrategyProvider(
5557
{
5658
core: mockCoreSetup,
59+
config$: mockConfig$,
5760
},
5861
mockApiCaller,
5962
mockSearch
6063
);
6164

6265
expect(spy).not.toBeCalled();
6366

64-
esSearch.search({ params: {}, debug: true });
67+
await esSearch.search({ params: {}, debug: true });
6568

6669
expect(spy).toBeCalled();
6770
});
6871

69-
it('calls the API caller with the params with defaults', () => {
72+
it('calls the API caller with the params with defaults', async () => {
7073
const params = { index: 'logstash-*' };
7174
const esSearch = esSearchStrategyProvider(
7275
{
7376
core: mockCoreSetup,
77+
config$: mockConfig$,
7478
},
7579
mockApiCaller,
7680
mockSearch
7781
);
7882

79-
esSearch.search({ params });
83+
await esSearch.search({ params });
8084

8185
expect(mockApiCaller).toBeCalled();
8286
expect(mockApiCaller.mock.calls[0][0]).toBe('search');
8387
expect(mockApiCaller.mock.calls[0][1]).toEqual({
8488
...params,
89+
timeout: '0ms',
8590
ignoreUnavailable: true,
8691
restTotalHitsAsInt: true,
8792
});
8893
});
8994

90-
it('calls the API caller with overridden defaults', () => {
91-
const params = { index: 'logstash-*', ignoreUnavailable: false };
95+
it('calls the API caller with overridden defaults', async () => {
96+
const params = { index: 'logstash-*', ignoreUnavailable: false, timeout: '1000ms' };
9297
const esSearch = esSearchStrategyProvider(
9398
{
9499
core: mockCoreSetup,
100+
config$: mockConfig$,
95101
},
96102
mockApiCaller,
97103
mockSearch
98104
);
99105

100-
esSearch.search({ params });
106+
await esSearch.search({ params });
101107

102108
expect(mockApiCaller).toBeCalled();
103109
expect(mockApiCaller.mock.calls[0][0]).toBe('search');
@@ -112,6 +118,7 @@ describe('ES search strategy', () => {
112118
const esSearch = esSearchStrategyProvider(
113119
{
114120
core: mockCoreSetup,
121+
config$: mockConfig$,
115122
},
116123
mockApiCaller,
117124
mockSearch

src/plugins/data/server/search/es_search/es_search_strategy.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19+
import { first } from 'rxjs/operators';
1920
import { APICaller } from 'kibana/server';
2021
import { SearchResponse } from 'elasticsearch';
2122
import { ES_SEARCH_STRATEGY } from '../../../common/search';
@@ -28,7 +29,9 @@ export const esSearchStrategyProvider: TSearchStrategyProvider<typeof ES_SEARCH_
2829
): ISearchStrategy<typeof ES_SEARCH_STRATEGY> => {
2930
return {
3031
search: async (request, options) => {
32+
const config = await context.config$.pipe(first()).toPromise();
3133
const params = {
34+
timeout: `${config.elasticsearch.shardTimeout.asMilliseconds()}ms`,
3235
ignoreUnavailable: true, // Don't fail if the index/indices don't exist
3336
restTotalHitsAsInt: true, // Get the number of hits as an int rather than a range
3437
...request.params,

src/plugins/data/server/search/i_search_context.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
* specific language governing permissions and limitations
1717
* under the License.
1818
*/
19-
import { CoreSetup } from '../../../../core/server';
19+
20+
import { Observable } from 'rxjs';
21+
import { CoreSetup, SharedGlobalConfig } from '../../../../core/server';
2022

2123
export interface ISearchContext {
2224
core: CoreSetup;
25+
config$: Observable<SharedGlobalConfig>;
2326
}

src/plugins/data/server/search/search_service.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,11 @@ export class SearchService implements Plugin<ISearchSetup, void> {
8383
};
8484

8585
api.registerSearchStrategyContext(this.initializerContext.opaqueId, 'core', () => core);
86+
api.registerSearchStrategyContext(
87+
this.initializerContext.opaqueId,
88+
'config$',
89+
() => this.initializerContext.config.legacy.globalConfig$
90+
);
8691

8792
// ES search capabilities are written in a way that it could easily be a separate plugin,
8893
// however these two plugins are tightly coupled due to the default search strategy using

0 commit comments

Comments
 (0)