Skip to content

Commit

Permalink
[8.12] [APM] Remove usage of internal client when fetching agent conf…
Browse files Browse the repository at this point in the history
…ig etags metrics (#173001) | [APM] Unskip apm api test suite (#173055) (#173120)

Backports the following to `8.12`:

- #173055
- #173001

<!--BACKPORT {commits} BACKPORT-->
  • Loading branch information
sorenlouv authored Dec 12, 2023
1 parent c87b62d commit 837ecf8
Show file tree
Hide file tree
Showing 11 changed files with 119 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export async function decoratePackagePolicyWithAgentConfigAndSourceMap({
apmIndices: APMIndices;
}) {
const [agentConfigurations, { artifacts }] = await Promise.all([
listConfigurations(internalESClient, apmIndices),
listConfigurations({ internalESClient, apmIndices }),
listSourceMapArtifacts({ fleetPluginStart }),
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export async function syncAgentConfigsToApmPackagePolicies({
const [savedObjectsClient, agentConfigurations, packagePolicies] =
await Promise.all([
getInternalSavedObjectsClient(coreStart),
listConfigurations(internalESClient, apmIndices),
listConfigurations({ internalESClient, apmIndices }),
getApmPackagePolicies({ coreStart, fleetPluginStart }),
]);

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

import type { SearchHit } from '@kbn/es-types';
import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { APMEventClient } from '../../../lib/helpers/create_es_client/create_apm_event_client';
import { AgentConfiguration } from '../../../../common/agent_configuration/configuration_types';
import {
SERVICE_ENVIRONMENT,
Expand All @@ -15,16 +15,16 @@ import {
import { APMInternalESClient } from '../../../lib/helpers/create_es_client/create_internal_es_client';
import { APM_AGENT_CONFIGURATION_INDEX } from '../apm_indices/apm_system_index_constants';
import { convertConfigSettingsToString } from './convert_settings_to_string';
import { getConfigsAppliedToAgentsThroughFleet } from './get_config_applied_to_agent_through_fleet';
import { getAgentConfigEtagMetrics } from './get_agent_config_etag_metrics';

export async function findExactConfiguration({
service,
internalESClient,
apmIndices,
apmEventClient,
}: {
service: AgentConfiguration['service'];
internalESClient: APMInternalESClient;
apmIndices: APMIndices;
apmEventClient: APMEventClient;
}) {
const serviceNameFilter = service.name
? { term: { [SERVICE_NAME]: service.name } }
Expand All @@ -43,13 +43,10 @@ export async function findExactConfiguration({
},
};

const [agentConfig, configsAppliedToAgentsThroughFleet] = await Promise.all([
internalESClient.search<AgentConfiguration, typeof params>(
'find_exact_agent_configuration',
params
),
getConfigsAppliedToAgentsThroughFleet(internalESClient, apmIndices),
]);
const agentConfig = await internalESClient.search<
AgentConfiguration,
typeof params
>('find_exact_agent_configuration', params);

const hit = agentConfig.hits.hits[0] as
| SearchHit<AgentConfiguration>
Expand All @@ -59,11 +56,33 @@ export async function findExactConfiguration({
return;
}

const appliedByAgent = await getIsAppliedByAgent({
apmEventClient,
agentConfiguration: hit._source,
});

return {
id: hit._id,
...convertConfigSettingsToString(hit)._source,
applied_by_agent:
hit._source.applied_by_agent ||
configsAppliedToAgentsThroughFleet.hasOwnProperty(hit._source.etag),
applied_by_agent: appliedByAgent,
};
}

async function getIsAppliedByAgent({
apmEventClient,
agentConfiguration,
}: {
apmEventClient: APMEventClient;
agentConfiguration: AgentConfiguration;
}) {
if (agentConfiguration.applied_by_agent) {
return true;
}

const appliedEtags = await getAgentConfigEtagMetrics(
apmEventClient,
agentConfiguration.etag
);

return appliedEtags.includes(agentConfiguration.etag);
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,26 @@

import { termQuery, rangeQuery } from '@kbn/observability-plugin/server';
import datemath from '@kbn/datemath';
import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { ProcessorEvent } from '@kbn/observability-plugin/common';
import { APMEventClient } from '../../../lib/helpers/create_es_client/create_apm_event_client';
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,
apmIndices: APMIndices
export async function getAgentConfigEtagMetrics(
apmEventClient: APMEventClient,
etag?: string
) {
const params = {
index: apmIndices.metric,
track_total_hits: 0,
size: 0,
apm: {
events: [ProcessorEvent.metric],
},
body: {
track_total_hits: 0,
size: 0,
query: {
bool: {
filter: [
...termQuery(METRICSET_NAME, 'agent_config'),
...termQuery('labels.etag', etag),
...rangeQuery(
datemath.parse('now-15m')!.valueOf(),
datemath.parse('now')!.valueOf()
Expand All @@ -42,18 +45,14 @@ export async function getConfigsAppliedToAgentsThroughFleet(
},
};

const response = await internalESClient.search(
'get_config_applied_to_agent_through_fleet',
const response = await apmEventClient.search(
'get_agent_config_etag_metrics',
params
);

return (
response.aggregations?.config_by_etag.buckets.reduce(
(configsAppliedToAgentsThroughFleet, bucket) => {
configsAppliedToAgentsThroughFleet[bucket.key as string] = true;
return configsAppliedToAgentsThroughFleet;
},
{} as Record<string, true>
) ?? {}
response.aggregations?.config_by_etag.buckets.map(
({ key }) => key as string
) ?? []
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,27 +6,33 @@
*/

import { APMIndices } from '@kbn/apm-data-access-plugin/server';
import { APMEventClient } from '../../../lib/helpers/create_es_client/create_apm_event_client';
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 { getAgentConfigEtagMetrics } from './get_agent_config_etag_metrics';
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,
apmIndices: APMIndices
) {
export async function listConfigurations({
internalESClient,
apmEventClient,
apmIndices,
}: {
internalESClient: APMInternalESClient;
apmEventClient?: APMEventClient;
apmIndices: APMIndices;
}) {
const params = {
index: APM_AGENT_CONFIGURATION_INDEX,
size: 200,
};

const [agentConfigs, configsAppliedToAgentsThroughFleet] = await Promise.all([
const [agentConfigs, appliedEtags = []] = await Promise.all([
internalESClient.search<AgentConfiguration>(
'list_agent_configuration',
params
),
getConfigsAppliedToAgentsThroughFleet(internalESClient, apmIndices),
apmEventClient ? getAgentConfigEtagMetrics(apmEventClient) : undefined,
]);

return agentConfigs.hits.hits
Expand All @@ -36,7 +42,7 @@ export async function listConfigurations(
...hit._source,
applied_by_agent:
hit._source.applied_by_agent ||
configsAppliedToAgentsThroughFleet.hasOwnProperty(hit._source.etag),
appliedEtags.includes(hit._source.etag),
};
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ describe('agent configuration queries', () => {
it('fetches configurations', async () => {
mock = await inspectSearchParams(
({ mockInternalESClient, mockIndices }) =>
listConfigurations(mockInternalESClient, mockIndices)
listConfigurations({
internalESClient: mockInternalESClient,
apmIndices: mockIndices,
})
);

expect(mock.params).toMatchSnapshot();
Expand Down Expand Up @@ -94,11 +97,11 @@ describe('agent configuration queries', () => {
describe('findExactConfiguration', () => {
it('find configuration by service.name', async () => {
mock = await inspectSearchParams(
({ mockInternalESClient, mockIndices }) =>
({ mockInternalESClient, mockApmEventClient }) =>
findExactConfiguration({
service: { name: 'foo' },
internalESClient: mockInternalESClient,
apmIndices: mockIndices,
apmEventClient: mockApmEventClient,
})
);

Expand All @@ -107,11 +110,11 @@ describe('agent configuration queries', () => {

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

Expand All @@ -120,11 +123,11 @@ describe('agent configuration queries', () => {

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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,15 @@ const agentConfigurationRoute = createApmServerRoute({
throwNotFoundIfAgentConfigNotAvailable(resources.featureFlags);

const apmIndices = await resources.getApmIndices();
const internalESClient = await createInternalESClientWithResources(
resources
);
const configurations = await listConfigurations(
const [internalESClient, apmEventClient] = await Promise.all([
createInternalESClientWithResources(resources),
getApmEventClient(resources),
]);
const configurations = await listConfigurations({
internalESClient,
apmIndices
);
apmIndices,
apmEventClient,
});

return { configurations };
},
Expand All @@ -75,14 +77,14 @@ const getSingleAgentConfigurationRoute = createApmServerRoute({
const { params, logger } = resources;
const { name, environment } = params.query;
const service = { name, environment };
const apmIndices = await resources.getApmIndices();
const internalESClient = await createInternalESClientWithResources(
resources
);
const [internalESClient, apmEventClient] = await Promise.all([
createInternalESClientWithResources(resources),
getApmEventClient(resources),
]);
const exactConfig = await findExactConfiguration({
service,
internalESClient,
apmIndices,
apmEventClient,
});

if (!exactConfig) {
Expand Down Expand Up @@ -114,13 +116,14 @@ const deleteAgentConfigurationRoute = createApmServerRoute({
const { params, logger, core, telemetryUsageCounter } = resources;
const { service } = params.body;
const apmIndices = await resources.getApmIndices();
const internalESClient = await createInternalESClientWithResources(
resources
);
const [internalESClient, apmEventClient] = await Promise.all([
createInternalESClientWithResources(resources),
getApmEventClient(resources),
]);
const exactConfig = await findExactConfiguration({
service,
internalESClient,
apmIndices,
apmEventClient,
});
if (!exactConfig) {
logger.info(
Expand Down Expand Up @@ -171,16 +174,17 @@ const createOrUpdateAgentConfigurationRoute = createApmServerRoute({
const { params, logger, core, telemetryUsageCounter } = resources;
const { body, query } = params;
const apmIndices = await resources.getApmIndices();
const internalESClient = await createInternalESClientWithResources(
resources
);
const [internalESClient, apmEventClient] = await Promise.all([
createInternalESClientWithResources(resources),
getApmEventClient(resources),
]);

// if the config already exists, it is fetched and updated
// this is to avoid creating two configs with identical service params
const exactConfig = await findExactConfiguration({
service: body.service,
internalESClient,
apmIndices,
apmEventClient,
});

// if the config exists ?overwrite=true is required
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,8 @@ export default function ApiTest({ getService }: FtrProviderContext) {

after(() => synthtraceEsClient.clean());

describe('compare latency value between service inventory and service maps', () => {
// FLAKY: https://github.com/elastic/kibana/issues/172772
describe.skip('compare latency value between service inventory and service maps', () => {
before(async () => {
[errorTransactionValues, errorRateMetricValues] = await Promise.all([
getErrorRateValues('transaction'),
Expand Down
8 changes: 5 additions & 3 deletions x-pack/test/apm_api_integration/tests/inspect/inspect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { FtrProviderContext } from '../../common/ftr_provider_context';

import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata';

export default function customLinksTests({ getService }: FtrProviderContext) {
export default function inspectFlagTests({ getService }: FtrProviderContext) {
const registry = getService('registry');
const apmApiClient = getService('apmApiClient');

Expand Down Expand Up @@ -64,7 +64,7 @@ export default function customLinksTests({ getService }: FtrProviderContext) {
});
});

describe('elasticsearch calls made with internal user are not return', () => {
describe('elasticsearch calls made with internal user should not leak internal queries', () => {
it('for custom links', async () => {
const { status, body } = await apmApiClient.readUser({
endpoint: 'GET /internal/apm/settings/custom_links',
Expand Down Expand Up @@ -92,7 +92,9 @@ export default function customLinksTests({ getService }: FtrProviderContext) {
});

expect(status).to.be(200);
expect(body._inspect).to.eql([]);
expect(body._inspect?.map((res) => res.stats?.indexPattern.value)).to.eql([
['metrics-apm*', 'apm-*'],
]);
});
});
});
Expand Down
Loading

0 comments on commit 837ecf8

Please sign in to comment.