From 57173178eba43281600650883146a9cca960a6dd Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 26 Nov 2020 15:06:06 +0100 Subject: [PATCH 1/5] adding anomalies to transaction duration chart --- .../shared/charts/timeseries_chart.tsx | 51 ++++++++++++++++++- .../charts/transaction_charts/index.tsx | 3 +- .../public/selectors/chart_selectors.test.ts | 14 ++--- .../apm/public/selectors/chart_selectors.ts | 44 ++++++++-------- 4 files changed, 78 insertions(+), 34 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx index ea6f2a4a233e5..b086104ff0525 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx @@ -16,6 +16,7 @@ import { niceTimeFormatter, Placement, Position, + RectAnnotation, ScaleType, Settings, YDomainRange, @@ -27,7 +28,7 @@ import React, { useEffect } from 'react'; import { useHistory } from 'react-router-dom'; import { useChartTheme } from '../../../../../observability/public'; import { asAbsoluteDateTime } from '../../../../common/utils/formatters'; -import { TimeSeries } from '../../../../typings/timeseries'; +import { RectCoordinate, TimeSeries } from '../../../../typings/timeseries'; import { FETCH_STATUS } from '../../../hooks/useFetcher'; import { useTheme } from '../../../hooks/useTheme'; import { useUrlParams } from '../../../hooks/useUrlParams'; @@ -53,6 +54,7 @@ interface Props { yTickFormat?: (y: number) => string; showAnnotations?: boolean; yDomain?: YDomainRange; + anomalySeries?: TimeSeries[]; } export function TimeseriesChart({ @@ -65,6 +67,7 @@ export function TimeseriesChart({ yTickFormat, showAnnotations = true, yDomain, + anomalySeries, }: Props) { const history = useHistory(); const chartRef = React.createRef(); @@ -102,7 +105,12 @@ export function TimeseriesChart({ onBrushEnd({ x, history })} - theme={chartTheme} + theme={{ + ...chartTheme, + areaSeriesStyle: { + line: { visible: false }, + }, + }} onPointerUpdate={setPointerEvent} externalPointerEvents={{ tooltip: { visible: true, placement: Placement.Bottom }, @@ -169,6 +177,45 @@ export function TimeseriesChart({ /> ); })} + + {anomalySeries && + anomalySeries.map((anomalySerie) => { + if (anomalySerie.type === 'area') { + return ( + false} + /> + ); + } + return ( + ({ + coordinates: { + x0, + x1, + }, + }) + )} + style={{ + fill: anomalySerie.color, + }} + /> + ); + })} ); diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/index.tsx index 3f8071ec39f0f..ac117bbbd922a 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/index.tsx @@ -46,7 +46,7 @@ export function TransactionCharts({ }: TransactionChartProps) { const { transactionType } = urlParams; - const { responseTimeSeries, tpmSeries } = charts; + const { responseTimeSeries, tpmSeries, anomalySeries } = charts; const { formatter, toggleSerie } = useFormatter(responseTimeSeries); @@ -79,6 +79,7 @@ export function TransactionCharts({ id="transactionDuration" timeseries={responseTimeSeries || []} yLabelFormat={getResponseTimeTickFormatter(formatter)} + anomalySeries={anomalySeries} onToggleLegend={(serie) => { if (serie) { toggleSerie(serie); diff --git a/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts b/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts index a17faebc9aefa..c35d866878748 100644 --- a/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts +++ b/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts @@ -23,13 +23,12 @@ describe('chart selectors', () => { it('should return anomalyScoreSeries', () => { const data = [{ x0: 0, x: 10 }]; expect(getAnomalyScoreSeries(data)).toEqual({ - areaColor: 'rgba(231,102,76,0.1)', - color: 'none', + color: '#e7664c', data: [{ x0: 0, x: 10 }], hideLegend: true, hideTooltipValue: true, title: 'Anomaly score', - type: 'areaMaxHeight', + type: 'rectAnnotation', }); }); }); @@ -55,9 +54,7 @@ describe('chart selectors', () => { }; it('should produce correct series', () => { - expect( - getResponseTimeSeries({ apmTimeseries, anomalyTimeseries: undefined }) - ).toEqual([ + expect(getResponseTimeSeries({ apmTimeseries })).toEqual([ { color: '#6092c0', data: [ @@ -92,10 +89,7 @@ describe('chart selectors', () => { }); it('should return 3 series', () => { - expect( - getResponseTimeSeries({ apmTimeseries, anomalyTimeseries: undefined }) - .length - ).toBe(3); + expect(getResponseTimeSeries({ apmTimeseries }).length).toBe(3); }); }); diff --git a/x-pack/plugins/apm/public/selectors/chart_selectors.ts b/x-pack/plugins/apm/public/selectors/chart_selectors.ts index 663fbc9028108..735ba91ea7dc2 100644 --- a/x-pack/plugins/apm/public/selectors/chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/chart_selectors.ts @@ -34,6 +34,7 @@ export interface ITransactionChartData { tpmSeries?: ITpmBucket[]; responseTimeSeries?: TimeSeries[]; mlJobId: string | undefined; + anomalySeries?: TimeSeries[]; } const INITIAL_DATA: Partial = { @@ -58,16 +59,33 @@ export function getTransactionCharts( transactionCharts.responseTimeSeries = getResponseTimeSeries({ apmTimeseries, + }); + + transactionCharts.anomalySeries = getResponseTimeAnnomalySeries({ anomalyTimeseries, }); } return transactionCharts; } +function getResponseTimeAnnomalySeries({ + anomalyTimeseries, +}: { + anomalyTimeseries: TimeSeriesAPIResponse['anomalyTimeseries']; +}): TimeSeries[] | undefined { + if (anomalyTimeseries) { + return [ + getAnomalyBoundariesSeries(anomalyTimeseries?.anomalyBoundaries), + getAnomalyScoreSeries(anomalyTimeseries?.anomalyScore), + ]; + } +} + export function getResponseTimeSeries({ apmTimeseries, - anomalyTimeseries, -}: TimeSeriesAPIResponse) { +}: { + apmTimeseries: TimeSeriesAPIResponse['apmTimeseries']; +}) { const { overallAvgDuration } = apmTimeseries; const { avg, p95, p99 } = apmTimeseries.responseTimes; @@ -107,16 +125,6 @@ export function getResponseTimeSeries({ }, ]; - if (anomalyTimeseries) { - // insert after Avg. series - series.splice( - 1, - 0, - getAnomalyBoundariesSeries(anomalyTimeseries.anomalyBoundaries), - getAnomalyScoreSeries(anomalyTimeseries.anomalyScore) - ); - } - return series; } @@ -125,12 +133,9 @@ export function getAnomalyScoreSeries(data: RectCoordinate[]) { title: i18n.translate('xpack.apm.transactions.chart.anomalyScoreLabel', { defaultMessage: 'Anomaly score', }), - hideLegend: true, - hideTooltipValue: true, data, - type: 'areaMaxHeight', - color: 'none', - areaColor: rgba(theme.euiColorVis9, 0.1), + type: 'rectAnnotation', + color: theme.euiColorVis9, }; } @@ -142,12 +147,9 @@ function getAnomalyBoundariesSeries(data: Coordinate[]) { defaultMessage: 'Anomaly Boundaries', } ), - hideLegend: true, - hideTooltipValue: true, data, type: 'area', - color: 'none', - areaColor: rgba(theme.euiColorVis1, 0.1), + color: rgba(theme.euiColorVis1, 0.5), }; } From bf00555aff63f9257372ad976f67564795261bda Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 26 Nov 2020 16:15:31 +0100 Subject: [PATCH 2/5] removing extra : --- .../components/shared/charts/transaction_charts/ml_header.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/ml_header.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/ml_header.tsx index 52b0470d31552..ee5cd8960d335 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/ml_header.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/transaction_charts/ml_header.tsx @@ -84,7 +84,7 @@ export function MLHeader({ hasValidMlLicense, mlJobId }: Props) { transactionType={transactionType} > {i18n.translate('xpack.apm.metrics.transactionChart.viewJob', { - defaultMessage: 'View Job:', + defaultMessage: 'View Job', })} From 1003ed7dbd8af92efe679be5b17417b30fc73a13 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Thu, 26 Nov 2020 16:20:22 +0100 Subject: [PATCH 3/5] fixing test --- x-pack/plugins/apm/public/selectors/chart_selectors.test.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts b/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts index c35d866878748..c9e6177f2c721 100644 --- a/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts +++ b/x-pack/plugins/apm/public/selectors/chart_selectors.test.ts @@ -25,8 +25,6 @@ describe('chart selectors', () => { expect(getAnomalyScoreSeries(data)).toEqual({ color: '#e7664c', data: [{ x0: 0, x: 10 }], - hideLegend: true, - hideTooltipValue: true, title: 'Anomaly score', type: 'rectAnnotation', }); From 1e65b471e7014561dba9ba42c1b23db81332526a Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 30 Nov 2020 13:23:48 +0100 Subject: [PATCH 4/5] addressing pr comments --- .../shared/charts/timeseries_chart.tsx | 23 +++++++++++-------- .../apm/public/selectors/chart_selectors.ts | 19 ++++++++++----- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx index b086104ff0525..b977b19f8793f 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx @@ -34,6 +34,7 @@ import { useTheme } from '../../../hooks/useTheme'; import { useUrlParams } from '../../../hooks/useUrlParams'; import { useAnnotations } from '../../../hooks/use_annotations'; import { useChartPointerEvent } from '../../../hooks/use_chart_pointer_event'; +import { AnomalySeries } from '../../../selectors/chart_selectors'; import { unit } from '../../../style/variables'; import { ChartContainer } from './chart_container'; import { onBrushEnd } from './helper/helper'; @@ -54,7 +55,7 @@ interface Props { yTickFormat?: (y: number) => string; showAnnotations?: boolean; yDomain?: YDomainRange; - anomalySeries?: TimeSeries[]; + anomalySeries?: AnomalySeries; } export function TimeseriesChart({ @@ -179,19 +180,21 @@ export function TimeseriesChart({ })} {anomalySeries && - anomalySeries.map((anomalySerie) => { - if (anomalySerie.type === 'area') { + Object.keys(anomalySeries).map((key) => { + const anomalyType = key as keyof AnomalySeries; + const anomaly = anomalySeries[anomalyType]; + if (anomalyType === 'bounderies') { return ( false} @@ -200,9 +203,9 @@ export function TimeseriesChart({ } return ( ({ coordinates: { x0, @@ -211,7 +214,7 @@ export function TimeseriesChart({ }) )} style={{ - fill: anomalySerie.color, + fill: anomaly.color, }} /> ); diff --git a/x-pack/plugins/apm/public/selectors/chart_selectors.ts b/x-pack/plugins/apm/public/selectors/chart_selectors.ts index 735ba91ea7dc2..2fdcaf9e4e675 100644 --- a/x-pack/plugins/apm/public/selectors/chart_selectors.ts +++ b/x-pack/plugins/apm/public/selectors/chart_selectors.ts @@ -30,11 +30,16 @@ export interface ITpmBucket { color: string; } +export interface AnomalySeries { + scores: TimeSeries; + bounderies: TimeSeries; +} + export interface ITransactionChartData { tpmSeries?: ITpmBucket[]; responseTimeSeries?: TimeSeries[]; mlJobId: string | undefined; - anomalySeries?: TimeSeries[]; + anomalySeries?: AnomalySeries; } const INITIAL_DATA: Partial = { @@ -72,12 +77,14 @@ function getResponseTimeAnnomalySeries({ anomalyTimeseries, }: { anomalyTimeseries: TimeSeriesAPIResponse['anomalyTimeseries']; -}): TimeSeries[] | undefined { +}): AnomalySeries | undefined { if (anomalyTimeseries) { - return [ - getAnomalyBoundariesSeries(anomalyTimeseries?.anomalyBoundaries), - getAnomalyScoreSeries(anomalyTimeseries?.anomalyScore), - ]; + return { + bounderies: getAnomalyBoundariesSeries( + anomalyTimeseries.anomalyBoundaries + ), + scores: getAnomalyScoreSeries(anomalyTimeseries.anomalyScore), + }; } } From 1ebf81612ccaf010baa6c678cb7fa811d568ebbf Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Mon, 30 Nov 2020 14:34:48 +0100 Subject: [PATCH 5/5] addressing pr comments --- .../shared/charts/timeseries_chart.tsx | 69 ++++++++----------- 1 file changed, 29 insertions(+), 40 deletions(-) diff --git a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx index b977b19f8793f..a857707ca0c75 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/timeseries_chart.tsx @@ -179,46 +179,35 @@ export function TimeseriesChart({ ); })} - {anomalySeries && - Object.keys(anomalySeries).map((key) => { - const anomalyType = key as keyof AnomalySeries; - const anomaly = anomalySeries[anomalyType]; - if (anomalyType === 'bounderies') { - return ( - false} - /> - ); - } - return ( - ({ - coordinates: { - x0, - x1, - }, - }) - )} - style={{ - fill: anomaly.color, - }} - /> - ); - })} + {anomalySeries?.bounderies && ( + false} + /> + )} + + {anomalySeries?.scores && ( + ({ + coordinates: { x0, x1 }, + }) + )} + style={{ fill: anomalySeries.scores.color }} + /> + )} );