[Search service] Add timeout parameter from config to requests (#52352)

* Add timeout parameter to requests

* export SharedGlobalConfig from `core/server`
This commit is contained in:
Lukas Olson 2019-12-11 10:57:19 -07:00 committed by GitHub
parent b91c24c0b3
commit 8395596159
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 60 additions and 11 deletions

View file

@ -197,5 +197,6 @@ The plugin integrates with the core system via lifecycle events: `setup`<!-- -->
| [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) |
| [SavedObjectsClientFactory](./kibana-plugin-server.savedobjectsclientfactory.md) | Describes the factory used to create instances of the Saved Objects Client. |
| [SavedObjectsClientWrapperFactory](./kibana-plugin-server.savedobjectsclientwrapperfactory.md) | Describes the factory used to create instances of Saved Objects Client Wrappers. |
| [SharedGlobalConfig](./kibana-plugin-server.sharedglobalconfig.md) | |
| [UiSettingsType](./kibana-plugin-server.uisettingstype.md) | UI element type to represent the settings. |

View file

@ -0,0 +1,16 @@
<!-- Do not edit this file. It is automatically generated by API Documenter. -->
[Home](./index.md) &gt; [kibana-plugin-server](./kibana-plugin-server.md) &gt; [SharedGlobalConfig](./kibana-plugin-server.sharedglobalconfig.md)
## SharedGlobalConfig type
<b>Signature:</b>
```typescript
export declare type SharedGlobalConfig = RecursiveReadonly<{
kibana: Pick<KibanaConfigType, typeof SharedGlobalConfigKeys.kibana[number]>;
elasticsearch: Pick<ElasticsearchConfigType, typeof SharedGlobalConfigKeys.elasticsearch[number]>;
path: Pick<PathConfigType, typeof SharedGlobalConfigKeys.path[number]>;
}>;
```

View file

@ -143,6 +143,7 @@ export {
PluginInitializerContext,
PluginManifest,
PluginName,
SharedGlobalConfig,
} from './plugins';
export {

View file

@ -206,6 +206,9 @@ export const SharedGlobalConfigKeys = {
path: ['data'] as const,
};
/**
* @public
*/
export type SharedGlobalConfig = RecursiveReadonly<{
kibana: Pick<KibanaConfigType, typeof SharedGlobalConfigKeys.kibana[number]>;
elasticsearch: Pick<ElasticsearchConfigType, typeof SharedGlobalConfigKeys.elasticsearch[number]>;

View file

@ -1748,6 +1748,13 @@ export interface SessionStorageFactory<T> {
asScoped: (request: KibanaRequest) => SessionStorage<T>;
}
// @public (undocumented)
export type SharedGlobalConfig = RecursiveReadonly_2<{
kibana: Pick<KibanaConfigType_2, typeof SharedGlobalConfigKeys.kibana[number]>;
elasticsearch: Pick<ElasticsearchConfigType, typeof SharedGlobalConfigKeys.elasticsearch[number]>;
path: Pick<PathConfigType, typeof SharedGlobalConfigKeys.path[number]>;
}>;
// @public
export interface UiSettingsParams {
category?: string[];
@ -1785,6 +1792,9 @@ export const validBodyOutput: readonly ["data", "stream"];
//
// 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
// 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
// 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
// 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
// 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
// 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
// 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
```

View file

@ -38,7 +38,7 @@ export function createApi({
}
// Give providers access to other search strategies by injecting this function
const strategy = await strategyProvider(caller, api.search);
return strategy.search(request);
return strategy.search(request, options);
},
};
return api;

View file

@ -17,7 +17,7 @@
* under the License.
*/
import { coreMock } from '../../../../../core/server/mocks';
import { coreMock, pluginInitializerContextConfigMock } from '../../../../../core/server/mocks';
import { esSearchStrategyProvider } from './es_search_strategy';
describe('ES search strategy', () => {
@ -31,6 +31,7 @@ describe('ES search strategy', () => {
},
});
const mockSearch = jest.fn();
const mockConfig$ = pluginInitializerContextConfigMock<any>({}).legacy.globalConfig$;
beforeEach(() => {
mockApiCaller.mockClear();
@ -41,6 +42,7 @@ describe('ES search strategy', () => {
const esSearch = esSearchStrategyProvider(
{
core: mockCoreSetup,
config$: mockConfig$,
},
mockApiCaller,
mockSearch
@ -49,11 +51,12 @@ describe('ES search strategy', () => {
expect(typeof esSearch.search).toBe('function');
});
it('logs the response if `debug` is set to `true`', () => {
it('logs the response if `debug` is set to `true`', async () => {
const spy = jest.spyOn(console, 'log');
const esSearch = esSearchStrategyProvider(
{
core: mockCoreSetup,
config$: mockConfig$,
},
mockApiCaller,
mockSearch
@ -61,43 +64,46 @@ describe('ES search strategy', () => {
expect(spy).not.toBeCalled();
esSearch.search({ params: {}, debug: true });
await esSearch.search({ params: {}, debug: true });
expect(spy).toBeCalled();
});
it('calls the API caller with the params with defaults', () => {
it('calls the API caller with the params with defaults', async () => {
const params = { index: 'logstash-*' };
const esSearch = esSearchStrategyProvider(
{
core: mockCoreSetup,
config$: mockConfig$,
},
mockApiCaller,
mockSearch
);
esSearch.search({ params });
await esSearch.search({ params });
expect(mockApiCaller).toBeCalled();
expect(mockApiCaller.mock.calls[0][0]).toBe('search');
expect(mockApiCaller.mock.calls[0][1]).toEqual({
...params,
timeout: '0ms',
ignoreUnavailable: true,
restTotalHitsAsInt: true,
});
});
it('calls the API caller with overridden defaults', () => {
const params = { index: 'logstash-*', ignoreUnavailable: false };
it('calls the API caller with overridden defaults', async () => {
const params = { index: 'logstash-*', ignoreUnavailable: false, timeout: '1000ms' };
const esSearch = esSearchStrategyProvider(
{
core: mockCoreSetup,
config$: mockConfig$,
},
mockApiCaller,
mockSearch
);
esSearch.search({ params });
await esSearch.search({ params });
expect(mockApiCaller).toBeCalled();
expect(mockApiCaller.mock.calls[0][0]).toBe('search');
@ -112,6 +118,7 @@ describe('ES search strategy', () => {
const esSearch = esSearchStrategyProvider(
{
core: mockCoreSetup,
config$: mockConfig$,
},
mockApiCaller,
mockSearch

View file

@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import { first } from 'rxjs/operators';
import { APICaller } from 'kibana/server';
import { SearchResponse } from 'elasticsearch';
import { ES_SEARCH_STRATEGY } from '../../../common/search';
@ -28,7 +29,9 @@ export const esSearchStrategyProvider: TSearchStrategyProvider<typeof ES_SEARCH_
): ISearchStrategy<typeof ES_SEARCH_STRATEGY> => {
return {
search: async (request, options) => {
const config = await context.config$.pipe(first()).toPromise();
const params = {
timeout: `${config.elasticsearch.shardTimeout.asMilliseconds()}ms`,
ignoreUnavailable: true, // Don't fail if the index/indices don't exist
restTotalHitsAsInt: true, // Get the number of hits as an int rather than a range
...request.params,

View file

@ -16,8 +16,11 @@
* specific language governing permissions and limitations
* under the License.
*/
import { CoreSetup } from '../../../../core/server';
import { Observable } from 'rxjs';
import { CoreSetup, SharedGlobalConfig } from '../../../../core/server';
export interface ISearchContext {
core: CoreSetup;
config$: Observable<SharedGlobalConfig>;
}

View file

@ -83,6 +83,11 @@ export class SearchService implements Plugin<ISearchSetup, void> {
};
api.registerSearchStrategyContext(this.initializerContext.opaqueId, 'core', () => core);
api.registerSearchStrategyContext(
this.initializerContext.opaqueId,
'config$',
() => this.initializerContext.config.legacy.globalConfig$
);
// ES search capabilities are written in a way that it could easily be a separate plugin,
// however these two plugins are tightly coupled due to the default search strategy using