From b72b2d1e495ac5bf9b38ed7e92468ce18a53f439 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Wed, 31 Mar 2021 17:03:47 -0400 Subject: [PATCH 1/4] fixing obs transaction per minute value --- .../apm_observability_overview_fetchers.ts | 12 +--- ...nates.ts => get_transaction_per_minute.ts} | 55 ++++++++++++++----- .../server/routes/observability_overview.ts | 8 +-- .../components/app/section/apm/index.tsx | 12 +++- 4 files changed, 60 insertions(+), 27 deletions(-) rename x-pack/plugins/apm/server/lib/observability_overview/{get_transaction_coordinates.ts => get_transaction_per_minute.ts} (50%) 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 index 55ead8d942aca..3a02efd05e5a5 100644 --- 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_per_minute.ts similarity index 50% rename from x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts rename to x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts index aac18e2bdfe4c..8e2e4e4d6ddab 100644 --- a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_coordinates.ts +++ b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts @@ -5,6 +5,11 @@ * 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 { Coordinates } from '../../../../observability/typings/common'; import { Setup, SetupTimeRange } from '../helpers/setup_request'; @@ -12,7 +17,7 @@ import { getProcessorEventForAggregatedTransactions } from '../helpers/aggregate import { calculateThroughput } from '../helpers/calculate_throughput'; import { withApmSpan } from '../../utils/with_apm_span'; -export function getTransactionCoordinates({ +export function getTransactionPerMinute({ setup, bucketSize, searchAggregatedTransactions, @@ -20,7 +25,7 @@ export function getTransactionCoordinates({ setup: Setup & SetupTimeRange; bucketSize: string; searchAggregatedTransactions: boolean; -}): Promise { +}): Promise<{ value: number | undefined; timeseries: Coordinates[] }> { return withApmSpan( 'observability_overview_get_transaction_distribution', async () => { @@ -42,23 +47,47 @@ export function getTransactionCoordinates({ }, }, aggs: { - distribution: { - date_histogram: { - field: '@timestamp', - fixed_interval: bucketSize, - min_doc_count: 0, + transactionType: { + terms: { + field: TRANSACTION_TYPE, + }, + aggs: { + timeseries: { + date_histogram: { + field: '@timestamp', + fixed_interval: bucketSize, + min_doc_count: 0, + }, + }, }, }, }, }, }); - return ( - aggregations?.distribution.buckets.map((bucket) => ({ - x: bucket.key, - y: calculateThroughput({ start, end, value: bucket.doc_count }), - })) || [] - ); + 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: calculateThroughput({ start, end, value: bucket.doc_count }), + })) || [], + }; } ); } diff --git a/x-pack/plugins/apm/server/routes/observability_overview.ts b/x-pack/plugins/apm/server/routes/observability_overview.ts index b9c0a76b6fb90..1f5e2c497c110 100644 --- 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 { getTransactionPerMinute } from '../lib/observability_overview/get_transaction_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({ + getTransactionPerMinute({ setup, bucketSize, searchAggregatedTransactions, }), ]); - return { serviceCount, transactionCoordinates }; + return { serviceCount, transactionPerMinute }; }); }, }); 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 index 91a536840ecbd..84cce68dd420f 100644 --- 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,16 @@ 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'; + } + return numeral(value).format('0,0.0'); +} + export function APMSection({ bucketSize }: Props) { const theme = useContext(ThemeContext); const chartTheme = useChartTheme(); @@ -93,7 +103,7 @@ export function APMSection({ bucketSize }: Props) { Date: Thu, 1 Apr 2021 09:55:58 -0400 Subject: [PATCH 2/4] addressing PR comments --- .../get_transaction_per_minute.ts | 8 +++--- .../components/app/section/apm/index.test.tsx | 26 +++++++++++++++++++ .../components/app/section/apm/index.tsx | 3 +++ .../typings/fetch_overview_data/index.ts | 2 +- 4 files changed, 35 insertions(+), 4 deletions(-) diff --git a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts index 8e2e4e4d6ddab..8fe0ab1884819 100644 --- a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts +++ b/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts @@ -11,7 +11,6 @@ import { } from '../../../common/transaction_types'; import { TRANSACTION_TYPE } from '../../../common/elasticsearch_fieldnames'; import { rangeQuery } from '../../../server/utils/queries'; -import { Coordinates } from '../../../../observability/typings/common'; import { Setup, SetupTimeRange } from '../helpers/setup_request'; import { getProcessorEventForAggregatedTransactions } from '../helpers/aggregated_transactions'; import { calculateThroughput } from '../helpers/calculate_throughput'; @@ -25,7 +24,7 @@ export function getTransactionPerMinute({ setup: Setup & SetupTimeRange; bucketSize: string; searchAggregatedTransactions: boolean; -}): Promise<{ value: number | undefined; timeseries: Coordinates[] }> { +}) { return withApmSpan( 'observability_overview_get_transaction_distribution', async () => { @@ -58,6 +57,9 @@ export function getTransactionPerMinute({ fixed_interval: bucketSize, min_doc_count: 0, }, + aggs: { + throughput: { rate: { unit: 'minute' as const } }, + }, }, }, }, @@ -85,7 +87,7 @@ export function getTransactionPerMinute({ timeseries: topTransactionTypeBucket?.timeseries.buckets.map((bucket) => ({ x: bucket.key, - y: calculateThroughput({ start, end, value: bucket.doc_count }), + y: bucket.throughput.value, })) || [], }; } 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 index e5f100be285e1..d29481a39eb72 100644 --- 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(); + + 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 index 84cce68dd420f..e71468d3b028c 100644 --- a/x-pack/plugins/observability/public/components/app/section/apm/index.tsx +++ b/x-pack/plugins/observability/public/components/app/section/apm/index.tsx @@ -38,6 +38,9 @@ function formatTpmStat(value?: number) { if (value <= 0.1) { return '< 0.1'; } + if (value > 1000) { + return numeral(value).format('0.00a'); + } return numeral(value).format('0,0.0'); } 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 index f4e824784fe09..e9960833a1c4f 100644 --- 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 { From dd8c4663c14e58ae5c6ecad01ebcc98da5e22f6b Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 2 Apr 2021 12:21:12 -0400 Subject: [PATCH 3/4] fixing unit test --- ...pm_observability_overview_fetchers.test.ts | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 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 index 1821e92ee5a78..29fabc51fd582 100644 --- 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); From 31ee141da7c06c29bf6bbcbf288d64b5ac2b8ade Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 2 Apr 2021 14:21:58 -0400 Subject: [PATCH 4/4] addressing PR comments --- ...nute.ts => get_transactions_per_minute.ts} | 4 ++-- .../server/routes/observability_overview.ts | 4 ++-- .../observability_overview.ts | 19 ++++++++++--------- 3 files changed, 14 insertions(+), 13 deletions(-) rename x-pack/plugins/apm/server/lib/observability_overview/{get_transaction_per_minute.ts => get_transactions_per_minute.ts} (96%) diff --git a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts b/x-pack/plugins/apm/server/lib/observability_overview/get_transactions_per_minute.ts similarity index 96% rename from x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts rename to x-pack/plugins/apm/server/lib/observability_overview/get_transactions_per_minute.ts index 8fe0ab1884819..da8ac7c50b594 100644 --- a/x-pack/plugins/apm/server/lib/observability_overview/get_transaction_per_minute.ts +++ b/x-pack/plugins/apm/server/lib/observability_overview/get_transactions_per_minute.ts @@ -16,7 +16,7 @@ import { getProcessorEventForAggregatedTransactions } from '../helpers/aggregate import { calculateThroughput } from '../helpers/calculate_throughput'; import { withApmSpan } from '../../utils/with_apm_span'; -export function getTransactionPerMinute({ +export function getTransactionsPerMinute({ setup, bucketSize, searchAggregatedTransactions, @@ -26,7 +26,7 @@ export function getTransactionPerMinute({ searchAggregatedTransactions: boolean; }) { return withApmSpan( - 'observability_overview_get_transaction_distribution', + 'observability_overview_get_transactions_per_minute', async () => { const { apmEventClient, start, end } = setup; diff --git a/x-pack/plugins/apm/server/routes/observability_overview.ts b/x-pack/plugins/apm/server/routes/observability_overview.ts index 1f5e2c497c110..1aac2c09d01c5 100644 --- 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 { getTransactionPerMinute } from '../lib/observability_overview/get_transaction_per_minute'; +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'; @@ -44,7 +44,7 @@ export const observabilityOverviewRoute = createRoute({ setup, searchAggregatedTransactions, }), - getTransactionPerMinute({ + getTransactionsPerMinute({ setup, bucketSize, searchAggregatedTransactions, 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 index 2af5f51953b23..746e746ccd827 100644 --- 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, }, ] `);