From c1bbce2521ef079523845e160c6b540d9b837e87 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Thu, 2 Feb 2023 14:57:14 +0100 Subject: [PATCH 01/12] switch get environment function to use terms_enum api instead of aggregation --- .../get_environments.test.ts.snap | 85 +------------------ .../routes/environments/get_environments.ts | 53 +++++------- 2 files changed, 21 insertions(+), 117 deletions(-) diff --git a/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap b/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap index cd874cdc5550c..2f5bf4530bb89 100644 --- a/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap +++ b/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap @@ -1,86 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`getEnvironments fetches environments 1`] = ` -Object { - "apm": Object { - "events": Array [ - "transaction", - "metric", - "error", - ], - }, - "body": Object { - "aggs": Object { - "environments": Object { - "terms": Object { - "field": "service.environment", - "missing": "ENVIRONMENT_NOT_DEFINED", - "size": 50, - }, - }, - }, - "query": Object { - "bool": Object { - "filter": Array [ - Object { - "range": Object { - "@timestamp": Object { - "format": "epoch_millis", - "gte": 0, - "lte": 50000, - }, - }, - }, - Object { - "term": Object { - "service.name": "foo", - }, - }, - ], - }, - }, - "size": 0, - "track_total_hits": false, - }, -} -`; +exports[`getEnvironments fetches environments 1`] = `undefined`; -exports[`getEnvironments fetches environments without a service name 1`] = ` -Object { - "apm": Object { - "events": Array [ - "transaction", - "metric", - "error", - ], - }, - "body": Object { - "aggs": Object { - "environments": Object { - "terms": Object { - "field": "service.environment", - "missing": "ENVIRONMENT_NOT_DEFINED", - "size": 50, - }, - }, - }, - "query": Object { - "bool": Object { - "filter": Array [ - Object { - "range": Object { - "@timestamp": Object { - "format": "epoch_millis", - "gte": 0, - "lte": 50000, - }, - }, - }, - ], - }, - }, - "size": 0, - "track_total_hits": false, - }, -} -`; +exports[`getEnvironments fetches environments without a service name 1`] = `undefined`; 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..ef0c806725117 100644 --- a/x-pack/plugins/apm/server/routes/environments/get_environments.ts +++ b/x-pack/plugins/apm/server/routes/environments/get_environments.ts @@ -5,21 +5,21 @@ * 2.0. */ -import { rangeQuery, termQuery } from '@kbn/observability-plugin/server'; +import { rangeQuery } from '@kbn/observability-plugin/server'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; -import { - SERVICE_ENVIRONMENT, - SERVICE_NAME, -} from '../../../common/es_fields/apm'; -import { ENVIRONMENT_NOT_DEFINED } from '../../../common/environment_filter_values'; +import { SERVICE_ENVIRONMENT } from '../../../common/es_fields/apm'; import { getProcessorEventForTransactions } from '../../lib/helpers/transactions'; import { Environment } from '../../../common/environment_rt'; -import { APMEventClient } from '../../lib/helpers/create_es_client/create_apm_event_client'; +import { + APMEventClient, + APMEventESTermsEnumRequest, +} 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's selector, * filtered by range. */ + export async function getEnvironments({ searchAggregatedTransactions, serviceName, @@ -39,7 +39,7 @@ export async function getEnvironments({ ? 'get_environments_for_service' : 'get_environments'; - const params = { + const params: APMEventESTermsEnumRequest = { apm: { events: [ getProcessorEventForTransactions(searchAggregatedTransactions), @@ -47,36 +47,21 @@ export async function getEnvironments({ ProcessorEvent.error, ], }, - body: { - track_total_hits: false, - size: 0, - query: { - bool: { - filter: [ - ...rangeQuery(start, end), - ...termQuery(SERVICE_NAME, serviceName), - ], - }, - }, - aggs: { - environments: { - terms: { - field: SERVICE_ENVIRONMENT, - missing: ENVIRONMENT_NOT_DEFINED.value, - size, - }, - }, + size, + field: SERVICE_ENVIRONMENT, + index_filter: { + bool: { + filter: [...rangeQuery(start, end)], }, }, }; - const resp = await apmEventClient.search(operationName, params); - const aggs = resp.aggregations; - const environmentsBuckets = aggs?.environments.buckets || []; + if (serviceName) { + params.string = serviceName; + } - const environments = environmentsBuckets.map( - (environmentBucket) => environmentBucket.key as string - ); + const resp = await apmEventClient.termsEnum(operationName, params); + const environments = resp.terms || []; return environments as Environment[]; } From ad1cd3e4e325bebd2b878845729df7aba40f4766 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Thu, 2 Feb 2023 16:07:34 +0100 Subject: [PATCH 02/12] pass service as empty string if unavailable --- .../server/routes/environments/get_environments.ts | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) 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 ef0c806725117..57e7ca546ea42 100644 --- a/x-pack/plugins/apm/server/routes/environments/get_environments.ts +++ b/x-pack/plugins/apm/server/routes/environments/get_environments.ts @@ -10,10 +10,7 @@ import { ProcessorEvent } from '@kbn/observability-plugin/common'; import { SERVICE_ENVIRONMENT } from '../../../common/es_fields/apm'; import { getProcessorEventForTransactions } from '../../lib/helpers/transactions'; import { Environment } from '../../../common/environment_rt'; -import { - APMEventClient, - APMEventESTermsEnumRequest, -} from '../../lib/helpers/create_es_client/create_apm_event_client'; +import { APMEventClient } from '../../lib/helpers/create_es_client/create_apm_event_client'; /** * This is used for getting the list of environments for the environment's selector, @@ -22,7 +19,7 @@ import { export async function getEnvironments({ searchAggregatedTransactions, - serviceName, + serviceName = '', apmEventClient, size, start, @@ -39,7 +36,7 @@ export async function getEnvironments({ ? 'get_environments_for_service' : 'get_environments'; - const params: APMEventESTermsEnumRequest = { + const params = { apm: { events: [ getProcessorEventForTransactions(searchAggregatedTransactions), @@ -49,6 +46,7 @@ export async function getEnvironments({ }, size, field: SERVICE_ENVIRONMENT, + string: serviceName, index_filter: { bool: { filter: [...rangeQuery(start, end)], @@ -56,10 +54,6 @@ export async function getEnvironments({ }, }; - if (serviceName) { - params.string = serviceName; - } - const resp = await apmEventClient.termsEnum(operationName, params); const environments = resp.terms || []; From 9a66237e07268e23f4792d549877e75553b7e676 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Mon, 6 Feb 2023 13:13:55 +0100 Subject: [PATCH 03/12] change environment fetcher logic to use suggestion api when no service name is passed and getEnvironment aggregation api when service name is passed --- .../public/hooks/use_environments_fetcher.tsx | 38 ++++++++- .../get_environments.test.ts.snap | 85 ++++++++++++++++++- .../routes/environments/get_environments.ts | 47 +++++++--- 3 files changed, 154 insertions(+), 16 deletions(-) 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..7ea00277967a6 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -6,6 +6,8 @@ */ import { useFetcher } from './use_fetcher'; +import { SERVICE_ENVIRONMENT } from '@kbn/apm-plugin/common/es_fields/apm'; +import { Environment } from '../../common/environment_rt'; const INITIAL_DATA = { environments: [] }; @@ -18,6 +20,18 @@ export function useEnvironmentsFetcher({ start?: string; end?: string; }) { + if (serviceName) { + return getEnvironmentsForGivenService(start, end, serviceName); + } + + return getEnvironmentsUsingSuggestionsApi(start, end); +} + +const getEnvironmentsForGivenService = ( + start?: string, + end?: string, + serviceName?: string +) => { const { data = INITIAL_DATA, status } = useFetcher( (callApmApi) => { if (start && end) { @@ -36,4 +50,26 @@ export function useEnvironmentsFetcher({ ); return { environments: data.environments, status }; -} +}; + +const getEnvironmentsUsingSuggestionsApi = (start?: string, end?: string) => { + const { data = { terms: [] }, status } = useFetcher( + (callApmApi) => { + if (start && end) { + return callApmApi('GET /internal/apm/suggestions', { + params: { + query: { + start, + end, + fieldName: SERVICE_ENVIRONMENT, + fieldValue: '', + }, + }, + }); + } + }, + [start, end] + ); + + return { environments: data.terms as Environment[], status }; +}; diff --git a/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap b/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap index 2f5bf4530bb89..cd874cdc5550c 100644 --- a/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap +++ b/x-pack/plugins/apm/server/routes/environments/__snapshots__/get_environments.test.ts.snap @@ -1,5 +1,86 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`getEnvironments fetches environments 1`] = `undefined`; +exports[`getEnvironments fetches environments 1`] = ` +Object { + "apm": Object { + "events": Array [ + "transaction", + "metric", + "error", + ], + }, + "body": Object { + "aggs": Object { + "environments": Object { + "terms": Object { + "field": "service.environment", + "missing": "ENVIRONMENT_NOT_DEFINED", + "size": 50, + }, + }, + }, + "query": Object { + "bool": Object { + "filter": Array [ + Object { + "range": Object { + "@timestamp": Object { + "format": "epoch_millis", + "gte": 0, + "lte": 50000, + }, + }, + }, + Object { + "term": Object { + "service.name": "foo", + }, + }, + ], + }, + }, + "size": 0, + "track_total_hits": false, + }, +} +`; -exports[`getEnvironments fetches environments without a service name 1`] = `undefined`; +exports[`getEnvironments fetches environments without a service name 1`] = ` +Object { + "apm": Object { + "events": Array [ + "transaction", + "metric", + "error", + ], + }, + "body": Object { + "aggs": Object { + "environments": Object { + "terms": Object { + "field": "service.environment", + "missing": "ENVIRONMENT_NOT_DEFINED", + "size": 50, + }, + }, + }, + "query": Object { + "bool": Object { + "filter": Array [ + Object { + "range": Object { + "@timestamp": Object { + "format": "epoch_millis", + "gte": 0, + "lte": 50000, + }, + }, + }, + ], + }, + }, + "size": 0, + "track_total_hits": false, + }, +} +`; 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 57e7ca546ea42..056bfa731880b 100644 --- a/x-pack/plugins/apm/server/routes/environments/get_environments.ts +++ b/x-pack/plugins/apm/server/routes/environments/get_environments.ts @@ -5,21 +5,24 @@ * 2.0. */ -import { rangeQuery } from '@kbn/observability-plugin/server'; +import { rangeQuery, termQuery } from '@kbn/observability-plugin/server'; import { ProcessorEvent } from '@kbn/observability-plugin/common'; -import { SERVICE_ENVIRONMENT } from '../../../common/es_fields/apm'; +import { + SERVICE_ENVIRONMENT, + SERVICE_NAME, +} from '../../../common/es_fields/apm'; +import { ENVIRONMENT_NOT_DEFINED } from '../../../common/environment_filter_values'; import { getProcessorEventForTransactions } from '../../lib/helpers/transactions'; 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 environment's selector, + * This is used for getting the list of environments for the environment selector, * filtered by range. */ - export async function getEnvironments({ searchAggregatedTransactions, - serviceName = '', + serviceName, apmEventClient, size, start, @@ -44,18 +47,36 @@ export async function getEnvironments({ ProcessorEvent.error, ], }, - size, - field: SERVICE_ENVIRONMENT, - string: serviceName, - index_filter: { - bool: { - filter: [...rangeQuery(start, end)], + body: { + track_total_hits: false, + size: 0, + query: { + bool: { + filter: [ + ...rangeQuery(start, end), + ...termQuery(SERVICE_NAME, serviceName), + ], + }, + }, + aggs: { + environments: { + terms: { + field: SERVICE_ENVIRONMENT, + missing: ENVIRONMENT_NOT_DEFINED.value, + size, + }, + }, }, }, }; - const resp = await apmEventClient.termsEnum(operationName, params); - const environments = resp.terms || []; + const resp = await apmEventClient.search(operationName, params); + const aggs = resp.aggregations; + const environmentsBuckets = aggs?.environments.buckets || []; + + const environments = environmentsBuckets.map( + (environmentBucket) => environmentBucket.key as string + ); return environments as Environment[]; } From 9627da039d9ac7b3378b7778a0e2a99c4b07db02 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Mon, 6 Feb 2023 12:43:47 +0000 Subject: [PATCH 04/12] [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' --- x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 7ea00277967a6..44277dd1014f8 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -5,8 +5,8 @@ * 2.0. */ +import { SERVICE_ENVIRONMENT } from '../../common/es_fields/apm'; import { useFetcher } from './use_fetcher'; -import { SERVICE_ENVIRONMENT } from '@kbn/apm-plugin/common/es_fields/apm'; import { Environment } from '../../common/environment_rt'; const INITIAL_DATA = { environments: [] }; From 9ac0a17ce03bd9a699e28e3fb8a5920de79ab550 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Mon, 6 Feb 2023 15:20:18 +0100 Subject: [PATCH 05/12] adjusting environment fetching logic based on useFetcher --- .../public/hooks/use_environments_fetcher.tsx | 63 +++++++------------ 1 file changed, 23 insertions(+), 40 deletions(-) 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 7ea00277967a6..f8b3a75f0df07 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -9,8 +9,6 @@ import { useFetcher } from './use_fetcher'; import { SERVICE_ENVIRONMENT } from '@kbn/apm-plugin/common/es_fields/apm'; import { Environment } from '../../common/environment_rt'; -const INITIAL_DATA = { environments: [] }; - export function useEnvironmentsFetcher({ serviceName, start, @@ -20,21 +18,13 @@ export function useEnvironmentsFetcher({ start?: string; end?: string; }) { - if (serviceName) { - return getEnvironmentsForGivenService(start, end, serviceName); - } - - return getEnvironmentsUsingSuggestionsApi(start, end); -} - -const getEnvironmentsForGivenService = ( - start?: string, - end?: string, - serviceName?: string -) => { - const { data = INITIAL_DATA, status } = useFetcher( + const { data, status } = useFetcher( (callApmApi) => { - if (start && end) { + if (!start || !end) { + return; + } + + if (serviceName) { return callApmApi('GET /internal/apm/environments', { params: { query: { @@ -45,31 +35,24 @@ const getEnvironmentsForGivenService = ( }, }); } - }, - [start, end, serviceName] - ); - - return { environments: data.environments, status }; -}; - -const getEnvironmentsUsingSuggestionsApi = (start?: string, end?: string) => { - const { data = { terms: [] }, status } = useFetcher( - (callApmApi) => { - if (start && end) { - return callApmApi('GET /internal/apm/suggestions', { - params: { - query: { - start, - end, - fieldName: SERVICE_ENVIRONMENT, - fieldValue: '', - }, + return callApmApi('GET /internal/apm/suggestions', { + params: { + query: { + start, + end, + fieldName: SERVICE_ENVIRONMENT, + fieldValue: '', }, - }); - } + }, + }); }, - [start, end] + [start, end, serviceName] ); - return { environments: data.terms as Environment[], status }; -}; + return { + environments: ((data as any)?.environments || + (data as any)?.terms || + []) as Environment[], + status, + }; +} From f00787eb03fb45a83babda0a17e39991e92f3b74 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 7 Feb 2023 10:58:56 +0100 Subject: [PATCH 06/12] remove any type to more specific type --- .../apm/public/hooks/use_environments_fetcher.tsx | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) 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 c58ce0c4999e0..2d65d80d9512a 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -8,7 +8,6 @@ import { SERVICE_ENVIRONMENT } from '../../common/es_fields/apm'; import { useFetcher } from './use_fetcher'; import { Environment } from '../../common/environment_rt'; - export function useEnvironmentsFetcher({ serviceName, start, @@ -33,6 +32,8 @@ export function useEnvironmentsFetcher({ serviceName, }, }, + }).then((response) => { + return { environments: response.environments }; }); } return callApmApi('GET /internal/apm/suggestions', { @@ -44,15 +45,15 @@ export function useEnvironmentsFetcher({ fieldValue: '', }, }, + }).then((response) => { + return { environments: response.terms }; }); }, [start, end, serviceName] ); return { - environments: ((data as any)?.environments || - (data as any)?.terms || - []) as Environment[], + environments: (data?.environments ?? []) as Environment[], status, }; } From 8758f45f888f497d51c96b08f4d5f1873ef1488d Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 7 Feb 2023 14:24:19 +0100 Subject: [PATCH 07/12] fix logic which broke the inspector component --- x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 2d65d80d9512a..8f28d04a81657 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -33,7 +33,8 @@ export function useEnvironmentsFetcher({ }, }, }).then((response) => { - return { environments: response.environments }; + // Spreading of original response is needed in order to display the api call in the inspector + return { ...response, environments: response.environments }; }); } return callApmApi('GET /internal/apm/suggestions', { @@ -46,7 +47,7 @@ export function useEnvironmentsFetcher({ }, }, }).then((response) => { - return { environments: response.terms }; + return { ...response, environments: response.terms }; }); }, [start, end, serviceName] From 0a1e90764c2f4f42a9a612e7e5545bac23a9feb7 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 7 Feb 2023 14:34:47 +0100 Subject: [PATCH 08/12] simplified type for getEnvironments on the server side --- x-pack/plugins/apm/server/routes/environments/route.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/apm/server/routes/environments/route.ts b/x-pack/plugins/apm/server/routes/environments/route.ts index 32b7d2a5117d5..6dd67fadc0ea9 100644 --- a/x-pack/plugins/apm/server/routes/environments/route.ts +++ b/x-pack/plugins/apm/server/routes/environments/route.ts @@ -12,6 +12,7 @@ import { getEnvironments } from './get_environments'; import { rangeRt } from '../default_api_types'; import { createApmServerRoute } from '../apm_routes/create_apm_server_route'; import { getApmEventClient } from '../../lib/helpers/get_apm_event_client'; +import { Environment } from '@kbn/apm-plugin/common/environment_rt'; const environmentsRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/environments', @@ -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; From 122dcaf10bff9b9a416817f95d9fd7139f2dca99 Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 7 Feb 2023 13:55:21 +0000 Subject: [PATCH 09/12] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- x-pack/plugins/apm/server/routes/environments/route.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/apm/server/routes/environments/route.ts b/x-pack/plugins/apm/server/routes/environments/route.ts index 6dd67fadc0ea9..8757d7f99f544 100644 --- a/x-pack/plugins/apm/server/routes/environments/route.ts +++ b/x-pack/plugins/apm/server/routes/environments/route.ts @@ -7,12 +7,12 @@ 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'; import { createApmServerRoute } from '../apm_routes/create_apm_server_route'; import { getApmEventClient } from '../../lib/helpers/get_apm_event_client'; -import { Environment } from '@kbn/apm-plugin/common/environment_rt'; const environmentsRoute = createApmServerRoute({ endpoint: 'GET /internal/apm/environments', From 315ec23d011ba9deddc2bac9a8724f41267eb656 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 7 Feb 2023 16:28:03 +0100 Subject: [PATCH 10/12] fix final code reviews --- .../public/hooks/use_environments_fetcher.tsx | 14 +++++---- .../plugins/apm/public/hooks/use_fetcher.tsx | 31 ++++++++++++------- 2 files changed, 28 insertions(+), 17 deletions(-) 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 8f28d04a81657..4dc32754cbfe4 100644 --- a/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/use_environments_fetcher.tsx @@ -8,6 +8,11 @@ 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'; + +type EnvironmentsAPIResponse = APIReturnType<'GET /internal/apm/environments'>; + +const INITIAL_DATA: EnvironmentsAPIResponse = { environments: [] }; export function useEnvironmentsFetcher({ serviceName, start, @@ -17,7 +22,7 @@ export function useEnvironmentsFetcher({ start?: string; end?: string; }) { - const { data, status } = useFetcher( + const { data = INITIAL_DATA, status } = useFetcher( (callApmApi) => { if (!start || !end) { return; @@ -32,9 +37,6 @@ export function useEnvironmentsFetcher({ serviceName, }, }, - }).then((response) => { - // Spreading of original response is needed in order to display the api call in the inspector - return { ...response, environments: response.environments }; }); } return callApmApi('GET /internal/apm/suggestions', { @@ -47,14 +49,14 @@ export function useEnvironmentsFetcher({ }, }, }).then((response) => { - return { ...response, environments: response.terms }; + return { environments: response.terms }; }); }, [start, end, serviceName] ); return { - environments: (data?.environments ?? []) as Environment[], + 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, From 4e723d13c080ff697c95b334e3f3a7d466494403 Mon Sep 17 00:00:00 2001 From: achyutjhunjhunwala Date: Tue, 7 Feb 2023 18:29:11 +0100 Subject: [PATCH 11/12] add api tests for getEnvironments --- .../tests/environment/generate_data.ts | 60 ++++++++++++ .../tests/environment/get_environment.spec.ts | 94 +++++++++++++++++++ 2 files changed, 154 insertions(+) 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/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..09e03452bbf3e --- /dev/null +++ b/x-pack/test/apm_api_integration/tests/environment/generate_data.ts @@ -0,0 +1,60 @@ +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", + ] + `); + }); + }); + }); + }); +} From f413e8d3f227084c6b929e888382fa341eca497f Mon Sep 17 00:00:00 2001 From: kibanamachine <42973632+kibanamachine@users.noreply.github.com> Date: Tue, 7 Feb 2023 17:35:27 +0000 Subject: [PATCH 12/12] [CI] Auto-commit changed files from 'node scripts/precommit_hook.js --ref HEAD~1..HEAD --fix' --- .../apm_api_integration/tests/environment/generate_data.ts | 7 +++++++ 1 file changed, 7 insertions(+) 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 index 09e03452bbf3e..6e9ae2831e6be 100644 --- a/x-pack/test/apm_api_integration/tests/environment/generate_data.ts +++ b/x-pack/test/apm_api_integration/tests/environment/generate_data.ts @@ -1,3 +1,10 @@ +/* + * 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';