[APM] Error rate on service list page is not in sync with the value at the transaction page (#80814) (#81149)

cauemarcondes · web-flow · commit b793dff110d1 · 2020-10-20T16:13:03.000+02:00
* fixing error rate

* addressing pr comments

* addressing pr comments

* fixing TS issue

* fixing api test
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
@@ -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
@@ -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<typeof getOutcomeAggregation>,
+    {}
+  >
+) {
+  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<typeof getOutcomeAggregation> };
+    },
+    {}
+  >['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
@@ -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
@@ -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<typeof outcomes, {}>
-  ) {
-    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
@@ -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,28 +74,16 @@ export async function getErrorRate({
       size: 0,
       query: { bool: { filter } },
       aggs: {
-        total_transactions: {
+        outcomes,
+        timeseries: {
           date_histogram: {
             field: '@timestamp',
             fixed_interval: getBucketSize(start, end).intervalString,
             min_doc_count: 0,
             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
@@ -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();
         });
       });
     });
