[APM] Add useMemo and minor feedback for environment filters (#36970) (#37153)

* [APM] Add useMemo and minor feedback for environment filters

* Fix types

* Rename to `loadEnvironmentsFilter`

# Conflicts:
#	x-pack/plugins/apm/server/new-platform/plugin.ts
#	x-pack/plugins/apm/server/routes/services.ts

x-pack/plugins/apm/public/components/shared/EnvironmentFilter/index.tsx

@@ -8,7 +8,7 @@ import { EuiSelect, EuiFormLabel } from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import React from 'react';
 import { useFetcher } from '../../../hooks/useFetcher';
-import { loadServiceEnvironments } from '../../../services/rest/apm/services';
+import { loadEnvironmentsFilter } from '../../../services/rest/apm/ui_filters';
 import { useLocation } from '../../../hooks/useLocation';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { history } from '../../../utils/history';
@@ -33,48 +33,42 @@ function updateEnvironmentUrl(
   });
 }
 
+const ALL_OPTION = {
+  value: ENVIRONMENT_ALL,
+  text: i18n.translate('xpack.apm.filter.environment.allLabel', {
+    defaultMessage: 'All'
+  })
+};
+
+const NOT_DEFINED_OPTION = {
+  value: ENVIRONMENT_NOT_DEFINED,
+  text: i18n.translate('xpack.apm.filter.environment.notDefinedLabel', {
+    defaultMessage: 'Not defined'
+  })
+};
+
+const SEPARATOR_OPTION = {
+  text: `- ${i18n.translate(
+    'xpack.apm.filter.environment.selectEnvironmentLabel',
+    { defaultMessage: 'Select environment' }
+  )} -`,
+  disabled: true
+};
+
 function getOptions(environments: string[]) {
-  const ALL_OPTION = {
-    value: ENVIRONMENT_ALL,
-    text: i18n.translate('xpack.apm.filter.environment.allLabel', {
-      defaultMessage: 'All'
-    })
-  };
-
-  const NOT_DEFINED_OPTION = {
-    value: ENVIRONMENT_NOT_DEFINED,
-    text: i18n.translate('xpack.apm.filter.environment.notDefinedLabel', {
-      defaultMessage: 'Not defined'
-    })
-  };
-
-  const hasUndefinedEnv = environments.includes(ENVIRONMENT_NOT_DEFINED);
-  const commonOptions = hasUndefinedEnv
-    ? [ALL_OPTION, NOT_DEFINED_OPTION]
-    : [ALL_OPTION];
-  const definedEnvs = environments.filter(
-    env => env !== ENVIRONMENT_NOT_DEFINED
-  );
-  const environmentOptions = definedEnvs.map(environment => ({
-    value: environment,
-    text: environment
-  }));
+  const environmentOptions = environments
+    .filter(env => env !== ENVIRONMENT_NOT_DEFINED)
+    .map(environment => ({
+      value: environment,
+      text: environment
+    }));
 
   return [
-    ...commonOptions, // all, not defined
-    ...(environmentOptions.length // separate common and environment options
-      ? [
-          {
-            text: `- ${i18n.translate(
-              'xpack.apm.filter.environment.selectEnvironmentLabel',
-              {
-                defaultMessage: 'Select environment'
-              }
-            )} -`,
-            disabled: true
-          }
-        ]
+    ALL_OPTION,
+    ...(environments.includes(ENVIRONMENT_NOT_DEFINED)
+      ? [NOT_DEFINED_OPTION]
       : []),
+    ...(environmentOptions.length > 0 ? [SEPARATOR_OPTION] : []),
     ...environmentOptions
   ];
 }
@@ -94,7 +88,7 @@ export const EnvironmentFilter: React.FC = () => {
   const { data: environments = [], status = 'loading' } = useFetcher(
     () => {
       if (start && end) {
-        return loadServiceEnvironments({
+        return loadEnvironmentsFilter({
           start,
           end,
           serviceName: realServiceName

x-pack/plugins/apm/public/context/LoadingIndicatorContext.tsx

@@ -44,6 +44,9 @@ export function LoadingIndicatorProvider({
   const [statuses, dispatchStatus] = useReducer(reducer, {});
   const isLoading = useMemo(() => getIsAnyLoading(statuses), [statuses]);
   const shouldShowLoadingIndicator = useDelayedVisibility(isLoading);
+  const contextValue = React.useMemo(() => ({ statuses, dispatchStatus }), [
+    statuses
+  ]);
 
   return (
     <Fragment>
@@ -54,7 +57,7 @@ export function LoadingIndicatorProvider({
       )}
 
       <LoadingIndicatorContext.Provider
-        value={{ statuses, dispatchStatus }}
+        value={contextValue}
         children={children}
       />
     </Fragment>

x-pack/plugins/apm/public/context/UrlParamsContext/index.tsx

@@ -28,14 +28,8 @@ interface TimeRangeRefreshAction {
   time: TimeRange;
 }
 
-function useUiFilters(urlParams: IUrlParams): UIFilters {
-  return useMemo(
-    () => ({
-      kuery: urlParams.kuery,
-      environment: urlParams.environment
-    }),
-    [urlParams]
-  );
+function useUiFilters({ kuery, environment }: IUrlParams): UIFilters {
+  return useMemo(() => ({ kuery, environment }), [kuery, environment]);
 }
 
 const defaultRefresh = (time: TimeRange) => {};
@@ -74,6 +68,10 @@ const UrlParamsProvider: React.FC<{}> = ({ children }) => {
     resolveUrlParams(location, {})
   );
   const uiFilters = useUiFilters(urlParams);
+  const contextValue = React.useMemo(
+    () => ({ urlParams, refreshTimeRange, uiFilters }),
+    [urlParams]
+  );
 
   function refreshTimeRange(time: TimeRange) {
     dispatch({ type: TIME_RANGE_REFRESH, time });
@@ -86,12 +84,7 @@ const UrlParamsProvider: React.FC<{}> = ({ children }) => {
     [location]
   );
 
-  return (
-    <UrlParamsContext.Provider
-      children={children}
-      value={{ urlParams, refreshTimeRange, uiFilters }}
-    />
-  );
+  return <UrlParamsContext.Provider children={children} value={contextValue} />;
 };
 
 export { UrlParamsContext, UrlParamsProvider, useUiFilters };

x-pack/plugins/apm/public/services/rest/apm/services.ts

@@ -6,7 +6,6 @@
 
 import { ServiceAPIResponse } from '../../../../server/lib/services/get_service';
 import { ServiceListAPIResponse } from '../../../../server/lib/services/get_services';
-import { ServiceEnvironmentsAPIResponse } from '../../../../server/lib/services/get_service_environments';
 import { callApi } from '../callApi';
 import { getUiFiltersES } from '../../ui_filters/get_ui_filters_es';
 import { UIFilters } from '../../../../typings/ui-filters';
@@ -50,23 +49,3 @@ export async function loadServiceDetails({
     }
   });
 }
-
-export async function loadServiceEnvironments({
-  serviceName,
-  start,
-  end
-}: {
-  serviceName?: string;
-  start: string;
-  end: string;
-}) {
-  const pathname = `/api/apm/services/environments`;
-  return callApi<ServiceEnvironmentsAPIResponse>({
-    pathname,
-    query: {
-      start,
-      end,
-      serviceName
-    }
-  });
-}

x-pack/plugins/apm/public/services/rest/apm/ui_filters.ts

@@ -0,0 +1,27 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import { EnvironmentUIFilterAPIResponse } from '../../../../server/lib/ui_filters/get_environments';
+import { callApi } from '../callApi';
+
+export async function loadEnvironmentsFilter({
+  serviceName,
+  start,
+  end
+}: {
+  serviceName?: string;
+  start: string;
+  end: string;
+}) {
+  return callApi<EnvironmentUIFilterAPIResponse>({
+    pathname: '/api/apm/ui_filters/environments',
+    query: {
+      start,
+      end,
+      serviceName
+    }
+  });
+}

x-pack/plugins/apm/server/lib/traces/get_top_traces.ts

@@ -16,7 +16,7 @@ import { getTransactionGroups } from '../transaction_groups';
 
 export type TraceListAPIResponse = PromiseReturnType<typeof getTopTraces>;
 export async function getTopTraces(setup: Setup) {
-  const { start, end } = setup;
+  const { start, end, uiFiltersES } = setup;
 
   const bodyQuery = {
     bool: {
@@ -24,7 +24,8 @@ export async function getTopTraces(setup: Setup) {
       must_not: { exists: { field: PARENT_ID } },
       filter: [
         { range: rangeFilter(start, end) },
-        { term: { [PROCESSOR_EVENT]: 'transaction' } }
+        { term: { [PROCESSOR_EVENT]: 'transaction' } },
+        ...uiFiltersES
       ],
       should: [{ term: { [TRANSACTION_SAMPLED]: true } }]
     }

x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/fetcher.test.ts.snap

@@ -53,15 +53,6 @@ Array [
           },
         },
         "query": Object {
-          "bool": Object {
-            "filter": Array [
-              Object {
-                "term": Object {
-                  "service.environment": "test",
-                },
-              },
-            ],
-          },
           "my": "bodyQuery",
         },
         "size": 0,

x-pack/plugins/apm/server/lib/transaction_groups/fetcher.test.ts

@@ -27,11 +27,7 @@ describe('transactionGroupsFetcher', () => {
         }),
         has: () => true
       },
-      uiFiltersES: [
-        {
-          term: { 'service.environment': 'test' }
-        }
-      ]
+      uiFiltersES: [{ term: { 'service.environment': 'test' } }]
     };
     const bodyQuery = { my: 'bodyQuery' };
     res = await transactionGroupsFetcher(setup, bodyQuery);

x-pack/plugins/apm/server/lib/transaction_groups/fetcher.ts

@@ -38,7 +38,7 @@ interface Aggs {
 
 export type ESResponse = PromiseReturnType<typeof transactionGroupsFetcher>;
 export function transactionGroupsFetcher(setup: Setup, bodyQuery: StringMap) {
-  const { uiFiltersES, client, config } = setup;
+  const { client, config } = setup;
   const params: SearchParams = {
     index: config.get<string>('apm_oss.transactionIndices'),
     body: {
@@ -72,18 +72,5 @@ export function transactionGroupsFetcher(setup: Setup, bodyQuery: StringMap) {
     }
   };
 
-  params.body.query = {
-    ...params.body.query,
-    bool: {
-      ...params.body.query.bool,
-      filter: [
-        ...(params.body.query.bool && params.body.query.bool.filter
-          ? params.body.query.bool.filter
-          : []),
-        ...uiFiltersES
-      ]
-    }
-  };
-
   return client<void, Aggs>('search', params);
 }

x-pack/plugins/apm/server/lib/transactions/get_top_transactions/index.ts

@@ -4,7 +4,6 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { ESFilter } from 'elasticsearch';
 import {
   PROCESSOR_EVENT,
   SERVICE_NAME,
@@ -29,24 +28,24 @@ export async function getTopTransactions({
   transactionType,
   serviceName
 }: IOptions) {
-  const { start, end } = setup;
-  const filter: ESFilter[] = [
-    { term: { [SERVICE_NAME]: serviceName } },
-    { term: { [PROCESSOR_EVENT]: 'transaction' } },
-    { range: rangeFilter(start, end) }
-  ];
+  const { start, end, uiFiltersES } = setup;
+
+  const bodyQuery = {
+    bool: {
+      filter: [
+        { term: { [SERVICE_NAME]: serviceName } },
+        { term: { [PROCESSOR_EVENT]: 'transaction' } },
+        { range: rangeFilter(start, end) },
+        ...uiFiltersES
+      ]
+    }
+  };
 
   if (transactionType) {
-    filter.push({
+    bodyQuery.bool.filter.push({
       term: { [TRANSACTION_TYPE]: transactionType }
     });
   }
 
-  const bodyQuery = {
-    bool: {
-      filter
-    }
-  };
-
   return getTransactionGroups(setup, bodyQuery);
 }

x-pack/plugins/apm/server/lib/ui_filters/get_environments.ts

@@ -16,13 +16,10 @@ import { rangeFilter } from '../helpers/range_filter';
 import { Setup } from '../helpers/setup_request';
 import { ENVIRONMENT_NOT_DEFINED } from '../../../common/environment_filter_values';
 
-export type ServiceEnvironmentsAPIResponse = PromiseReturnType<
-  typeof getServiceEnvironments
+export type EnvironmentUIFilterAPIResponse = PromiseReturnType<
+  typeof getEnvironments
 >;
-export async function getServiceEnvironments(
-  setup: Setup,
-  serviceName?: string
-) {
+export async function getEnvironments(setup: Setup, serviceName?: string) {
   const { start, end, client, config } = setup;
 
   const filter: ESFilter[] = [

x-pack/plugins/apm/server/new-platform/plugin.ts

@@ -13,9 +13,11 @@ import { initMetricsApi } from '../routes/metrics';
 import { initServicesApi } from '../routes/services';
 import { initTracesApi } from '../routes/traces';
 import { initTransactionGroupsApi } from '../routes/transaction_groups';
+import { initUIFiltersApi } from '../routes/ui_filters';
 
 export class Plugin {
   public setup(core: CoreSetup) {
+    initUIFiltersApi(core);
     initTransactionGroupsApi(core);
     initTracesApi(core);
     initServicesApi(core);

x-pack/plugins/apm/server/routes/services.ts

@@ -5,7 +5,6 @@
  */
 
 import Boom from 'boom';
-import Joi from 'joi';
 import { CoreSetup } from 'src/core/server';
 import { AgentName } from '../../typings/es_schemas/ui/fields/Agent';
 import { createApmTelementry, storeApmTelemetry } from '../lib/apm_telemetry';
@@ -13,7 +12,6 @@ import { withDefaultValidators } from '../lib/helpers/input_validation';
 import { setupRequest } from '../lib/helpers/setup_request';
 import { getService } from '../lib/services/get_service';
 import { getServices } from '../lib/services/get_services';
-import { getServiceEnvironments } from '../lib/services/get_service_environments';
 
 const ROOT = '/api/apm/services';
 const defaultErrorHandler = (err: Error) => {
@@ -63,26 +61,4 @@ export function initServicesApi(core: CoreSetup) {
       return getService(serviceName, setup).catch(defaultErrorHandler);
     }
   });
-
-  server.route({
-    method: 'GET',
-    path: `${ROOT}/environments`,
-    options: {
-      validate: {
-        query: withDefaultValidators({
-          serviceName: Joi.string()
-        })
-      },
-      tags: ['access:apm']
-    },
-    handler: req => {
-      const setup = setupRequest(req);
-      const { serviceName } = req.query as {
-        serviceName?: string;
-      };
-      return getServiceEnvironments(setup, serviceName).catch(
-        defaultErrorHandler
-      );
-    }
-  });
 }

x-pack/plugins/apm/server/routes/ui_filters.ts

@@ -0,0 +1,41 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+
+import Boom from 'boom';
+import Joi from 'joi';
+import { CoreSetup } from 'src/core/server';
+import { withDefaultValidators } from '../lib/helpers/input_validation';
+import { setupRequest } from '../lib/helpers/setup_request';
+import { getEnvironments } from '../lib/ui_filters/get_environments';
+
+const defaultErrorHandler = (err: Error) => {
+  // eslint-disable-next-line
+  console.error(err.stack);
+  throw Boom.boomify(err, { statusCode: 400 });
+};
+
+export function initUIFiltersApi(core: CoreSetup) {
+  const { server } = core.http;
+  server.route({
+    method: 'GET',
+    path: '/api/apm/ui_filters/environments',
+    options: {
+      validate: {
+        query: withDefaultValidators({
+          serviceName: Joi.string()
+        })
+      },
+      tags: ['access:apm']
+    },
+    handler: req => {
+      const setup = setupRequest(req);
+      const { serviceName } = req.query as {
+        serviceName?: string;
+      };
+      return getEnvironments(setup, serviceName).catch(defaultErrorHandler);
+    }
+  });
+}