From 064829675aeffddb5bcd4b66a3af7d6f3617a30d Mon Sep 17 00:00:00 2001 From: Achyut Jhunjhunwala Date: Wed, 8 Feb 2023 09:33:22 +0100 Subject: [PATCH] [APM] switch get environment function to use terms_enum api (#150175) Fixes https://github.com/elastic/kibana/issues/144544 ## Summary To get the list of Environments, aggregation was used on search api. With this change, list of environments will now be retrieved using the [Terms Enum API](https://www.elastic.co/guide/en/elasticsearch/reference/current/search-terms-enum.html#search-terms-enum) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com> --- .../public/hooks/use_environments_fetcher.tsx | 29 +++++- .../plugins/apm/public/hooks/use_fetcher.tsx | 31 +++--- .../routes/environments/get_environments.ts | 2 +- .../apm/server/routes/environments/route.ts | 7 +- .../tests/environment/generate_data.ts | 67 +++++++++++++ .../tests/environment/get_environment.spec.ts | 94 +++++++++++++++++++ 6 files changed, 210 insertions(+), 20 deletions(-) create mode 100644 x-pack/test/apm_api_integration/tests/environment/generate_data.ts create mode 100644 x-pack/test/apm_api_integration/tests/environment/get_environment.spec.ts diff --git a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx index 50ad012e1aa02..4dc32754cbfe4 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -5,10 +5,14 @@ * 2.0. */ +import { SERVICE_ENVIRONMENT } from '../../common/es_fields/apm'; import { useFetcher } from './use_fetcher'; +import { Environment } from '../../common/environment_rt'; +import { APIReturnType } from '../services/rest/create_call_apm_api'; -const INITIAL_DATA = { environments: [] }; +type EnvironmentsAPIResponse = APIReturnType<'GET /internal/apm/environments'>; +const INITIAL_DATA: EnvironmentsAPIResponse = { environments: [] }; export function useEnvironmentsFetcher({ serviceName, start, @@ -20,7 +24,11 @@ export function useEnvironmentsFetcher({ }) { const { data = INITIAL_DATA, status } = useFetcher( (callApmApi) => { - if (start && end) { + if (!start || !end) { + return; + } + + if (serviceName) { return callApmApi('GET /internal/apm/environments', { params: { query: { @@ -31,9 +39,24 @@ export function useEnvironmentsFetcher({ }, }); } + return callApmApi('GET /internal/apm/suggestions', { + params: { + query: { + start, + end, + fieldName: SERVICE_ENVIRONMENT, + fieldValue: '', + }, + }, + }).then((response) => { + return { environments: response.terms }; + }); }, [start, end, serviceName] ); - return { environments: data.environments, status }; + return { + environments: data.environments as Environment[], + status, + }; } diff --git a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx index 80f2451863b42..28e06341965f6 100644 --- a/x-pack/plugins/apm/public/hooks/use_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_fetcher.tsx @@ -54,13 +54,28 @@ function getDetailsFromErrorResponse( } const createAutoAbortedAPMClient = ( - signal: AbortSignal + signal: AbortSignal, + addInspectorRequest: (result: FetcherResult) => void ): AutoAbortedAPMClient => { return ((endpoint, options) => { return callApmApi(endpoint, { ...options, signal, - } as any); + } as any) + .catch((err) => { + addInspectorRequest({ + status: FETCH_STATUS.FAILURE, + data: err.body?.attributes, + }); + throw err; + }) + .then((response) => { + addInspectorRequest({ + data: response, + status: FETCH_STATUS.SUCCESS, + }); + return response; + }); }) as AutoAbortedAPMClient; }; @@ -102,7 +117,9 @@ export function useFetcher( const signal = controller.signal; - const promise = fn(createAutoAbortedAPMClient(signal)); + const promise = fn( + createAutoAbortedAPMClient(signal, addInspectorRequest) + ); // if `fn` doesn't return a promise it is a signal that data fetching was not initiated. // This can happen if the data fetching is conditional (based on certain inputs). // In these cases it is not desirable to invoke the global loading spinner, or change the status to success @@ -179,14 +196,6 @@ export function useFetcher( /* eslint-enable react-hooks/exhaustive-deps */ ]); - useEffect(() => { - if (result.error) { - addInspectorRequest({ ...result, data: result.error.body?.attributes }); - } else { - addInspectorRequest(result); - } - }, [addInspectorRequest, result]); - return useMemo(() => { return { ...result, diff --git a/x-pack/plugins/apm/server/routes/environments/get_environments.ts b/x-pack/plugins/apm/server/routes/environments/get_environments.ts index f04d67fe7d339..056bfa731880b 100644 --- a/x-pack/plugins/apm/server/routes/environments/get_environments.ts +++ b/x-pack/plugins/apm/server/routes/environments/get_environments.ts @@ -17,7 +17,7 @@ import { Environment } from '../../../common/environment_rt'; import { APMEventClient } from '../../lib/helpers/create_es_client/create_apm_event_client'; /** - * This is used for getting the list of environments for the environments selector, + * This is used for getting the list of environments for the environment selector, * filtered by range. */ export async function getEnvironments({ diff --git a/x-pack/plugins/apm/server/routes/environments/route.ts b/x-pack/plugins/apm/server/routes/environments/route.ts index 32b7d2a5117d5..8757d7f99f544 100644 --- a/x-pack/plugins/apm/server/routes/environments/route.ts +++ b/x-pack/plugins/apm/server/routes/environments/route.ts @@ -7,6 +7,7 @@ import * as t from 'io-ts'; import { maxSuggestions } from '@kbn/observability-plugin/common'; +import { Environment } from '../../../common/environment_rt'; import { getSearchTransactionsEvents } from '../../lib/helpers/transactions'; import { getEnvironments } from './get_environments'; import { rangeRt } from '../default_api_types'; @@ -27,11 +28,7 @@ const environmentsRoute = createApmServerRoute({ handler: async ( resources ): Promise<{ - environments: Array< - | 'ENVIRONMENT_NOT_DEFINED' - | 'ENVIRONMENT_ALL' - | t.Branded - >; + environments: Environment[]; }> => { const apmEventClient = await getApmEventClient(resources); const { context, params, config } = resources; diff --git a/x-pack/test/apm_api_integration/tests/environment/generate_data.ts b/x-pack/test/apm_api_integration/tests/environment/generate_data.ts new file mode 100644 index 0000000000000..6e9ae2831e6be --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/environment/generate_data.ts @@ -0,0 +1,67 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { apm, timerange } from '@kbn/apm-synthtrace-client'; +import type { ApmSynthtraceEsClient } from '@kbn/apm-synthtrace'; + +// Generate synthetic data for the environment test suite +export async function generateData({ + synthtraceEsClient, + start, + end, +}: { + synthtraceEsClient: ApmSynthtraceEsClient; + start: number; + end: number; +}) { + const environmentNames = ['production', 'development', 'staging']; + const serviceNames = ['go', 'java', 'node']; + + const services = environmentNames.flatMap((environment) => { + return serviceNames.flatMap((serviceName) => { + return apm + .service({ + name: serviceName, + environment, + agentName: serviceName, + }) + .instance('instance-a'); + }); + }); + + const goServiceWithAdditionalEnvironment = apm + .service({ + name: 'go', + environment: 'custom-go-environment', + agentName: 'go', + }) + .instance('instance-a'); + + // Generate a transaction for each service + const docs = timerange(start, end) + .ratePerMinute(1) + .generator((timestamp) => { + const loopGeneratedDocs = services.flatMap((service) => { + return service + .transaction({ transactionName: 'GET /api/product/:id' }) + .timestamp(timestamp) + .duration(1000); + }); + + const customDoc = goServiceWithAdditionalEnvironment + .transaction({ + transactionName: 'GET /api/go/memory', + transactionType: 'custom-go-type', + }) + .timestamp(timestamp) + .duration(1000); + + return [...loopGeneratedDocs, customDoc]; + }); + + await synthtraceEsClient.index(docs); +} diff --git a/x-pack/test/apm_api_integration/tests/environment/get_environment.spec.ts b/x-pack/test/apm_api_integration/tests/environment/get_environment.spec.ts new file mode 100644 index 0000000000000..5a3e850c88d0a --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/environment/get_environment.spec.ts @@ -0,0 +1,94 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ +import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../common/ftr_provider_context'; +import { generateData } from './generate_data'; + +const startNumber = new Date('2021-01-01T00:00:00.000Z').getTime(); +const endNumber = new Date('2021-01-01T00:05:00.000Z').getTime() - 1; + +const start = new Date(startNumber).toISOString(); +const end = new Date(endNumber).toISOString(); + +export default function environmentsAPITests({ getService }: FtrProviderContext) { + const registry = getService('registry'); + const apmApiClient = getService('apmApiClient'); + const synthtraceEsClient = getService('synthtraceEsClient'); + + registry.when('environments when data is loaded', { config: 'basic', archives: [] }, async () => { + before(async () => { + await generateData({ + synthtraceEsClient, + start: startNumber, + end: endNumber, + }); + }); + + after(() => synthtraceEsClient.clean()); + + describe('get environments', () => { + describe('when service name is not specified', () => { + it('returns all environments', async () => { + const { body } = await apmApiClient.readUser({ + endpoint: 'GET /internal/apm/environments', + params: { + query: { start, end }, + }, + }); + expect(body.environments.length).to.be.equal(4); + expectSnapshot(body.environments).toMatchInline(` + Array [ + "development", + "production", + "staging", + "custom-go-environment", + ] + `); + }); + }); + + describe('when service name is specified', () => { + it('returns service specific environments for go', async () => { + const { body } = await apmApiClient.readUser({ + endpoint: 'GET /internal/apm/environments', + params: { + query: { start, end, serviceName: 'go' }, + }, + }); + + expect(body.environments.length).to.be.equal(4); + expectSnapshot(body.environments).toMatchInline(` + Array [ + "custom-go-environment", + "development", + "production", + "staging", + ] + `); + }); + + it('returns service specific environments for java', async () => { + const { body } = await apmApiClient.readUser({ + endpoint: 'GET /internal/apm/environments', + params: { + query: { start, end, serviceName: 'java' }, + }, + }); + + expect(body.environments.length).to.be.equal(3); + expectSnapshot(body.environments).toMatchInline(` + Array [ + "development", + "production", + "staging", + ] + `); + }); + }); + }); + }); +}