From 7695ae92582e9861c5a3b0acadae7d114394f0d0 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Sat, 13 Feb 2021 14:49:52 -0500 Subject: [PATCH 01/16] adding time comparison to latency chart --- .../shared/charts/latency_chart/index.tsx | 21 +- .../use_transaction_latency_chart_fetcher.ts | 25 +- .../selectors/latency_chart_selector.test.ts | 128 ++++----- .../selectors/latency_chart_selectors.ts | 43 ++- .../transactions/get_latency_charts/index.ts | 18 +- .../plugins/apm/server/routes/transactions.ts | 37 ++- .../transactions/__snapshots__/latency.snap | 168 ++++++++++- .../tests/transactions/latency.ts | 263 ++++++++++++++---- 8 files changed, 564 insertions(+), 139 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx index 3cccf543c3e60..659360b80d80c 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx @@ -13,6 +13,7 @@ import { LatencyAggregationType } from '../../../../../common/latency_aggregatio import { getDurationFormatter } from '../../../../../common/utils/formatters'; import { useLicenseContext } from '../../../../context/license/use_license_context'; import { useUrlParams } from '../../../../context/url_params_context/use_url_params'; +import { useTheme } from '../../../../hooks/use_theme'; import { useTransactionLatencyChartsFetcher } from '../../../../hooks/use_transaction_latency_chart_fetcher'; import { TimeseriesChart } from '../../../shared/charts/timeseries_chart'; import { @@ -21,6 +22,7 @@ import { } from '../../../shared/charts/transaction_charts/helper'; import { MLHeader } from '../../../shared/charts/transaction_charts/ml_header'; import * as urlHelpers from '../../../shared/Links/url_helpers'; +import { getComparisonChartTheme } from '../../time_comparison/get_time_range_comparison'; interface Props { height?: number; @@ -34,8 +36,10 @@ const options: Array<{ value: LatencyAggregationType; text: string }> = [ export function LatencyChart({ height }: Props) { const history = useHistory(); + const theme = useTheme(); + const comparisonChartTheme = getComparisonChartTheme(theme); const { urlParams } = useUrlParams(); - const { latencyAggregationType } = urlParams; + const { latencyAggregationType, comparisonEnabled } = urlParams; const license = useLicenseContext(); const { @@ -43,9 +47,14 @@ export function LatencyChart({ height }: Props) { latencyChartsStatus, } = useTransactionLatencyChartsFetcher(); - const { latencyTimeseries, anomalyTimeseries, mlJobId } = latencyChartsData; + const { + currentPeriod, + previousPeriod, + anomalyTimeseries, + mlJobId, + } = latencyChartsData; - const latencyMaxY = getMaxY(latencyTimeseries); + const latencyMaxY = getMaxY(currentPeriod); const latencyFormatter = getDurationFormatter(latencyMaxY); return ( @@ -99,7 +108,11 @@ export function LatencyChart({ height }: Props) { height={height} fetchStatus={latencyChartsStatus} id="latencyChart" - timeseries={latencyTimeseries} + customTheme={comparisonChartTheme} + timeseries={[ + ...currentPeriod, + ...(comparisonEnabled && previousPeriod ? [previousPeriod] : []), + ]} yLabelFormat={getResponseTimeTickFormatter(latencyFormatter)} anomalyTimeseries={anomalyTimeseries} /> diff --git a/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts b/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts index d5974ee3543a7..b1dbd09067683 100644 --- a/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts +++ b/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts @@ -12,16 +12,35 @@ import { useUrlParams } from '../context/url_params_context/use_url_params'; import { useApmServiceContext } from '../context/apm_service/use_apm_service_context'; import { getLatencyChartSelector } from '../selectors/latency_chart_selectors'; import { useTheme } from './use_theme'; +import { LatencyAggregationType } from '../../common/latency_aggregation_types'; +import { getTimeRangeComparison } from '../components/shared/time_comparison/get_time_range_comparison'; export function useTransactionLatencyChartsFetcher() { const { serviceName } = useParams<{ serviceName?: string }>(); const { transactionType } = useApmServiceContext(); const theme = useTheme(); const { - urlParams: { start, end, transactionName, latencyAggregationType }, + urlParams: { + start, + end, + transactionName, + latencyAggregationType, + comparisonType, + }, uiFilters, } = useUrlParams(); + const { + comparisonStart = undefined, + comparisonEnd = undefined, + } = comparisonType + ? getTimeRangeComparison({ + start, + end, + comparisonType, + }) + : {}; + const { data, error, status } = useFetcher( (callApmApi) => { if ( @@ -43,6 +62,8 @@ export function useTransactionLatencyChartsFetcher() { transactionName, uiFilters: JSON.stringify(uiFilters), latencyAggregationType, + comparisonStart, + comparisonEnd, }, }, }); @@ -56,6 +77,8 @@ export function useTransactionLatencyChartsFetcher() { transactionType, uiFilters, latencyAggregationType, + comparisonStart, + comparisonEnd, ] ); diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts index d4c9ba0878f28..ca21f6989ea81 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts @@ -18,12 +18,19 @@ const theme = { euiColorVis5: 'red', euiColorVis7: 'black', euiColorVis9: 'yellow', + euiColorLightestShade: 'green', }, } as EuiTheme; const latencyChartData = { - overallAvgDuration: 1, - latencyTimeseries: [{ x: 1, y: 10 }], + currentPeriod: { + overallAvgDuration: 1, + latencyTimeseries: [{ x: 1, y: 10 }], + }, + previousPeriod: { + overallAvgDuration: 1, + latencyTimeseries: [{ x: 1, y: 10 }], + }, anomalyTimeseries: { jobId: '1', anomalyBoundaries: [{ x: 1, y: 2, y0: 1 }], @@ -36,21 +43,21 @@ describe('getLatencyChartSelector', () => { it('returns default values when data is undefined', () => { const latencyChart = getLatencyChartSelector({ theme }); expect(latencyChart).toEqual({ - latencyTimeseries: [], + currentPeriod: [], mlJobId: undefined, anomalyTimeseries: undefined, }); }); it('returns average timeseries', () => { - const { anomalyTimeseries, ...latencyWithouAnomaly } = latencyChartData; + const { anomalyTimeseries, ...latencyWithoutAnomaly } = latencyChartData; const latencyTimeseries = getLatencyChartSelector({ - latencyChart: latencyWithouAnomaly as LatencyChartsResponse, + latencyChart: latencyWithoutAnomaly as LatencyChartsResponse, theme, latencyAggregationType: LatencyAggregationType.avg, }); expect(latencyTimeseries).toEqual({ - latencyTimeseries: [ + currentPeriod: [ { title: 'Average', data: [{ x: 1, y: 10 }], @@ -59,46 +66,65 @@ describe('getLatencyChartSelector', () => { color: 'blue', }, ], + previousPeriod: { + color: 'green', + data: [{ x: 1, y: 10 }], + type: 'area', + title: 'Previous period', + }, }); }); it('returns 95th percentile timeseries', () => { - const { anomalyTimeseries, ...latencyWithouAnomaly } = latencyChartData; + const { anomalyTimeseries, ...latencyWithoutAnomaly } = latencyChartData; const latencyTimeseries = getLatencyChartSelector({ - latencyChart: latencyWithouAnomaly as LatencyChartsResponse, + latencyChart: latencyWithoutAnomaly as LatencyChartsResponse, theme, latencyAggregationType: LatencyAggregationType.p95, }); expect(latencyTimeseries).toEqual({ - latencyTimeseries: [ + currentPeriod: [ { title: '95th percentile', - data: [{ x: 1, y: 10 }], titleShort: '95th', + data: [{ x: 1, y: 10 }], type: 'linemark', color: 'red', }, ], + previousPeriod: { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, }); }); it('returns 99th percentile timeseries', () => { - const { anomalyTimeseries, ...latencyWithouAnomaly } = latencyChartData; + const { anomalyTimeseries, ...latencyWithoutAnomaly } = latencyChartData; const latencyTimeseries = getLatencyChartSelector({ - latencyChart: latencyWithouAnomaly as LatencyChartsResponse, + latencyChart: latencyWithoutAnomaly as LatencyChartsResponse, theme, latencyAggregationType: LatencyAggregationType.p99, }); + expect(latencyTimeseries).toEqual({ - latencyTimeseries: [ + currentPeriod: [ { title: '99th percentile', - data: [{ x: 1, y: 10 }], titleShort: '99th', + data: [{ x: 1, y: 10 }], type: 'linemark', color: 'black', }, ], + previousPeriod: { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, }); }); }); @@ -111,76 +137,54 @@ describe('getLatencyChartSelector', () => { latencyAggregationType: LatencyAggregationType.p99, }); expect(latencyTimeseries).toEqual({ + currentPeriod: [ + { + title: '99th percentile', + titleShort: '99th', + data: [{ x: 1, y: 10 }], + type: 'linemark', + color: 'black', + }, + ], + previousPeriod: { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, + mlJobId: '1', anomalyTimeseries: { boundaries: [ { - color: 'rgba(0,0,0,0)', - areaSeriesStyle: { - point: { - opacity: 0, - }, - }, - data: [ - { - x: 1, - y: 1, - }, - ], + type: 'area', fit: 'lookahead', hideLegend: true, hideTooltipValue: true, stackAccessors: ['y'], + areaSeriesStyle: { point: { opacity: 0 } }, title: 'anomalyBoundariesLower', - type: 'area', + data: [{ x: 1, y: 1 }], + color: 'rgba(0,0,0,0)', }, { - color: 'rgba(0,0,255,0.5)', - areaSeriesStyle: { - point: { - opacity: 0, - }, - }, - data: [ - { - x: 1, - y: 1, - }, - ], + type: 'area', fit: 'lookahead', hideLegend: true, hideTooltipValue: true, stackAccessors: ['y'], + areaSeriesStyle: { point: { opacity: 0 } }, title: 'anomalyBoundariesUpper', - type: 'area', + data: [{ x: 1, y: 1 }], + color: 'rgba(0,0,255,0.5)', }, ], scores: { - color: 'yellow', - data: [ - { - x: 1, - x0: 2, - }, - ], title: 'anomalyScores', type: 'rectAnnotation', + data: [{ x: 1, x0: 2 }], + color: 'yellow', }, }, - latencyTimeseries: [ - { - color: 'black', - data: [ - { - x: 1, - y: 10, - }, - ], - title: '99th percentile', - titleShort: '99th', - type: 'linemark', - }, - ], - mlJobId: '1', }); }); }); diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts index 858d44de8bb7a..ecf5dcef67dfb 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts @@ -15,8 +15,9 @@ import { APIReturnType } from '../services/rest/createCallApmApi'; export type LatencyChartsResponse = APIReturnType<'GET /api/apm/services/{serviceName}/transactions/charts/latency'>; -export interface LatencyChartData { - latencyTimeseries: Array>; +interface LatencyChartData { + currentPeriod: Array>; + previousPeriod?: APMChartSpec; mlJobId?: string; anomalyTimeseries?: { boundaries: APMChartSpec[]; scores: APMChartSpec }; } @@ -30,19 +31,27 @@ export function getLatencyChartSelector({ theme: EuiTheme; latencyAggregationType?: string; }): LatencyChartData { - if (!latencyChart?.latencyTimeseries || !latencyAggregationType) { + if ( + !latencyChart?.currentPeriod.latencyTimeseries || + !latencyAggregationType + ) { return { - latencyTimeseries: [], + currentPeriod: [], + previousPeriod: undefined, mlJobId: undefined, anomalyTimeseries: undefined, }; } return { - latencyTimeseries: getLatencyTimeseries({ - latencyChart, + currentPeriod: getLatencyTimeseries({ + latencyChart: latencyChart.currentPeriod, theme, latencyAggregationType, }), + previousPeriod: getPreviousPeriodTimeseries({ + previousPeriod: latencyChart.previousPeriod, + theme, + }), mlJobId: latencyChart.anomalyTimeseries?.jobId, anomalyTimeseries: getAnomalyTimeseries({ anomalyTimeseries: latencyChart.anomalyTimeseries, @@ -51,12 +60,32 @@ export function getLatencyChartSelector({ }; } +function getPreviousPeriodTimeseries({ + previousPeriod, + theme, +}: { + previousPeriod: LatencyChartsResponse['previousPeriod']; + theme: EuiTheme; +}) { + return { + data: previousPeriod.latencyTimeseries ?? [], + type: 'area', + color: theme.eui.euiColorLightestShade, + title: i18n.translate( + 'xpack.apm.serviceOverview.latencyChartTitle.previousPeriodLabel', + { + defaultMessage: 'Previous period', + } + ), + }; +} + function getLatencyTimeseries({ latencyChart, theme, latencyAggregationType, }: { - latencyChart: LatencyChartsResponse; + latencyChart: LatencyChartsResponse['currentPeriod']; theme: EuiTheme; latencyAggregationType: string; }) { diff --git a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts index ee27d00fdc0d4..df38d575a1eff 100644 --- a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts +++ b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts @@ -20,7 +20,7 @@ import { getTransactionDurationFieldForAggregatedTransactions, } from '../../../lib/helpers/aggregated_transactions'; import { getBucketSize } from '../../../lib/helpers/get_bucket_size'; -import { Setup, SetupTimeRange } from '../../../lib/helpers/setup_request'; +import { Setup } from '../../../lib/helpers/setup_request'; import { withApmSpan } from '../../../utils/with_apm_span'; import { getLatencyAggregation, @@ -37,15 +37,19 @@ function searchLatency({ setup, searchAggregatedTransactions, latencyAggregationType, + start, + end, }: { serviceName: string; transactionType: string | undefined; transactionName: string | undefined; - setup: Setup & SetupTimeRange; + setup: Setup; searchAggregatedTransactions: boolean; latencyAggregationType: LatencyAggregationType; + start: number; + end: number; }) { - const { start, end, apmEventClient } = setup; + const { apmEventClient } = setup; const { intervalString } = getBucketSize({ start, end }); const filter: ESFilter[] = [ @@ -108,13 +112,17 @@ export function getLatencyTimeseries({ setup, searchAggregatedTransactions, latencyAggregationType, + start, + end, }: { serviceName: string; transactionType: string | undefined; transactionName: string | undefined; - setup: Setup & SetupTimeRange; + setup: Setup; searchAggregatedTransactions: boolean; latencyAggregationType: LatencyAggregationType; + start: number; + end: number; }) { return withApmSpan('get_latency_charts', async () => { const response = await searchLatency({ @@ -124,6 +132,8 @@ export function getLatencyTimeseries({ setup, searchAggregatedTransactions, latencyAggregationType, + start, + end, }); if (!response.aggregations) { diff --git a/x-pack/plugins/apm/server/routes/transactions.ts b/x-pack/plugins/apm/server/routes/transactions.ts index bef96cb7f0767..5f154b0adccaa 100644 --- a/x-pack/plugins/apm/server/routes/transactions.ts +++ b/x-pack/plugins/apm/server/routes/transactions.ts @@ -24,8 +24,9 @@ import { getLatencyTimeseries } from '../lib/transactions/get_latency_charts'; import { getThroughputCharts } from '../lib/transactions/get_throughput_charts'; import { getTransactionGroupList } from '../lib/transaction_groups'; import { getErrorRate } from '../lib/transaction_groups/get_error_rate'; +import { offsetPreviousPeriodCoordinates } from '../utils/offset_previous_period_coordinate'; import { createRoute } from './create_route'; -import { rangeRt, uiFiltersRt } from './default_api_types'; +import { comparisonRangeRt, rangeRt, uiFiltersRt } from './default_api_types'; /** * Returns a list of transactions grouped by name @@ -168,6 +169,7 @@ export const transactionLatencyChatsRoute = createRoute({ }), uiFiltersRt, rangeRt, + comparisonRangeRt, ]), }), options: { tags: ['access:apm'] }, @@ -179,6 +181,8 @@ export const transactionLatencyChatsRoute = createRoute({ transactionType, transactionName, latencyAggregationType, + comparisonStart, + comparisonEnd, } = context.params.query; if (!setup.uiFilters.environment) { @@ -191,6 +195,8 @@ export const transactionLatencyChatsRoute = createRoute({ setup ); + const { start, end } = setup; + const options = { serviceName, transactionType, @@ -200,9 +206,15 @@ export const transactionLatencyChatsRoute = createRoute({ logger, }; - const [latencyData, anomalyTimeseries] = await Promise.all([ + const [ + currentPeriod, + anomalyTimeseries, + previousPeriod, + ] = await Promise.all([ getLatencyTimeseries({ ...options, + start, + end, latencyAggregationType: latencyAggregationType as LatencyAggregationType, }), getAnomalySeries(options).catch((error) => { @@ -210,13 +222,26 @@ export const transactionLatencyChatsRoute = createRoute({ logger.error(error); return undefined; }), + comparisonStart && comparisonEnd + ? getLatencyTimeseries({ + ...options, + start: comparisonStart, + end: comparisonEnd, + latencyAggregationType: latencyAggregationType as LatencyAggregationType, + }).then((latency) => ({ + ...latency, + latencyTimeseries: offsetPreviousPeriodCoordinates({ + currentPeriodStart: start, + previousPeriodStart: comparisonStart, + previousPeriodTimeseries: latency.latencyTimeseries, + }), + })) + : { latencyTimeseries: [], overallAvgDuration: null }, ]); - const { latencyTimeseries, overallAvgDuration } = latencyData; - return { - latencyTimeseries, - overallAvgDuration, + currentPeriod, + previousPeriod, anomalyTimeseries, }; }, diff --git a/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap b/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap index a384ca2c9364e..fce8faea966a3 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap +++ b/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap @@ -1,5 +1,171 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`APM API tests basic apm_8.0.0 Latency with a basic license when data is loaded time comparison returns some data 1`] = ` +Array [ + Object { + "x": 1607436780000, + "y": 51029, + }, + Object { + "x": 1607436820000, + "y": 38124, + }, + Object { + "x": 1607436860000, + "y": 16327, + }, + Object { + "x": 1607436980000, + "y": 35617.5, + }, + Object { + "x": 1607436990000, + "y": 34599.75, + }, + Object { + "x": 1607437100000, + "y": 26980, + }, + Object { + "x": 1607437110000, + "y": 42808, + }, + Object { + "x": 1607437130000, + "y": 22230.5, + }, + Object { + "x": 1607437220000, + "y": 34973, + }, + Object { + "x": 1607437230000, + "y": 19284.2, + }, + Object { + "x": 1607437240000, + "y": 9280, + }, + Object { + "x": 1607437250000, + "y": 42777, + }, + Object { + "x": 1607437260000, + "y": 10702, + }, + Object { + "x": 1607437340000, + "y": 22452, + }, + Object { + "x": 1607437470000, + "y": 14495.5, + }, + Object { + "x": 1607437480000, + "y": 11644.5714285714, + }, + Object { + "x": 1607437570000, + "y": 17359.6666666667, + }, + Object { + "x": 1607437590000, + "y": 11394.2, + }, +] +`; + +exports[`APM API tests basic apm_8.0.0 Latency with a basic license when data is loaded time comparison returns some data 2`] = ` +Array [ + Object { + "x": 1607436800000, + "y": 23448.25, + }, + Object { + "x": 1607436820000, + "y": 25181, + }, + Object { + "x": 1607436840000, + "y": 16834, + }, + Object { + "x": 1607436910000, + "y": 21582, + }, + Object { + "x": 1607437040000, + "y": 31800, + }, + Object { + "x": 1607437050000, + "y": 21341, + }, + Object { + "x": 1607437060000, + "y": 21108.5, + }, + Object { + "x": 1607437150000, + "y": 12147.3333333333, + }, + Object { + "x": 1607437160000, + "y": 23941.5, + }, + Object { + "x": 1607437180000, + "y": 18244, + }, + Object { + "x": 1607437240000, + "y": 24359.5, + }, + Object { + "x": 1607437280000, + "y": 27767, + }, + Object { + "x": 1607437290000, + "y": 21909.6666666667, + }, + Object { + "x": 1607437390000, + "y": 31521, + }, + Object { + "x": 1607437410000, + "y": 20227.5, + }, + Object { + "x": 1607437420000, + "y": 18664, + }, + Object { + "x": 1607437510000, + "y": 14197.5, + }, + Object { + "x": 1607437520000, + "y": 19199.8571428571, + }, + Object { + "x": 1607437540000, + "y": 63745.75, + }, + Object { + "x": 1607437640000, + "y": 63220, + }, + Object { + "x": 1607437660000, + "y": 20040, + }, +] +`; + exports[`APM API tests trial apm_8.0.0 Transaction latency with a trial license when data is loaded when not defined environments seleted should return the correct anomaly boundaries 1`] = ` Array [ Object { @@ -43,4 +209,4 @@ Array [ "y0": 5732.00699123528, }, ] -`; +`; \ No newline at end of file diff --git a/x-pack/test/apm_api_integration/tests/transactions/latency.ts b/x-pack/test/apm_api_integration/tests/transactions/latency.ts index 3003c6f902f39..6791792be3a7b 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/latency.ts +++ b/x-pack/test/apm_api_integration/tests/transactions/latency.ts @@ -6,30 +6,39 @@ */ import expect from '@kbn/expect'; +import url from 'url'; +import moment from 'moment'; +import { APIReturnType } from '../../../../plugins/apm/public/services/rest/createCallApmApi'; import { PromiseReturnType } from '../../../../plugins/observability/typings/common'; import { FtrProviderContext } from '../../common/ftr_provider_context'; import archives_metadata from '../../common/fixtures/es_archiver/archives_metadata'; import { registry } from '../../common/registry'; +type LatencyChartReturnType = APIReturnType<'GET /api/apm/services/{serviceName}/transactions/charts/latency'>; + export default function ApiTest({ getService }: FtrProviderContext) { const supertest = getService('supertest'); const archiveName = 'apm_8.0.0'; - const range = archives_metadata[archiveName]; - - // url parameters - const start = encodeURIComponent(range.start); - const end = encodeURIComponent(range.end); + const { start, end } = archives_metadata[archiveName]; registry.when( 'Latency with a basic license when data is not loaded ', { config: 'basic', archives: [] }, () => { - const uiFilters = encodeURIComponent(JSON.stringify({ environment: 'testing' })); + const uiFilters = JSON.stringify({ environment: 'testing' }); it('returns 400 when latencyAggregationType is not informed', async () => { const response = await supertest.get( - `/api/apm/services/opbeans-node/transactions/charts/latency?start=${start}&end=${end}&uiFilters=${uiFilters}&transactionType=request` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + transactionType: 'request', + }, + }) ); expect(response.status).to.be(400); @@ -37,7 +46,15 @@ export default function ApiTest({ getService }: FtrProviderContext) { it('returns 400 when transactionType is not informed', async () => { const response = await supertest.get( - `/api/apm/services/opbeans-node/transactions/charts/latency?start=${start}&end=${end}&uiFilters=${uiFilters}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + }, + }) ); expect(response.status).to.be(400); @@ -45,13 +62,24 @@ export default function ApiTest({ getService }: FtrProviderContext) { it('handles the empty state', async () => { const response = await supertest.get( - `/api/apm/services/opbeans-node/transactions/charts/latency?start=${start}&end=${end}&uiFilters=${uiFilters}&latencyAggregationType=avg&transactionType=request` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType: 'request', + }, + }) ); expect(response.status).to.be(200); - expect(response.body.overallAvgDuration).to.be(null); - expect(response.body.latencyTimeseries.length).to.be(0); + const latencyChartReturn = response.body as LatencyChartReturnType; + + expect(latencyChartReturn.currentPeriod.overallAvgDuration).to.be(null); + expect(latencyChartReturn.currentPeriod.latencyTimeseries.length).to.be(0); }); } ); @@ -62,47 +90,112 @@ export default function ApiTest({ getService }: FtrProviderContext) { () => { let response: PromiseReturnType; - const uiFilters = encodeURIComponent(JSON.stringify({ environment: 'testing' })); + const uiFilters = JSON.stringify({ environment: 'testing' }); describe('average latency type', () => { before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-node/transactions/charts/latency?start=${start}&end=${end}&uiFilters=${uiFilters}&transactionType=request&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType: 'request', + }, + }) ); }); it('returns average duration and timeseries', async () => { expect(response.status).to.be(200); - expect(response.body.overallAvgDuration).not.to.be(null); - expect(response.body.latencyTimeseries.length).to.be.eql(61); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn.currentPeriod.overallAvgDuration).not.to.be(null); + expect(latencyChartReturn.currentPeriod.latencyTimeseries.length).to.be.eql(61); }); }); describe('95th percentile latency type', () => { before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-node/transactions/charts/latency?start=${start}&end=${end}&uiFilters=${uiFilters}&transactionType=request&latencyAggregationType=p95` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'p95', + transactionType: 'request', + }, + }) ); }); it('returns average duration and timeseries', async () => { expect(response.status).to.be(200); - expect(response.body.overallAvgDuration).not.to.be(null); - expect(response.body.latencyTimeseries.length).to.be.eql(61); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn.currentPeriod.overallAvgDuration).not.to.be(null); + expect(latencyChartReturn.currentPeriod.latencyTimeseries.length).to.be.eql(61); }); }); describe('99th percentile latency type', () => { before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-node/transactions/charts/latency?start=${start}&end=${end}&uiFilters=${uiFilters}&transactionType=request&latencyAggregationType=p99` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'p99', + transactionType: 'request', + }, + }) ); }); it('returns average duration and timeseries', async () => { expect(response.status).to.be(200); - expect(response.body.overallAvgDuration).not.to.be(null); - expect(response.body.latencyTimeseries.length).to.be.eql(61); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn.currentPeriod.overallAvgDuration).not.to.be(null); + expect(latencyChartReturn.currentPeriod.latencyTimeseries.length).to.be.eql(61); + }); + }); + + describe('time comparison', () => { + before(async () => { + response = await supertest.get( + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + uiFilters, + latencyAggregationType: 'avg', + transactionType: 'request', + start: moment(end).subtract(15, 'minutes').toISOString(), + end, + comparisonStart: start, + comparisonEnd: moment(start).add(15, 'minutes').toISOString(), + }, + }) + ); + }); + + it('returns some data', async () => { + expect(response.status).to.be(200); + const latencyChartReturn = response.body as LatencyChartReturnType; + const currentPeriodNonNullDataPoints = latencyChartReturn.currentPeriod.latencyTimeseries.filter( + ({ y }) => y !== null + ); + expect(currentPeriodNonNullDataPoints.length).to.be.greaterThan(0); + const previousPeriodNonNullDataPoints = latencyChartReturn.previousPeriod.latencyTimeseries.filter( + ({ y }) => y !== null + ); + expect(previousPeriodNonNullDataPoints.length).to.be.greaterThan(0); + + expectSnapshot(currentPeriodNonNullDataPoints).toMatch(); + expectSnapshot(previousPeriodNonNullDataPoints).toMatch(); }); }); } @@ -117,10 +210,19 @@ export default function ApiTest({ getService }: FtrProviderContext) { const transactionType = 'request'; describe('without environment', () => { - const uiFilters = encodeURIComponent(JSON.stringify({})); + const uiFilters = JSON.stringify({}); before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-java/transactions/charts/latency?start=${start}&end=${end}&transactionType=${transactionType}&uiFilters=${uiFilters}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType, + }, + }) ); }); it('should return an error response', () => { @@ -131,7 +233,15 @@ export default function ApiTest({ getService }: FtrProviderContext) { describe('without uiFilters', () => { before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-java/transactions/charts/latency?start=${start}&end=${end}&transactionType=${transactionType}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + latencyAggregationType: 'avg', + transactionType, + }, + }) ); }); it('should return an error response', () => { @@ -140,10 +250,19 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); describe('with environment selected in uiFilters', () => { - const uiFilters = encodeURIComponent(JSON.stringify({ environment: 'production' })); + const uiFilters = JSON.stringify({ environment: 'production' }); before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-java/transactions/charts/latency?start=${start}&end=${end}&transactionType=${transactionType}&uiFilters=${uiFilters}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType, + }, + }) ); }); @@ -152,27 +271,39 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('should return the ML job id for anomalies of the selected environment', () => { - expect(response.body).to.have.property('anomalyTimeseries'); - expect(response.body.anomalyTimeseries).to.have.property('jobId'); - expectSnapshot(response.body.anomalyTimeseries.jobId).toMatchInline( + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.have.property('anomalyTimeseries'); + expect(latencyChartReturn.anomalyTimeseries).to.have.property('jobId'); + expectSnapshot(latencyChartReturn.anomalyTimeseries?.jobId).toMatchInline( `"apm-production-1369-high_mean_transaction_duration"` ); }); it('should return a non-empty anomaly series', () => { - expect(response.body).to.have.property('anomalyTimeseries'); - expect(response.body.anomalyTimeseries.anomalyBoundaries?.length).to.be.greaterThan(0); - expectSnapshot(response.body.anomalyTimeseries.anomalyBoundaries).toMatch(); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.have.property('anomalyTimeseries'); + expect(latencyChartReturn.anomalyTimeseries?.anomalyBoundaries?.length).to.be.greaterThan( + 0 + ); + expectSnapshot(latencyChartReturn.anomalyTimeseries?.anomalyBoundaries).toMatch(); }); }); describe('when not defined environments seleted', () => { - const uiFilters = encodeURIComponent( - JSON.stringify({ environment: 'ENVIRONMENT_NOT_DEFINED' }) - ); + const uiFilters = JSON.stringify({ environment: 'ENVIRONMENT_NOT_DEFINED' }); + before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-python/transactions/charts/latency?start=${start}&end=${end}&transactionType=${transactionType}&uiFilters=${uiFilters}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType, + }, + }) ); }); @@ -181,24 +312,35 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('should return the ML job id for anomalies with no defined environment', () => { - expect(response.body).to.have.property('anomalyTimeseries'); - expect(response.body.anomalyTimeseries).to.have.property('jobId'); - expectSnapshot(response.body.anomalyTimeseries.jobId).toMatchInline( + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.have.property('anomalyTimeseries'); + expect(latencyChartReturn.anomalyTimeseries).to.have.property('jobId'); + expectSnapshot(latencyChartReturn.anomalyTimeseries?.jobId).toMatchInline( `"apm-environment_not_defined-5626-high_mean_transaction_duration"` ); }); it('should return the correct anomaly boundaries', () => { - expect(response.body).to.have.property('anomalyTimeseries'); - expectSnapshot(response.body.anomalyTimeseries.anomalyBoundaries).toMatch(); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.have.property('anomalyTimeseries'); + expectSnapshot(latencyChartReturn.anomalyTimeseries?.anomalyBoundaries).toMatch(); }); }); describe('with all environments selected', () => { - const uiFilters = encodeURIComponent(JSON.stringify({ environment: 'ENVIRONMENT_ALL' })); + const uiFilters = JSON.stringify({ environment: 'ENVIRONMENT_ALL' }); before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-java/transactions/charts/latency?start=${start}&end=${end}&transactionType=${transactionType}&uiFilters=${uiFilters}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType, + }, + }) ); }); @@ -207,17 +349,26 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('should not return anomaly timeseries data', () => { - expect(response.body).to.not.have.property('anomalyTimeseries'); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.not.have.property('anomalyTimeseries'); }); }); describe('with environment selected and empty kuery filter', () => { - const uiFilters = encodeURIComponent( - JSON.stringify({ kuery: '', environment: 'production' }) - ); + const uiFilters = JSON.stringify({ kuery: '', environment: 'production' }); + before(async () => { response = await supertest.get( - `/api/apm/services/opbeans-java/transactions/charts/latency?start=${start}&end=${end}&transactionType=${transactionType}&uiFilters=${uiFilters}&latencyAggregationType=avg` + url.format({ + pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + query: { + start, + end, + uiFilters, + latencyAggregationType: 'avg', + transactionType, + }, + }) ); }); @@ -226,17 +377,21 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('should return the ML job id for anomalies of the selected environment', () => { - expect(response.body).to.have.property('anomalyTimeseries'); - expect(response.body.anomalyTimeseries).to.have.property('jobId'); - expectSnapshot(response.body.anomalyTimeseries.jobId).toMatchInline( + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.have.property('anomalyTimeseries'); + expect(latencyChartReturn.anomalyTimeseries).to.have.property('jobId'); + expectSnapshot(latencyChartReturn.anomalyTimeseries?.jobId).toMatchInline( `"apm-production-1369-high_mean_transaction_duration"` ); }); it('should return a non-empty anomaly series', () => { - expect(response.body).to.have.property('anomalyTimeseries'); - expect(response.body.anomalyTimeseries.anomalyBoundaries?.length).to.be.greaterThan(0); - expectSnapshot(response.body.anomalyTimeseries.anomalyBoundaries).toMatch(); + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn).to.have.property('anomalyTimeseries'); + expect(latencyChartReturn.anomalyTimeseries?.anomalyBoundaries?.length).to.be.greaterThan( + 0 + ); + expectSnapshot(latencyChartReturn.anomalyTimeseries?.anomalyBoundaries).toMatch(); }); }); } From 004ba522575973396c9060a1ebd1ede6adf7c9cc Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Sat, 13 Feb 2021 14:50:21 -0500 Subject: [PATCH 02/16] adding time comparison to latency chart --- .../apm/public/hooks/use_transaction_latency_chart_fetcher.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts b/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts index b1dbd09067683..073244b224241 100644 --- a/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts +++ b/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts @@ -12,7 +12,6 @@ import { useUrlParams } from '../context/url_params_context/use_url_params'; import { useApmServiceContext } from '../context/apm_service/use_apm_service_context'; import { getLatencyChartSelector } from '../selectors/latency_chart_selectors'; import { useTheme } from './use_theme'; -import { LatencyAggregationType } from '../../common/latency_aggregation_types'; import { getTimeRangeComparison } from '../components/shared/time_comparison/get_time_range_comparison'; export function useTransactionLatencyChartsFetcher() { From f0132d13699c78d3499fc372e73e1b2cbf05e671 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Sat, 13 Feb 2021 16:10:25 -0500 Subject: [PATCH 03/16] fixing TS --- x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts index ecf5dcef67dfb..5293643f843bd 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts @@ -15,7 +15,7 @@ import { APIReturnType } from '../services/rest/createCallApmApi'; export type LatencyChartsResponse = APIReturnType<'GET /api/apm/services/{serviceName}/transactions/charts/latency'>; -interface LatencyChartData { +export interface LatencyChartData { currentPeriod: Array>; previousPeriod?: APMChartSpec; mlJobId?: string; From e33ef149ab92edb7e8c412015aaacf0f22c203ac Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Sun, 14 Feb 2021 10:38:07 -0500 Subject: [PATCH 04/16] fixing api test --- .../tests/transactions/latency.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/transactions/latency.ts b/x-pack/test/apm_api_integration/tests/transactions/latency.ts index 6791792be3a7b..838cedea39bd2 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/latency.ts +++ b/x-pack/test/apm_api_integration/tests/transactions/latency.ts @@ -214,7 +214,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { before(async () => { response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + pathname: `/api/apm/services/opbeans-java/transactions/charts/latency`, query: { start, end, @@ -234,7 +234,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { before(async () => { response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + pathname: `/api/apm/services/opbeans-java/transactions/charts/latency`, query: { start, end, @@ -254,7 +254,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { before(async () => { response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + pathname: `/api/apm/services/opbeans-java/transactions/charts/latency`, query: { start, end, @@ -295,7 +295,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { before(async () => { response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + pathname: `/api/apm/services/opbeans-python/transactions/charts/latency`, query: { start, end, @@ -332,7 +332,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { before(async () => { response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + pathname: `/api/apm/services/opbeans-java/transactions/charts/latency`, query: { start, end, @@ -360,7 +360,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { before(async () => { response = await supertest.get( url.format({ - pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, + pathname: `/api/apm/services/opbeans-java/transactions/charts/latency`, query: { start, end, From a54b36c33e3ef27113a95372f90ed31863883011 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 15 Feb 2021 11:22:04 -0500 Subject: [PATCH 05/16] addressing PR comments --- .../shared/charts/latency_chart/index.tsx | 12 ++-- .../selectors/latency_chart_selector.test.ts | 57 ++++++++++------- .../selectors/latency_chart_selectors.ts | 28 +++++---- .../transactions/get_latency_charts/index.ts | 63 ++++++++++++++++++- .../plugins/apm/server/routes/transactions.ts | 29 ++------- 5 files changed, 122 insertions(+), 67 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx index 659360b80d80c..42398c69456ce 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx @@ -54,7 +54,12 @@ export function LatencyChart({ height }: Props) { mlJobId, } = latencyChartsData; - const latencyMaxY = getMaxY(currentPeriod); + const timeseries = [ + ...currentPeriod, + ...(comparisonEnabled ? previousPeriod : []), + ]; + + const latencyMaxY = getMaxY(timeseries); const latencyFormatter = getDurationFormatter(latencyMaxY); return ( @@ -109,10 +114,7 @@ export function LatencyChart({ height }: Props) { fetchStatus={latencyChartsStatus} id="latencyChart" customTheme={comparisonChartTheme} - timeseries={[ - ...currentPeriod, - ...(comparisonEnabled && previousPeriod ? [previousPeriod] : []), - ]} + timeseries={timeseries} yLabelFormat={getResponseTimeTickFormatter(latencyFormatter)} anomalyTimeseries={anomalyTimeseries} /> diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts index ca21f6989ea81..fde68b67d5d6d 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts @@ -44,6 +44,7 @@ describe('getLatencyChartSelector', () => { const latencyChart = getLatencyChartSelector({ theme }); expect(latencyChart).toEqual({ currentPeriod: [], + previousPeriod: [], mlJobId: undefined, anomalyTimeseries: undefined, }); @@ -66,12 +67,14 @@ describe('getLatencyChartSelector', () => { color: 'blue', }, ], - previousPeriod: { - color: 'green', - data: [{ x: 1, y: 10 }], - type: 'area', - title: 'Previous period', - }, + previousPeriod: [ + { + color: 'green', + data: [{ x: 1, y: 10 }], + type: 'area', + title: 'Previous period', + }, + ], }); }); @@ -92,12 +95,14 @@ describe('getLatencyChartSelector', () => { color: 'red', }, ], - previousPeriod: { - data: [{ x: 1, y: 10 }], - type: 'area', - color: 'green', - title: 'Previous period', - }, + previousPeriod: [ + { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, + ], }); }); @@ -119,12 +124,14 @@ describe('getLatencyChartSelector', () => { color: 'black', }, ], - previousPeriod: { - data: [{ x: 1, y: 10 }], - type: 'area', - color: 'green', - title: 'Previous period', - }, + previousPeriod: [ + { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, + ], }); }); }); @@ -146,12 +153,14 @@ describe('getLatencyChartSelector', () => { color: 'black', }, ], - previousPeriod: { - data: [{ x: 1, y: 10 }], - type: 'area', - color: 'green', - title: 'Previous period', - }, + previousPeriod: [ + { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, + ], mlJobId: '1', anomalyTimeseries: { boundaries: [ diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts index 5293643f843bd..c32ad8077c511 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts @@ -17,7 +17,7 @@ export type LatencyChartsResponse = APIReturnType<'GET /api/apm/services/{servic export interface LatencyChartData { currentPeriod: Array>; - previousPeriod?: APMChartSpec; + previousPeriod: Array>; mlJobId?: string; anomalyTimeseries?: { boundaries: APMChartSpec[]; scores: APMChartSpec }; } @@ -37,7 +37,7 @@ export function getLatencyChartSelector({ ) { return { currentPeriod: [], - previousPeriod: undefined, + previousPeriod: [], mlJobId: undefined, anomalyTimeseries: undefined, }; @@ -67,17 +67,19 @@ function getPreviousPeriodTimeseries({ previousPeriod: LatencyChartsResponse['previousPeriod']; theme: EuiTheme; }) { - return { - data: previousPeriod.latencyTimeseries ?? [], - type: 'area', - color: theme.eui.euiColorLightestShade, - title: i18n.translate( - 'xpack.apm.serviceOverview.latencyChartTitle.previousPeriodLabel', - { - defaultMessage: 'Previous period', - } - ), - }; + return [ + { + data: previousPeriod.latencyTimeseries ?? [], + type: 'area', + color: theme.eui.euiColorLightestShade, + title: i18n.translate( + 'xpack.apm.serviceOverview.latencyChartTitle.previousPeriodLabel', + { + defaultMessage: 'Previous period', + } + ), + }, + ]; } function getLatencyTimeseries({ diff --git a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts index df38d575a1eff..bba41d68272a5 100644 --- a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts +++ b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts @@ -20,7 +20,8 @@ import { getTransactionDurationFieldForAggregatedTransactions, } from '../../../lib/helpers/aggregated_transactions'; import { getBucketSize } from '../../../lib/helpers/get_bucket_size'; -import { Setup } from '../../../lib/helpers/setup_request'; +import { Setup, SetupTimeRange } from '../../../lib/helpers/setup_request'; +import { offsetPreviousPeriodCoordinates } from '../../../utils/offset_previous_period_coordinate'; import { withApmSpan } from '../../../utils/with_apm_span'; import { getLatencyAggregation, @@ -157,3 +158,63 @@ export function getLatencyTimeseries({ }; }); } + +export async function getLatencyPeriods({ + serviceName, + transactionType, + transactionName, + setup, + searchAggregatedTransactions, + latencyAggregationType, + comparisonStart, + comparisonEnd, +}: { + serviceName: string; + transactionType: string | undefined; + transactionName: string | undefined; + setup: Setup & SetupTimeRange; + searchAggregatedTransactions: boolean; + latencyAggregationType: LatencyAggregationType; + comparisonStart?: number; + comparisonEnd?: number; +}) { + const { start, end } = setup; + const options = { + serviceName, + transactionType, + transactionName, + setup, + searchAggregatedTransactions, + }; + + const currentPeriodPromise = getLatencyTimeseries({ + ...options, + start, + end, + latencyAggregationType: latencyAggregationType as LatencyAggregationType, + }); + + const previousPeriodPromise = + comparisonStart && comparisonEnd + ? getLatencyTimeseries({ + ...options, + start: comparisonStart, + end: comparisonEnd, + latencyAggregationType: latencyAggregationType as LatencyAggregationType, + }).then((latency) => ({ + ...latency, + latencyTimeseries: offsetPreviousPeriodCoordinates({ + currentPeriodStart: start, + previousPeriodStart: comparisonStart, + previousPeriodTimeseries: latency.latencyTimeseries, + }), + })) + : { latencyTimeseries: [], overallAvgDuration: null }; + + const [currentPeriod, previousPeriod] = await Promise.all([ + currentPeriodPromise, + previousPeriodPromise, + ]); + + return { currentPeriod, previousPeriod }; +} diff --git a/x-pack/plugins/apm/server/routes/transactions.ts b/x-pack/plugins/apm/server/routes/transactions.ts index 5f154b0adccaa..2300dd6d880af 100644 --- a/x-pack/plugins/apm/server/routes/transactions.ts +++ b/x-pack/plugins/apm/server/routes/transactions.ts @@ -20,11 +20,10 @@ import { getServiceTransactionGroupComparisonStatistics } from '../lib/services/ import { getTransactionBreakdown } from '../lib/transactions/breakdown'; import { getTransactionDistribution } from '../lib/transactions/distribution'; import { getAnomalySeries } from '../lib/transactions/get_anomaly_data'; -import { getLatencyTimeseries } from '../lib/transactions/get_latency_charts'; +import { getLatencyPeriods } from '../lib/transactions/get_latency_charts'; import { getThroughputCharts } from '../lib/transactions/get_throughput_charts'; import { getTransactionGroupList } from '../lib/transaction_groups'; import { getErrorRate } from '../lib/transaction_groups/get_error_rate'; -import { offsetPreviousPeriodCoordinates } from '../utils/offset_previous_period_coordinate'; import { createRoute } from './create_route'; import { comparisonRangeRt, rangeRt, uiFiltersRt } from './default_api_types'; @@ -195,8 +194,6 @@ export const transactionLatencyChatsRoute = createRoute({ setup ); - const { start, end } = setup; - const options = { serviceName, transactionType, @@ -207,36 +204,20 @@ export const transactionLatencyChatsRoute = createRoute({ }; const [ - currentPeriod, + { currentPeriod, previousPeriod }, anomalyTimeseries, - previousPeriod, ] = await Promise.all([ - getLatencyTimeseries({ + getLatencyPeriods({ ...options, - start, - end, latencyAggregationType: latencyAggregationType as LatencyAggregationType, + comparisonStart, + comparisonEnd, }), getAnomalySeries(options).catch((error) => { logger.warn(`Unable to retrieve anomalies for latency charts.`); logger.error(error); return undefined; }), - comparisonStart && comparisonEnd - ? getLatencyTimeseries({ - ...options, - start: comparisonStart, - end: comparisonEnd, - latencyAggregationType: latencyAggregationType as LatencyAggregationType, - }).then((latency) => ({ - ...latency, - latencyTimeseries: offsetPreviousPeriodCoordinates({ - currentPeriodStart: start, - previousPeriodStart: comparisonStart, - previousPeriodTimeseries: latency.latencyTimeseries, - }), - })) - : { latencyTimeseries: [], overallAvgDuration: null }, ]); return { From 286e5e5f40ee83d109486950c122aa67bae25f35 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Fri, 19 Feb 2021 11:39:29 -0500 Subject: [PATCH 06/16] adding api test --- x-pack/test/apm_api_integration/tests/transactions/latency.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/test/apm_api_integration/tests/transactions/latency.ts b/x-pack/test/apm_api_integration/tests/transactions/latency.ts index 8145e7b5955a6..96ae269e3e814 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/latency.ts +++ b/x-pack/test/apm_api_integration/tests/transactions/latency.ts @@ -83,6 +83,7 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(latencyChartReturn.currentPeriod.overallAvgDuration).to.be(null); expect(latencyChartReturn.currentPeriod.latencyTimeseries.length).to.be(0); + expect(latencyChartReturn.previousPeriod.latencyTimeseries.length).to.be(0); }); } ); From c2e37407a52024d02a19203445c0487f7de54149 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 22 Feb 2021 15:59:25 -0500 Subject: [PATCH 07/16] addressing PR comments --- .../components/app/service_overview/index.tsx | 6 +- .../shared/charts/latency_chart/index.tsx | 7 +- .../selectors/latency_chart_selector.test.ts | 125 ++++++++---------- .../selectors/latency_chart_selectors.ts | 99 +++++++------- .../transactions/get_latency_charts/index.ts | 20 +-- x-pack/plugins/apm/server/routes/services.ts | 13 +- .../plugins/apm/server/routes/transactions.ts | 9 +- .../offset_previous_period_coordinate.test.ts | 4 - .../offset_previous_period_coordinate.ts | 8 +- 9 files changed, 130 insertions(+), 161 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/service_overview/index.tsx b/x-pack/plugins/apm/public/components/app/service_overview/index.tsx index a0ea00ae5c3ad..f6ec2fb24018f 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/index.tsx @@ -57,7 +57,11 @@ export function ServiceOverview({ return ( - + diff --git a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx index 42398c69456ce..b16a81f0b04cd 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx @@ -9,6 +9,7 @@ import { EuiFlexGroup, EuiFlexItem, EuiSelect, EuiTitle } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import { useHistory } from 'react-router-dom'; +import { APMChartSpec, Coordinate } from '../../../../../typings/timeseries'; import { LatencyAggregationType } from '../../../../../common/latency_aggregation_types'; import { getDurationFormatter } from '../../../../../common/utils/formatters'; import { useLicenseContext } from '../../../../context/license/use_license_context'; @@ -55,9 +56,9 @@ export function LatencyChart({ height }: Props) { } = latencyChartsData; const timeseries = [ - ...currentPeriod, - ...(comparisonEnabled ? previousPeriod : []), - ]; + currentPeriod, + comparisonEnabled ? previousPeriod : undefined, + ].filter((data) => data) as Array>; const latencyMaxY = getMaxY(timeseries); const latencyFormatter = getDurationFormatter(latencyMaxY); diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts index fde68b67d5d6d..252ced2be5e0e 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selector.test.ts @@ -43,8 +43,8 @@ describe('getLatencyChartSelector', () => { it('returns default values when data is undefined', () => { const latencyChart = getLatencyChartSelector({ theme }); expect(latencyChart).toEqual({ - currentPeriod: [], - previousPeriod: [], + currentPeriod: undefined, + previousPeriod: undefined, mlJobId: undefined, anomalyTimeseries: undefined, }); @@ -58,23 +58,20 @@ describe('getLatencyChartSelector', () => { latencyAggregationType: LatencyAggregationType.avg, }); expect(latencyTimeseries).toEqual({ - currentPeriod: [ - { - title: 'Average', - data: [{ x: 1, y: 10 }], - legendValue: '1 μs', - type: 'linemark', - color: 'blue', - }, - ], - previousPeriod: [ - { - color: 'green', - data: [{ x: 1, y: 10 }], - type: 'area', - title: 'Previous period', - }, - ], + currentPeriod: { + title: 'Average', + data: [{ x: 1, y: 10 }], + legendValue: '1 μs', + type: 'linemark', + color: 'blue', + }, + + previousPeriod: { + color: 'green', + data: [{ x: 1, y: 10 }], + type: 'area', + title: 'Previous period', + }, }); }); @@ -86,23 +83,19 @@ describe('getLatencyChartSelector', () => { latencyAggregationType: LatencyAggregationType.p95, }); expect(latencyTimeseries).toEqual({ - currentPeriod: [ - { - title: '95th percentile', - titleShort: '95th', - data: [{ x: 1, y: 10 }], - type: 'linemark', - color: 'red', - }, - ], - previousPeriod: [ - { - data: [{ x: 1, y: 10 }], - type: 'area', - color: 'green', - title: 'Previous period', - }, - ], + currentPeriod: { + title: '95th percentile', + titleShort: '95th', + data: [{ x: 1, y: 10 }], + type: 'linemark', + color: 'red', + }, + previousPeriod: { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, }); }); @@ -115,23 +108,19 @@ describe('getLatencyChartSelector', () => { }); expect(latencyTimeseries).toEqual({ - currentPeriod: [ - { - title: '99th percentile', - titleShort: '99th', - data: [{ x: 1, y: 10 }], - type: 'linemark', - color: 'black', - }, - ], - previousPeriod: [ - { - data: [{ x: 1, y: 10 }], - type: 'area', - color: 'green', - title: 'Previous period', - }, - ], + currentPeriod: { + title: '99th percentile', + titleShort: '99th', + data: [{ x: 1, y: 10 }], + type: 'linemark', + color: 'black', + }, + previousPeriod: { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, }); }); }); @@ -144,23 +133,19 @@ describe('getLatencyChartSelector', () => { latencyAggregationType: LatencyAggregationType.p99, }); expect(latencyTimeseries).toEqual({ - currentPeriod: [ - { - title: '99th percentile', - titleShort: '99th', - data: [{ x: 1, y: 10 }], - type: 'linemark', - color: 'black', - }, - ], - previousPeriod: [ - { - data: [{ x: 1, y: 10 }], - type: 'area', - color: 'green', - title: 'Previous period', - }, - ], + currentPeriod: { + title: '99th percentile', + titleShort: '99th', + data: [{ x: 1, y: 10 }], + type: 'linemark', + color: 'black', + }, + previousPeriod: { + data: [{ x: 1, y: 10 }], + type: 'area', + color: 'green', + title: 'Previous period', + }, mlJobId: '1', anomalyTimeseries: { boundaries: [ diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts index c32ad8077c511..4f18681da5bcd 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts @@ -16,8 +16,8 @@ import { APIReturnType } from '../services/rest/createCallApmApi'; export type LatencyChartsResponse = APIReturnType<'GET /api/apm/services/{serviceName}/transactions/charts/latency'>; export interface LatencyChartData { - currentPeriod: Array>; - previousPeriod: Array>; + currentPeriod?: APMChartSpec; + previousPeriod?: APMChartSpec; mlJobId?: string; anomalyTimeseries?: { boundaries: APMChartSpec[]; scores: APMChartSpec }; } @@ -36,8 +36,8 @@ export function getLatencyChartSelector({ !latencyAggregationType ) { return { - currentPeriod: [], - previousPeriod: [], + currentPeriod: undefined, + previousPeriod: undefined, mlJobId: undefined, anomalyTimeseries: undefined, }; @@ -67,19 +67,17 @@ function getPreviousPeriodTimeseries({ previousPeriod: LatencyChartsResponse['previousPeriod']; theme: EuiTheme; }) { - return [ - { - data: previousPeriod.latencyTimeseries ?? [], - type: 'area', - color: theme.eui.euiColorLightestShade, - title: i18n.translate( - 'xpack.apm.serviceOverview.latencyChartTitle.previousPeriodLabel', - { - defaultMessage: 'Previous period', - } - ), - }, - ]; + return { + data: previousPeriod.latencyTimeseries ?? [], + type: 'area', + color: theme.eui.euiColorLightestShade, + title: i18n.translate( + 'xpack.apm.serviceOverview.latencyChartTitle.previousPeriodLabel', + { + defaultMessage: 'Previous period', + } + ), + }; } function getLatencyTimeseries({ @@ -96,49 +94,42 @@ function getLatencyTimeseries({ switch (latencyAggregationType) { case 'avg': { - return [ - { - title: i18n.translate( - 'xpack.apm.transactions.latency.chart.averageLabel', - { defaultMessage: 'Average' } - ), - data: latencyTimeseries, - legendValue: asDuration(overallAvgDuration), - type: 'linemark', - color: theme.eui.euiColorVis1, - }, - ]; + return { + title: i18n.translate( + 'xpack.apm.transactions.latency.chart.averageLabel', + { defaultMessage: 'Average' } + ), + data: latencyTimeseries, + legendValue: asDuration(overallAvgDuration), + type: 'linemark', + color: theme.eui.euiColorVis1, + }; } case 'p95': { - return [ - { - title: i18n.translate( - 'xpack.apm.transactions.latency.chart.95thPercentileLabel', - { defaultMessage: '95th percentile' } - ), - titleShort: '95th', - data: latencyTimeseries, - type: 'linemark', - color: theme.eui.euiColorVis5, - }, - ]; + return { + title: i18n.translate( + 'xpack.apm.transactions.latency.chart.95thPercentileLabel', + { defaultMessage: '95th percentile' } + ), + titleShort: '95th', + data: latencyTimeseries, + type: 'linemark', + color: theme.eui.euiColorVis5, + }; } case 'p99': { - return [ - { - title: i18n.translate( - 'xpack.apm.transactions.latency.chart.99thPercentileLabel', - { defaultMessage: '99th percentile' } - ), - titleShort: '99th', - data: latencyTimeseries, - type: 'linemark', - color: theme.eui.euiColorVis7, - }, - ]; + return { + title: i18n.translate( + 'xpack.apm.transactions.latency.chart.99thPercentileLabel', + { defaultMessage: '99th percentile' } + ), + titleShort: '99th', + data: latencyTimeseries, + type: 'linemark', + color: theme.eui.euiColorVis7, + }; } } - return []; } function getAnomalyTimeseries({ diff --git a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts index 34a75bb361264..b9e4869bafcb1 100644 --- a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts +++ b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts @@ -207,14 +207,7 @@ export async function getLatencyPeriods({ start: comparisonStart, end: comparisonEnd, latencyAggregationType: latencyAggregationType as LatencyAggregationType, - }).then((latency) => ({ - ...latency, - latencyTimeseries: offsetPreviousPeriodCoordinates({ - currentPeriodStart: start, - previousPeriodStart: comparisonStart, - previousPeriodTimeseries: latency.latencyTimeseries, - }), - })) + }) : { latencyTimeseries: [], overallAvgDuration: null }; const [currentPeriod, previousPeriod] = await Promise.all([ @@ -222,5 +215,14 @@ export async function getLatencyPeriods({ previousPeriodPromise, ]); - return { currentPeriod, previousPeriod }; + return { + currentPeriod, + previousPeriod: { + ...previousPeriod, + latencyTimeseries: offsetPreviousPeriodCoordinates({ + currentPeriodStart: currentPeriod.latencyTimeseries[0]?.x, + previousPeriodTimeseries: previousPeriod.latencyTimeseries, + }), + }, + }; } diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index 24c7c6e3e23d7..d2e799eacda6c 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -381,19 +381,16 @@ export const serviceThroughputRoute = createRoute({ environment, start: comparisonStart, end: comparisonEnd, - }).then((coordinates) => - offsetPreviousPeriodCoordinates({ - currentPeriodStart: start, - previousPeriodStart: comparisonStart, - previousPeriodTimeseries: coordinates, - }) - ) + }) : [], ]); return { currentPeriod, - previousPeriod, + previousPeriod: offsetPreviousPeriodCoordinates({ + currentPeriodStart: currentPeriod[0]?.x, + previousPeriodTimeseries: previousPeriod, + }), }; }, }); diff --git a/x-pack/plugins/apm/server/routes/transactions.ts b/x-pack/plugins/apm/server/routes/transactions.ts index 0683166eb957f..effa6f8fc49fe 100644 --- a/x-pack/plugins/apm/server/routes/transactions.ts +++ b/x-pack/plugins/apm/server/routes/transactions.ts @@ -170,17 +170,12 @@ export const transactionLatencyChartsRoute = createRoute({ serviceName: t.string, }), query: t.intersection([ - t.partial({ - transactionName: t.string, - }), t.type({ transactionType: t.string, latencyAggregationType: latencyAggregationTypeRt, }), - environmentRt, - uiFiltersRt, - rangeRt, - comparisonRangeRt, + t.partial({ transactionName: t.string }), + t.intersection([environmentRt, uiFiltersRt, rangeRt, comparisonRangeRt]), ]), }), options: { tags: ['access:apm'] }, diff --git a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts index 6436c7c5193ec..d58b1a8a8ef7a 100644 --- a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts +++ b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts @@ -7,7 +7,6 @@ import { Coordinate } from '../../typings/timeseries'; import { offsetPreviousPeriodCoordinates } from './offset_previous_period_coordinate'; -const previousPeriodStart = new Date('2021-01-27T14:45:00.000Z').valueOf(); const currentPeriodStart = new Date('2021-01-28T14:45:00.000Z').valueOf(); describe('mergePeriodsTimeseries', () => { @@ -16,7 +15,6 @@ describe('mergePeriodsTimeseries', () => { expect( offsetPreviousPeriodCoordinates({ currentPeriodStart, - previousPeriodStart, previousPeriodTimeseries: undefined, }) ).toEqual([]); @@ -26,7 +24,6 @@ describe('mergePeriodsTimeseries', () => { expect( offsetPreviousPeriodCoordinates({ currentPeriodStart, - previousPeriodStart, previousPeriodTimeseries: [], }) ).toEqual([]); @@ -44,7 +41,6 @@ describe('mergePeriodsTimeseries', () => { expect( offsetPreviousPeriodCoordinates({ currentPeriodStart, - previousPeriodStart, previousPeriodTimeseries, }) ).toEqual([ diff --git a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts index 837e3d02056f0..0f5f1f42fe123 100644 --- a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts +++ b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts @@ -10,19 +10,17 @@ import { Coordinate } from '../../typings/timeseries'; export function offsetPreviousPeriodCoordinates({ currentPeriodStart, - previousPeriodStart, previousPeriodTimeseries, }: { - currentPeriodStart: number; - previousPeriodStart: number; + currentPeriodStart?: number; previousPeriodTimeseries?: Coordinate[]; }) { - if (!previousPeriodTimeseries) { + if (!previousPeriodTimeseries?.length) { return []; } const dateOffset = moment(currentPeriodStart).diff( - moment(previousPeriodStart) + moment(previousPeriodTimeseries[0].x) ); return previousPeriodTimeseries.map(({ x, y }) => { From afa95148993cf669eb0a87a38b9c583ee5e293b3 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 25 Feb 2021 12:49:35 -0500 Subject: [PATCH 08/16] fixing api test --- .../tests/transactions/__snapshots__/latency.snap | 15 --------------- .../tests/transactions/latency.ts | 3 --- 2 files changed, 18 deletions(-) diff --git a/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap b/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap index c3b221a6eaabb..85f2c4cbd3f2d 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap +++ b/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap @@ -165,18 +165,3 @@ Array [ }, ] `; - -exports[`APM API tests trial apm_8.0.0 Transaction latency with a trial license when data is loaded with environment selected and empty kuery filter should return a non-empty anomaly series 1`] = ` -Array [ - Object { - "x": 1607436000000, - "y": 1625128.56211579, - "y0": 7533.02707532227, - }, - Object { - "x": 1607436900000, - "y": 1660982.24115757, - "y0": 5732.00699123528, - }, -] -`; diff --git a/x-pack/test/apm_api_integration/tests/transactions/latency.ts b/x-pack/test/apm_api_integration/tests/transactions/latency.ts index 7c9168d785f8b..b657ecf41d553 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/latency.ts +++ b/x-pack/test/apm_api_integration/tests/transactions/latency.ts @@ -90,8 +90,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { () => { let response: PromiseReturnType; - const uiFilters = JSON.stringify({}); - describe('average latency type', () => { before(async () => { response = await supertest.get( @@ -170,7 +168,6 @@ export default function ApiTest({ getService }: FtrProviderContext) { url.format({ pathname: `/api/apm/services/opbeans-node/transactions/charts/latency`, query: { - uiFilters, latencyAggregationType: 'avg', transactionType: 'request', start: moment(end).subtract(15, 'minutes').toISOString(), From 648f0f40c55e1c10a2d082277892f517555376b2 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 8 Mar 2021 14:08:14 -0500 Subject: [PATCH 09/16] rounding date diff --- .../components/shared/time_comparison/index.tsx | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx index 0b6c1a2c52a98..44ba1d3104805 100644 --- a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx @@ -84,12 +84,14 @@ function getSelectOptions({ }), }; - const dateDiff = getDateDifference({ - start: momentStart, - end: momentEnd, - unitOfTime: 'days', - precise: true, - }); + const dateDiff = Math.round( + getDateDifference({ + start: momentStart, + end: momentEnd, + unitOfTime: 'days', + precise: true, + }) + ); const isRangeToNow = rangeTo === 'now'; From 6d9d249ea9a15584d975d5f554dd3831698eb211 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 8 Mar 2021 15:04:16 -0500 Subject: [PATCH 10/16] addressing PR comments --- .../components/shared/time_comparison/index.tsx | 4 ++-- .../apm/public/selectors/latency_chart_selectors.ts | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx index 44ba1d3104805..1769119593c0e 100644 --- a/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx @@ -84,13 +84,13 @@ function getSelectOptions({ }), }; - const dateDiff = Math.round( + const dateDiff = Number( getDateDifference({ start: momentStart, end: momentEnd, unitOfTime: 'days', precise: true, - }) + }).toFixed(2) ); const isRangeToNow = rangeTo === 'now'; diff --git a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts index 4f18681da5bcd..2ee4a717106eb 100644 --- a/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/latency_chart_selectors.ts @@ -30,17 +30,12 @@ export function getLatencyChartSelector({ latencyChart?: LatencyChartsResponse; theme: EuiTheme; latencyAggregationType?: string; -}): LatencyChartData { +}): Partial { if ( !latencyChart?.currentPeriod.latencyTimeseries || !latencyAggregationType ) { - return { - currentPeriod: undefined, - previousPeriod: undefined, - mlJobId: undefined, - anomalyTimeseries: undefined, - }; + return {}; } return { currentPeriod: getLatencyTimeseries({ @@ -73,9 +68,7 @@ function getPreviousPeriodTimeseries({ color: theme.eui.euiColorLightestShade, title: i18n.translate( 'xpack.apm.serviceOverview.latencyChartTitle.previousPeriodLabel', - { - defaultMessage: 'Previous period', - } + { defaultMessage: 'Previous period' } ), }; } From 186b3d4e5abca1a0359b8422b9eb95d956988dec Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 8 Mar 2021 19:40:12 -0500 Subject: [PATCH 11/16] fixing api test --- .../transactions/__snapshots__/latency.snap | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap b/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap index 85f2c4cbd3f2d..468776bf692f4 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap +++ b/x-pack/test/apm_api_integration/tests/transactions/__snapshots__/latency.snap @@ -165,3 +165,33 @@ Array [ }, ] `; + +exports[`APM API tests trial apm_8.0.0 Transaction latency with a trial license when data is loaded when not defined environments is seleted should return the correct anomaly boundaries 1`] = ` +Array [ + Object { + "x": 1607436000000, + "y": 0, + "y0": 0, + }, + Object { + "x": 1607436900000, + "y": 0, + "y0": 0, + }, +] +`; + +exports[`APM API tests trial apm_8.0.0 Transaction latency with a trial license when data is loaded with environment selected should return a non-empty anomaly series 1`] = ` +Array [ + Object { + "x": 1607436000000, + "y": 136610.507897203, + "y0": 22581.5157631454, + }, + Object { + "x": 1607436900000, + "y": 136610.507897203, + "y0": 22581.5157631454, + }, +] +`; From 23b99c0214f83ce4edf1c1cd270e36831b4d4b23 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 9 Mar 2021 11:48:43 -0500 Subject: [PATCH 12/16] refactoring --- .../use_transaction_latency_chart_fetcher.ts | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts b/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts index a6049d03b5341..16a82b1d4972b 100644 --- a/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts +++ b/x-pack/plugins/apm/public/hooks/use_transaction_latency_chart_fetcher.ts @@ -30,16 +30,11 @@ export function useTransactionLatencyChartsFetcher() { }, } = useUrlParams(); - const { - comparisonStart = undefined, - comparisonEnd = undefined, - } = comparisonType - ? getTimeRangeComparison({ - start, - end, - comparisonType, - }) - : {}; + const { comparisonStart, comparisonEnd } = getTimeRangeComparison({ + start, + end, + comparisonType, + }); const { data, error, status } = useFetcher( (callApmApi) => { From 6788723950716757bb2f011156039a2263ca1970 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 9 Mar 2021 13:43:46 -0500 Subject: [PATCH 13/16] fixing ts issue --- .../get_service_transaction_group_comparison_statistics.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts b/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts index ebdb5fb5bbdc2..9f6c85fcb1858 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts @@ -247,7 +247,6 @@ export async function getServiceTransactionGroupComparisonStatisticsPeriods({ getOffsetXCoordinate: (timeseries: Coordinate[]) => offsetPreviousPeriodCoordinates({ currentPeriodStart: start, - previousPeriodStart: comparisonStart, previousPeriodTimeseries: timeseries, }), }) From 22d1a9fdea4fdbd461b0f0310ac34cc58b99621d Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 9 Mar 2021 18:34:51 -0500 Subject: [PATCH 14/16] fixing offset function --- ...transaction_group_comparison_statistics.ts | 44 +++++++++++-------- .../transactions/get_latency_charts/index.ts | 2 +- x-pack/plugins/apm/server/routes/services.ts | 2 +- .../offset_previous_period_coordinate.test.ts | 10 +++-- .../offset_previous_period_coordinate.ts | 7 ++- .../tests/transactions/latency.ts | 7 +++ ...ansactions_groups_comparison_statistics.ts | 33 +++++++++----- 7 files changed, 69 insertions(+), 36 deletions(-) diff --git a/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts b/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts index 9f6c85fcb1858..130066a26c3e9 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { keyBy } from 'lodash'; +import { head, keyBy } from 'lodash'; import { EVENT_OUTCOME, SERVICE_NAME, @@ -47,7 +47,6 @@ export async function getServiceTransactionGroupComparisonStatistics({ latencyAggregationType, start, end, - getOffsetXCoordinate, }: { environment?: string; kuery?: string; @@ -60,7 +59,6 @@ export async function getServiceTransactionGroupComparisonStatistics({ latencyAggregationType: LatencyAggregationType; start: number; end: number; - getOffsetXCoordinate?: (timeseries: Coordinate[]) => Coordinate[]; }): Promise< Array<{ transactionName: string; @@ -175,15 +173,9 @@ export async function getServiceTransactionGroupComparisonStatistics({ bucket.transaction_group_total_duration.value || 0; return { transactionName, - latency: getOffsetXCoordinate - ? getOffsetXCoordinate(latency) - : latency, - throughput: getOffsetXCoordinate - ? getOffsetXCoordinate(throughput) - : throughput, - errorRate: getOffsetXCoordinate - ? getOffsetXCoordinate(errorRate) - : errorRate, + latency, + throughput, + errorRate, impact: totalDuration ? (transactionGroupTotalDuration * 100) / totalDuration : 0, @@ -244,11 +236,6 @@ export async function getServiceTransactionGroupComparisonStatisticsPeriods({ ...commonProps, start: comparisonStart, end: comparisonEnd, - getOffsetXCoordinate: (timeseries: Coordinate[]) => - offsetPreviousPeriodCoordinates({ - currentPeriodStart: start, - previousPeriodTimeseries: timeseries, - }), }) : []; @@ -257,8 +244,29 @@ export async function getServiceTransactionGroupComparisonStatisticsPeriods({ previousPeriodPromise, ]); + const firtCurrentPeriod = currentPeriod.length ? currentPeriod[0] : undefined; + return { currentPeriod: keyBy(currentPeriod, 'transactionName'), - previousPeriod: keyBy(previousPeriod, 'transactionName'), + previousPeriod: keyBy( + previousPeriod.map((data) => { + return { + ...data, + errorRate: offsetPreviousPeriodCoordinates({ + currentPeriodTimeseries: firtCurrentPeriod?.errorRate, + previousPeriodTimeseries: data.errorRate, + }), + throughput: offsetPreviousPeriodCoordinates({ + currentPeriodTimeseries: firtCurrentPeriod?.throughput, + previousPeriodTimeseries: data.throughput, + }), + latency: offsetPreviousPeriodCoordinates({ + currentPeriodTimeseries: firtCurrentPeriod?.latency, + previousPeriodTimeseries: data.latency, + }), + }; + }), + 'transactionName' + ), }; } diff --git a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts index ee3bfd9fdadf0..31b5c6ff64dfd 100644 --- a/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts +++ b/x-pack/plugins/apm/server/lib/transactions/get_latency_charts/index.ts @@ -229,7 +229,7 @@ export async function getLatencyPeriods({ previousPeriod: { ...previousPeriod, latencyTimeseries: offsetPreviousPeriodCoordinates({ - currentPeriodStart: currentPeriod.latencyTimeseries[0]?.x, + currentPeriodTimeseries: currentPeriod.latencyTimeseries, previousPeriodTimeseries: previousPeriod.latencyTimeseries, }), }, diff --git a/x-pack/plugins/apm/server/routes/services.ts b/x-pack/plugins/apm/server/routes/services.ts index 147764b16fdad..a84c8dc274248 100644 --- a/x-pack/plugins/apm/server/routes/services.ts +++ b/x-pack/plugins/apm/server/routes/services.ts @@ -415,7 +415,7 @@ export const serviceThroughputRoute = createRoute({ return { currentPeriod, previousPeriod: offsetPreviousPeriodCoordinates({ - currentPeriodStart: currentPeriod[0]?.x, + currentPeriodTimeseries: currentPeriod, previousPeriodTimeseries: previousPeriod, }), }; diff --git a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts index d58b1a8a8ef7a..f965751333838 100644 --- a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts +++ b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.test.ts @@ -7,14 +7,16 @@ import { Coordinate } from '../../typings/timeseries'; import { offsetPreviousPeriodCoordinates } from './offset_previous_period_coordinate'; -const currentPeriodStart = new Date('2021-01-28T14:45:00.000Z').valueOf(); +const currentPeriodTimeseries: Coordinate[] = [ + { x: new Date('2021-01-28T14:45:00.000Z').valueOf(), y: 0 }, +]; describe('mergePeriodsTimeseries', () => { describe('returns empty array', () => { it('when previous timeseries is not defined', () => { expect( offsetPreviousPeriodCoordinates({ - currentPeriodStart, + currentPeriodTimeseries, previousPeriodTimeseries: undefined, }) ).toEqual([]); @@ -23,7 +25,7 @@ describe('mergePeriodsTimeseries', () => { it('when previous timeseries is empty', () => { expect( offsetPreviousPeriodCoordinates({ - currentPeriodStart, + currentPeriodTimeseries, previousPeriodTimeseries: [], }) ).toEqual([]); @@ -40,7 +42,7 @@ describe('mergePeriodsTimeseries', () => { expect( offsetPreviousPeriodCoordinates({ - currentPeriodStart, + currentPeriodTimeseries, previousPeriodTimeseries, }) ).toEqual([ diff --git a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts index 90b188e5547f9..4d095a79394a1 100644 --- a/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts +++ b/x-pack/plugins/apm/server/utils/offset_previous_period_coordinate.ts @@ -9,15 +9,18 @@ import moment from 'moment'; import { Coordinate } from '../../typings/timeseries'; export function offsetPreviousPeriodCoordinates({ - currentPeriodStart = 0, + currentPeriodTimeseries, previousPeriodTimeseries, }: { - currentPeriodStart?: number; + currentPeriodTimeseries?: Coordinate[]; previousPeriodTimeseries?: Coordinate[]; }) { if (!previousPeriodTimeseries?.length) { return []; } + const currentPeriodStart = currentPeriodTimeseries?.length + ? currentPeriodTimeseries[0].x + : 0; const dateDiff = currentPeriodStart - previousPeriodTimeseries[0].x; diff --git a/x-pack/test/apm_api_integration/tests/transactions/latency.ts b/x-pack/test/apm_api_integration/tests/transactions/latency.ts index b657ecf41d553..cb72f7f9844bc 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/latency.ts +++ b/x-pack/test/apm_api_integration/tests/transactions/latency.ts @@ -194,6 +194,13 @@ export default function ApiTest({ getService }: FtrProviderContext) { expectSnapshot(currentPeriodNonNullDataPoints).toMatch(); expectSnapshot(previousPeriodNonNullDataPoints).toMatch(); }); + + it('matches x-axis on current period and previous period', () => { + const latencyChartReturn = response.body as LatencyChartReturnType; + expect(latencyChartReturn.currentPeriod.latencyTimeseries.map(({ x }) => x)).to.be.eql( + latencyChartReturn.previousPeriod.latencyTimeseries.map(({ x }) => x) + ); + }); }); } ); diff --git a/x-pack/test/apm_api_integration/tests/transactions/transactions_groups_comparison_statistics.ts b/x-pack/test/apm_api_integration/tests/transactions/transactions_groups_comparison_statistics.ts index 1065bc9405838..72fb0e832412d 100644 --- a/x-pack/test/apm_api_integration/tests/transactions/transactions_groups_comparison_statistics.ts +++ b/x-pack/test/apm_api_integration/tests/transactions/transactions_groups_comparison_statistics.ts @@ -77,8 +77,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect(Object.keys(currentPeriod).sort()).to.be.eql(transactionNames.sort()); - const currentPeriodItems = Object.values(currentPeriod).map((data) => data); - const previousPeriodItems = Object.values(previousPeriod).map((data) => data); + const currentPeriodItems = Object.values(currentPeriod); + const previousPeriodItems = Object.values(previousPeriod); expect(previousPeriodItems.length).to.be.eql(0); @@ -210,8 +210,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('returns correct latency data', () => { - const currentPeriodItems = Object.values(currentPeriod).map((data) => data); - const previousPeriodItems = Object.values(previousPeriod).map((data) => data); + const currentPeriodItems = Object.values(currentPeriod); + const previousPeriodItems = Object.values(previousPeriod); const currentPeriodFirstItem = currentPeriodItems[0]; const previousPeriodFirstItem = previousPeriodItems[0]; @@ -227,8 +227,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('returns correct throughput data', () => { - const currentPeriodItems = Object.values(currentPeriod).map((data) => data); - const previousPeriodItems = Object.values(previousPeriod).map((data) => data); + const currentPeriodItems = Object.values(currentPeriod); + const previousPeriodItems = Object.values(previousPeriod); const currentPeriodFirstItem = currentPeriodItems[0]; const previousPeriodFirstItem = previousPeriodItems[0]; @@ -244,8 +244,8 @@ export default function ApiTest({ getService }: FtrProviderContext) { }); it('returns correct error rate data', () => { - const currentPeriodItems = Object.values(currentPeriod).map((data) => data); - const previousPeriodItems = Object.values(previousPeriod).map((data) => data); + const currentPeriodItems = Object.values(currentPeriod); + const previousPeriodItems = Object.values(previousPeriod); const currentPeriodFirstItem = currentPeriodItems[0]; const previousPeriodFirstItem = previousPeriodItems[0]; @@ -256,13 +256,26 @@ export default function ApiTest({ getService }: FtrProviderContext) { expect( removeEmptyCoordinates(previousPeriodFirstItem.errorRate).length ).to.be.greaterThan(0); + expectSnapshot(currentPeriodFirstItem.errorRate).toMatch(); expectSnapshot(previousPeriodFirstItem.errorRate).toMatch(); }); + it('matches x-axis on current period and previous period', () => { + const currentPeriodItems = Object.values(currentPeriod); + const previousPeriodItems = Object.values(previousPeriod); + + const currentPeriodFirstItem = currentPeriodItems[0]; + const previousPeriodFirstItem = previousPeriodItems[0]; + + expect(currentPeriodFirstItem.errorRate.map(({ x }) => x)).to.be.eql( + previousPeriodFirstItem.errorRate.map(({ x }) => x) + ); + }); + it('returns correct impact data', () => { - const currentPeriodItems = Object.values(currentPeriod).map((data) => data); - const previousPeriodItems = Object.values(previousPeriod).map((data) => data); + const currentPeriodItems = Object.values(currentPeriod); + const previousPeriodItems = Object.values(previousPeriod); const currentPeriodFirstItem = currentPeriodItems[0]; const previousPeriodFirstItem = previousPeriodItems[0]; From 91598bc807421c7aa5c5029fa94c6bd2280c73d0 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 9 Mar 2021 18:35:11 -0500 Subject: [PATCH 15/16] fixing offset function --- .../get_service_transaction_group_comparison_statistics.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts b/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts index 130066a26c3e9..54e882d1dd6da 100644 --- a/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts +++ b/x-pack/plugins/apm/server/lib/services/get_service_transaction_group_comparison_statistics.ts @@ -5,7 +5,7 @@ * 2.0. */ -import { head, keyBy } from 'lodash'; +import { keyBy } from 'lodash'; import { EVENT_OUTCOME, SERVICE_NAME, From f67dc7f6295eca3a5720b973e60303cd105d44cf Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Wed, 10 Mar 2021 09:03:13 -0500 Subject: [PATCH 16/16] addressing PR comments --- .../components/shared/charts/latency_chart/index.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx index b16a81f0b04cd..3f61273729e64 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/latency_chart/index.tsx @@ -9,7 +9,6 @@ import { EuiFlexGroup, EuiFlexItem, EuiSelect, EuiTitle } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import React from 'react'; import { useHistory } from 'react-router-dom'; -import { APMChartSpec, Coordinate } from '../../../../../typings/timeseries'; import { LatencyAggregationType } from '../../../../../common/latency_aggregation_types'; import { getDurationFormatter } from '../../../../../common/utils/formatters'; import { useLicenseContext } from '../../../../context/license/use_license_context'; @@ -35,6 +34,10 @@ const options: Array<{ value: LatencyAggregationType; text: string }> = [ { value: LatencyAggregationType.p99, text: '99th percentile' }, ]; +function filterNil(value: T | null | undefined): value is T { + return value != null; +} + export function LatencyChart({ height }: Props) { const history = useHistory(); const theme = useTheme(); @@ -58,7 +61,7 @@ export function LatencyChart({ height }: Props) { const timeseries = [ currentPeriod, comparisonEnabled ? previousPeriod : undefined, - ].filter((data) => data) as Array>; + ].filter(filterNil); const latencyMaxY = getMaxY(timeseries); const latencyFormatter = getDurationFormatter(latencyMaxY);