From c069d57c3bf0ee2479385a292f45e3859bf532f2 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Tue, 24 Nov 2020 16:43:05 +0100 Subject: [PATCH 1/2] fixing charts --- .../TransactionBreakdownGraph/index.tsx | 35 ++++++++++++--- .../shared/charts/annotations/index.tsx | 45 ------------------- .../shared/charts/timeseries_chart.tsx | 36 +++++++++++++-- .../transaction_error_rate_chart/index.tsx | 1 + .../public/context/charts_sync_context.tsx | 6 ++- 5 files changed, 69 insertions(+), 54 deletions(-) delete mode 100644 x-pack/plugins/apm/public/components/shared/charts/annotations/index.tsx diff --git a/x-pack/plugins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownGraph/index.tsx b/x-pack/plugins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownGraph/index.tsx index 677e4b7593ff1..0f5f34ec34dc2 100644 --- a/x-pack/plugins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownGraph/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/TransactionBreakdown/TransactionBreakdownGraph/index.tsx @@ -5,26 +5,33 @@ */ import { + AnnotationDomainTypes, AreaSeries, Axis, Chart, CurveType, + LineAnnotation, niceTimeFormatter, Placement, Position, ScaleType, Settings, } from '@elastic/charts'; +import { EuiIcon } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; import moment from 'moment'; import React, { useEffect } from 'react'; import { useHistory } from 'react-router-dom'; -import { asPercent } from '../../../../../common/utils/formatters'; +import { + asAbsoluteDateTime, + asPercent, +} from '../../../../../common/utils/formatters'; import { TimeSeries } from '../../../../../typings/timeseries'; import { FETCH_STATUS } from '../../../../hooks/useFetcher'; +import { useTheme } from '../../../../hooks/useTheme'; import { useUrlParams } from '../../../../hooks/useUrlParams'; -import { useChartsSync as useChartsSync2 } from '../../../../hooks/use_charts_sync'; +import { useChartsSync } from '../../../../hooks/use_charts_sync'; import { unit } from '../../../../style/variables'; -import { Annotations } from '../../charts/annotations'; import { ChartContainer } from '../../charts/chart_container'; import { onBrushEnd } from '../../charts/helper/helper'; @@ -38,8 +45,9 @@ interface Props { export function TransactionBreakdownGraph({ fetchStatus, timeseries }: Props) { const history = useHistory(); const chartRef = React.createRef(); - const { event, setEvent } = useChartsSync2(); + const { event, setEvent, annotations } = useChartsSync(); const { urlParams } = useUrlParams(); + const theme = useTheme(); const { start, end } = urlParams; useEffect(() => { @@ -53,6 +61,8 @@ export function TransactionBreakdownGraph({ fetchStatus, timeseries }: Props) { const xFormatter = niceTimeFormatter([min, max]); + const annotationColor = theme.eui.euiColorSecondary; + return ( asPercent(y ?? 0, 1)} /> - + ({ + dataValue: annotation['@timestamp'], + header: asAbsoluteDateTime(annotation['@timestamp']), + details: `${i18n.translate('xpack.apm.chart.annotation.version', { + defaultMessage: 'Version', + })} ${annotation.text}`, + }))} + style={{ + line: { strokeWidth: 1, stroke: annotationColor, opacity: 1 }, + }} + marker={} + markerPosition={Position.Top} + /> {timeseries?.length ? ( timeseries.map((serie) => { diff --git a/x-pack/plugins/apm/public/components/shared/charts/annotations/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/annotations/index.tsx deleted file mode 100644 index 683c66b2a96fe..0000000000000 --- a/x-pack/plugins/apm/public/components/shared/charts/annotations/index.tsx +++ /dev/null @@ -1,45 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -import { - AnnotationDomainTypes, - LineAnnotation, - Position, -} from '@elastic/charts'; -import { EuiIcon } from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import React from 'react'; -import { asAbsoluteDateTime } from '../../../../../common/utils/formatters'; -import { useTheme } from '../../../../hooks/useTheme'; -import { useAnnotations } from '../../../../hooks/use_annotations'; - -export function Annotations() { - const { annotations } = useAnnotations(); - const theme = useTheme(); - - if (!annotations.length) { - return null; - } - - const color = theme.eui.euiColorSecondary; - - return ( - ({ - dataValue: annotation['@timestamp'], - header: asAbsoluteDateTime(annotation['@timestamp']), - details: `${i18n.translate('xpack.apm.chart.annotation.version', { - defaultMessage: 'Version', - })} ${annotation.text}`, - }))} - style={{ line: { strokeWidth: 1, stroke: color, opacity: 1 } }} - marker={} - markerPosition={Position.Top} - /> - ); -} 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 918e940651dee..078952afa7387 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 @@ -5,11 +5,13 @@ */ import { + AnnotationDomainTypes, AreaSeries, Axis, Chart, CurveType, LegendItemListener, + LineAnnotation, LineSeries, niceTimeFormatter, Placement, @@ -17,16 +19,20 @@ import { ScaleType, Settings, SettingsSpec, + YDomainRange, } from '@elastic/charts'; +import { EuiIcon } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; import moment from 'moment'; import React, { useEffect } from 'react'; import { useHistory } from 'react-router-dom'; +import { asAbsoluteDateTime } from '../../../../common/utils/formatters'; import { TimeSeries } from '../../../../typings/timeseries'; import { FETCH_STATUS } from '../../../hooks/useFetcher'; +import { useTheme } from '../../../hooks/useTheme'; import { useUrlParams } from '../../../hooks/useUrlParams'; import { useChartsSync } from '../../../hooks/use_charts_sync'; import { unit } from '../../../style/variables'; -import { Annotations } from './annotations'; import { ChartContainer } from './chart_container'; import { onBrushEnd } from './helper/helper'; @@ -45,6 +51,7 @@ interface Props { */ yTickFormat?: (y: number) => string; showAnnotations?: boolean; + yDomain?: YDomainRange; } export function TimeseriesChart({ @@ -56,11 +63,14 @@ export function TimeseriesChart({ yLabelFormat, yTickFormat, showAnnotations = true, + yDomain, }: Props) { const history = useHistory(); const chartRef = React.createRef(); - const { event, setEvent } = useChartsSync(); + const { event, setEvent, annotations } = useChartsSync(); const { urlParams } = useUrlParams(); + const theme = useTheme(); + const { start, end } = urlParams; useEffect(() => { @@ -89,6 +99,8 @@ export function TimeseriesChart({ y === null || y === undefined ); + const annotationColor = theme.eui.euiColorSecondary; + return ( @@ -118,6 +130,7 @@ export function TimeseriesChart({ tickFormat={xFormatter} /> - {showAnnotations && } + {showAnnotations && ( + ({ + dataValue: annotation['@timestamp'], + header: asAbsoluteDateTime(annotation['@timestamp']), + details: `${i18n.translate('xpack.apm.chart.annotation.version', { + defaultMessage: 'Version', + })} ${annotation.text}`, + }))} + style={{ + line: { strokeWidth: 1, stroke: annotationColor, opacity: 1 }, + }} + marker={} + markerPosition={Position.Top} + /> + )} {timeseries.map((serie) => { const Series = serie.type === 'area' ? AreaSeries : LineSeries; diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx index b9028ff2e9e8c..00472df95c4b1 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/transaction_error_rate_chart/index.tsx @@ -91,6 +91,7 @@ export function TransactionErrorRateChart({ ]} yLabelFormat={yLabelFormat} yTickFormat={yTickFormat} + yDomain={{ min: 0, max: 1 }} /> ); diff --git a/x-pack/plugins/apm/public/context/charts_sync_context.tsx b/x-pack/plugins/apm/public/context/charts_sync_context.tsx index d983a857a26ec..6197790c5e2f1 100644 --- a/x-pack/plugins/apm/public/context/charts_sync_context.tsx +++ b/x-pack/plugins/apm/public/context/charts_sync_context.tsx @@ -11,10 +11,13 @@ import React, { SetStateAction, useState, } from 'react'; +import { Annotation } from '../../common/annotations'; +import { useAnnotations } from '../hooks/use_annotations'; export const ChartsSyncContext = createContext<{ event: any; setEvent: Dispatch>; + annotations: Annotation[]; } | null>(null); export function ChartsSyncContextProvider({ @@ -23,10 +26,11 @@ export function ChartsSyncContextProvider({ children: ReactNode; }) { const [event, setEvent] = useState({}); + const { annotations } = useAnnotations(); return ( ); From 68490eef00656f0cf8f331faf3d8c6dd37e0a761 Mon Sep 17 00:00:00 2001 From: cauemarcondes Date: Wed, 25 Nov 2020 10:00:50 +0100 Subject: [PATCH 2/2] addressing pr comments --- .../ErrorGroupDetails/Distribution/index.tsx | 7 +- .../TransactionDetails/Distribution/index.tsx | 2 +- .../shared/charts/timeseries_chart.tsx | 6 +- .../transaction_breakdown_chart_contents.tsx | 5 +- .../charts/transaction_charts/index.tsx | 119 +++++++++--------- .../public/context/annotations_context.tsx | 49 ++++++++ .../public/context/charts_sync_context.tsx | 6 +- .../apm/public/hooks/use_annotations.ts | 34 ++--- 8 files changed, 134 insertions(+), 94 deletions(-) create mode 100644 x-pack/plugins/apm/public/context/annotations_context.tsx diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution/index.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution/index.tsx index 99316e3520a76..159f111bee04c 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution/index.tsx @@ -90,7 +90,12 @@ export function ErrorDistribution({ distribution, title }: Props) { showOverlappingTicks tickFormat={xFormatter} /> - + formatYShort(value)} /> (); - const { event, setEvent, annotations } = useChartsSync(); + const { event, setEvent } = useChartsSync(); + const { annotations } = useAnnotations(); const chartTheme = useChartTheme(); const { urlParams } = useUrlParams(); const theme = useTheme(); @@ -122,6 +124,7 @@ export function TimeseriesChart({ position={Position.Bottom} showOverlappingTicks tickFormat={xFormatter} + gridLine={{ visible: false }} /> {showAnnotations && ( diff --git a/x-pack/plugins/apm/public/components/shared/charts/transaction_breakdown_chart/transaction_breakdown_chart_contents.tsx b/x-pack/plugins/apm/public/components/shared/charts/transaction_breakdown_chart/transaction_breakdown_chart_contents.tsx index 3cbec6cef772b..9d56944a3d93d 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/transaction_breakdown_chart/transaction_breakdown_chart_contents.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/transaction_breakdown_chart/transaction_breakdown_chart_contents.tsx @@ -31,6 +31,7 @@ import { TimeSeries } from '../../../../../typings/timeseries'; import { FETCH_STATUS } from '../../../../hooks/useFetcher'; import { useTheme } from '../../../../hooks/useTheme'; import { useUrlParams } from '../../../../hooks/useUrlParams'; +import { useAnnotations } from '../../../../hooks/use_annotations'; import { useChartsSync } from '../../../../hooks/use_charts_sync'; import { unit } from '../../../../style/variables'; import { ChartContainer } from '../../charts/chart_container'; @@ -51,7 +52,8 @@ export function TransactionBreakdownChartContents({ }: Props) { const history = useHistory(); const chartRef = React.createRef(); - const { event, setEvent, annotations } = useChartsSync(); + const { event, setEvent } = useChartsSync(); + const { annotations } = useAnnotations(); const chartTheme = useChartTheme(); const { urlParams } = useUrlParams(); const theme = useTheme(); @@ -93,6 +95,7 @@ export function TransactionBreakdownChartContents({ position={Position.Bottom} showOverlappingTicks tickFormat={xFormatter} + gridLine={{ visible: false }} /> - - - - - - - - {responseTimeLabel(transactionType)} - - - - {(license) => ( - - )} - - - { - if (serie) { - toggleSerie(serie); - } - }} - /> - - + + + + + + + + + {responseTimeLabel(transactionType)} + + + + {(license) => ( + + )} + + + { + if (serie) { + toggleSerie(serie); + } + }} + /> + + - - - - {tpmLabel(transactionType)} - - - - - + + + + {tpmLabel(transactionType)} + + + + + - + - - - - - - - - - + + + + + + + + + + ); } diff --git a/x-pack/plugins/apm/public/context/annotations_context.tsx b/x-pack/plugins/apm/public/context/annotations_context.tsx new file mode 100644 index 0000000000000..4e09a3d227b11 --- /dev/null +++ b/x-pack/plugins/apm/public/context/annotations_context.tsx @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React, { createContext } from 'react'; +import { useParams } from 'react-router-dom'; +import { Annotation } from '../../common/annotations'; +import { useFetcher } from '../hooks/useFetcher'; +import { useUrlParams } from '../hooks/useUrlParams'; +import { callApmApi } from '../services/rest/createCallApmApi'; + +export const AnnotationsContext = createContext({ annotations: [] } as { + annotations: Annotation[]; +}); + +const INITIAL_STATE = { annotations: [] }; + +export function AnnotationsContextProvider({ + children, +}: { + children: React.ReactNode; +}) { + const { serviceName } = useParams<{ serviceName?: string }>(); + const { urlParams, uiFilters } = useUrlParams(); + const { start, end } = urlParams; + const { environment } = uiFilters; + + const { data = INITIAL_STATE } = useFetcher(() => { + if (start && end && serviceName) { + return callApmApi({ + endpoint: 'GET /api/apm/services/{serviceName}/annotation/search', + params: { + path: { + serviceName, + }, + query: { + start, + end, + environment, + }, + }, + }); + } + }, [start, end, environment, serviceName]); + + return ; +} diff --git a/x-pack/plugins/apm/public/context/charts_sync_context.tsx b/x-pack/plugins/apm/public/context/charts_sync_context.tsx index 6197790c5e2f1..d983a857a26ec 100644 --- a/x-pack/plugins/apm/public/context/charts_sync_context.tsx +++ b/x-pack/plugins/apm/public/context/charts_sync_context.tsx @@ -11,13 +11,10 @@ import React, { SetStateAction, useState, } from 'react'; -import { Annotation } from '../../common/annotations'; -import { useAnnotations } from '../hooks/use_annotations'; export const ChartsSyncContext = createContext<{ event: any; setEvent: Dispatch>; - annotations: Annotation[]; } | null>(null); export function ChartsSyncContextProvider({ @@ -26,11 +23,10 @@ export function ChartsSyncContextProvider({ children: ReactNode; }) { const [event, setEvent] = useState({}); - const { annotations } = useAnnotations(); return ( ); diff --git a/x-pack/plugins/apm/public/hooks/use_annotations.ts b/x-pack/plugins/apm/public/hooks/use_annotations.ts index e8f6785706a91..1cd9a7e65dda2 100644 --- a/x-pack/plugins/apm/public/hooks/use_annotations.ts +++ b/x-pack/plugins/apm/public/hooks/use_annotations.ts @@ -3,36 +3,16 @@ * or more contributor license agreements. Licensed under the Elastic License; * you may not use this file except in compliance with the Elastic License. */ -import { useParams } from 'react-router-dom'; -import { callApmApi } from '../services/rest/createCallApmApi'; -import { useFetcher } from './useFetcher'; -import { useUrlParams } from './useUrlParams'; -const INITIAL_STATE = { annotations: [] }; +import { useContext } from 'react'; +import { AnnotationsContext } from '../context/annotations_context'; export function useAnnotations() { - const { serviceName } = useParams<{ serviceName?: string }>(); - const { urlParams, uiFilters } = useUrlParams(); - const { start, end } = urlParams; - const { environment } = uiFilters; + const context = useContext(AnnotationsContext); - const { data = INITIAL_STATE } = useFetcher(() => { - if (start && end && serviceName) { - return callApmApi({ - endpoint: 'GET /api/apm/services/{serviceName}/annotation/search', - params: { - path: { - serviceName, - }, - query: { - start, - end, - environment, - }, - }, - }); - } - }, [start, end, environment, serviceName]); + if (!context) { + throw new Error('Missing Annotations context provider'); + } - return data; + return context; }