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

Refactor Plugins to access elasticsearch from CoreStart #59915

Merged
merged 28 commits into from
Apr 15, 2020
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
b621af9
x-pack/watcher: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
ebf9f6c
x-pack/upgrade_assistant: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
e2f4436
x-pack/actions: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
9278e55
x-pack/alerting: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
92fae4c
x-pack/lens: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
73eac20
expressions: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
275bc9e
x-pack/remote_clusters: remove unused Elasticsearch dependency on Cor…
rudolf Mar 11, 2020
5b7039a
x-pack/oss_telemetry: use Elasticsearch from CoreStart
rudolf Mar 11, 2020
06c19f5
Cleanup after #59886
rudolf Mar 17, 2020
78fc4d2
x-pack/watcher: create custom client only once
rudolf Mar 18, 2020
ae5af50
Merge branch 'master' into refactor-elasticsearch-from-start
elasticmachine Mar 18, 2020
9653cf7
Revert "x-pack/watcher: create custom client only once"
rudolf Mar 19, 2020
87bdfc7
Revert "x-pack/watcher: use Elasticsearch from CoreStart"
rudolf Mar 19, 2020
d0f6d35
Merge branch 'master' into refactor-elasticsearch-from-start
rudolf Mar 19, 2020
cdbcf86
x-pack/task_manager: use Elasticsearch from CoreStart
rudolf Mar 19, 2020
a6a0c43
x-pack/event_log: use Elasticsearch from CoreStart
rudolf Mar 20, 2020
02b4a8c
x-pack/alerting: use Elasticsearch from CoreStart
rudolf Mar 20, 2020
417627a
x-pack/apm: use Elasticsearch from CoreStart
rudolf Mar 20, 2020
6e48799
x-pack/actions: use Elasticsearch from CoreStart
rudolf Mar 20, 2020
4d011f1
Merge branch 'master' into refactor-elasticsearch-from-start
elasticmachine Mar 23, 2020
d3f5ef9
PR Feedback
rudolf Mar 23, 2020
4898cfc
APM review nits
rudolf Mar 23, 2020
1714a8f
Remove unused variable
rudolf Mar 24, 2020
d63ea30
Merge branch 'master' into refactor-elasticsearch-from-start
rudolf Mar 24, 2020
39a3924
Remove unused variable
rudolf Mar 25, 2020
b4d5dee
Merge branch 'master' into refactor-elasticsearch-from-start
rudolf Mar 25, 2020
91f0f5d
Merge branch 'master' into refactor-elasticsearch-from-start
rudolf Apr 14, 2020
7f57073
x-pack/apm: better typesafety
rudolf Apr 15, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/core/server/elasticsearch/elasticsearch_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,12 @@ import { CoreService } from '../../types';
import { merge } from '../../utils';
import { CoreContext } from '../core_context';
import { Logger } from '../logging';
import { ClusterClient, ScopeableRequest } from './cluster_client';
import {
ClusterClient,
ScopeableRequest,
IClusterClient,
ICustomClusterClient,
} from './cluster_client';
import { ElasticsearchClientConfig } from './elasticsearch_client_config';
import { ElasticsearchConfig, ElasticsearchConfigType } from './elasticsearch_config';
import { InternalHttpServiceSetup, GetAuthHeaders } from '../http/';
Expand All @@ -57,12 +62,14 @@ export class ElasticsearchService
implements CoreService<InternalElasticsearchServiceSetup, ElasticsearchServiceStart> {
private readonly log: Logger;
private readonly config$: Observable<ElasticsearchConfig>;
private subscription: Subscription | undefined;
private subscription?: Subscription;
private stop$ = new Subject();
private kibanaVersion: string;
createClient: InternalElasticsearchServiceSetup['createClient'] | undefined;
dataClient: InternalElasticsearchServiceSetup['dataClient'] | undefined;
adminClient: InternalElasticsearchServiceSetup['adminClient'] | undefined;
private createClient?: (
type: string,
clientConfig?: Partial<ElasticsearchClientConfig>
) => ICustomClusterClient;
private adminClient?: IClusterClient;

constructor(private readonly coreContext: CoreContext) {
this.kibanaVersion = coreContext.env.packageInfo.version;
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ export class Server {
uiSettings: uiSettingsStart,
};

const pluginsStart = await this.plugins.start(this.coreStart!);
const pluginsStart = await this.plugins.start(this.coreStart);

await this.legacy.start({
core: {
Expand Down
23 changes: 11 additions & 12 deletions src/plugins/expressions/server/legacy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { register, registryFactory, Registry, Fn } from '@kbn/interpreter/common

import Boom from 'boom';
import { schema } from '@kbn/config-schema';
import { CoreSetup, Logger } from 'src/core/server';
import { CoreSetup, Logger, APICaller } from 'src/core/server';
import { ExpressionsServerSetupDependencies } from './plugin';
import { typeSpecs, ExpressionType } from '../common';
import { serializeProvider } from '../common';
Expand Down Expand Up @@ -97,7 +97,10 @@ export const createLegacyServerEndpoints = (
* @param {*} handlers - The Canvas handlers
* @param {*} fnCall - Describes the function being run `{ functionName, args, context }`
*/
async function runFunction(handlers: any, fnCall: any) {
async function runFunction(
handlers: { environment: string; elasticsearchClient: APICaller },
fnCall: any
) {
const { functionName, args, context } = fnCall;
const { deserialize } = serializeProvider(registries.types.toJS());
const fnDef = registries.serverFunctions.toJS()[functionName];
Expand All @@ -112,18 +115,14 @@ export const createLegacyServerEndpoints = (
* results back using ND-JSON.
*/
plugins.bfetch.addBatchProcessingRoute(`/api/interpreter/fns`, request => {
const scopedClient = core.elasticsearch.dataClient.asScoped(request);
const handlers = {
environment: 'server',
elasticsearchClient: async (
endpoint: string,
clientParams: Record<string, any> = {},
options?: any
) => scopedClient.callAsCurrentUser(endpoint, clientParams, options),
};

return {
onBatchItem: async (fnCall: any) => {
const [coreStart] = await core.getStartServices();
const handlers = {
environment: 'server',
elasticsearchClient: coreStart.elasticsearch.legacy.client.asScoped(request)
.callAsCurrentUser,
};
const result = await runFunction(handlers, fnCall);
if (typeof result === 'undefined') {
throw new Error(`Function ${fnCall.functionName} did not return anything.`);
Expand Down
3 changes: 3 additions & 0 deletions x-pack/plugins/actions/server/plugin.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ describe('Actions Plugin', () => {
savedObjects: {
client: {},
},
elasticsearch: {
adminClient: jest.fn(),
},
},
} as any,
httpServerMock.createKibanaRequest(),
Expand Down
22 changes: 10 additions & 12 deletions x-pack/plugins/actions/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,13 @@ import {
Plugin,
CoreSetup,
CoreStart,
IClusterClient,
KibanaRequest,
Logger,
SharedGlobalConfig,
RequestHandler,
IContextProvider,
SavedObjectsServiceStart,
ElasticsearchServiceStart,
} from '../../../../src/core/server';

import {
Expand Down Expand Up @@ -87,7 +87,6 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi

private readonly logger: Logger;
private serverBasePath?: string;
private adminClient?: IClusterClient;
private taskRunnerFactory?: TaskRunnerFactory;
private actionTypeRegistry?: ActionTypeRegistry;
private actionExecutor?: ActionExecutor;
Expand Down Expand Up @@ -161,7 +160,6 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
this.actionTypeRegistry = actionTypeRegistry;
this.serverBasePath = core.http.basePath.serverBasePath;
this.actionExecutor = actionExecutor;
this.adminClient = core.elasticsearch.adminClient;
this.spaces = plugins.spaces?.spacesService;

registerBuiltInActionTypes({
Expand All @@ -186,7 +184,7 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi

core.http.registerRouteHandlerContext(
'actions',
this.createRouteHandlerContext(await this.kibanaIndex)
this.createRouteHandlerContext(core, await this.kibanaIndex)
);

// Routes
Expand All @@ -212,15 +210,14 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
actionTypeRegistry,
taskRunnerFactory,
kibanaIndex,
adminClient,
isESOUsingEphemeralEncryptionKey,
} = this;

actionExecutor!.initialize({
logger,
eventLogger: this.eventLogger!,
spaces: this.spaces,
getServices: this.getServicesFactory(core.savedObjects),
getServices: this.getServicesFactory(core.savedObjects, core.elasticsearch),
encryptedSavedObjectsPlugin: plugins.encryptedSavedObjects,
actionTypeRegistry: actionTypeRegistry!,
});
Expand Down Expand Up @@ -253,26 +250,27 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
savedObjectsClient: core.savedObjects.getScopedClient(request),
actionTypeRegistry: actionTypeRegistry!,
defaultKibanaIndex: await kibanaIndex,
scopedClusterClient: adminClient!.asScoped(request),
scopedClusterClient: core.elasticsearch.legacy.client.asScoped(request),
});
},
};
}

private getServicesFactory(
savedObjects: SavedObjectsServiceStart
savedObjects: SavedObjectsServiceStart,
elasticsearch: ElasticsearchServiceStart
): (request: KibanaRequest) => Services {
const { adminClient } = this;
return request => ({
callCluster: adminClient!.asScoped(request).callAsCurrentUser,
callCluster: elasticsearch.legacy.client.asScoped(request).callAsCurrentUser,
savedObjectsClient: savedObjects.getScopedClient(request),
});
}

private createRouteHandlerContext = (
coreSetup: CoreSetup,
rudolf marked this conversation as resolved.
Show resolved Hide resolved
defaultKibanaIndex: string
): IContextProvider<RequestHandler<any, any, any>, 'actions'> => {
const { actionTypeRegistry, adminClient, isESOUsingEphemeralEncryptionKey } = this;
const { actionTypeRegistry, isESOUsingEphemeralEncryptionKey } = this;
return async function actionsRouteHandlerContext(context, request) {
return {
getActionsClient: () => {
Expand All @@ -285,7 +283,7 @@ export class ActionsPlugin implements Plugin<Promise<PluginSetupContract>, Plugi
savedObjectsClient: context.core.savedObjects.client,
actionTypeRegistry: actionTypeRegistry!,
defaultKibanaIndex,
scopedClusterClient: adminClient!.asScoped(request),
scopedClusterClient: context.core.elasticsearch.adminClient,
rudolf marked this conversation as resolved.
Show resolved Hide resolved
});
},
listTypes: actionTypeRegistry!.list.bind(actionTypeRegistry!),
Expand Down
8 changes: 6 additions & 2 deletions x-pack/plugins/actions/server/usage/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Logger, CoreSetup } from 'kibana/server';
import { Logger, CoreSetup, APICaller } from 'kibana/server';
import moment from 'moment';
import {
RunContext,
Expand Down Expand Up @@ -62,7 +62,11 @@ async function scheduleTasks(logger: Logger, taskManager: TaskManagerStartContra
export function telemetryTaskRunner(logger: Logger, core: CoreSetup, kibanaIndex: string) {
return ({ taskInstance }: RunContext) => {
const { state } = taskInstance;
const callCluster = core.elasticsearch.adminClient.callAsInternalUser;
const callCluster = (...args: Parameters<APICaller>) => {
return core.getStartServices().then(([{ elasticsearch: { legacy: { client } } }]) =>
client.callAsInternalUser(...args)
);
};
return {
async run() {
return Promise.all([
Expand Down
12 changes: 5 additions & 7 deletions x-pack/plugins/alerting/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { TaskRunnerFactory } from './task_runner';
import { AlertsClientFactory } from './alerts_client_factory';
import { LicenseState } from './lib/license_state';
import {
IClusterClient,
KibanaRequest,
Logger,
PluginInitializerContext,
Expand All @@ -29,6 +28,7 @@ import {
IContextProvider,
RequestHandler,
SharedGlobalConfig,
ElasticsearchServiceStart,
} from '../../../../src/core/server';

import {
Expand Down Expand Up @@ -83,7 +83,6 @@ export class AlertingPlugin {
private readonly logger: Logger;
private alertTypeRegistry?: AlertTypeRegistry;
private readonly taskRunnerFactory: TaskRunnerFactory;
private adminClient?: IClusterClient;
private serverBasePath?: string;
private licenseState: LicenseState | null = null;
private isESOUsingEphemeralEncryptionKey?: boolean;
Expand All @@ -107,7 +106,6 @@ export class AlertingPlugin {
}

public async setup(core: CoreSetup, plugins: AlertingPluginsSetup): Promise<PluginSetupContract> {
this.adminClient = core.elasticsearch.adminClient;
this.licenseState = new LicenseState(plugins.licensing.license$);
this.spaces = plugins.spaces?.spacesService;
this.security = plugins.security;
Expand Down Expand Up @@ -204,7 +202,7 @@ export class AlertingPlugin {

taskRunnerFactory.initialize({
logger,
getServices: this.getServicesFactory(core.savedObjects),
getServices: this.getServicesFactory(core.savedObjects, core.elasticsearch),
spaceIdToNamespace: this.spaceIdToNamespace,
executeAction: plugins.actions.execute,
encryptedSavedObjectsPlugin: plugins.encryptedSavedObjects,
Expand Down Expand Up @@ -243,11 +241,11 @@ export class AlertingPlugin {
};

private getServicesFactory(
savedObjects: SavedObjectsServiceStart
savedObjects: SavedObjectsServiceStart,
elasticsearch: ElasticsearchServiceStart
): (request: KibanaRequest) => Services {
const { adminClient } = this;
return request => ({
callCluster: adminClient!.asScoped(request).callAsCurrentUser,
callCluster: elasticsearch.legacy.client.asScoped(request).callAsCurrentUser,
savedObjectsClient: savedObjects.getScopedClient(request),
});
}
Expand Down
9 changes: 7 additions & 2 deletions x-pack/plugins/alerting/server/usage/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { Logger, CoreSetup } from 'kibana/server';
import { Logger, CoreSetup, APICaller } from 'kibana/server';
import moment from 'moment';
import {
RunContext,
Expand Down Expand Up @@ -65,7 +65,12 @@ async function scheduleTasks(logger: Logger, taskManager: TaskManagerStartContra
export function telemetryTaskRunner(logger: Logger, core: CoreSetup, kibanaIndex: string) {
return ({ taskInstance }: RunContext) => {
const { state } = taskInstance;
const callCluster = core.elasticsearch.adminClient.callAsInternalUser;
const callCluster = (...args: Parameters<APICaller>) => {
return core.getStartServices().then(([{ elasticsearch: { legacy: { client } } }]) =>
client.callAsInternalUser(...args)
);
};

return {
async run() {
return Promise.all([
Expand Down
54 changes: 36 additions & 18 deletions x-pack/plugins/apm/server/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,13 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { PluginInitializerContext, Plugin, CoreSetup } from 'src/core/server';
import {
PluginInitializerContext,
Plugin,
CoreSetup,
CoreStart,
Logger
} from 'src/core/server';
import { Observable, combineLatest, AsyncSubject } from 'rxjs';
import { map, take } from 'rxjs/operators';
import { Server } from 'hapi';
Expand Down Expand Up @@ -33,6 +39,8 @@ export interface APMPluginContract {
}

export class APMPlugin implements Plugin<APMPluginContract> {
private currentConfig?: APMConfig;
private logger?: Logger;
legacySetup$: AsyncSubject<LegacySetup>;
constructor(private readonly initContext: PluginInitializerContext) {
this.initContext = initContext;
Expand All @@ -49,30 +57,23 @@ export class APMPlugin implements Plugin<APMPluginContract> {
usageCollection?: UsageCollectionSetup;
}
) {
const logger = this.initContext.logger.get('apm');
const logger = (this.logger = this.initContext.logger.get('apm'));
Copy link
Member

Choose a reason for hiding this comment

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

Like below, let's stick to a single assignment:

Suggested change
const logger = (this.logger = this.initContext.logger.get('apm'));
this.logger = this.initContext.logger.get('apm');

const config$ = this.initContext.config.create<APMXPackConfig>();
const mergedConfig$ = combineLatest(plugins.apm_oss.config$, config$).pipe(
map(([apmOssConfig, apmConfig]) => mergeConfigs(apmOssConfig, apmConfig))
);

this.legacySetup$.subscribe(__LEGACY => {
createApmApi().init(core, { config$: mergedConfig$, logger, __LEGACY });
createApmApi().init(core, {
config$: mergedConfig$,
logger,
__LEGACY
});
});

const currentConfig = await mergedConfig$.pipe(take(1)).toPromise();

// create agent configuration index without blocking setup lifecycle
createApmAgentConfigurationIndex({
esClient: core.elasticsearch.dataClient,
config: currentConfig,
logger
});
// create custom action index without blocking setup lifecycle
createApmCustomLinkIndex({
esClient: core.elasticsearch.dataClient,
config: currentConfig,
logger
});
const currentConfig = (this.currentConfig = await mergedConfig$
.pipe(take(1))
.toPromise());
Copy link
Member

Choose a reason for hiding this comment

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

For simplicity let's stick to a single assignment

Suggested change
const currentConfig = (this.currentConfig = await mergedConfig$
.pipe(take(1))
.toPromise());
this.currentConfig = await mergedConfig$
.pipe(take(1))
.toPromise();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If using only the class variable, typescript is unable to infer if the class variable is defined in async context requiring a ! to override the possibly undefined values. So I preferred the type safety of another local variable. I've addressed your suggestions in 4898cfc let me know what you think?


plugins.home.tutorials.registerTutorial(
tutorialProvider({
Expand Down Expand Up @@ -115,6 +116,23 @@ export class APMPlugin implements Plugin<APMPluginContract> {
};
}

public start() {}
public start(core: CoreStart) {
if (this.currentConfig == null || this.logger == null) {
throw new Error('APMPlugin needs to be setup before calling start()');
}

// create agent configuration index without blocking setup lifecycle
createApmAgentConfigurationIndex({
esClient: core.elasticsearch.legacy.client,
config: this.currentConfig,
logger: this.logger
});
// create custom action index without blocking setup lifecycle
createApmCustomLinkIndex({
esClient: core.elasticsearch.legacy.client,
config: this.currentConfig,
logger: this.logger
});
}
public stop() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ beforeEach(() => {
clusterClient = elasticsearchServiceMock.createClusterClient();
clusterClientAdapter = new ClusterClientAdapter({
logger,
clusterClient,
clusterClientPromise: new Promise(resolve => resolve(clusterClient)),
rudolf marked this conversation as resolved.
Show resolved Hide resolved
});
});

Expand Down
Loading