Review feedback

dgieselaar · dgieselaar · commit 50ef730658dc · 2020-07-31T12:11:25.000+02:00
diff --git a/x-pack/plugins/apm/common/processor_event.ts b/x-pack/plugins/apm/common/processor_event.ts
@@ -12,6 +12,12 @@ export enum ProcessorEvent {
   onboarding = 'onboarding',
   sourcemap = 'sourcemap',
 }
+/**
+ * Processor events that are searchable in the UI via the query bar.
+ *
+ * Some client-sideroutes will define 1 or more processor events that
+ * will be used to fetch the dynamic index pattern for the query bar.
+ */
 
 export type UIProcessorEvent =
   | ProcessorEvent.transaction
diff --git a/x-pack/plugins/apm/common/projections.ts b/x-pack/plugins/apm/common/projections.ts
@@ -4,13 +4,13 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-export enum PROJECTION {
-  SERVICES = 'services',
-  TRANSACTION_GROUPS = 'transactionGroups',
-  TRACES = 'traces',
-  TRANSACTIONS = 'transactions',
-  METRICS = 'metrics',
-  ERROR_GROUPS = 'errorGroups',
-  SERVICE_NODES = 'serviceNodes',
-  RUM_OVERVIEW = 'rumOverview',
+export enum Projection {
+  services = 'services',
+  transactionGroups = 'transactionGroups',
+  traces = 'traces',
+  transactions = 'transactions',
+  metrics = 'metrics',
+  errorGroups = 'errorGroups',
+  serviceNodes = 'serviceNodes',
+  rumOverview = 'rumOverview',
 }
diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx
@@ -14,7 +14,7 @@ import {
 import { i18n } from '@kbn/i18n';
 import React, { useMemo } from 'react';
 import { useTrackPageview } from '../../../../../observability/public';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { useFetcher } from '../../../hooks/useFetcher';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { callApmApi } from '../../../services/rest/createCallApmApi';
@@ -79,7 +79,7 @@ function ErrorGroupOverview() {
       params: {
         serviceName,
       },
-      projection: PROJECTION.ERROR_GROUPS,
+      projection: Projection.errorGroups,
     };
 
     return config;
diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/index.tsx b/x-pack/plugins/apm/public/components/app/RumDashboard/index.tsx
@@ -13,7 +13,7 @@ import {
 } from '@elastic/eui';
 import { useTrackPageview } from '../../../../../observability/public';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { RumDashboard } from './RumDashboard';
 import { ServiceNameFilter } from '../../shared/LocalUIFilters/ServiceNameFilter';
 import { useUrlParams } from '../../../hooks/useUrlParams';
@@ -28,7 +28,7 @@ export function RumOverview() {
   const localUIFiltersConfig = useMemo(() => {
     const config: React.ComponentProps<typeof LocalUIFilters> = {
       filterNames: ['transactionUrl', 'location', 'device', 'os', 'browser'],
-      projection: PROJECTION.RUM_OVERVIEW,
+      projection: Projection.rumOverview,
     };
 
     return config;
diff --git a/x-pack/plugins/apm/public/components/app/ServiceMetrics/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceMetrics/index.tsx
@@ -16,7 +16,7 @@ import { useServiceMetricCharts } from '../../../hooks/useServiceMetricCharts';
 import { MetricsChart } from '../../shared/charts/MetricsChart';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { ChartsSyncContextProvider } from '../../../context/ChartsSyncContext';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
 
 interface ServiceMetricsProps {
@@ -36,7 +36,7 @@ export function ServiceMetrics({ agentName }: ServiceMetricsProps) {
         serviceName,
         serviceNodeName,
       },
-      projection: PROJECTION.METRICS,
+      projection: Projection.metrics,
       showCount: false,
     }),
     [serviceName, serviceNodeName]
diff --git a/x-pack/plugins/apm/public/components/app/ServiceNodeOverview/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceNodeOverview/index.tsx
@@ -15,7 +15,7 @@ import { i18n } from '@kbn/i18n';
 import styled from 'styled-components';
 import { UNIDENTIFIED_SERVICE_NODES_LABEL } from '../../../../common/i18n';
 import { SERVICE_NODE_NAME_MISSING } from '../../../../common/service_nodes';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { ManagedTable, ITableColumn } from '../../shared/ManagedTable';
@@ -46,7 +46,7 @@ function ServiceNodeOverview() {
       params: {
         serviceName,
       },
-      projection: PROJECTION.SERVICE_NODES,
+      projection: Projection.serviceNodes,
     }),
     [serviceName]
   );
diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceOverview/index.tsx
@@ -15,7 +15,7 @@ import { NoServicesMessage } from './NoServicesMessage';
 import { ServiceList } from './ServiceList';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { useTrackPageview } from '../../../../../observability/public';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
 import { useApmPluginContext } from '../../../hooks/useApmPluginContext';
 
@@ -88,7 +88,7 @@ export function ServiceOverview() {
   const localFiltersConfig: React.ComponentProps<typeof LocalUIFilters> = useMemo(
     () => ({
       filterNames: ['host', 'agentName'],
-      projection: PROJECTION.SERVICES,
+      projection: Projection.services,
     }),
     []
   );
diff --git a/x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx
@@ -11,7 +11,7 @@ import { TraceList } from './TraceList';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { useTrackPageview } from '../../../../../observability/public';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { APIReturnType } from '../../../services/rest/createCallApmApi';
 
 type TracesAPIResponse = APIReturnType<'/api/apm/traces'>;
@@ -48,7 +48,7 @@ export function TraceOverview() {
   const localUIFiltersConfig = useMemo(() => {
     const config: React.ComponentProps<typeof LocalUIFilters> = {
       filterNames: ['transactionResult', 'host', 'containerId', 'podName'],
-      projection: PROJECTION.TRACES,
+      projection: Projection.traces,
     };
 
     return config;
diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx
@@ -27,7 +27,7 @@ import { FETCH_STATUS } from '../../../hooks/useFetcher';
 import { TransactionBreakdown } from '../../shared/TransactionBreakdown';
 import { ChartsSyncContextProvider } from '../../../context/ChartsSyncContext';
 import { useTrackPageview } from '../../../../../observability/public';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
 import { HeightRetainer } from '../../shared/HeightRetainer';
 import { ErroneousTransactionsRateChart } from '../../shared/charts/ErroneousTransactionsRateChart';
@@ -52,7 +52,7 @@ export function TransactionDetails() {
   const localUIFiltersConfig = useMemo(() => {
     const config: React.ComponentProps<typeof LocalUIFilters> = {
       filterNames: ['transactionResult', 'serviceVersion'],
-      projection: PROJECTION.TRANSACTIONS,
+      projection: Projection.transactions,
       params: {
         transactionName,
         transactionType,
diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx
@@ -35,7 +35,7 @@ import { ChartsSyncContextProvider } from '../../../context/ChartsSyncContext';
 import { useTrackPageview } from '../../../../../observability/public';
 import { fromQuery, toQuery } from '../../shared/Links/url_helpers';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 import { useUrlParams } from '../../../hooks/useUrlParams';
 import { useServiceTransactionTypes } from '../../../hooks/useServiceTransactionTypes';
 import { TransactionTypeFilter } from '../../shared/LocalUIFilters/TransactionTypeFilter';
@@ -103,7 +103,7 @@ export function TransactionOverview() {
         serviceName,
         transactionType,
       },
-      projection: PROJECTION.TRANSACTION_GROUPS,
+      projection: Projection.transactionGroups,
     }),
     [serviceName, transactionType]
   );
diff --git a/x-pack/plugins/apm/public/components/shared/LocalUIFilters/index.tsx b/x-pack/plugins/apm/public/components/shared/LocalUIFilters/index.tsx
@@ -17,10 +17,10 @@ import styled from 'styled-components';
 import { LocalUIFilterName } from '../../../../server/lib/ui_filters/local_ui_filters/config';
 import { Filter } from './Filter';
 import { useLocalUIFilters } from '../../../hooks/useLocalUIFilters';
-import { PROJECTION } from '../../../../common/projections';
+import { Projection } from '../../../../common/projections';
 
 interface Props {
-  projection: PROJECTION;
+  projection: Projection;
   filterNames: LocalUIFilterName[];
   params?: Record<string, string | number | boolean | undefined>;
   showCount?: boolean;
diff --git a/x-pack/plugins/apm/public/hooks/useLocalUIFilters.ts b/x-pack/plugins/apm/public/hooks/useLocalUIFilters.ts
@@ -17,7 +17,7 @@ import {
 import { history } from '../utils/history';
 import { toQuery, fromQuery } from '../components/shared/Links/url_helpers';
 import { removeUndefinedProps } from '../context/UrlParamsContext/helpers';
-import { PROJECTION } from '../../common/projections';
+import { Projection } from '../../common/projections';
 import { pickKeys } from '../../common/utils/pick_keys';
 import { useCallApi } from './useCallApi';
 
@@ -35,7 +35,7 @@ export function useLocalUIFilters({
   filterNames,
   params,
 }: {
-  projection: PROJECTION;
+  projection: Projection;
   filterNames: LocalUIFilterName[];
   params?: Record<string, string | number | boolean | undefined>;
 }) {
diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/add_filter_to_exclude_legacy_data.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/add_filter_to_exclude_legacy_data.ts
@@ -15,17 +15,11 @@ import {
   Adds a range query to the ES request to exclude legacy data
 */
 
-export function addFilterForLegacyData(
+export function addFilterToExcludeLegacyData(
   params: ESSearchRequest & {
     body: { query: { bool: { filter: ESFilter[] } } };
-  },
-  { includeLegacyData = false } = {}
-) {
-  // search across all data (including data)
-  if (includeLegacyData) {
-    return params;
   }
-
+) {
   const nextParams = cloneDeep(params);
 
   // add filter for omitting pre-7.x data
diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/index.ts
@@ -4,43 +4,29 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 
-import { defaultsDeep, uniq, cloneDeep } from 'lodash';
 import { ValuesType } from 'utility-types';
-import { PROCESSOR_EVENT } from '../../../../../common/elasticsearch_fieldnames';
 import { APMBaseDoc } from '../../../../../typings/es_schemas/raw/apm_base_doc';
 import { APMError } from '../../../../../typings/es_schemas/ui/apm_error';
 import { KibanaRequest } from '../../../../../../../../src/core/server';
 import { ProcessorEvent } from '../../../../../common/processor_event';
 import {
-  ESFilter,
   ESSearchRequest,
   ESSearchResponse,
 } from '../../../../../typings/elasticsearch';
-import {
-  ApmIndicesConfig,
-  ApmIndicesName,
-} from '../../../settings/apm_indices/get_apm_indices';
+import { ApmIndicesConfig } from '../../../settings/apm_indices/get_apm_indices';
 import { APMRequestHandlerContext } from '../../../../routes/typings';
-import { addFilterForLegacyData } from './add_filter_for_legacy_data';
+import { addFilterToExcludeLegacyData } from './add_filter_to_exclude_legacy_data';
 import { callClientWithDebug } from '../call_client_with_debug';
 import { Transaction } from '../../../../../typings/es_schemas/ui/transaction';
 import { Span } from '../../../../../typings/es_schemas/ui/span';
+import { unpackProcessorEvents } from './unpack_processor_events';
 
 export type APMEventESSearchRequest = Omit<ESSearchRequest, 'index'> & {
   apm: {
     events: ProcessorEvent[];
   };
 };
 
-export const processorEventIndexMap: Record<ProcessorEvent, ApmIndicesName> = {
-  [ProcessorEvent.transaction]: 'apm_oss.transactionIndices',
-  [ProcessorEvent.span]: 'apm_oss.spanIndices',
-  [ProcessorEvent.metric]: 'apm_oss.metricsIndices',
-  [ProcessorEvent.error]: 'apm_oss.errorIndices',
-  [ProcessorEvent.sourcemap]: 'apm_oss.sourcemapIndices',
-  [ProcessorEvent.onboarding]: 'apm_oss.onboardingIndices',
-};
-
 type TypeOfProcessorEvent<T extends ProcessorEvent> = {
   [ProcessorEvent.error]: APMError;
   [ProcessorEvent.transaction]: Transaction;
@@ -84,40 +70,17 @@ export function createApmEventClient({
       params: TParams,
       { includeLegacyData } = { includeLegacyData: false }
     ): Promise<TypedSearchResponse<TParams>> {
-      const { apm, ...esParams } = params;
+      const withProcessorEventFilter = unpackProcessorEvents(params, indices);
 
-      const index = uniq(
-        apm.events.map((event) => indices[processorEventIndexMap[event]])
-      );
-
-      const nextParams: ESSearchRequest & {
-        body: { query: { bool: { filter: ESFilter[] } } };
-      } = defaultsDeep(cloneDeep(esParams), {
-        body: {
-          query: {
-            bool: {
-              filter: [],
-            },
-          },
-        },
-      });
-
-      nextParams.body.query.bool.filter.push({
-        terms: {
-          [PROCESSOR_EVENT]: apm.events,
-        },
-      });
-
-      const withLegacyData = addFilterForLegacyData(nextParams, {
-        includeLegacyData,
-      });
+      const withPossibleLegacyDataFilter = !includeLegacyData
+        ? addFilterToExcludeLegacyData(withProcessorEventFilter)
+        : withProcessorEventFilter;
 
       return callClientWithDebug({
         apiCaller: client.callAsCurrentUser,
         operationName: 'search',
         params: {
-          ...withLegacyData,
-          index,
+          ...withPossibleLegacyDataFilter,
           ignore_throttled: !includeFrozen,
         },
         request,
diff --git a/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/unpack_processor_events.ts b/x-pack/plugins/apm/server/lib/helpers/create_es_client/create_apm_event_client/unpack_processor_events.ts
@@ -0,0 +1,61 @@
+/*
+ * 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 { uniq, defaultsDeep, cloneDeep } from 'lodash';
+import { PROCESSOR_EVENT } from '../../../../../common/elasticsearch_fieldnames';
+import { ProcessorEvent } from '../../../../../common/processor_event';
+import {
+  ESSearchRequest,
+  ESFilter,
+} from '../../../../../typings/elasticsearch';
+import { APMEventESSearchRequest } from '.';
+import {
+  ApmIndicesConfig,
+  ApmIndicesName,
+} from '../../../settings/apm_indices/get_apm_indices';
+
+export const processorEventIndexMap: Record<ProcessorEvent, ApmIndicesName> = {
+  [ProcessorEvent.transaction]: 'apm_oss.transactionIndices',
+  [ProcessorEvent.span]: 'apm_oss.spanIndices',
+  [ProcessorEvent.metric]: 'apm_oss.metricsIndices',
+  [ProcessorEvent.error]: 'apm_oss.errorIndices',
+  [ProcessorEvent.sourcemap]: 'apm_oss.sourcemapIndices',
+  [ProcessorEvent.onboarding]: 'apm_oss.onboardingIndices',
+};
+
+export function unpackProcessorEvents(
+  request: APMEventESSearchRequest,
+  indices: ApmIndicesConfig
+) {
+  const { apm, ...params } = request;
+
+  const index = uniq(
+    apm.events.map((event) => indices[processorEventIndexMap[event]])
+  );
+
+  const withFilterForProcessorEvent: ESSearchRequest & {
+    body: { query: { bool: { filter: ESFilter[] } } };
+  } = defaultsDeep(cloneDeep(params), {
+    body: {
+      query: {
+        bool: {
+          filter: [],
+        },
+      },
+    },
+  });
+
+  withFilterForProcessorEvent.body.query.bool.filter.push({
+    terms: {
+      [PROCESSOR_EVENT]: apm.events,
+    },
+  });
+
+  return {
+    index,
+    ...withFilterForProcessorEvent,
+  };
+}
diff --git a/x-pack/plugins/apm/server/lib/helpers/setup_request.test.ts b/x-pack/plugins/apm/server/lib/helpers/setup_request.test.ts
