Skip to content

Commit

Permalink
[APM] Refactor internal es client (#170733)
Browse files Browse the repository at this point in the history
Related #170031. 

The internal Elasticsearch client in APM currently takes `apmIndices` as
a required argument. This is due to legacy reasons and no longer needed
and should therefore be removed.

Co-authored-by: Katerina <[email protected]>
  • Loading branch information
sorenlouv and kpatticha authored Nov 15, 2023
1 parent aef7148 commit b710f78
Show file tree
Hide file tree
Showing 12 changed files with 110 additions and 142 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { unwrapEsResponse } from '@kbn/observability-plugin/server';
import type { ESSearchResponse, ESSearchRequest } from '@kbn/es-types';
import type { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { ElasticsearchClient } from '@kbn/core-elasticsearch-server';
import { APMRouteHandlerResources } from '../../../../routes/apm_routes/register_apm_server_routes';
import {
Expand All @@ -21,39 +20,31 @@ import { cancelEsRequestOnAbort } from '../cancel_es_request_on_abort';
export type APMIndexDocumentParams<T> = estypes.IndexRequest<T>;

export type APMInternalESClient = Awaited<
ReturnType<typeof createInternalESClientWithContext>
ReturnType<typeof createInternalESClientWithResources>
>;

export async function createInternalESClientWithContext({
debug,
apmIndices,
export async function createInternalESClientWithResources({
params,
request,
context,
}: {
debug: boolean;
apmIndices: APMIndices;
request: APMRouteHandlerResources['request'];
context: APMRouteHandlerResources['context'];
}) {
}: APMRouteHandlerResources) {
const coreContext = await context.core;
const { asInternalUser } = coreContext.elasticsearch.client;
const debug = params.query._inspect;

return createInternalESClient({
debug,
apmIndices,
request,
elasticsearchClient: asInternalUser,
});
}

export async function createInternalESClient({
debug,
apmIndices,
request,
elasticsearchClient,
}: {
debug: boolean;
apmIndices: APMIndices;
request?: APMRouteHandlerResources['request'];
elasticsearchClient: ElasticsearchClient;
}) {
Expand Down Expand Up @@ -92,7 +83,6 @@ export async function createInternalESClient({
}

return {
apmIndices,
search: async <
TDocument = unknown,
TSearchRequest extends ESSearchRequest = ESSearchRequest
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
KibanaRequest,
} from '@kbn/core/server';
import { PackagePolicy } from '@kbn/fleet-plugin/common';
import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import {
APM_SERVER_SCHEMA_SAVED_OBJECT_TYPE,
APM_SERVER_SCHEMA_SAVED_OBJECT_ID,
Expand All @@ -33,6 +34,7 @@ export async function createCloudApmPackgePolicy({
logger,
internalESClient,
request,
apmIndices,
}: {
cloudPluginSetup: APMPluginSetupDependencies['cloud'];
fleetPluginStart: NonNullable<APMPluginStartDependencies['fleet']>;
Expand All @@ -41,6 +43,7 @@ export async function createCloudApmPackgePolicy({
logger: Logger;
internalESClient: APMInternalESClient;
request: KibanaRequest;
apmIndices: APMIndices;
}): Promise<PackagePolicy> {
const { attributes } = await savedObjectsClient.get(
APM_SERVER_SCHEMA_SAVED_OBJECT_TYPE,
Expand All @@ -61,6 +64,7 @@ export async function createCloudApmPackgePolicy({
internalESClient,
packagePolicy: apmPackagePolicyDefinition,
fleetPluginStart,
apmIndices,
});
logger.info(`Fleet migration on Cloud - apmPackagePolicy create start`);
const apmPackagePolicy = await fleetPluginStart.packagePolicyService.create(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* 2.0.
*/

import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { NewPackagePolicy } from '@kbn/fleet-plugin/common';
import { APMInternalESClient } from '../../lib/helpers/create_es_client/create_internal_es_client';
import { APMPluginStartDependencies } from '../../types';
Expand All @@ -19,13 +20,15 @@ export async function decoratePackagePolicyWithAgentConfigAndSourceMap({
packagePolicy,
internalESClient,
fleetPluginStart,
apmIndices,
}: {
packagePolicy: NewPackagePolicy;
internalESClient: APMInternalESClient;
fleetPluginStart: NonNullable<APMPluginStartDependencies['fleet']>;
apmIndices: APMIndices;
}) {
const [agentConfigurations, { artifacts }] = await Promise.all([
listConfigurations(internalESClient),
listConfigurations(internalESClient, apmIndices),
listSourceMapArtifacts({ fleetPluginStart }),
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ function onPackagePolicyCreateOrUpdate({

const internalESClient = await createInternalESClient({
debug: false,
apmIndices,
elasticsearchClient: asInternalUser,
});

return decoratePackagePolicyWithAgentConfigAndSourceMap({
internalESClient,
fleetPluginStart,
packagePolicy,
apmIndices,
});
};
}
13 changes: 5 additions & 8 deletions x-pack/plugins/apm/server/routes/fleet/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
APM_SERVER_SCHEMA_SAVED_OBJECT_TYPE,
} from '../../../common/apm_saved_object_constants';
import { ApmFeatureFlags } from '../../../common/apm_feature_flags';
import { createInternalESClientWithContext } from '../../lib/helpers/create_es_client/create_internal_es_client';
import { createInternalESClientWithResources } from '../../lib/helpers/create_es_client/create_internal_es_client';
import { getInternalSavedObjectsClient } from '../../lib/helpers/get_internal_saved_objects_client';
import { createApmServerRoute } from '../apm_routes/create_apm_server_route';
import { createCloudApmPackgePolicy } from './create_cloud_apm_package_policy';
Expand Down Expand Up @@ -172,20 +172,17 @@ const createCloudApmPackagePolicyRoute = createApmServerRoute({
throw Boom.forbidden(CLOUD_SUPERUSER_REQUIRED_MESSAGE);
}

const internalESClient = await createInternalESClientWithContext({
context,
request,
debug: resources.params.query._inspect,
apmIndices,
});

const internalESClient = await createInternalESClientWithResources(
resources
);
const cloudApmPackagePolicy = await createCloudApmPackgePolicy({
cloudPluginSetup,
fleetPluginStart,
savedObjectsClient,
esClient,
logger,
internalESClient,
apmIndices,
request,
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import { CoreStart, SavedObjectsClientContract } from '@kbn/core/server';
import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { TelemetryUsageCounter } from '../typings';
import { APMPluginStartDependencies } from '../../types';
import { getInternalSavedObjectsClient } from '../../lib/helpers/get_internal_saved_objects_client';
Expand All @@ -18,11 +19,13 @@ export async function syncAgentConfigsToApmPackagePolicies({
coreStartPromise,
fleetPluginStart,
internalESClient,
apmIndices,
telemetryUsageCounter,
}: {
coreStartPromise: Promise<CoreStart>;
fleetPluginStart: NonNullable<APMPluginStartDependencies['fleet']>;
internalESClient: APMInternalESClient;
apmIndices: APMIndices;
telemetryUsageCounter?: TelemetryUsageCounter;
}) {
if (telemetryUsageCounter) {
Expand All @@ -36,7 +39,7 @@ export async function syncAgentConfigsToApmPackagePolicies({
const [savedObjectsClient, agentConfigurations, packagePolicies] =
await Promise.all([
getInternalSavedObjectsClient(coreStart),
listConfigurations(internalESClient),
listConfigurations(internalESClient, apmIndices),
getApmPackagePolicies({ coreStart, fleetPluginStart }),
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import type { SearchHit } from '@kbn/es-types';
import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { AgentConfiguration } from '../../../../common/agent_configuration/configuration_types';
import {
SERVICE_ENVIRONMENT,
Expand All @@ -19,9 +20,11 @@ import { getConfigsAppliedToAgentsThroughFleet } from './get_config_applied_to_a
export async function findExactConfiguration({
service,
internalESClient,
apmIndices,
}: {
service: AgentConfiguration['service'];
internalESClient: APMInternalESClient;
apmIndices: APMIndices;
}) {
const serviceNameFilter = service.name
? { term: { [SERVICE_NAME]: service.name } }
Expand All @@ -45,7 +48,7 @@ export async function findExactConfiguration({
'find_exact_agent_configuration',
params
),
getConfigsAppliedToAgentsThroughFleet(internalESClient),
getConfigsAppliedToAgentsThroughFleet(internalESClient, apmIndices),
]);

const hit = agentConfig.hits.hits[0] as
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@

import { termQuery, rangeQuery } from '@kbn/observability-plugin/server';
import datemath from '@kbn/datemath';
import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { METRICSET_NAME } from '../../../../common/es_fields/apm';
import { APMInternalESClient } from '../../../lib/helpers/create_es_client/create_internal_es_client';

export async function getConfigsAppliedToAgentsThroughFleet(
internalESClient: APMInternalESClient
internalESClient: APMInternalESClient,
apmIndices: APMIndices
) {
const params = {
index: internalESClient.apmIndices.metric,
index: apmIndices.metric,
track_total_hits: 0,
size: 0,
body: {
query: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,16 @@
* 2.0.
*/

import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { AgentConfiguration } from '../../../../common/agent_configuration/configuration_types';
import { convertConfigSettingsToString } from './convert_settings_to_string';
import { getConfigsAppliedToAgentsThroughFleet } from './get_config_applied_to_agent_through_fleet';
import { APMInternalESClient } from '../../../lib/helpers/create_es_client/create_internal_es_client';
import { APM_AGENT_CONFIGURATION_INDEX } from '../apm_indices/apm_system_index_constants';

export async function listConfigurations(
internalESClient: APMInternalESClient
internalESClient: APMInternalESClient,
apmIndices: APMIndices
) {
const params = {
index: APM_AGENT_CONFIGURATION_INDEX,
Expand All @@ -24,7 +26,7 @@ export async function listConfigurations(
'list_agent_configuration',
params
),
getConfigsAppliedToAgentsThroughFleet(internalESClient),
getConfigsAppliedToAgentsThroughFleet(internalESClient, apmIndices),
]);

return agentConfigs.hits.hits
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,9 @@ describe('agent configuration queries', () => {

describe('listConfigurations', () => {
it('fetches configurations', async () => {
mock = await inspectSearchParams(({ mockInternalESClient }) =>
listConfigurations(mockInternalESClient)
mock = await inspectSearchParams(
({ mockInternalESClient, mockIndices }) =>
listConfigurations(mockInternalESClient, mockIndices)
);

expect(mock.params).toMatchSnapshot();
Expand Down Expand Up @@ -92,33 +93,39 @@ describe('agent configuration queries', () => {

describe('findExactConfiguration', () => {
it('find configuration by service.name', async () => {
mock = await inspectSearchParams(({ mockInternalESClient }) =>
findExactConfiguration({
service: { name: 'foo' },
internalESClient: mockInternalESClient,
})
mock = await inspectSearchParams(
({ mockInternalESClient, mockIndices }) =>
findExactConfiguration({
service: { name: 'foo' },
internalESClient: mockInternalESClient,
apmIndices: mockIndices,
})
);

expect(mock.params).toMatchSnapshot();
});

it('find configuration by service.environment', async () => {
mock = await inspectSearchParams(({ mockInternalESClient }) =>
findExactConfiguration({
service: { environment: 'bar' },
internalESClient: mockInternalESClient,
})
mock = await inspectSearchParams(
({ mockInternalESClient, mockIndices }) =>
findExactConfiguration({
service: { environment: 'bar' },
internalESClient: mockInternalESClient,
apmIndices: mockIndices,
})
);

expect(mock.params).toMatchSnapshot();
});

it('find configuration by service.name and service.environment', async () => {
mock = await inspectSearchParams(({ mockInternalESClient }) =>
findExactConfiguration({
service: { name: 'foo', environment: 'bar' },
internalESClient: mockInternalESClient,
})
mock = await inspectSearchParams(
({ mockInternalESClient, mockIndices }) =>
findExactConfiguration({
service: { name: 'foo', environment: 'bar' },
internalESClient: mockInternalESClient,
apmIndices: mockIndices,
})
);

expect(mock.params).toMatchSnapshot();
Expand Down
Loading

0 comments on commit b710f78

Please sign in to comment.