Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[APM] Add callout to inform users of high cardinality in unique transaction names #69112

Merged
12 changes: 10 additions & 2 deletions x-pack/plugins/apm/public/components/app/TraceOverview/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
transactionGroups: [],
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({
Expand Down Expand Up @@ -56,7 +64,7 @@ export function TraceOverview() {
<EuiFlexItem grow={7}>
<EuiPanel>
<TraceList
items={data}
items={data.transactionGroups}
isLoading={status === FETCH_STATUS.LOADING}
/>
</EuiPanel>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,14 @@ import {
EuiFlexGroup,
EuiFlexItem,
EuiHorizontalRule,
EuiCallOut,
EuiLink,
EuiCode,
} from '@elastic/eui';
import { Location } from 'history';
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';
Expand Down Expand Up @@ -154,9 +158,34 @@ export function TransactionOverview() {
<h3>Transactions</h3>
</EuiTitle>
<EuiSpacer size="s" />
{!transactionListData.isAggregationAccurate && (
<EuiCallOut
title={i18n.translate(
'xpack.apm.transactionCardinalityWarning',
{
defaultMessage:
'This view shows a subset of reported transactions.',
}
)}
color="danger"
iconType="alert"
>
<p>
The number of unique transaction names exceeds the configured
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we translate this text too?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should. good catch

value of {transactionListData.bucketSize}. Try reconfiguring
your agents to group similar transactions or increase the
value of
<EuiCode>
xpack.apm.ui.transactionGroupBucketSize
</EuiCode>.{' '}
<EuiLink href="#">Learn more in the docs.</EuiLink>.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should add the docs link instead of #.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooh, good catch!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess still waiting for #67691

</p>
</EuiCallOut>
)}
<EuiSpacer size="s" />
<TransactionList
isLoading={transactionListStatus === 'loading'}
items={transactionListData}
items={transactionListData.transactionGroups}
/>
</EuiPanel>
</EuiFlexItem>
Expand Down
28 changes: 23 additions & 5 deletions x-pack/plugins/apm/public/hooks/useTransactionList.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -21,7 +20,13 @@ const getRelativeImpact = (
1
);

function getWithRelativeImpact(items: TransactionGroupListAPIResponse) {
type TransactionsAPIResponse = APIReturnType<
'/api/apm/services/{serviceName}/transaction_groups'
>;

function getWithRelativeImpact(
items: TransactionsAPIResponse['transactionGroups']
) {
const impacts = items
.map(({ impact }) => impact)
.filter((impact) => impact !== null) as number[];
Expand All @@ -40,10 +45,16 @@ function getWithRelativeImpact(items: TransactionGroupListAPIResponse) {
});
}

const DEFAULT_RESPONSE: TransactionsAPIResponse = {
transactionGroups: [],
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({
Expand All @@ -63,7 +74,14 @@ export function useTransactionList(urlParams: IUrlParams) {
[serviceName, start, end, transactionType, uiFilters]
);

const memoizedData = useMemo(() => getWithRelativeImpact(data), [data]);
const memoizedData = useMemo(
() => ({
transactionGroups: getWithRelativeImpact(data.transactionGroups),
isAggregationAccurate: data.isAggregationAccurate,
bucketSize: data.bucketSize,
}),
[data]
);
return {
data: memoizedData,
status,
Expand Down
10 changes: 9 additions & 1 deletion x-pack/plugins/apm/public/services/rest/createCallApmApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'> & {
Expand Down Expand Up @@ -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;

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -39,21 +39,24 @@ 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();
});
});

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();
});
Expand Down
7 changes: 4 additions & 3 deletions x-pack/plugins/apm/server/lib/transaction_groups/fetcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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 } } }]
Expand Down
8 changes: 3 additions & 5 deletions x-pack/plugins/apm/server/lib/transaction_groups/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,27 +18,31 @@ describe('transaction group queries', () => {
});

it('fetches top transactions', async () => {
const bucketSize = 100;
mock = await inspectSearchParams((setup) =>
transactionGroupsFetcher(
{
type: 'top_transactions',
serviceName: 'foo',
transactionType: 'bar',
},
setup
setup,
bucketSize
)
);

expect(mock.params).toMatchSnapshot();
});

it('fetches top traces', async () => {
const bucketSize = 100;
mock = await inspectSearchParams((setup) =>
transactionGroupsFetcher(
{
type: 'top_traces',
},
setup
setup,
bucketSize
)
);

Expand Down
Loading