From 144c0b2d7964020c494127a192f45d5411facbae Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Wed, 3 May 2023 23:46:24 +0200 Subject: [PATCH 1/6] fix preview chart in latency threshold --- .../transaction_duration_rule_type/index.tsx | 19 ++++-- .../ui_components/chart_preview/index.tsx | 31 +++++---- .../plugins/apm/server/routes/alerts/route.ts | 3 + .../get_all_groupby_fields.ts | 22 +++++++ .../get_transaction_duration_chart_preview.ts | 64 +++++++++---------- ...register_transaction_duration_rule_type.ts | 15 ++--- 6 files changed, 92 insertions(+), 62 deletions(-) create mode 100644 x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_all_groupby_fields.ts diff --git a/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx b/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx index ea1a5d8138f18..b812b883f9ae7 100644 --- a/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx +++ b/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx @@ -121,6 +121,7 @@ export function TransactionDurationRuleType(props: Props) { interval, start, end, + groupBy: params.groupBy, }, }, } @@ -135,6 +136,7 @@ export function TransactionDurationRuleType(props: Props) { params.transactionName, params.windowSize, params.windowUnit, + params.groupBy, ] ); @@ -168,7 +170,7 @@ export function TransactionDurationRuleType(props: Props) { allowAll={false} currentValue={params.serviceName} onChange={(value) => { - if (value !== params.serviceName) { + if (value !== params.serviceName && value !== '') { setRuleParams('serviceName', value); setRuleParams('transactionType', undefined); setRuleParams('transactionName', undefined); @@ -178,17 +180,26 @@ export function TransactionDurationRuleType(props: Props) { />, setRuleParams('transactionType', value)} + onChange={(value) => + setRuleParams('transactionType', value !== '' ? value : undefined) + } serviceName={params.serviceName} />, setRuleParams('environment', value)} + onChange={(value) => + setRuleParams( + 'environment', + value !== '' ? value : ENVIRONMENT_ALL.value + ) + } serviceName={params.serviceName} />, setRuleParams('transactionName', value)} + onChange={(value) => + setRuleParams('transactionName', value !== '' ? value : undefined) + } serviceName={params.serviceName} />, data.map(({ x }) => x)); @@ -48,12 +46,6 @@ export function ChartPreview({ const xMax = Math.max(...timestamps); const xFormatter = niceTimeFormatter([xMin, xMax]); - function updateYMax() { - // Make the maximum Y value either the actual max or 20% more than the threshold - const values = series.flatMap(({ data }) => data.map((d) => d.y ?? 0)); - setYMax(Math.max(...values, threshold * 1.2)); - } - const style = { fill: theme.eui.euiColorVis2, line: { @@ -76,24 +68,35 @@ export function ChartPreview({ ]; const timeZone = getTimeZone(uiSettings); - const legendSize = Math.ceil(series.length / 2) * 30; + const legendSize = + series.length > 1 ? Math.ceil(series.length / 2) * 30 : series.length * 35; const chartSize = 150; + const domainYMax = () => { + // Make the maximum Y value either the actual max or 20% more than the threshold + const values = series.flatMap(({ data }) => data.map((d) => d.y ?? 0)); + return Math.max(...values, threshold * 1.2); + }; + + const domain = { + max: domainYMax(), + min: 0, + }; + return ( <> 1 ? chartSize + legendSize : chartSize, + height: chartSize + legendSize, }} data-test-subj="ChartPreview" > 1} + showLegend={true} legendPosition={'bottom'} legendSize={legendSize} - onLegendItemClick={updateYMax} /> {series.map(({ name, data }, index) => ( ; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_all_groupby_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_all_groupby_fields.ts new file mode 100644 index 0000000000000..f3ba336c7b5bb --- /dev/null +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_all_groupby_fields.ts @@ -0,0 +1,22 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { + SERVICE_ENVIRONMENT, + SERVICE_NAME, + TRANSACTION_TYPE, +} from '../../../../../common/es_fields/apm'; + +export const getAllGroupByFields = (groupBy: string[] | undefined = []) => { + const predefinedGroupby = [ + SERVICE_NAME, + SERVICE_ENVIRONMENT, + TRANSACTION_TYPE, + ]; + + return Array.from(new Set([...predefinedGroupby, ...groupBy])); +}; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts index f7b226216aef0..966436bddcbd2 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts @@ -10,7 +10,6 @@ import { rangeQuery, termQuery } from '@kbn/observability-plugin/server'; import { AggregationType } from '../../../../../common/rules/apm_rule_types'; import { SERVICE_NAME, - SERVICE_ENVIRONMENT, TRANSACTION_TYPE, TRANSACTION_NAME, } from '../../../../../common/es_fields/apm'; @@ -23,12 +22,13 @@ import { getProcessorEventForTransactions, } from '../../../../lib/helpers/transactions'; import { - ENVIRONMENT_NOT_DEFINED, - getEnvironmentLabel, -} from '../../../../../common/environment_filter_values'; -import { averageOrPercentileAgg } from './average_or_percentile_agg'; + averageOrPercentileAgg, + getMultiTermsSortOrder, +} from './average_or_percentile_agg'; import { APMConfig } from '../../../..'; import { APMEventClient } from '../../../../lib/helpers/create_es_client/create_apm_event_client'; +import { getGroupByTerms } from '../utils/get_groupby_terms'; +import { getAllGroupByFields } from './get_all_groupby_fields'; export type TransactionDurationChartPreviewResponse = Array<{ name: string; @@ -53,6 +53,7 @@ export async function getTransactionDurationChartPreview({ interval, start, end, + groupBy, } = alertParams; const searchAggregatedTransactions = await getSearchTransactionsEvents({ config, @@ -77,6 +78,8 @@ export async function getTransactionDurationChartPreview({ searchAggregatedTransactions ); + const allGroupByFields = getAllGroupByFields(groupBy); + const aggs = { timeseries: { date_histogram: { @@ -89,23 +92,18 @@ export async function getTransactionDurationChartPreview({ }, }, aggs: { - environment: { - terms: { - field: SERVICE_ENVIRONMENT, - missing: ENVIRONMENT_NOT_DEFINED.value, - size: 10, - order: { - [aggregationType === AggregationType.Avg - ? 'avgLatency' - : `pctLatency.${ - aggregationType === AggregationType.P95 ? 95 : 99 - }`]: 'desc', - } as Record, + series: { + multi_terms: { + terms: [...getGroupByTerms(allGroupByFields)], + size: 3, + ...getMultiTermsSortOrder(aggregationType), + }, + aggs: { + ...averageOrPercentileAgg({ + aggregationType, + transactionDurationField, + }), }, - aggs: averageOrPercentileAgg({ - aggregationType, - transactionDurationField, - }), }, }, }, @@ -125,19 +123,19 @@ export async function getTransactionDurationChartPreview({ return []; } - const environmentDataMap = resp.aggregations.timeseries.buckets.reduce( + const seriesDataMap = resp.aggregations.timeseries.buckets.reduce( (acc, bucket) => { const x = bucket.key; - bucket.environment.buckets.forEach((environmentBucket) => { - const env = environmentBucket.key as string; + bucket.series.buckets.forEach((seriesBucket) => { + const bucketKey = seriesBucket.key.join('_'); const y = - 'avgLatency' in environmentBucket - ? environmentBucket.avgLatency.value - : environmentBucket.pctLatency.values[0].value; - if (acc[env]) { - acc[env].push({ x, y }); + 'avgLatency' in seriesBucket + ? seriesBucket.avgLatency.value + : seriesBucket.pctLatency.values[0].value; + if (acc[bucketKey]) { + acc[bucketKey].push({ x, y }); } else { - acc[env] = [{ x, y }]; + acc[bucketKey] = [{ x, y }]; } }); @@ -146,8 +144,8 @@ export async function getTransactionDurationChartPreview({ {} as Record> ); - return Object.keys(environmentDataMap).map((env) => ({ - name: getEnvironmentLabel(env), - data: environmentDataMap[env], + return Object.keys(seriesDataMap).map((key) => ({ + name: key, + data: seriesDataMap[key], })); } diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index 0ac465261c954..bfaaef3aa757a 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -64,6 +64,7 @@ import { getMultiTermsSortOrder, } from './average_or_percentile_agg'; import { getGroupByActionVariables } from '../utils/get_groupby_action_variables'; +import { getAllGroupByFields } from './get_all_groupby_fields'; const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.TransactionDuration]; @@ -103,15 +104,7 @@ export function registerTransactionDurationRuleType({ minimumLicenseRequired: 'basic', isExportable: true, executor: async ({ params: ruleParams, services, spaceId }) => { - const predefinedGroupby = [ - SERVICE_NAME, - SERVICE_ENVIRONMENT, - TRANSACTION_TYPE, - ]; - - const allGroupbyFields = Array.from( - new Set([...predefinedGroupby, ...(ruleParams.groupBy ?? [])]) - ); + const allGroupByFields = getAllGroupByFields(ruleParams.groupBy); const config = await firstValueFrom(config$); @@ -172,7 +165,7 @@ export function registerTransactionDurationRuleType({ aggs: { series: { multi_terms: { - terms: [...getGroupByTerms(allGroupbyFields)], + terms: [...getGroupByTerms(allGroupByFields)], size: 1000, ...getMultiTermsSortOrder(ruleParams.aggregationType), }, @@ -205,7 +198,7 @@ export function registerTransactionDurationRuleType({ for (const bucket of response.aggregations.series.buckets) { const groupByFields = bucket.key.reduce( (obj, bucketKey, bucketIndex) => { - obj[allGroupbyFields[bucketIndex]] = bucketKey; + obj[allGroupByFields[bucketIndex]] = bucketKey; return obj; }, {} as Record From 8186cc25197e0ce146584fa7b3db55bb1004f3a2 Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Thu, 4 May 2023 12:31:45 +0200 Subject: [PATCH 2/6] add tests --- .../tests/alerts/chart_preview.spec.ts | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) 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 f95bb8de59a89..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 @@ -5,6 +5,12 @@ * 2.0. */ +import { + SERVICE_ENVIRONMENT, + SERVICE_NAME, + TRANSACTION_NAME, + TRANSACTION_TYPE, +} from '@kbn/apm-plugin/common/es_fields/apm'; import expect from '@kbn/expect'; import archives from '../../common/fixtures/es_archiver/archives_metadata'; import { FtrProviderContext } from '../../common/ftr_provider_context'; @@ -246,5 +252,98 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.status).to.be(200); expect(response.body.latencyChartPreview).to.eql([]); }); + + it('transaction_duration with no group by parameter', async () => { + const options = getOptions(); + const response = await apmApiClient.readUser({ + ...options, + endpoint: 'GET /internal/apm/rule_types/transaction_duration/chart_preview', + }); + + expect(response.status).to.be(200); + expect(response.body.latencyChartPreview.length).to.equal(1); + expect( + response.body.latencyChartPreview.map( + (item: { name: string; data: Array<{ x: number; y: number | null }> }) => item.name + ) + ).to.eql(['opbeans-java_production_request']); + }); + + it('transaction_duration with default group by fields', async () => { + const options = { + params: { + query: { + ...getOptions().params.query, + groupBy: [SERVICE_NAME, SERVICE_ENVIRONMENT, TRANSACTION_TYPE], + }, + }, + }; + + const response = await apmApiClient.readUser({ + ...options, + endpoint: 'GET /internal/apm/rule_types/transaction_duration/chart_preview', + }); + + expect(response.status).to.be(200); + expect(response.body.latencyChartPreview.length).to.equal(1); + expect( + response.body.latencyChartPreview.map( + (item: { name: string; data: Array<{ x: number; y: number | null }> }) => item.name + ) + ).to.eql(['opbeans-java_production_request']); + }); + + it('transaction_duration with group by on transaction name', async () => { + const options = { + params: { + query: { + ...getOptions().params.query, + groupBy: [SERVICE_NAME, SERVICE_ENVIRONMENT, TRANSACTION_TYPE, TRANSACTION_NAME], + }, + }, + }; + + const response = await apmApiClient.readUser({ + ...options, + endpoint: 'GET /internal/apm/rule_types/transaction_duration/chart_preview', + }); + + expect(response.status).to.be(200); + expect(response.body.latencyChartPreview.length).to.equal(3); + expect( + response.body.latencyChartPreview.map( + (item: { name: string; data: Array<{ x: number; y: number | null }> }) => item.name + ) + ).to.eql([ + 'opbeans-java_production_request_DispatcherServlet#doGet', + 'opbeans-java_production_request_APIRestController#stats', + 'opbeans-java_production_request_APIRestController#customers', + ]); + }); + + it('transaction_duration with group by on transaction name and filter on transaction name', async () => { + const options = { + params: { + query: { + ...getOptions().params.query, + transactionName: 'DispatcherServlet#doGet', + groupBy: [SERVICE_NAME, SERVICE_ENVIRONMENT, TRANSACTION_TYPE, TRANSACTION_NAME], + }, + }, + }; + + const response = await apmApiClient.readUser({ + ...options, + endpoint: 'GET /internal/apm/rule_types/transaction_duration/chart_preview', + }); + + expect(response.status).to.be(200); + expect(response.body.latencyChartPreview.length).to.equal(1); + expect( + response.body.latencyChartPreview.map( + (item: { name: string; data: Array<{ x: number; y: number | null }> }) => item.name + ) + ).to.eql(['opbeans-java_production_request_DispatcherServlet#doGet']); + }); }); } From 217e958f1cd7f01c795e708557c63ed7e6c20914 Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Fri, 5 May 2023 23:39:13 +0200 Subject: [PATCH 3/6] refactoring --- .../transaction_duration_rule_type/index.tsx | 10 ++----- .../plugins/apm/server/routes/alerts/route.ts | 1 - .../get_all_groupby_fields.ts | 22 --------------- .../get_transaction_duration_chart_preview.ts | 24 ++++++++++++---- ...register_transaction_duration_rule_type.ts | 7 +++-- .../utils/get_all_groupby_fields.ts | 28 +++++++++++++++++++ 6 files changed, 54 insertions(+), 38 deletions(-) delete mode 100644 x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_all_groupby_fields.ts create mode 100644 x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts diff --git a/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx b/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx index b812b883f9ae7..35ca8ae161faa 100644 --- a/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx +++ b/x-pack/plugins/apm/public/components/alerting/rule_types/transaction_duration_rule_type/index.tsx @@ -170,7 +170,7 @@ export function TransactionDurationRuleType(props: Props) { allowAll={false} currentValue={params.serviceName} onChange={(value) => { - if (value !== params.serviceName && value !== '') { + if (value !== params.serviceName) { setRuleParams('serviceName', value); setRuleParams('transactionType', undefined); setRuleParams('transactionName', undefined); @@ -180,9 +180,7 @@ export function TransactionDurationRuleType(props: Props) { />, - setRuleParams('transactionType', value !== '' ? value : undefined) - } + onChange={(value) => setRuleParams('transactionType', value)} serviceName={params.serviceName} />, , - setRuleParams('transactionName', value !== '' ? value : undefined) - } + onChange={(value) => setRuleParams('transactionName', value)} serviceName={params.serviceName} />, { - const predefinedGroupby = [ - SERVICE_NAME, - SERVICE_ENVIRONMENT, - TRANSACTION_TYPE, - ]; - - return Array.from(new Set([...predefinedGroupby, ...groupBy])); -}; diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts index 966436bddcbd2..6d39a59fe73a3 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/get_transaction_duration_chart_preview.ts @@ -7,7 +7,10 @@ import { QueryDslQueryContainer } from '@elastic/elasticsearch/lib/api/typesWithBodyKey'; import { rangeQuery, termQuery } from '@kbn/observability-plugin/server'; -import { AggregationType } from '../../../../../common/rules/apm_rule_types'; +import { + AggregationType, + ApmRuleType, +} from '../../../../../common/rules/apm_rule_types'; import { SERVICE_NAME, TRANSACTION_TYPE, @@ -28,7 +31,7 @@ import { import { APMConfig } from '../../../..'; import { APMEventClient } from '../../../../lib/helpers/create_es_client/create_apm_event_client'; import { getGroupByTerms } from '../utils/get_groupby_terms'; -import { getAllGroupByFields } from './get_all_groupby_fields'; +import { getAllGroupByFields } from '../utils/get_all_groupby_fields'; export type TransactionDurationChartPreviewResponse = Array<{ name: string; @@ -64,9 +67,15 @@ export async function getTransactionDurationChartPreview({ const query = { bool: { filter: [ - ...termQuery(SERVICE_NAME, serviceName), - ...termQuery(TRANSACTION_TYPE, transactionType), - ...termQuery(TRANSACTION_NAME, transactionName), + ...termQuery(SERVICE_NAME, serviceName, { + queryEmptyString: false, + }), + ...termQuery(TRANSACTION_TYPE, transactionType, { + queryEmptyString: false, + }), + ...termQuery(TRANSACTION_NAME, transactionName, { + queryEmptyString: false, + }), ...rangeQuery(start, end), ...environmentQuery(environment), ...getDocumentTypeFilterForTransactions(searchAggregatedTransactions), @@ -78,7 +87,10 @@ export async function getTransactionDurationChartPreview({ searchAggregatedTransactions ); - const allGroupByFields = getAllGroupByFields(groupBy); + const allGroupByFields = getAllGroupByFields( + ApmRuleType.TransactionDuration, + groupBy + ); const aggs = { timeseries: { diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts index bfaaef3aa757a..682457d4b19c4 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/transaction_duration/register_transaction_duration_rule_type.ts @@ -64,7 +64,7 @@ import { getMultiTermsSortOrder, } from './average_or_percentile_agg'; import { getGroupByActionVariables } from '../utils/get_groupby_action_variables'; -import { getAllGroupByFields } from './get_all_groupby_fields'; +import { getAllGroupByFields } from '../utils/get_all_groupby_fields'; const ruleTypeConfig = RULE_TYPES_CONFIG[ApmRuleType.TransactionDuration]; @@ -104,7 +104,10 @@ export function registerTransactionDurationRuleType({ minimumLicenseRequired: 'basic', isExportable: true, executor: async ({ params: ruleParams, services, spaceId }) => { - const allGroupByFields = getAllGroupByFields(ruleParams.groupBy); + const allGroupByFields = getAllGroupByFields( + ApmRuleType.TransactionDuration, + ruleParams.groupBy + ); const config = await firstValueFrom(config$); diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts new file mode 100644 index 0000000000000..1500763caf750 --- /dev/null +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts @@ -0,0 +1,28 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0; you may not use this file except in compliance with the Elastic License + * 2.0. + */ + +import { ApmRuleType } from '../../../../../common/rules/apm_rule_types'; +import { + SERVICE_ENVIRONMENT, + SERVICE_NAME, + TRANSACTION_TYPE, +} from '../../../../../common/es_fields/apm'; + +export const getAllGroupByFields = ( + ruleType: string, + groupBy: string[] | undefined = [] +) => { + const predefinedGroupby = + ruleType === ApmRuleType.TransactionDuration || + ruleType === ApmRuleType.TransactionErrorRate + ? [SERVICE_NAME, SERVICE_ENVIRONMENT, TRANSACTION_TYPE] + : ruleType === ApmRuleType.ErrorCount + ? [SERVICE_NAME, SERVICE_ENVIRONMENT] + : []; + + return Array.from(new Set([...predefinedGroupby, ...groupBy])); +}; From b9b93e55af6a1385c05a15f41fdc19cd7fe2886e Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Fri, 5 May 2023 23:40:31 +0200 Subject: [PATCH 4/6] fix error rate preview chart --- .../index.tsx | 4 +- ...et_transaction_error_rate_chart_preview.ts | 106 ++++++++++++++---- .../tests/alerts/chart_preview.spec.ts | 11 +- 3 files changed, 90 insertions(+), 31 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 e115043353b1f..c558a79e73d28 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,6 +95,7 @@ export function TransactionErrorRateRuleType(props: Props) { interval, start, end, + groupBy: params.groupBy, }, }, } @@ -108,6 +109,7 @@ export function TransactionErrorRateRuleType(props: Props) { params.serviceName, params.windowSize, params.windowUnit, + params.groupBy, ] ); @@ -173,7 +175,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 3996ad5994710..d83a8ba69b037 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,10 +6,12 @@ */ 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'; @@ -18,17 +20,15 @@ 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 { ApmDocumentType } from '../../../../../common/document_type'; +import { getAllGroupByFields } from '../utils/get_all_groupby_fields'; +import { EventOutcome } from '../../../../../common/event_outcome'; +import { getGroupByTerms } from '../utils/get_groupby_terms'; export type TransactionErrorRateChartPreviewResponse = Array<{ - x: number; - y: number; + name: string; + data: Array<{ x: number; y: number | null }>; }>; export async function getTransactionErrorRateChartPreview({ @@ -48,16 +48,20 @@ 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)], @@ -68,14 +72,25 @@ export async function getTransactionErrorRateChartPreview({ query: { bool: { filter: [ - ...termQuery(SERVICE_NAME, serviceName), - ...termQuery(TRANSACTION_TYPE, transactionType), - ...termQuery(TRANSACTION_NAME, transactionName), + ...termQuery(SERVICE_NAME, serviceName, { + queryEmptyString: false, + }), + ...termQuery(TRANSACTION_TYPE, transactionType, { + queryEmptyString: false, + }), + ...termQuery(TRANSACTION_NAME, transactionName, { + queryEmptyString: false, + }), ...rangeQuery(start, end), ...environmentQuery(environment), ...getDocumentTypeFilterForTransactions( searchAggregatedTransactions ), + { + terms: { + [EVENT_OUTCOME]: [EventOutcome.failure, EventOutcome.success], + }, + }, ], }, }, @@ -89,11 +104,22 @@ export async function getTransactionErrorRateChartPreview({ max: end, }, }, - aggs: getOutcomeAggregation( - searchAggregatedTransactions - ? ApmDocumentType.TransactionMetric - : ApmDocumentType.TransactionEvent - ), + aggs: { + series: { + multi_terms: { + terms: [...getGroupByTerms(allGroupByFields)], + size: 3, + order: { _count: 'desc' as const }, + }, + aggs: { + outcomes: { + terms: { + field: EVENT_OUTCOME, + }, + }, + }, + }, + }, }, }, }, @@ -108,10 +134,44 @@ export async function getTransactionErrorRateChartPreview({ return []; } - return resp.aggregations.timeseries.buckets.map((bucket) => { - return { - x: bucket.key, - y: calculateFailedTransactionRate(bucket), - }; - }); + 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], + })); } + +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 bc96f7dda7d7d..198300ef7d217 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,7 +84,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(response.status).to.be(200); expect( response.body.errorRateChartPreview.some( - (item: { x: number; y: number | null }) => item.x && item.y + (item: { name: string; data: Array<{ x: number; y: number | null }> }) => + item.data.some((coordinate) => coordinate.x && coordinate.y) ) ).to.equal(true); }); @@ -110,7 +111,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); expect(response.status).to.be(200); - expect(response.body.errorRateChartPreview[0]).to.eql({ + expect(response.body.errorRateChartPreview[0].data[0]).to.eql({ x: 1627974600000, y: 1, }); @@ -137,11 +138,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); expect(response.status).to.be(200); - expect( - response.body.errorRateChartPreview.every( - (item: { x: number; y: number | null }) => item.y === null - ) - ).to.equal(true); + expect(response.body.errorRateChartPreview).to.eql([]); }); it('error_count (with data)', async () => { From 768f9aaaa17705bb7cb4c1c536341703a61391c0 Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Tue, 16 May 2023 13:16:27 +0200 Subject: [PATCH 5/6] 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 () => { From bbdc9a9c0600a14b730732304eb10ac582247e68 Mon Sep 17 00:00:00 2001 From: Bena Kansara Date: Tue, 16 May 2023 13:26:35 +0200 Subject: [PATCH 6/6] refactoring --- .../routes/alerts/rule_types/utils/get_all_groupby_fields.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts b/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts index 1500763caf750..6d4ccda93ee0a 100644 --- a/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts +++ b/x-pack/plugins/apm/server/routes/alerts/rule_types/utils/get_all_groupby_fields.ts @@ -5,6 +5,7 @@ * 2.0. */ +import { union } from 'lodash'; import { ApmRuleType } from '../../../../../common/rules/apm_rule_types'; import { SERVICE_ENVIRONMENT, @@ -16,7 +17,7 @@ export const getAllGroupByFields = ( ruleType: string, groupBy: string[] | undefined = [] ) => { - const predefinedGroupby = + const predefinedGroupBy = ruleType === ApmRuleType.TransactionDuration || ruleType === ApmRuleType.TransactionErrorRate ? [SERVICE_NAME, SERVICE_ENVIRONMENT, TRANSACTION_TYPE] @@ -24,5 +25,5 @@ export const getAllGroupByFields = ( ? [SERVICE_NAME, SERVICE_ENVIRONMENT] : []; - return Array.from(new Set([...predefinedGroupby, ...groupBy])); + return union(predefinedGroupBy, groupBy); };