[OBS]home page is showing incorrect value of APM throughput (tpm) (#9…

…5991) (#96217) * fixing obs transaction per minute value * addressing PR comments * fixing unit test * addressing PR comments Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Cauê Marcondes <[email protected]>
elastic · Apr 5, 2021 · e3d0830 · e3d0830
1 parent 328cc41
commit e3d0830
Show file tree

Hide file tree

Showing 9 changed files with 166 additions and 95 deletions.
diff --git a/x-pack/plugins/apm/public/services/rest/apm_observability_overview_fetchers.test.ts b/x-pack/plugins/apm/public/services/rest/apm_observability_overview_fetchers.test.ts
@@ -46,11 +46,14 @@ describe('Observability dashboard data', () => {
       callApmApiMock.mockImplementation(() =>
         Promise.resolve({
           serviceCount: 10,
-          transactionCoordinates: [
-            { x: 1, y: 1 },
-            { x: 2, y: 2 },
-            { x: 3, y: 3 },
-          ],
+          transactionPerMinute: {
+            value: 2,
+            timeseries: [
+              { x: 1, y: 1 },
+              { x: 2, y: 2 },
+              { x: 3, y: 3 },
+            ],
+          },
         })
       );
       const response = await fetchObservabilityOverviewPageData(params);
@@ -81,7 +84,7 @@ describe('Observability dashboard data', () => {
       callApmApiMock.mockImplementation(() =>
         Promise.resolve({
           serviceCount: 0,
-          transactionCoordinates: [],
+          transactionPerMinute: { value: null, timeseries: [] },
         })
       );
       const response = await fetchObservabilityOverviewPageData(params);
@@ -108,7 +111,10 @@ describe('Observability dashboard data', () => {
       callApmApiMock.mockImplementation(() =>
         Promise.resolve({
           serviceCount: 0,
-          transactionCoordinates: [{ x: 1 }, { x: 2 }, { x: 3 }],
+          transactionPerMinute: {
+            value: 0,
+            timeseries: [{ x: 1 }, { x: 2 }, { x: 3 }],
+          },
         })
       );
       const response = await fetchObservabilityOverviewPageData(params);

diff --git a/x-pack/plugins/apm/public/services/rest/apm_observability_overview_fetchers.ts b/x-pack/plugins/apm/public/services/rest/apm_observability_overview_fetchers.ts
@@ -5,7 +5,6 @@
  * 2.0.
  */
 
-import { mean } from 'lodash';
 import {
   ApmFetchDataResponse,
   FetchDataParams,
@@ -31,7 +30,7 @@ export const fetchObservabilityOverviewPageData = async ({
     },
   });
 
-  const { serviceCount, transactionCoordinates } = data;
+  const { serviceCount, transactionPerMinute } = data;
 
   return {
     appLink: `/app/apm/services?rangeFrom=${relativeTime.start}&rangeTo=${relativeTime.end}`,
@@ -42,17 +41,12 @@ export const fetchObservabilityOverviewPageData = async ({
       },
       transactions: {
         type: 'number',
-        value:
-          mean(
-            transactionCoordinates
-              .map(({ y }) => y)
-              .filter((y) => y && isFinite(y))
-          ) || 0,
+        value: transactionPerMinute.value || 0,
       },
     },
     series: {
       transactions: {
-        coordinates: transactionCoordinates,
+        coordinates: transactionPerMinute.timeseries,
       },
     },
   };

diff --git a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts
diff --git a/x-pack/plugins/apm/server/lib/observability_overview/get_transactions_per_minute.ts b/x-pack/plugins/apm/server/lib/observability_overview/get_transactions_per_minute.ts
@@ -0,0 +1,95 @@
+/*
+ * 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 {
+  TRANSACTION_PAGE_LOAD,
+  TRANSACTION_REQUEST,
+} from '../../../common/transaction_types';
+import { TRANSACTION_TYPE } from '../../../common/elasticsearch_fieldnames';
+import { rangeQuery } from '../../../server/utils/queries';
+import { Setup, SetupTimeRange } from '../helpers/setup_request';
+import { getProcessorEventForAggregatedTransactions } from '../helpers/aggregated_transactions';
+import { calculateThroughput } from '../helpers/calculate_throughput';
+import { withApmSpan } from '../../utils/with_apm_span';
+
+export function getTransactionsPerMinute({
+  setup,
+  bucketSize,
+  searchAggregatedTransactions,
+}: {
+  setup: Setup & SetupTimeRange;
+  bucketSize: string;
+  searchAggregatedTransactions: boolean;
+}) {
+  return withApmSpan(
+    'observability_overview_get_transactions_per_minute',
+    async () => {
+      const { apmEventClient, start, end } = setup;
+
+      const { aggregations } = await apmEventClient.search({
+        apm: {
+          events: [
+            getProcessorEventForAggregatedTransactions(
+              searchAggregatedTransactions
+            ),
+          ],
+        },
+        body: {
+          size: 0,
+          query: {
+            bool: {
+              filter: rangeQuery(start, end),
+            },
+          },
+          aggs: {
+            transactionType: {
+              terms: {
+                field: TRANSACTION_TYPE,
+              },
+              aggs: {
+                timeseries: {
+                  date_histogram: {
+                    field: '@timestamp',
+                    fixed_interval: bucketSize,
+                    min_doc_count: 0,
+                  },
+                  aggs: {
+                    throughput: { rate: { unit: 'minute' as const } },
+                  },
+                },
+              },
+            },
+          },
+        },
+      });
+
+      if (!aggregations || !aggregations.transactionType.buckets) {
+        return { value: undefined, timeseries: [] };
+      }
+
+      const topTransactionTypeBucket =
+        aggregations.transactionType.buckets.find(
+          ({ key: transactionType }) =>
+            transactionType === TRANSACTION_REQUEST ||
+            transactionType === TRANSACTION_PAGE_LOAD
+        ) || aggregations.transactionType.buckets[0];
+
+      return {
+        value: calculateThroughput({
+          start,
+          end,
+          value: topTransactionTypeBucket?.doc_count || 0,
+        }),
+        timeseries:
+          topTransactionTypeBucket?.timeseries.buckets.map((bucket) => ({
+            x: bucket.key,
+            y: bucket.throughput.value,
+          })) || [],
+      };
+    }
+  );
+}
diff --git a/x-pack/plugins/apm/server/routes/observability_overview.ts b/x-pack/plugins/apm/server/routes/observability_overview.ts
@@ -8,7 +8,7 @@
 import * as t from 'io-ts';
 import { setupRequest } from '../lib/helpers/setup_request';
 import { getServiceCount } from '../lib/observability_overview/get_service_count';
-import { getTransactionCoordinates } from '../lib/observability_overview/get_transaction_coordinates';
+import { getTransactionsPerMinute } from '../lib/observability_overview/get_transactions_per_minute';
 import { getHasData } from '../lib/observability_overview/has_data';
 import { createRoute } from './create_route';
 import { rangeRt } from './default_api_types';
@@ -39,18 +39,18 @@ export const observabilityOverviewRoute = createRoute({
     );
 
     return withApmSpan('observability_overview', async () => {
-      const [serviceCount, transactionCoordinates] = await Promise.all([
+      const [serviceCount, transactionPerMinute] = await Promise.all([
         getServiceCount({
           setup,
           searchAggregatedTransactions,
         }),
-        getTransactionCoordinates({
+        getTransactionsPerMinute({
           setup,
           bucketSize,
           searchAggregatedTransactions,
         }),
       ]);
-      return { serviceCount, transactionCoordinates };
+      return { serviceCount, transactionPerMinute };
     });
   },
 });
diff --git a/x-pack/plugins/observability/public/components/app/section/apm/index.test.tsx b/x-pack/plugins/observability/public/components/app/section/apm/index.test.tsx
@@ -56,6 +56,32 @@ describe('APMSection', () => {
       } as unknown) as ObservabilityPublicPluginsStart,
     }));
   });
+
+  it('renders transaction stat less then 1k', () => {
+    const resp = {
+      appLink: '/app/apm',
+      stats: {
+        services: { value: 11, type: 'number' },
+        transactions: { value: 900, type: 'number' },
+      },
+      series: {
+        transactions: { coordinates: [] },
+      },
+    };
+    jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
+      data: resp,
+      status: fetcherHook.FETCH_STATUS.SUCCESS,
+      refetch: jest.fn(),
+    });
+    const { getByText, queryAllByTestId } = render(<APMSection bucketSize="60s" />);
+
+    expect(getByText('APM')).toBeInTheDocument();
+    expect(getByText('View in app')).toBeInTheDocument();
+    expect(getByText('Services 11')).toBeInTheDocument();
+    expect(getByText('Throughput 900.0 tpm')).toBeInTheDocument();
+    expect(queryAllByTestId('loading')).toEqual([]);
+  });
+
   it('renders with transaction series and stats', () => {
     jest.spyOn(fetcherHook, 'useFetcher').mockReturnValue({
       data: response,

diff --git a/x-pack/plugins/observability/public/components/app/section/apm/index.tsx b/x-pack/plugins/observability/public/components/app/section/apm/index.tsx
@@ -31,6 +31,19 @@ function formatTpm(value?: number) {
   return numeral(value).format('0.00a');
 }
 
+function formatTpmStat(value?: number) {
+  if (!value || value === 0) {
+    return '0';
+  }
+  if (value <= 0.1) {
+    return '< 0.1';
+  }
+  if (value > 1000) {
+    return numeral(value).format('0.00a');
+  }
+  return numeral(value).format('0,0.0');
+}
+
 export function APMSection({ bucketSize }: Props) {
   const theme = useContext(ThemeContext);
   const chartTheme = useChartTheme();
@@ -93,7 +106,7 @@ export function APMSection({ bucketSize }: Props) {
         </EuiFlexItem>
         <EuiFlexItem grow={false}>
           <StyledStat
-            title={`${formatTpm(stats?.transactions.value)} tpm`}
+            title={`${formatTpmStat(stats?.transactions.value)} tpm`}
             description={i18n.translate('xpack.observability.overview.apm.throughput', {
               defaultMessage: 'Throughput',
             })}

diff --git a/x-pack/plugins/observability/public/typings/fetch_overview_data/index.ts b/x-pack/plugins/observability/public/typings/fetch_overview_data/index.ts
@@ -15,7 +15,7 @@ export interface Stat {
 
 export interface Coordinates {
   x: number;
-  y?: number;
+  y?: number | null;
 }
 
 export interface Series {

diff --git a/x-pack/test/apm_api_integration/tests/observability_overview/observability_overview.ts b/x-pack/test/apm_api_integration/tests/observability_overview/observability_overview.ts
@@ -33,7 +33,7 @@ export default function ApiTest({ getService }: FtrProviderContext) {
           expect(response.status).to.be(200);
 
           expect(response.body.serviceCount).to.be(0);
-          expect(response.body.transactionCoordinates.length).to.be(0);
+          expect(response.body.transactionPerMinute.timeseries.length).to.be(0);
         });
       });
     }
@@ -50,14 +50,15 @@ export default function ApiTest({ getService }: FtrProviderContext) {
         expect(response.status).to.be(200);
 
         expect(response.body.serviceCount).to.be.greaterThan(0);
-        expect(response.body.transactionCoordinates.length).to.be.greaterThan(0);
+        expect(response.body.transactionPerMinute.timeseries.length).to.be.greaterThan(0);
 
         expectSnapshot(response.body.serviceCount).toMatchInline(`9`);
 
-        expectSnapshot(response.body.transactionCoordinates.length).toMatchInline(`31`);
+        expectSnapshot(response.body.transactionPerMinute.value).toMatchInline(`64.8`);
+        expectSnapshot(response.body.transactionPerMinute.timeseries.length).toMatchInline(`31`);
 
         expectSnapshot(
-          response.body.transactionCoordinates
+          response.body.transactionPerMinute.timeseries
             .slice(0, 5)
             .map(({ x, y }: { x: number; y: number }) => ({
               x: new Date(x).toISOString(),
@@ -67,23 +68,23 @@ export default function ApiTest({ getService }: FtrProviderContext) {
           Array [
             Object {
               "x": "2020-12-08T13:57:00.000Z",
-              "y": 0.166666666666667,
+              "y": 2,
             },
             Object {
               "x": "2020-12-08T13:58:00.000Z",
-              "y": 5.23333333333333,
+              "y": 61,
             },
             Object {
               "x": "2020-12-08T13:59:00.000Z",
-              "y": 4.4,
+              "y": 36,
             },
             Object {
               "x": "2020-12-08T14:00:00.000Z",
-              "y": 5.73333333333333,
+              "y": 75,
             },
             Object {
               "x": "2020-12-08T14:01:00.000Z",
-              "y": 4.33333333333333,
+              "y": 36,
             },
           ]
         `);