From 6b29f9f7700395853b5a99a2730b58446b1b046e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Tue, 20 Oct 2020 15:13:11 +0100 Subject: [PATCH] [APM] Error rate on service list page is not in sync with the value at the transaction page (#80814) (#81148) * fixing error rate * addressing pr comments * addressing pr comments * fixing TS issue * fixing api test --- .../ErroneousTransactionsRateChart/index.tsx | 4 +- .../lib/helpers/transaction_error_rate.ts | 65 ++++++++++++++++ .../get_service_map_service_node_info.test.ts | 2 +- .../get_services/get_services_items_stats.ts | 45 +++-------- .../lib/transaction_groups/get_error_rate.ts | 78 ++++++++----------- .../tests/transaction_groups/error_rate.ts | 18 ++--- 6 files changed, 117 insertions(+), 95 deletions(-) create mode 100644 x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.ts diff --git a/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx index e64357c085209..8aec4184f924d 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/ErroneousTransactionsRateChart/index.tsx @@ -17,7 +17,7 @@ import { callApmApi } from '../../../../services/rest/createCallApmApi'; // @ts-expect-error import CustomPlot from '../CustomPlot'; -const tickFormatY = (y?: number) => { +const tickFormatY = (y?: number | null) => { return asPercent(y || 0, 1); }; @@ -56,7 +56,7 @@ export function ErroneousTransactionsRateChart() { [syncedChartsProps] ); - const errorRates = data?.erroneousTransactionsRate || []; + const errorRates = data?.transactionErrorRate || []; const maxRate = max(errorRates.map((errorRate) => errorRate.y)); return ( diff --git a/x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.ts b/x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.ts new file mode 100644 index 0000000000000..ccc7ba7e22b50 --- /dev/null +++ b/x-pack/plugins/apm/server/lib/helpers/transaction_error_rate.ts @@ -0,0 +1,65 @@ +/* + * 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 { EVENT_OUTCOME } from '../../../common/elasticsearch_fieldnames'; +import { EventOutcome } from '../../../common/event_outcome'; +import { + AggregationOptionsByType, + AggregationResultOf, +} from '../../../typings/elasticsearch/aggregations'; +import { getTransactionDurationFieldForAggregatedTransactions } from './aggregated_transactions'; + +export function getOutcomeAggregation({ + searchAggregatedTransactions, +}: { + searchAggregatedTransactions: boolean; +}) { + return { + terms: { field: EVENT_OUTCOME }, + aggs: { + count: { + value_count: { + field: getTransactionDurationFieldForAggregatedTransactions( + searchAggregatedTransactions + ), + }, + }, + }, + }; +} + +export function calculateTransactionErrorPercentage( + outcomeResponse: AggregationResultOf< + ReturnType, + {} + > +) { + const outcomes = Object.fromEntries( + outcomeResponse.buckets.map(({ key, count }) => [key, count.value]) + ); + + const failedTransactions = outcomes[EventOutcome.failure] ?? 0; + const successfulTransactions = outcomes[EventOutcome.success] ?? 0; + + return failedTransactions / (successfulTransactions + failedTransactions); +} + +export function getTransactionErrorRateTimeSeries( + buckets: AggregationResultOf< + { + date_histogram: AggregationOptionsByType['date_histogram']; + aggs: { outcomes: ReturnType }; + }, + {} + >['buckets'] +) { + return buckets.map((dateBucket) => { + return { + x: dateBucket.key, + y: calculateTransactionErrorPercentage(dateBucket.outcomes), + }; + }); +} diff --git a/x-pack/plugins/apm/server/lib/service_map/get_service_map_service_node_info.test.ts b/x-pack/plugins/apm/server/lib/service_map/get_service_map_service_node_info.test.ts index eb2ddbf38b274..f7ca40ef1052c 100644 --- a/x-pack/plugins/apm/server/lib/service_map/get_service_map_service_node_info.test.ts +++ b/x-pack/plugins/apm/server/lib/service_map/get_service_map_service_node_info.test.ts @@ -44,7 +44,7 @@ describe('getServiceMapServiceNodeInfo', () => { it('returns data', async () => { jest.spyOn(getErrorRateModule, 'getErrorRate').mockResolvedValueOnce({ average: 0.5, - erroneousTransactionsRate: [], + transactionErrorRate: [], noHits: false, }); diff --git a/x-pack/plugins/apm/server/lib/services/get_services/get_services_items_stats.ts b/x-pack/plugins/apm/server/lib/services/get_services/get_services_items_stats.ts index 65bc3f7e47171..7d190c5b66450 100644 --- a/x-pack/plugins/apm/server/lib/services/get_services/get_services_items_stats.ts +++ b/x-pack/plugins/apm/server/lib/services/get_services/get_services_items_stats.ts @@ -28,7 +28,11 @@ import { getMLJobIds, getServiceAnomalies, } from '../../service_map/get_service_anomalies'; -import { AggregationResultOf } from '../../../../typings/elasticsearch/aggregations'; +import { + calculateTransactionErrorPercentage, + getOutcomeAggregation, + getTransactionErrorRateTimeSeries, +} from '../../helpers/transaction_error_rate'; function getDateHistogramOpts(start: number, end: number) { return { @@ -261,20 +265,7 @@ export const getTransactionErrorRates = async ({ }: AggregationParams) => { const { apmEventClient, start, end } = setup; - const outcomes = { - terms: { - field: EVENT_OUTCOME, - }, - aggs: { - count: { - value_count: { - field: getTransactionDurationFieldForAggregatedTransactions( - searchAggregatedTransactions - ), - }, - }, - }, - }; + const outcomes = getOutcomeAggregation({ searchAggregatedTransactions }); const response = await apmEventClient.search( mergeProjection(projection, { @@ -326,21 +317,6 @@ export const getTransactionErrorRates = async ({ return []; } - function calculateTransactionErrorPercentage( - outcomeResponse: AggregationResultOf - ) { - const successfulTransactions = - outcomeResponse.buckets.find( - (bucket) => bucket.key === EventOutcome.success - )?.count.value ?? 0; - const failedTransactions = - outcomeResponse.buckets.find( - (bucket) => bucket.key === EventOutcome.failure - )?.count.value ?? 0; - - return failedTransactions / (successfulTransactions + failedTransactions); - } - return aggregations.services.buckets.map((serviceBucket) => { const transactionErrorRate = calculateTransactionErrorPercentage( serviceBucket.outcomes @@ -349,12 +325,9 @@ export const getTransactionErrorRates = async ({ serviceName: serviceBucket.key as string, transactionErrorRate: { value: transactionErrorRate, - timeseries: serviceBucket.timeseries.buckets.map((dateBucket) => { - return { - x: dateBucket.key, - y: calculateTransactionErrorPercentage(dateBucket.outcomes), - }; - }), + timeseries: getTransactionErrorRateTimeSeries( + serviceBucket.timeseries.buckets + ), }, }; }); diff --git a/x-pack/plugins/apm/server/lib/transaction_groups/get_error_rate.ts b/x-pack/plugins/apm/server/lib/transaction_groups/get_error_rate.ts index d5289430b2698..e9d273dad6262 100644 --- a/x-pack/plugins/apm/server/lib/transaction_groups/get_error_rate.ts +++ b/x-pack/plugins/apm/server/lib/transaction_groups/get_error_rate.ts @@ -3,21 +3,25 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { mean } from 'lodash'; -import { EventOutcome } from '../../../common/event_outcome'; + +import { Coordinate } from '../../../typings/timeseries'; + import { + EVENT_OUTCOME, + SERVICE_NAME, TRANSACTION_NAME, TRANSACTION_TYPE, - SERVICE_NAME, - EVENT_OUTCOME, } from '../../../common/elasticsearch_fieldnames'; +import { EventOutcome } from '../../../common/event_outcome'; import { rangeFilter } from '../../../common/utils/range_filter'; -import { Setup, SetupTimeRange } from '../helpers/setup_request'; +import { getProcessorEventForAggregatedTransactions } from '../helpers/aggregated_transactions'; import { getBucketSize } from '../helpers/get_bucket_size'; +import { Setup, SetupTimeRange } from '../helpers/setup_request'; import { - getProcessorEventForAggregatedTransactions, - getTransactionDurationFieldForAggregatedTransactions, -} from '../helpers/aggregated_transactions'; + calculateTransactionErrorPercentage, + getOutcomeAggregation, + getTransactionErrorRateTimeSeries, +} from '../helpers/transaction_error_rate'; export async function getErrorRate({ serviceName, @@ -31,7 +35,11 @@ export async function getErrorRate({ transactionName?: string; setup: Setup & SetupTimeRange; searchAggregatedTransactions: boolean; -}) { +}): Promise<{ + noHits: boolean; + transactionErrorRate: Coordinate[]; + average: number | null; +}> { const { start, end, esFilter, apmEventClient } = setup; const transactionNamefilter = transactionName @@ -52,6 +60,8 @@ export async function getErrorRate({ ...esFilter, ]; + const outcomes = getOutcomeAggregation({ searchAggregatedTransactions }); + const params = { apm: { events: [ @@ -64,7 +74,8 @@ export async function getErrorRate({ size: 0, query: { bool: { filter } }, aggs: { - total_transactions: { + outcomes, + timeseries: { date_histogram: { field: '@timestamp', fixed_interval: getBucketSize(start, end).intervalString, @@ -72,20 +83,7 @@ export async function getErrorRate({ extended_bounds: { min: start, max: end }, }, aggs: { - [EVENT_OUTCOME]: { - terms: { - field: EVENT_OUTCOME, - }, - aggs: { - count: { - value_count: { - field: getTransactionDurationFieldForAggregatedTransactions( - searchAggregatedTransactions - ), - }, - }, - }, - }, + outcomes, }, }, }, @@ -96,31 +94,17 @@ export async function getErrorRate({ const noHits = resp.hits.total.value === 0; - const erroneousTransactionsRate = - resp.aggregations?.total_transactions.buckets.map((bucket) => { - const successful = - bucket[EVENT_OUTCOME].buckets.find( - (eventOutcomeBucket) => - eventOutcomeBucket.key === EventOutcome.success - )?.count.value ?? 0; - - const failed = - bucket[EVENT_OUTCOME].buckets.find( - (eventOutcomeBucket) => - eventOutcomeBucket.key === EventOutcome.failure - )?.count.value ?? 0; + if (!resp.aggregations) { + return { noHits, transactionErrorRate: [], average: null }; + } - return { - x: bucket.key, - y: failed / (successful + failed), - }; - }) || []; + const transactionErrorRate = getTransactionErrorRateTimeSeries( + resp.aggregations.timeseries.buckets + ); - const average = mean( - erroneousTransactionsRate - .map((errorRate) => errorRate.y) - .filter((y) => isFinite(y)) + const average = calculateTransactionErrorPercentage( + resp.aggregations.outcomes ); - return { noHits, erroneousTransactionsRate, average }; + return { noHits, transactionErrorRate, average }; } diff --git a/x-pack/test/apm_api_integration/basic/tests/transaction_groups/error_rate.ts b/x-pack/test/apm_api_integration/basic/tests/transaction_groups/error_rate.ts index 8ef42052ba951..86309c91b0bc2 100644 --- a/x-pack/test/apm_api_integration/basic/tests/transaction_groups/error_rate.ts +++ b/x-pack/test/apm_api_integration/basic/tests/transaction_groups/error_rate.ts @@ -31,7 +31,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.body.noHits).to.be(true); - expect(response.body.erroneousTransactionsRate.length).to.be(0); + expect(response.body.transactionErrorRate.length).to.be(0); expect(response.body.average).to.be(null); }); }); @@ -41,7 +41,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { describe('returns the transaction error rate', () => { let errorRateResponse: { - erroneousTransactionsRate: Array<{ x: number; y: number | null }>; + transactionErrorRate: Array<{ x: number; y: number | null }>; average: number; }; before(async () => { @@ -54,9 +54,9 @@ export default function ApiTest({ getService }: FtrProviderContext) { it('returns some data', () => { expect(errorRateResponse.average).to.be.greaterThan(0); - expect(errorRateResponse.erroneousTransactionsRate.length).to.be.greaterThan(0); + expect(errorRateResponse.transactionErrorRate.length).to.be.greaterThan(0); - const nonNullDataPoints = errorRateResponse.erroneousTransactionsRate.filter( + const nonNullDataPoints = errorRateResponse.transactionErrorRate.filter( ({ y }) => y !== null ); @@ -65,26 +65,26 @@ export default function ApiTest({ getService }: FtrProviderContext) { it('has the correct start date', () => { expectSnapshot( - new Date(first(errorRateResponse.erroneousTransactionsRate)?.x ?? NaN).toISOString() + new Date(first(errorRateResponse.transactionErrorRate)?.x ?? NaN).toISOString() ).toMatchInline(`"2020-09-29T14:30:00.000Z"`); }); it('has the correct end date', () => { expectSnapshot( - new Date(last(errorRateResponse.erroneousTransactionsRate)?.x ?? NaN).toISOString() + new Date(last(errorRateResponse.transactionErrorRate)?.x ?? NaN).toISOString() ).toMatchInline(`"2020-09-29T15:00:00.000Z"`); }); it('has the correct number of buckets', () => { - expectSnapshot(errorRateResponse.erroneousTransactionsRate.length).toMatchInline(`61`); + expectSnapshot(errorRateResponse.transactionErrorRate.length).toMatchInline(`61`); }); it('has the correct calculation for average', () => { - expectSnapshot(errorRateResponse.average).toMatchInline(`0.200076804915515`); + expectSnapshot(errorRateResponse.average).toMatchInline(`0.152173913043478`); }); it('has the correct error rate', () => { - expectSnapshot(errorRateResponse.erroneousTransactionsRate).toMatch(); + expectSnapshot(errorRateResponse.transactionErrorRate).toMatch(); }); }); });