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.

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

[Search] add server logs #72454

Merged
merged 14 commits into from
Jul 28, 2020
4 changes: 2 additions & 2 deletions src/plugins/data/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,11 @@ export class DataServerPlugin implements Plugin<DataPluginSetup, DataPluginStart
private readonly logger: Logger;

constructor(initializerContext: PluginInitializerContext<ConfigSchema>) {
this.searchService = new SearchService(initializerContext);
this.logger = initializerContext.logger.get('data');
this.searchService = new SearchService(initializerContext, this.logger);
this.scriptsService = new ScriptsService();
this.kqlTelemetryService = new KqlTelemetryService(initializerContext);
this.autocompleteService = new AutocompleteService(initializerContext);
this.logger = initializerContext.logger.get('data');
}

public setup(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ import { pluginInitializerContextConfigMock } from '../../../../../core/server/m
import { esSearchStrategyProvider } from './es_search_strategy';

describe('ES search strategy', () => {
const mockLogger: any = {
info: () => {},
};
const mockApiCaller = jest.fn().mockResolvedValue({
_shards: {
total: 10,
Expand All @@ -40,14 +43,14 @@ describe('ES search strategy', () => {
});

it('returns a strategy with `search`', async () => {
const esSearch = await esSearchStrategyProvider(mockConfig$);
const esSearch = await esSearchStrategyProvider(mockConfig$, mockLogger);

expect(typeof esSearch.search).toBe('function');
});

it('calls the API caller with the params with defaults', async () => {
const params = { index: 'logstash-*' };
const esSearch = await esSearchStrategyProvider(mockConfig$);
const esSearch = await esSearchStrategyProvider(mockConfig$, mockLogger);

await esSearch.search((mockContext as unknown) as RequestHandlerContext, { params });

Expand All @@ -63,7 +66,7 @@ describe('ES search strategy', () => {

it('calls the API caller with overridden defaults', async () => {
const params = { index: 'logstash-*', ignoreUnavailable: false, timeout: '1000ms' };
const esSearch = await esSearchStrategyProvider(mockConfig$);
const esSearch = await esSearchStrategyProvider(mockConfig$, mockLogger);

await esSearch.search((mockContext as unknown) as RequestHandlerContext, { params });

Expand All @@ -77,7 +80,7 @@ describe('ES search strategy', () => {

it('returns total, loaded, and raw response', async () => {
const params = { index: 'logstash-*' };
const esSearch = await esSearchStrategyProvider(mockConfig$);
const esSearch = await esSearchStrategyProvider(mockConfig$, mockLogger);

const response = await esSearch.search((mockContext as unknown) as RequestHandlerContext, {
params,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,18 @@
* under the License.
*/
import { first } from 'rxjs/operators';
import { SharedGlobalConfig } from 'kibana/server';
import { SharedGlobalConfig, Logger } from 'kibana/server';
import { SearchResponse } from 'elasticsearch';
import { Observable } from 'rxjs';
import { ISearchStrategy, getDefaultSearchParams, getTotalLoaded } from '..';

export const esSearchStrategyProvider = (
config$: Observable<SharedGlobalConfig>
config$: Observable<SharedGlobalConfig>,
logger: Logger
): ISearchStrategy => {
return {
search: async (context, request, options) => {
logger.info(`search ${JSON.stringify(request.params)}`);
const config = await config$.pipe(first()).toPromise();
const defaultParams = getDefaultSearchParams(config);

Expand Down
5 changes: 4 additions & 1 deletion src/plugins/data/server/search/search_service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ describe('Search service', () => {
let mockCoreSetup: MockedKeys<CoreSetup<object, DataPluginStart>>;

beforeEach(() => {
plugin = new SearchService(coreMock.createPluginInitializerContext({}));
const mockLogger: any = {
info: () => {},
};
plugin = new SearchService(coreMock.createPluginInitializerContext({}), mockLogger);
mockCoreSetup = coreMock.createSetup();
});

Expand Down
24 changes: 20 additions & 4 deletions src/plugins/data/server/search/search_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
PluginInitializerContext,
CoreSetup,
RequestHandlerContext,
Logger,
} from '../../../../core/server';
import { ISearchSetup, ISearchStart, ISearchStrategy } from './types';
import { registerSearchRoute } from './routes';
Expand All @@ -41,15 +42,18 @@ interface StrategyMap {
export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
private searchStrategies: StrategyMap = {};

constructor(private initializerContext: PluginInitializerContext) {}
constructor(
private initializerContext: PluginInitializerContext,
private readonly logger: Logger
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure that we need to keep it as a separate field. initializerContext already has logger field

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexwizp the one on the context is the LoggerFactory.
You need to initialize it with the plugin name, and it makes sense IMO to initialize it once.

) {}

public setup(
core: CoreSetup<object, DataPluginStart>,
{ usageCollection }: { usageCollection?: UsageCollectionSetup }
): ISearchSetup {
this.registerSearchStrategy(
ES_SEARCH_STRATEGY,
esSearchStrategyProvider(this.initializerContext.config.legacy.globalConfig$)
esSearchStrategyProvider(this.initializerContext.config.legacy.globalConfig$, this.logger)
lizozom marked this conversation as resolved.
Show resolved Hide resolved
);

core.savedObjects.registerType(searchTelemetry);
Expand All @@ -65,7 +69,11 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
return { registerSearchStrategy: this.registerSearchStrategy, usage };
}

private search(context: RequestHandlerContext, searchRequest: IEsSearchRequest, options: any) {
private search(
context: RequestHandlerContext,
searchRequest: IEsSearchRequest,
options: Record<string, any>
) {
return this.getSearchStrategy(options.strategy || ES_SEARCH_STRATEGY).search(
context,
searchRequest,
Expand All @@ -76,17 +84,25 @@ export class SearchService implements Plugin<ISearchSetup, ISearchStart> {
public start(): ISearchStart {
return {
getSearchStrategy: this.getSearchStrategy,
search: this.search,
search: (
context: RequestHandlerContext,
searchRequest: IEsSearchRequest,
options: Record<string, any>
) => {
return this.search(context, searchRequest, options);
},
};
}

public stop() {}

private registerSearchStrategy = (name: string, strategy: ISearchStrategy) => {
this.logger.info(`Register strategy ${name}`);
this.searchStrategies[name] = strategy;
};

private getSearchStrategy = (name: string): ISearchStrategy => {
this.logger.info(`Get strategy ${name}`);
const strategy = this.searchStrategies[name];
if (!strategy) {
throw new Error(`Search strategy ${name} not found`);
Expand Down
12 changes: 10 additions & 2 deletions x-pack/plugins/data_enhanced/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
CoreSetup,
CoreStart,
Plugin,
Logger,
} from '../../../../src/core/server';
import { ES_SEARCH_STRATEGY } from '../../../../src/plugins/data/common';
import { PluginSetup as DataPluginSetup } from '../../../../src/plugins/data/server';
Expand All @@ -19,12 +20,19 @@ interface SetupDependencies {
}

export class EnhancedDataServerPlugin implements Plugin<void, void, SetupDependencies> {
constructor(private initializerContext: PluginInitializerContext) {}
private readonly logger: Logger;

constructor(private initializerContext: PluginInitializerContext) {
this.logger = initializerContext.logger.get('data_enhanced');
}

public setup(core: CoreSetup, deps: SetupDependencies) {
deps.data.search.registerSearchStrategy(
ES_SEARCH_STRATEGY,
enhancedEsSearchStrategyProvider(this.initializerContext.config.legacy.globalConfig$)
enhancedEsSearchStrategyProvider(
this.initializerContext.config.legacy.globalConfig$,
this.logger
)
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ const mockRollupResponse = {

describe('ES search strategy', () => {
const mockApiCaller = jest.fn();
const mockLogger: any = {
info: () => {},
};
const mockContext = {
core: { elasticsearch: { legacy: { client: { callAsCurrentUser: mockApiCaller } } } },
};
Expand All @@ -41,7 +44,7 @@ describe('ES search strategy', () => {
});

it('returns a strategy with `search`', async () => {
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$);
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger);

expect(typeof esSearch.search).toBe('function');
});
Expand All @@ -50,7 +53,7 @@ describe('ES search strategy', () => {
mockApiCaller.mockResolvedValueOnce(mockAsyncResponse);

const params = { index: 'logstash-*', body: { query: {} } };
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$);
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger);

await esSearch.search((mockContext as unknown) as RequestHandlerContext, { params });

Expand All @@ -66,7 +69,7 @@ describe('ES search strategy', () => {
mockApiCaller.mockResolvedValueOnce(mockAsyncResponse);

const params = { index: 'logstash-*', body: { query: {} } };
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$);
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger);

await esSearch.search((mockContext as unknown) as RequestHandlerContext, { id: 'foo', params });

Expand All @@ -82,7 +85,7 @@ describe('ES search strategy', () => {
mockApiCaller.mockResolvedValueOnce(mockAsyncResponse);

const params = { index: 'foo-程', body: {} };
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$);
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger);

await esSearch.search((mockContext as unknown) as RequestHandlerContext, { params });

Expand All @@ -97,7 +100,7 @@ describe('ES search strategy', () => {
mockApiCaller.mockResolvedValueOnce(mockRollupResponse);

const params = { index: 'foo-程', body: {} };
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$);
const esSearch = await enhancedEsSearchStrategyProvider(mockConfig$, mockLogger);

await esSearch.search((mockContext as unknown) as RequestHandlerContext, {
indexType: 'rollup',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
LegacyAPICaller,
SharedGlobalConfig,
RequestHandlerContext,
Logger,
} from '../../../../../src/core/server';
import {
ISearchOptions,
Expand All @@ -30,13 +31,15 @@ export interface AsyncSearchResponse<T> {
}

export const enhancedEsSearchStrategyProvider = (
config$: Observable<SharedGlobalConfig>
config$: Observable<SharedGlobalConfig>,
logger: Logger
): ISearchStrategy => {
const search = async (
context: RequestHandlerContext,
request: IEnhancedEsSearchRequest,
options?: ISearchOptions
) => {
logger.info(`search ${JSON.stringify(request.params) || request.id}`);
const config = await config$.pipe(first()).toPromise();
const caller = context.core.elasticsearch.legacy.client.callAsCurrentUser;
const defaultParams = getDefaultSearchParams(config);
Expand All @@ -48,6 +51,7 @@ export const enhancedEsSearchStrategyProvider = (
};

const cancel = async (context: RequestHandlerContext, id: string) => {
logger.info(`cancel ${id}`);
const method = 'DELETE';
const path = encodeURI(`/_async_search/${id}`);
await context.core.elasticsearch.legacy.client.callAsCurrentUser('transport.request', {
Expand Down