From 768f9aaaa17705bb7cb4c1c536341703a61391c0 Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Tue, 16 May 2023 13:16:27 +0200 Subject: [PATCH] Revert "fix error rate preview chart" This reverts commit b9b93e55af6a1385c05a15f41fdc19cd7fe2886e. --- .../index.tsx | 4 +- ...et_transaction_error_rate_chart_preview.ts | 106 ++++-------------- .../tests/alerts/chart_preview.spec.ts | 11 +- 3 files changed, 31 insertions(+), 90 deletions(-) diff --git a/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_error_rate_rule_type/index.tsx b/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_error_rate_rule_type/index.tsx index c558a79e73d28..e115043353b1f 100644 --- a/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_error_rate_rule_type/index.tsx +++ b/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_error_rate_rule_type/index.tsx @@ -95,7 +95,6 @@ export function TransactionErrorRateRuleType(props: Props) { interval, start, end, - groupBy: params.groupBy, }, }, } @@ -109,7 +108,6 @@ export function TransactionErrorRateRuleType(props: Props) { params.serviceName, params.windowSize, params.windowUnit, - params.groupBy, ] ); @@ -175,7 +173,7 @@ export function TransactionErrorRateRuleType(props: Props) { const chartPreview = ( asPercent(d, 1)} threshold={thresholdAsPercent} uiSettings={services.uiSettings} diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/get_transaction_error_rate_chart_preview.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/get_transaction_error_rate_chart_preview.ts index d83a8ba69b037..3996ad5994710 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/get_transaction_error_rate_chart_preview.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_error_rate/get_transaction_error_rate_chart_preview.ts @@ -6,12 +6,10 @@ */ import { rangeQuery, termQuery } from '@kbn/observability-plugin/server'; -import { ApmRuleType } from '../../../../../common/rules/apm_rule_types'; import { SERVICE_NAME, TRANSACTION_TYPE, TRANSACTION_NAME, - EVENT_OUTCOME, } from '../../../../../common/es_fields/apm'; import { environmentQuery } from '../../../../../common/utils/environment_query'; import { AlertParams } from '../../route'; @@ -20,15 +18,17 @@ import { getDocumentTypeFilterForTransactions, getProcessorEventForTransactions, } from '../../../../lib/helpers/transactions'; +import { + calculateFailedTransactionRate, + getOutcomeAggregation, +} from '../../../../lib/helpers/transaction_error_rate'; import { APMConfig } from '../../../..'; import { APMEventClient } from '../../../../lib/helpers/create_es_client/create_apm_event_client'; -import { getAllGroupByFields } from '../utils/get_all_groupby_fields'; -import { EventOutcome } from '../../../../../common/event_outcome'; -import { getGroupByTerms } from '../utils/get_groupby_terms'; +import { ApmDocumentType } from '../../../../../common/document_type'; export type TransactionErrorRateChartPreviewResponse = Array<{ - name: string; - data: Array<{ x: number; y: number | null }>; + x: number; + y: number; }>; export async function getTransactionErrorRateChartPreview({ @@ -48,20 +48,16 @@ export async function getTransactionErrorRateChartPreview({ start, end, transactionName, - groupBy, } = alertParams; const searchAggregatedTransactions = await getSearchTransactionsEvents({ config, apmEventClient, kuery: '', + start, + end, }); - const allGroupByFields = getAllGroupByFields( - ApmRuleType.TransactionErrorRate, - groupBy - ); - const params = { apm: { events: [getProcessorEventForTransactions(searchAggregatedTransactions)], @@ -72,25 +68,14 @@ export async function getTransactionErrorRateChartPreview({ query: { bool: { filter: [ - ...termQuery(SERVICE_NAME, serviceName, { - queryEmptyString: false, - }), - ...termQuery(TRANSACTION_TYPE, transactionType, { - queryEmptyString: false, - }), - ...termQuery(TRANSACTION_NAME, transactionName, { - queryEmptyString: false, - }), + ...termQuery(SERVICE_NAME, serviceName), + ...termQuery(TRANSACTION_TYPE, transactionType), + ...termQuery(TRANSACTION_NAME, transactionName), ...rangeQuery(start, end), ...environmentQuery(environment), ...getDocumentTypeFilterForTransactions( searchAggregatedTransactions ), - { - terms: { - [EVENT_OUTCOME]: [EventOutcome.failure, EventOutcome.success], - }, - }, ], }, }, @@ -104,22 +89,11 @@ export async function getTransactionErrorRateChartPreview({ max: end, }, }, - aggs: { - series: { - multi_terms: { - terms: [...getGroupByTerms(allGroupByFields)], - size: 3, - order: { _count: 'desc' as const }, - }, - aggs: { - outcomes: { - terms: { - field: EVENT_OUTCOME, - }, - }, - }, - }, - }, + aggs: getOutcomeAggregation( + searchAggregatedTransactions + ? ApmDocumentType.TransactionMetric + : ApmDocumentType.TransactionEvent + ), }, }, }, @@ -134,44 +108,10 @@ export async function getTransactionErrorRateChartPreview({ return []; } - const seriesDataMap = resp.aggregations.timeseries.buckets.reduce( - (acc, bucket) => { - const x = bucket.key; - bucket.series.buckets.forEach((seriesBucket) => { - const bucketKey = seriesBucket.key.join('_'); - const y = calculateErrorRate(seriesBucket.outcomes.buckets); - - if (acc[bucketKey]) { - acc[bucketKey].push({ x, y }); - } else { - acc[bucketKey] = [{ x, y }]; - } - }); - - return acc; - }, - {} as Record> - ); - - return Object.keys(seriesDataMap).map((key) => ({ - name: key, - data: seriesDataMap[key], - })); + return resp.aggregations.timeseries.buckets.map((bucket) => { + return { + x: bucket.key, + y: calculateFailedTransactionRate(bucket), + }; + }); } - -const calculateErrorRate = ( - buckets: Array<{ - doc_count: number; - key: string | number; - }> -) => { - const failed = - buckets.find((outcomeBucket) => outcomeBucket.key === EventOutcome.failure) - ?.doc_count ?? 0; - - const succesful = - buckets.find((outcomeBucket) => outcomeBucket.key === EventOutcome.success) - ?.doc_count ?? 0; - - return (failed / (failed + succesful)) * 100; -}; diff --git a/x-pack/test/apm_api_integration/tests/alerts/chart_preview.spec.ts b/x-pack/test/apm_api_integration/tests/alerts/chart_preview.spec.ts index 198300ef7d217..bc96f7dda7d7d 100644 --- a/x-pack/test/apm_api_integration/tests/alerts/chart_preview.spec.ts +++ b/x-pack/test/apm_api_integration/tests/alerts/chart_preview.spec.ts @@ -84,8 +84,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.status).to.be(200); expect( response.body.errorRateChartPreview.some( - (item: { name: string; data: Array<{ x: number; y: number | null }> }) => - item.data.some((coordinate) => coordinate.x && coordinate.y) + (item: { x: number; y: number | null }) => item.x && item.y ) ).to.equal(true); }); @@ -111,7 +110,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); expect(response.status).to.be(200); - expect(response.body.errorRateChartPreview[0].data[0]).to.eql({ + expect(response.body.errorRateChartPreview[0]).to.eql({ x: 1627974600000, y: 1, }); @@ -138,7 +137,11 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); expect(response.status).to.be(200); - expect(response.body.errorRateChartPreview).to.eql([]); + expect( + response.body.errorRateChartPreview.every( + (item: { x: number; y: number | null }) => item.y === null + ) + ).to.equal(true); }); it('error_count (with data)', async () => {