From dae0803a25266a03368be42f5284ef79f978a112 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 25 Jun 2020 19:11:47 +0200 Subject: [PATCH] [APM] Add callout to inform users of high cardinality in unique transaction names (#69112) 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 * Fix tests Co-authored-by: Casper Hübertz Co-authored-by: Elastic Machine Co-authored-by: Brandon Morelli --- .../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 cb6003c58e90d14..cdebb3aac129b6e 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() { 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 fc5347d081316a8..a1e01b61d5c1b50 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() {

Transactions

+ {!transactionListData.isAggregationAccurate && ( + +

+ + xpack.apm.ui.transactionGroupBucketSize + + ), + }} + /> + + + {i18n.translate( + 'xpack.apm.transactionCardinalityWarning.docsLink', + { defaultMessage: 'Learn more in the docs' } + )} + +

+ + )} + diff --git a/x-pack/plugins/apm/public/hooks/useTransactionList.ts b/x-pack/plugins/apm/public/hooks/useTransactionList.ts index 202437ae722572a..ed6bb9309a557cb 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 44768c94f3b1dfa..8babc72ef129ce9 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; export type APMClientOptions = Omit & { @@ -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 64f06ad0a81cd08..087dc6afc9a587b 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 b93f842b878cb06..496533cf97e65dc 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 00702be6744ec8c..a26c3d85a3fc47a 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 d10c45ecbdbfb1b..595ee9d8da2dcf2 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; -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 30c4975120483e9..893e586b351a80c 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 58d770bebce979a..2c5aa79bb3483c4 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 e5ec9a8eae782b2..0bb29e27f0219e9 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 2f34d365e5be9ab..81dba39e9d71260 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, + }; }