Skip to content

Commit

Permalink
Ensure transactionType is used in overview (#85665) (#86413)
Browse files Browse the repository at this point in the history
* Ensure transactionType is used in overview

Fix the places where it was not being used:

- Transactions table
- Error rate chart
- Errors table

Also fix broken column sorting on transaction table.

* fix a sort

* test fix

* Snapshot update

* adds missing required param transactionType to API test

Co-authored-by: Kibana Machine <[email protected]>
Co-authored-by: Oliver Gupte <[email protected]>

Co-authored-by: Nathan L Smith <[email protected]>
Co-authored-by: Kibana Machine <[email protected]>
  • Loading branch information
3 people authored Dec 18, 2020
1 parent 7fba080 commit 8fbf280
Show file tree
Hide file tree
Showing 13 changed files with 105 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,8 @@ import * as useTransactionBreakdownHooks from '../../shared/charts/transaction_b
import { renderWithTheme } from '../../../utils/testHelpers';
import { ServiceOverview } from './';
import { waitFor } from '@testing-library/dom';
import * as callApmApi from '../../../services/rest/createCallApmApi';
import * as callApmApiModule from '../../../services/rest/createCallApmApi';
import * as useApmServiceContextHooks from '../../../context/apm_service/use_apm_service_context';

const KibanaReactContext = createKibanaReactContext({
usageCollection: { reportUiCounter: () => {} },
Expand Down Expand Up @@ -56,6 +57,13 @@ function Wrapper({ children }: { children?: ReactNode }) {

describe('ServiceOverview', () => {
it('renders', async () => {
jest
.spyOn(useApmServiceContextHooks, 'useApmServiceContext')
.mockReturnValue({
agentName: 'java',
transactionType: 'request',
transactionTypes: ['request'],
});
jest
.spyOn(useAnnotationsHooks, 'useAnnotationsContext')
.mockReturnValue({ annotations: [] });
Expand All @@ -78,17 +86,23 @@ describe('ServiceOverview', () => {
isAggregationAccurate: true,
},
'GET /api/apm/services/{serviceName}/dependencies': [],
// eslint-disable-next-line @typescript-eslint/naming-convention
'GET /api/apm/services/{serviceName}/service_overview_instances': [],
};

jest.spyOn(callApmApi, 'createCallApmApi').mockImplementation(() => {});
jest
.spyOn(callApmApiModule, 'createCallApmApi')
.mockImplementation(() => {});

jest.spyOn(callApmApi, 'callApmApi').mockImplementation(({ endpoint }) => {
const response = calls[endpoint as keyof typeof calls];
const callApmApi = jest
.spyOn(callApmApiModule, 'callApmApi')
.mockImplementation(({ endpoint }) => {
const response = calls[endpoint as keyof typeof calls];

return response
? Promise.resolve(response)
: Promise.reject(`Response for ${endpoint} is not defined`);
});
return response
? Promise.resolve(response)
: Promise.reject(`Response for ${endpoint} is not defined`);
});
jest
.spyOn(useTransactionBreakdownHooks, 'useTransactionBreakdown')
.mockReturnValue({
Expand All @@ -105,9 +119,7 @@ describe('ServiceOverview', () => {
);

await waitFor(() =>
expect(callApmApi.callApmApi).toHaveBeenCalledTimes(
Object.keys(calls).length
)
expect(callApmApi).toHaveBeenCalledTimes(Object.keys(calls).length)
);

expect((await findAllByText('Latency')).length).toBeGreaterThan(0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { i18n } from '@kbn/i18n';
import React, { useState } from 'react';
import { asInteger } from '../../../../../common/utils/formatters';
import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context';
import { useUrlParams } from '../../../../context/url_params_context/use_url_params';
import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher';
import { callApmApi } from '../../../../services/rest/createCallApmApi';
Expand Down Expand Up @@ -54,7 +55,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) {
urlParams: { start, end },
uiFilters,
} = useUrlParams();

const { transactionType } = useApmServiceContext();
const [tableOptions, setTableOptions] = useState<{
pageIndex: number;
sort: {
Expand Down Expand Up @@ -141,7 +142,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) {
},
status,
} = useFetcher(() => {
if (!start || !end) {
if (!start || !end || !transactionType) {
return;
}

Expand All @@ -158,6 +159,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) {
pageIndex: tableOptions.pageIndex,
sortField: tableOptions.sort.field,
sortDirection: tableOptions.sort.direction,
transactionType,
},
},
}).then((response) => {
Expand All @@ -181,6 +183,7 @@ export function ServiceOverviewErrorsTable({ serviceName }: Props) {
tableOptions.pageIndex,
tableOptions.sort.field,
tableOptions.sort.direction,
transactionType,
]);

const {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,15 @@ import {
callApmApi,
} from '../../../../services/rest/createCallApmApi';
import { px, unit } from '../../../../style/variables';
import { SparkPlot } from '../../../shared/charts/spark_plot';
import { ImpactBar } from '../../../shared/ImpactBar';
import { TransactionDetailLink } from '../../../shared/Links/apm/TransactionDetailLink';
import { TransactionOverviewLink } from '../../../shared/Links/apm/TransactionOverviewLink';
import { TableFetchWrapper } from '../../../shared/table_fetch_wrapper';
import { TableLinkFlexItem } from '../table_link_flex_item';
import { SparkPlot } from '../../../shared/charts/spark_plot';
import { ImpactBar } from '../../../shared/ImpactBar';
import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context';
import { TruncateWithTooltip } from '../../../shared/truncate_with_tooltip';
import { ServiceOverviewTableContainer } from '../service_overview_table_container';
import { TableLinkFlexItem } from '../table_link_flex_item';

type ServiceTransactionGroupItem = ValuesType<
APIReturnType<'GET /api/apm/services/{serviceName}/transactions/groups/overview'>['transactionGroups']
Expand All @@ -45,7 +46,7 @@ interface Props {
serviceName: string;
}

type SortField = 'latency' | 'throughput' | 'errorRate' | 'impact';
type SortField = 'name' | 'latency' | 'throughput' | 'errorRate' | 'impact';
type SortDirection = 'asc' | 'desc';

const PAGE_SIZE = 5;
Expand Down Expand Up @@ -85,7 +86,9 @@ function getLatencyAggregationTypeLabel(
}
}

export function ServiceOverviewTransactionsTable({ serviceName }: Props) {
export function ServiceOverviewTransactionsTable(props: Props) {
const { serviceName } = props;
const { transactionType } = useApmServiceContext();
const latencyAggregationType = useLatencyAggregationType();
const {
uiFilters,
Expand Down Expand Up @@ -114,7 +117,7 @@ export function ServiceOverviewTransactionsTable({ serviceName }: Props) {
},
status,
} = useFetcher(() => {
if (!start || !end || !latencyAggregationType) {
if (!start || !end || !latencyAggregationType || !transactionType) {
return;
}

Expand All @@ -132,6 +135,7 @@ export function ServiceOverviewTransactionsTable({ serviceName }: Props) {
pageIndex: tableOptions.pageIndex,
sortField: tableOptions.sort.field,
sortDirection: tableOptions.sort.direction,
transactionType,
latencyAggregationType,
},
},
Expand All @@ -156,6 +160,7 @@ export function ServiceOverviewTransactionsTable({ serviceName }: Props) {
tableOptions.pageIndex,
tableOptions.sort.field,
tableOptions.sort.direction,
transactionType,
latencyAggregationType,
]);

Expand All @@ -174,15 +179,15 @@ export function ServiceOverviewTransactionsTable({ serviceName }: Props) {
defaultMessage: 'Name',
}
),
render: (_, { name, transactionType }) => {
render: (_, { name, transactionType: type }) => {
return (
<TruncateWithTooltip
text={name}
content={
<TransactionDetailLink
serviceName={serviceName}
transactionName={name}
transactionType={transactionType}
transactionType={type}
>
{name}
</TransactionDetailLink>
Expand Down Expand Up @@ -227,7 +232,7 @@ export function ServiceOverviewTransactionsTable({ serviceName }: Props) {
},
},
{
field: 'error_rate',
field: 'errorRate',
name: i18n.translate(
'xpack.apm.serviceOverview.transactionsTableColumnErrorRate',
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { useTheme } from '../../../../hooks/use_theme';
import { useUrlParams } from '../../../../context/url_params_context/use_url_params';
import { callApmApi } from '../../../../services/rest/createCallApmApi';
import { TimeseriesChart } from '../timeseries_chart';
import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context';

function yLabelFormat(y?: number | null) {
return asPercent(y || 0, 1);
Expand All @@ -31,8 +32,8 @@ export function TransactionErrorRateChart({
const theme = useTheme();
const { serviceName } = useParams<{ serviceName?: string }>();
const { urlParams, uiFilters } = useUrlParams();

const { start, end, transactionType, transactionName } = urlParams;
const { transactionType } = useApmServiceContext();
const { start, end, transactionName } = urlParams;

const { data, status } = useFetcher(() => {
if (serviceName && start && end) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {
ERROR_GROUP_ID,
ERROR_LOG_MESSAGE,
SERVICE_NAME,
TRANSACTION_TYPE,
} from '../../../../common/elasticsearch_fieldnames';
import { Setup, SetupTimeRange } from '../../helpers/setup_request';
import { getBucketSize } from '../../helpers/get_bucket_size';
Expand All @@ -32,6 +33,7 @@ export async function getServiceErrorGroups({
pageIndex,
sortDirection,
sortField,
transactionType,
}: {
serviceName: string;
setup: Setup & SetupTimeRange;
Expand All @@ -40,6 +42,7 @@ export async function getServiceErrorGroups({
numBuckets: number;
sortDirection: 'asc' | 'desc';
sortField: 'name' | 'last_seen' | 'occurrences';
transactionType: string;
}) {
const { apmEventClient, start, end, esFilter } = setup;

Expand All @@ -55,6 +58,7 @@ export async function getServiceErrorGroups({
bool: {
filter: [
{ term: { [SERVICE_NAME]: serviceName } },
{ term: { [TRANSACTION_TYPE]: transactionType } },
{ range: rangeFilter(start, end) },
...esFilter,
],
Expand Down Expand Up @@ -127,6 +131,7 @@ export async function getServiceErrorGroups({
filter: [
{ terms: { [ERROR_GROUP_ID]: sortedErrorGroupIds } },
{ term: { [SERVICE_NAME]: serviceName } },
{ term: { [TRANSACTION_TYPE]: transactionType } },
{ range: rangeFilter(start, end) },
...esFilter,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export async function getTimeseriesDataForTransactionGroups({
searchAggregatedTransactions,
size,
numBuckets,
transactionType,
latencyAggregationType,
}: {
apmEventClient: APMEventClient;
Expand All @@ -49,6 +50,7 @@ export async function getTimeseriesDataForTransactionGroups({
searchAggregatedTransactions: boolean;
size: number;
numBuckets: number;
transactionType: string;
latencyAggregationType: LatencyAggregationType;
}) {
const { intervalString } = getBucketSize({ start, end, numBuckets });
Expand All @@ -72,6 +74,7 @@ export async function getTimeseriesDataForTransactionGroups({
filter: [
{ terms: { [TRANSACTION_NAME]: transactionNames } },
{ term: { [SERVICE_NAME]: serviceName } },
{ term: { [TRANSACTION_TYPE]: transactionType } },
{ range: rangeFilter(start, end) },
...esFilter,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
EVENT_OUTCOME,
SERVICE_NAME,
TRANSACTION_NAME,
TRANSACTION_TYPE,
} from '../../../../common/elasticsearch_fieldnames';
import {
getProcessorEventForAggregatedTransactions,
Expand All @@ -26,6 +27,7 @@ import {
} from '../../helpers/latency_aggregation_type';

export type ServiceOverviewTransactionGroupSortField =
| 'name'
| 'latency'
| 'throughput'
| 'errorRate'
Expand All @@ -46,6 +48,7 @@ export async function getTransactionGroupsForPage({
sortDirection,
pageIndex,
size,
transactionType,
latencyAggregationType,
}: {
apmEventClient: APMEventClient;
Expand All @@ -58,6 +61,7 @@ export async function getTransactionGroupsForPage({
sortDirection: 'asc' | 'desc';
pageIndex: number;
size: number;
transactionType: string;
latencyAggregationType: LatencyAggregationType;
}) {
const field = getTransactionDurationFieldForAggregatedTransactions(
Expand All @@ -78,6 +82,7 @@ export async function getTransactionGroupsForPage({
bool: {
filter: [
{ term: { [SERVICE_NAME]: serviceName } },
{ term: { [TRANSACTION_TYPE]: transactionType } },
{ range: rangeFilter(start, end) },
...esFilter,
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export async function getServiceTransactionGroups({
sortDirection,
sortField,
searchAggregatedTransactions,
transactionType,
latencyAggregationType,
}: {
serviceName: string;
Expand All @@ -32,6 +33,7 @@ export async function getServiceTransactionGroups({
sortDirection: 'asc' | 'desc';
sortField: ServiceOverviewTransactionGroupSortField;
searchAggregatedTransactions: boolean;
transactionType: string;
latencyAggregationType: LatencyAggregationType;
}) {
const { apmEventClient, start, end, esFilter } = setup;
Expand All @@ -51,6 +53,7 @@ export async function getServiceTransactionGroups({
sortDirection,
size,
searchAggregatedTransactions,
transactionType,
latencyAggregationType,
});

Expand All @@ -66,6 +69,7 @@ export async function getServiceTransactionGroups({
serviceName,
size,
transactionNames,
transactionType,
latencyAggregationType,
});

Expand Down
11 changes: 10 additions & 1 deletion x-pack/plugins/apm/server/routes/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ export const serviceErrorGroupsRoute = createRoute({
t.literal('occurrences'),
t.literal('name'),
]),
transactionType: t.string,
}),
]),
}),
Expand All @@ -266,7 +267,14 @@ export const serviceErrorGroupsRoute = createRoute({

const {
path: { serviceName },
query: { size, numBuckets, pageIndex, sortDirection, sortField },
query: {
numBuckets,
pageIndex,
size,
sortDirection,
sortField,
transactionType,
},
} = context.params;
return getServiceErrorGroups({
serviceName,
Expand All @@ -276,6 +284,7 @@ export const serviceErrorGroupsRoute = createRoute({
pageIndex,
sortDirection,
sortField,
transactionType,
});
},
});
Expand Down
Loading

0 comments on commit 8fbf280

Please sign in to comment.