From 40cc1ba306ef3c36e46601001031e4a7e2e93bca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= <sorenlouv@gmail.com>
Date: Sun, 28 Jun 2020 11:19:06 +0200
Subject: [PATCH] [APM] Add callout to inform users of high cardinality in
 unique transaction names (#69112) (#70153)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* [APM] Add callout

Showing a callout to inform the user we have detected a high cardinality in unique transaction names and enabling them how to fix it.

* Changed color and icon

* Updated copy and styling

* Check number of returned buckets

* Add translations and docs

* Update docs link

Co-authored-by: Brandon Morelli <bmorelli25@gmail.com>

* Fix tests

Co-authored-by: Casper Hübertz <casper@formgeist.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <bmorelli25@gmail.com>

Co-authored-by: Casper Hübertz <casper@formgeist.com>
Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>
Co-authored-by: Brandon Morelli <bmorelli25@gmail.com>
---
 .../components/app/TraceOverview/index.tsx    | 12 ++-
 .../app/TransactionOverview/index.tsx         | 46 ++++++++-
 .../apm/public/hooks/useTransactionList.ts    | 26 ++++-
 .../public/services/rest/createCallApmApi.ts  | 10 +-
 .../__snapshots__/fetcher.test.ts.snap        |  4 +-
 .../__snapshots__/queries.test.ts.snap        |  4 +-
 .../lib/transaction_groups/fetcher.test.ts    |  7 +-
 .../server/lib/transaction_groups/fetcher.ts  |  7 +-
 .../server/lib/transaction_groups/index.ts    |  8 +-
 .../lib/transaction_groups/queries.test.ts    |  8 +-
 .../lib/transaction_groups/transform.test.ts  | 96 ++++++++++++++-----
 .../lib/transaction_groups/transform.ts       | 29 ++++--
 12 files changed, 202 insertions(+), 55 deletions(-)

diff --git a/x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx
index cb6003c58e90d..cdebb3aac129b 100644
--- a/x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx
+++ b/x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx
@@ -12,11 +12,19 @@ import { useUrlParams } from '../../../hooks/useUrlParams';
 import { useTrackPageview } from '../../../../../observability/public';
 import { LocalUIFilters } from '../../shared/LocalUIFilters';
 import { PROJECTION } from '../../../../common/projections/typings';
+import { APIReturnType } from '../../../services/rest/createCallApmApi';
+
+type TracesAPIResponse = APIReturnType<'/api/apm/traces'>;
+const DEFAULT_RESPONSE: TracesAPIResponse = {
+  items: [],
+  isAggregationAccurate: true,
+  bucketSize: 0,
+};
 
 export function TraceOverview() {
   const { urlParams, uiFilters } = useUrlParams();
   const { start, end } = urlParams;
-  const { status, data = [] } = useFetcher(
+  const { status, data = DEFAULT_RESPONSE } = useFetcher(
     (callApmApi) => {
       if (start && end) {
         return callApmApi({
@@ -56,7 +64,7 @@ export function TraceOverview() {
         <EuiFlexItem grow={7}>
           <EuiPanel>
             <TraceList
-              items={data}
+              items={data.items}
               isLoading={status === FETCH_STATUS.LOADING}
             />
           </EuiPanel>
diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx
index fc5347d081316..a1e01b61d5c1b 100644
--- a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx
+++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx
@@ -11,16 +11,21 @@ import {
   EuiFlexGroup,
   EuiFlexItem,
   EuiHorizontalRule,
+  EuiCallOut,
+  EuiCode,
 } from '@elastic/eui';
 import { Location } from 'history';
+import { FormattedMessage } from '@kbn/i18n/react';
 import { first } from 'lodash';
 import React, { useMemo } from 'react';
+import { i18n } from '@kbn/i18n';
 import { useTransactionList } from '../../../hooks/useTransactionList';
 import { useTransactionCharts } from '../../../hooks/useTransactionCharts';
 import { IUrlParams } from '../../../context/UrlParamsContext/types';
 import { TransactionCharts } from '../../shared/charts/TransactionCharts';
 import { TransactionBreakdown } from '../../shared/TransactionBreakdown';
 import { TransactionList } from './List';
+import { ElasticDocsLink } from '../../shared/Links/ElasticDocsLink';
 import { useRedirect } from './useRedirect';
 import { history } from '../../../utils/history';
 import { useLocation } from '../../../hooks/useLocation';
@@ -140,9 +145,48 @@ export function TransactionOverview() {
               <h3>Transactions</h3>
             </EuiTitle>
             <EuiSpacer size="s" />
+            {!transactionListData.isAggregationAccurate && (
+              <EuiCallOut
+                title={i18n.translate(
+                  'xpack.apm.transactionCardinalityWarning.title',
+                  {
+                    defaultMessage:
+                      'This view shows a subset of reported transactions.',
+                  }
+                )}
+                color="danger"
+                iconType="alert"
+              >
+                <p>
+                  <FormattedMessage
+                    id="xpack.apm.transactionCardinalityWarning.body"
+                    defaultMessage="The number of unique transaction names exceeds the configured value of {bucketSize}. Try reconfiguring your agents to group similar transactions or increase the value of {codeBlock}"
+                    values={{
+                      bucketSize: transactionListData.bucketSize,
+                      codeBlock: (
+                        <EuiCode>
+                          xpack.apm.ui.transactionGroupBucketSize
+                        </EuiCode>
+                      ),
+                    }}
+                  />
+
+                  <ElasticDocsLink
+                    section="/kibana"
+                    path="/troubleshooting.html#troubleshooting-too-many-transactions"
+                  >
+                    {i18n.translate(
+                      'xpack.apm.transactionCardinalityWarning.docsLink',
+                      { defaultMessage: 'Learn more in the docs' }
+                    )}
+                  </ElasticDocsLink>
+                </p>
+              </EuiCallOut>
+            )}
+            <EuiSpacer size="s" />
             <TransactionList
               isLoading={transactionListStatus === 'loading'}
-              items={transactionListData}
+              items={transactionListData.items}
             />
           </EuiPanel>
         </EuiFlexItem>
diff --git a/x-pack/plugins/apm/public/hooks/useTransactionList.ts b/x-pack/plugins/apm/public/hooks/useTransactionList.ts
index 202437ae72257..ed6bb9309a557 100644
--- a/x-pack/plugins/apm/public/hooks/useTransactionList.ts
+++ b/x-pack/plugins/apm/public/hooks/useTransactionList.ts
@@ -8,8 +8,7 @@ import { useMemo } from 'react';
 import { IUrlParams } from '../context/UrlParamsContext/types';
 import { useUiFilters } from '../context/UrlParamsContext';
 import { useFetcher } from './useFetcher';
-// eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { TransactionGroupListAPIResponse } from '../../server/lib/transaction_groups';
+import { APIReturnType } from '../services/rest/createCallApmApi';
 
 const getRelativeImpact = (
   impact: number,
@@ -21,7 +20,11 @@ const getRelativeImpact = (
     1
   );
 
-function getWithRelativeImpact(items: TransactionGroupListAPIResponse) {
+type TransactionsAPIResponse = APIReturnType<
+  '/api/apm/services/{serviceName}/transaction_groups'
+>;
+
+function getWithRelativeImpact(items: TransactionsAPIResponse['items']) {
   const impacts = items
     .map(({ impact }) => impact)
     .filter((impact) => impact !== null) as number[];
@@ -40,10 +43,16 @@ function getWithRelativeImpact(items: TransactionGroupListAPIResponse) {
   });
 }
 
+const DEFAULT_RESPONSE: TransactionsAPIResponse = {
+  items: [],
+  isAggregationAccurate: true,
+  bucketSize: 0,
+};
+
 export function useTransactionList(urlParams: IUrlParams) {
   const { serviceName, transactionType, start, end } = urlParams;
   const uiFilters = useUiFilters(urlParams);
-  const { data = [], error, status } = useFetcher(
+  const { data = DEFAULT_RESPONSE, error, status } = useFetcher(
     (callApmApi) => {
       if (serviceName && start && end && transactionType) {
         return callApmApi({
@@ -63,7 +72,14 @@ export function useTransactionList(urlParams: IUrlParams) {
     [serviceName, start, end, transactionType, uiFilters]
   );
 
-  const memoizedData = useMemo(() => getWithRelativeImpact(data), [data]);
+  const memoizedData = useMemo(
+    () => ({
+      items: getWithRelativeImpact(data.items),
+      isAggregationAccurate: data.isAggregationAccurate,
+      bucketSize: data.bucketSize,
+    }),
+    [data]
+  );
   return {
     data: memoizedData,
     status,
diff --git a/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts b/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts
index 44768c94f3b1d..8babc72ef129c 100644
--- a/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts
+++ b/x-pack/plugins/apm/public/services/rest/createCallApmApi.ts
@@ -8,7 +8,7 @@ import { callApi, FetchOptions } from './callApi';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
 import { APMAPI } from '../../../server/routes/create_apm_api';
 // eslint-disable-next-line @kbn/eslint/no-restricted-paths
-import { Client } from '../../../server/routes/typings';
+import { Client, HttpMethod } from '../../../server/routes/typings';
 
 export type APMClient = Client<APMAPI['_S']>;
 export type APMClientOptions = Omit<FetchOptions, 'query' | 'body'> & {
@@ -43,3 +43,11 @@ export function createCallApmApi(http: HttpSetup) {
     });
   }) as APMClient;
 }
+
+// infer return type from API
+export type APIReturnType<
+  TPath extends keyof APMAPI['_S'],
+  TMethod extends HttpMethod = 'GET'
+> = APMAPI['_S'][TPath] extends { [key in TMethod]: { ret: any } }
+  ? APMAPI['_S'][TPath][TMethod]['ret']
+  : unknown;
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/fetcher.test.ts.snap b/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/fetcher.test.ts.snap
index 64f06ad0a81cd..087dc6afc9a58 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/fetcher.test.ts.snap
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/fetcher.test.ts.snap
@@ -46,7 +46,7 @@ Array [
               },
             },
             "composite": Object {
-              "size": 10000,
+              "size": 101,
               "sources": Array [
                 Object {
                   "service": Object {
@@ -159,7 +159,7 @@ Array [
               },
             },
             "composite": Object {
-              "size": 10000,
+              "size": 101,
               "sources": Array [
                 Object {
                   "transaction": Object {
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/queries.test.ts.snap b/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/queries.test.ts.snap
index b93f842b878cb..496533cf97e65 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/queries.test.ts.snap
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/__snapshots__/queries.test.ts.snap
@@ -44,7 +44,7 @@ Object {
           },
         },
         "composite": Object {
-          "size": 10000,
+          "size": 101,
           "sources": Array [
             Object {
               "service": Object {
@@ -153,7 +153,7 @@ Object {
           },
         },
         "composite": Object {
-          "size": 10000,
+          "size": 101,
           "sources": Array [
             Object {
               "transaction": Object {
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.test.ts b/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.test.ts
index 00702be6744ec..a26c3d85a3fc4 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.test.ts
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.test.ts
@@ -39,7 +39,8 @@ describe('transactionGroupsFetcher', () => {
   describe('type: top_traces', () => {
     it('should call client.search with correct query', async () => {
       const setup = getSetup();
-      await transactionGroupsFetcher({ type: 'top_traces' }, setup);
+      const bucketSize = 100;
+      await transactionGroupsFetcher({ type: 'top_traces' }, setup, bucketSize);
       expect(setup.client.search.mock.calls).toMatchSnapshot();
     });
   });
@@ -47,13 +48,15 @@ describe('transactionGroupsFetcher', () => {
   describe('type: top_transactions', () => {
     it('should call client.search with correct query', async () => {
       const setup = getSetup();
+      const bucketSize = 100;
       await transactionGroupsFetcher(
         {
           type: 'top_transactions',
           serviceName: 'opbeans-node',
           transactionType: 'request',
         },
-        setup
+        setup,
+        bucketSize
       );
       expect(setup.client.search.mock.calls).toMatchSnapshot();
     });
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.ts b/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.ts
index d10c45ecbdbfb..595ee9d8da2dc 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.ts
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/fetcher.ts
@@ -36,9 +36,10 @@ interface TopTraceOptions {
 export type Options = TopTransactionOptions | TopTraceOptions;
 
 export type ESResponse = PromiseReturnType<typeof transactionGroupsFetcher>;
-export function transactionGroupsFetcher(
+export async function transactionGroupsFetcher(
   options: Options,
-  setup: Setup & SetupTimeRange & SetupUIFilters
+  setup: Setup & SetupTimeRange & SetupUIFilters,
+  bucketSize: number
 ) {
   const { client } = setup;
 
@@ -71,7 +72,7 @@ export function transactionGroupsFetcher(
       aggs: {
         transaction_groups: {
           composite: {
-            size: 10000,
+            size: bucketSize + 1, // 1 extra bucket is added to check whether the total number of buckets exceed the specified bucket size.
             sources: [
               ...(isTopTraces
                 ? [{ service: { terms: { field: SERVICE_NAME } } }]
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/index.ts b/x-pack/plugins/apm/server/lib/transaction_groups/index.ts
index 30c4975120483..893e586b351a8 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/index.ts
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/index.ts
@@ -11,20 +11,18 @@ import {
 } from '../helpers/setup_request';
 import { transactionGroupsFetcher, Options } from './fetcher';
 import { transactionGroupsTransformer } from './transform';
-import { PromiseReturnType } from '../../../../observability/typings/common';
 
-export type TransactionGroupListAPIResponse = PromiseReturnType<
-  typeof getTransactionGroupList
->;
 export async function getTransactionGroupList(
   options: Options,
   setup: Setup & SetupTimeRange & SetupUIFilters
 ) {
   const { start, end } = setup;
-  const response = await transactionGroupsFetcher(options, setup);
+  const bucketSize = setup.config['xpack.apm.ui.transactionGroupBucketSize'];
+  const response = await transactionGroupsFetcher(options, setup, bucketSize);
   return transactionGroupsTransformer({
     response,
     start,
     end,
+    bucketSize,
   });
 }
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts b/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts
index 58d770bebce97..2c5aa79bb3483 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/queries.test.ts
@@ -18,6 +18,7 @@ describe('transaction group queries', () => {
   });
 
   it('fetches top transactions', async () => {
+    const bucketSize = 100;
     mock = await inspectSearchParams((setup) =>
       transactionGroupsFetcher(
         {
@@ -25,7 +26,8 @@ describe('transaction group queries', () => {
           serviceName: 'foo',
           transactionType: 'bar',
         },
-        setup
+        setup,
+        bucketSize
       )
     );
 
@@ -33,12 +35,14 @@ describe('transaction group queries', () => {
   });
 
   it('fetches top traces', async () => {
+    const bucketSize = 100;
     mock = await inspectSearchParams((setup) =>
       transactionGroupsFetcher(
         {
           type: 'top_traces',
         },
-        setup
+        setup,
+        bucketSize
       )
     );
 
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/transform.test.ts b/x-pack/plugins/apm/server/lib/transaction_groups/transform.test.ts
index e5ec9a8eae782..0bb29e27f0219 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/transform.test.ts
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/transform.test.ts
@@ -10,13 +10,20 @@ import { transactionGroupsTransformer } from './transform';
 
 describe('transactionGroupsTransformer', () => {
   it('should match snapshot', () => {
-    expect(
-      transactionGroupsTransformer({
-        response: transactionGroupsResponse,
-        start: 100,
-        end: 2000,
-      })
-    ).toMatchSnapshot();
+    const {
+      bucketSize,
+      isAggregationAccurate,
+      items,
+    } = transactionGroupsTransformer({
+      response: transactionGroupsResponse,
+      start: 100,
+      end: 2000,
+      bucketSize: 100,
+    });
+
+    expect(bucketSize).toBe(100);
+    expect(isAggregationAccurate).toBe(true);
+    expect(items).toMatchSnapshot();
   });
 
   it('should transform response correctly', () => {
@@ -43,17 +50,59 @@ describe('transactionGroupsTransformer', () => {
     } as unknown) as ESResponse;
 
     expect(
-      transactionGroupsTransformer({ response, start: 100, end: 20000 })
-    ).toEqual([
-      {
-        averageResponseTime: 255966.30555555556,
-        impact: 0,
-        name: 'POST /api/orders',
-        p95: 320238.5,
-        sample: 'sample source',
-        transactionsPerMinute: 542.713567839196,
+      transactionGroupsTransformer({
+        response,
+        start: 100,
+        end: 20000,
+        bucketSize: 100,
+      })
+    ).toEqual({
+      bucketSize: 100,
+      isAggregationAccurate: true,
+      items: [
+        {
+          averageResponseTime: 255966.30555555556,
+          impact: 0,
+          name: 'POST /api/orders',
+          p95: 320238.5,
+          sample: 'sample source',
+          transactionsPerMinute: 542.713567839196,
+        },
+      ],
+    });
+  });
+
+  it('`isAggregationAccurate` should be false if number of bucket is higher than `bucketSize`', () => {
+    const bucket = {
+      key: { transaction: 'POST /api/orders' },
+      doc_count: 180,
+      avg: { value: 255966.30555555556 },
+      p95: { values: { '95.0': 320238.5 } },
+      sum: { value: 3000000000 },
+      sample: {
+        hits: {
+          total: 180,
+          hits: [{ _source: 'sample source' }],
+        },
       },
-    ]);
+    };
+
+    const response = ({
+      aggregations: {
+        transaction_groups: {
+          buckets: [bucket, bucket, bucket, bucket], // four buckets returned
+        },
+      },
+    } as unknown) as ESResponse;
+
+    const { isAggregationAccurate } = transactionGroupsTransformer({
+      response,
+      start: 100,
+      end: 20000,
+      bucketSize: 3, // bucket size of three
+    });
+
+    expect(isAggregationAccurate).toEqual(false);
   });
 
   it('should calculate impact from sum', () => {
@@ -74,10 +123,13 @@ describe('transactionGroupsTransformer', () => {
       },
     } as unknown) as ESResponse;
 
-    expect(
-      transactionGroupsTransformer({ response, start: 100, end: 20000 }).map(
-        (bucket) => bucket.impact
-      )
-    ).toEqual([100, 25, 0]);
+    const { items } = transactionGroupsTransformer({
+      response,
+      start: 100,
+      end: 20000,
+      bucketSize: 100,
+    });
+
+    expect(items.map((bucket) => bucket.impact)).toEqual([100, 25, 0]);
   });
 });
diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/transform.ts b/x-pack/plugins/apm/server/lib/transaction_groups/transform.ts
index 2f34d365e5be9..81dba39e9d712 100644
--- a/x-pack/plugins/apm/server/lib/transaction_groups/transform.ts
+++ b/x-pack/plugins/apm/server/lib/transaction_groups/transform.ts
@@ -8,15 +8,15 @@ import moment from 'moment';
 import { sortByOrder } from 'lodash';
 import { ESResponse } from './fetcher';
 
-function calculateRelativeImpacts(transactionGroups: ITransactionGroup[]) {
-  const values = transactionGroups
+function calculateRelativeImpacts(items: ITransactionGroup[]) {
+  const values = items
     .map(({ impact }) => impact)
     .filter((value) => value !== null) as number[];
 
   const max = Math.max(...values);
   const min = Math.min(...values);
 
-  return transactionGroups.map((bucket) => ({
+  return items.map((bucket) => ({
     ...bucket,
     impact:
       bucket.impact !== null
@@ -60,17 +60,30 @@ export function transactionGroupsTransformer({
   response,
   start,
   end,
+  bucketSize,
 }: {
   response: ESResponse;
   start: number;
   end: number;
-}): ITransactionGroup[] {
+  bucketSize: number;
+}): {
+  items: ITransactionGroup[];
+  isAggregationAccurate: boolean;
+  bucketSize: number;
+} {
   const buckets = getBuckets(response);
   const duration = moment.duration(end - start);
   const minutes = duration.asMinutes();
-  const transactionGroups = buckets.map((bucket) =>
-    getTransactionGroup(bucket, minutes)
-  );
+  const items = buckets.map((bucket) => getTransactionGroup(bucket, minutes));
 
-  return calculateRelativeImpacts(transactionGroups);
+  const itemsWithRelativeImpact = calculateRelativeImpacts(items);
+
+  return {
+    items: itemsWithRelativeImpact,
+
+    // The aggregation is considered accurate if the configured bucket size is larger or equal to the number of buckets returned
+    // the actual number of buckets retrieved are `bucketsize + 1` to detect whether it's above the limit
+    isAggregationAccurate: bucketSize >= buckets.length,
+    bucketSize,
+  };
 }