From edfae05d380b628dba9ffa1b342e8c419ded2fb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cau=C3=AA=20Marcondes?= <55978943+cauemarcondes@users.noreply.github.com> Date: Thu, 21 Jan 2021 15:09:46 +0100 Subject: [PATCH] [APM] Service overview: Introduce time-series comparison (#88665) (#88948) * adding comparision select option * adding time comparison field on some pages * removing unused files * fixing unit test * adding unit tests * enabling comparison for more than 8 days * removing tooltip * refactoring search bar * moving useBreakPoint to common hooks folder, removing useShouldUSeMobileLayout hook * addressing PR comments * addressing PR comments * addressing PR comments * addressing PR comments Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../app/RumDashboard/RumDashboard.tsx | 2 +- .../app/TransactionDetails/index.tsx | 2 +- .../app/service_inventory/index.tsx | 2 +- .../service_inventory.test.tsx | 2 + .../components/app/service_overview/index.tsx | 8 +- .../service_overview_table_container.tsx | 6 +- .../use_should_use_mobile_layout.ts | 27 ---- .../app/transaction_overview/index.tsx | 2 +- .../transaction_overview.test.tsx | 4 +- .../components/shared/Links/url_helpers.ts | 2 + .../public/components/shared/search_bar.tsx | 39 ++++- .../shared/time_comparison/index.test.tsx | 142 +++++++++++++++++ .../shared/time_comparison/index.tsx | 144 ++++++++++++++++++ .../url_params_context/resolve_url_params.ts | 6 + .../context/url_params_context/types.ts | 2 + .../use_break_points.ts} | 0 16 files changed, 344 insertions(+), 46 deletions(-) delete mode 100644 x-pack/plugins/apm/public/components/app/service_overview/use_should_use_mobile_layout.ts create mode 100644 x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx create mode 100644 x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx rename x-pack/plugins/apm/public/{components/app/RumDashboard/hooks/useBreakPoints.ts => hooks/use_break_points.ts} (100%) diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/RumDashboard.tsx b/x-pack/plugins/apm/public/components/app/RumDashboard/RumDashboard.tsx index c810bd3e7c489..447f6d502fe40 100644 --- a/x-pack/plugins/apm/public/components/app/RumDashboard/RumDashboard.tsx +++ b/x-pack/plugins/apm/public/components/app/RumDashboard/RumDashboard.tsx @@ -18,7 +18,7 @@ import { UXMetrics } from './UXMetrics'; import { ImpactfulMetrics } from './ImpactfulMetrics'; import { PageLoadAndViews } from './Panels/PageLoadAndViews'; import { VisitorBreakdownsPanel } from './Panels/VisitorBreakdowns'; -import { useBreakPoints } from './hooks/useBreakPoints'; +import { useBreakPoints } from '../../../hooks/use_break_points'; import { getPercentileLabel } from './UXMetrics/translations'; import { useUrlParams } from '../../../context/url_params_context/use_url_params'; diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx index 6810b56fb8f87..b1a97dd34b887 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/index.tsx @@ -113,7 +113,7 @@ export function TransactionDetails({

{transactionName}

- + diff --git a/x-pack/plugins/apm/public/components/app/service_inventory/index.tsx b/x-pack/plugins/apm/public/components/app/service_inventory/index.tsx index 8776b7e9355f1..2f83d48049486 100644 --- a/x-pack/plugins/apm/public/components/app/service_inventory/index.tsx +++ b/x-pack/plugins/apm/public/components/app/service_inventory/index.tsx @@ -129,7 +129,7 @@ export function ServiceInventory() { return ( <> - + diff --git a/x-pack/plugins/apm/public/components/app/service_inventory/service_inventory.test.tsx b/x-pack/plugins/apm/public/components/app/service_inventory/service_inventory.test.tsx index e501dd3bb7a56..eb8068bc8114d 100644 --- a/x-pack/plugins/apm/public/components/app/service_inventory/service_inventory.test.tsx +++ b/x-pack/plugins/apm/public/components/app/service_inventory/service_inventory.test.tsx @@ -57,6 +57,8 @@ function wrapper({ children }: { children?: ReactNode }) { rangeTo: 'now', start: 'mystart', end: 'myend', + comparisonEnabled: true, + comparisonType: 'yesterday', }} > {children} 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 f7720589359c8..f265a4cb247fa 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 @@ -12,6 +12,7 @@ import { isRumAgentName } from '../../../../common/agent_name'; import { AnnotationsContextProvider } from '../../../context/annotations/annotations_context'; import { useApmServiceContext } from '../../../context/apm_service/use_apm_service_context'; import { ChartPointerEventContextProvider } from '../../../context/chart_pointer_event/chart_pointer_event_context'; +import { useBreakPoints } from '../../../hooks/use_break_points'; import { LatencyChart } from '../../shared/charts/latency_chart'; import { TransactionBreakdownChart } from '../../shared/charts/transaction_breakdown_chart'; import { TransactionErrorRateChart } from '../../shared/charts/transaction_error_rate_chart'; @@ -22,7 +23,6 @@ import { ServiceOverviewErrorsTable } from './service_overview_errors_table'; import { ServiceOverviewInstancesTable } from './service_overview_instances_table'; import { ServiceOverviewThroughputChart } from './service_overview_throughput_chart'; import { ServiceOverviewTransactionsTable } from './service_overview_transactions_table'; -import { useShouldUseMobileLayout } from './use_should_use_mobile_layout'; /** * The height a chart should be if it's next to a table with 5 rows and a title. @@ -44,8 +44,8 @@ export function ServiceOverview({ // The default EuiFlexGroup breaks at 768, but we want to break at 992, so we // observe the window width and set the flex directions of rows accordingly - const shouldUseMobileLayout = useShouldUseMobileLayout(); - const rowDirection = shouldUseMobileLayout ? 'column' : 'row'; + const { isMedium } = useBreakPoints(); + const rowDirection = isMedium ? 'column' : 'row'; const { transactionType } = useApmServiceContext(); const transactionTypeLabel = i18n.translate( @@ -57,7 +57,7 @@ export function ServiceOverview({ return ( - + {isRumAgent && ( diff --git a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table_container.tsx b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table_container.tsx index 76db81a70550d..bbd6dd1d498d3 100644 --- a/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table_container.tsx +++ b/x-pack/plugins/apm/public/components/app/service_overview/service_overview_table_container.tsx @@ -6,7 +6,7 @@ import React, { ReactNode } from 'react'; import styled from 'styled-components'; -import { useShouldUseMobileLayout } from './use_should_use_mobile_layout'; +import { useBreakPoints } from '../../../hooks/use_break_points'; /** * The height for a table on the overview page. Is the height of a 5-row basic @@ -58,12 +58,12 @@ export function ServiceOverviewTableContainer({ children?: ReactNode; isEmptyAndLoading: boolean; }) { - const shouldUseMobileLayout = useShouldUseMobileLayout(); + const { isMedium } = useBreakPoints(); return ( {children} diff --git a/x-pack/plugins/apm/public/components/app/service_overview/use_should_use_mobile_layout.ts b/x-pack/plugins/apm/public/components/app/service_overview/use_should_use_mobile_layout.ts deleted file mode 100644 index bd844a3f2e694..0000000000000 --- a/x-pack/plugins/apm/public/components/app/service_overview/use_should_use_mobile_layout.ts +++ /dev/null @@ -1,27 +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 { isWithinMaxBreakpoint } from '@elastic/eui'; -import { useEffect, useState } from 'react'; - -export function useShouldUseMobileLayout() { - const [shouldUseMobileLayout, setShouldUseMobileLayout] = useState( - isWithinMaxBreakpoint(window.innerWidth, 'm') - ); - - useEffect(() => { - const resizeHandler = () => { - setShouldUseMobileLayout(isWithinMaxBreakpoint(window.innerWidth, 'm')); - }; - window.addEventListener('resize', resizeHandler); - - return () => { - window.removeEventListener('resize', resizeHandler); - }; - }); - - return shouldUseMobileLayout; -} diff --git a/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx b/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx index 30fbfe9cc8708..5be08477b3bf5 100644 --- a/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/transaction_overview/index.tsx @@ -108,7 +108,7 @@ export function TransactionOverview({ serviceName }: TransactionOverviewProps) { return ( <> - + diff --git a/x-pack/plugins/apm/public/components/app/transaction_overview/transaction_overview.test.tsx b/x-pack/plugins/apm/public/components/app/transaction_overview/transaction_overview.test.tsx index bfb9c51bc245e..31d90330721da 100644 --- a/x-pack/plugins/apm/public/components/app/transaction_overview/transaction_overview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/transaction_overview/transaction_overview.test.tsx @@ -130,7 +130,7 @@ describe('TransactionOverview', () => { }); expect(history.location.search).toEqual( - '?transactionType=secondType&rangeFrom=now-15m&rangeTo=now' + '?transactionType=secondType&rangeFrom=now-15m&rangeTo=now&comparisonEnabled=true&comparisonType=yesterday' ); expect(getByText(container, 'firstType')).toBeInTheDocument(); expect(getByText(container, 'secondType')).toBeInTheDocument(); @@ -139,7 +139,7 @@ describe('TransactionOverview', () => { expect(history.push).toHaveBeenCalled(); expect(history.location.search).toEqual( - '?transactionType=firstType&rangeFrom=now-15m&rangeTo=now' + '?transactionType=firstType&rangeFrom=now-15m&rangeTo=now&comparisonEnabled=true&comparisonType=yesterday' ); }); }); diff --git a/x-pack/plugins/apm/public/components/shared/Links/url_helpers.ts b/x-pack/plugins/apm/public/components/shared/Links/url_helpers.ts index 8576d9ee86353..e899e4a07e96f 100644 --- a/x-pack/plugins/apm/public/components/shared/Links/url_helpers.ts +++ b/x-pack/plugins/apm/public/components/shared/Links/url_helpers.ts @@ -85,6 +85,8 @@ export type APMQueryParams = { searchTerm?: string; percentile?: 50 | 75 | 90 | 95 | 99; latencyAggregationType?: string; + comparisonEnabled?: boolean; + comparisonType?: string; } & { [key in LocalUIFilterName]?: string }; // forces every value of T[K] to be type: string diff --git a/x-pack/plugins/apm/public/components/shared/search_bar.tsx b/x-pack/plugins/apm/public/components/shared/search_bar.tsx index 6382f4937ac0e..48d329a853327 100644 --- a/x-pack/plugins/apm/public/components/shared/search_bar.tsx +++ b/x-pack/plugins/apm/public/components/shared/search_bar.tsx @@ -7,22 +7,49 @@ import { EuiFlexGroup, EuiFlexItem } from '@elastic/eui'; import React from 'react'; import styled from 'styled-components'; +import { px, unit } from '../../style/variables'; import { DatePicker } from './DatePicker'; import { KueryBar } from './KueryBar'; +import { TimeComparison } from './time_comparison'; +import { useBreakPoints } from '../../hooks/use_break_points'; const SearchBarFlexGroup = styled(EuiFlexGroup)` margin: ${({ theme }) => `${theme.eui.euiSizeM} ${theme.eui.euiSizeM} -${theme.eui.gutterTypes.gutterMedium} ${theme.eui.euiSizeM}`}; `; -export function SearchBar(props: { prepend?: React.ReactNode | string }) { +interface Props { + prepend?: React.ReactNode | string; + showTimeComparison?: boolean; +} + +function getRowDirection(showColumn: boolean) { + return showColumn ? 'column' : 'row'; +} + +export function SearchBar({ prepend, showTimeComparison = false }: Props) { + const { isMedium, isLarge } = useBreakPoints(); + const itemsStyle = { marginBottom: isLarge ? px(unit) : 0 }; return ( - - - + + + - - + + + {showTimeComparison && ( + + + + )} + + + + ); diff --git a/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx b/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx new file mode 100644 index 0000000000000..6348097a3e3ad --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.test.tsx @@ -0,0 +1,142 @@ +/* + * 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 { render } from '@testing-library/react'; +import React, { ReactNode } from 'react'; +import { MemoryRouter } from 'react-router-dom'; +import { EuiThemeProvider } from '../../../../../observability/public'; +import { MockUrlParamsContextProvider } from '../../../context/url_params_context/mock_url_params_context_provider'; +import { IUrlParams } from '../../../context/url_params_context/types'; +import { + expectTextsInDocument, + expectTextsNotInDocument, +} from '../../../utils/testHelpers'; +import { TimeComparison } from './'; +import * as urlHelpers from '../../shared/Links/url_helpers'; + +function getWrapper(params?: IUrlParams) { + return ({ children }: { children?: ReactNode }) => { + return ( + + + {children} + + + ); + }; +} + +describe('TimeComparison', () => { + const spy = jest.spyOn(urlHelpers, 'replace'); + beforeEach(() => { + jest.resetAllMocks(); + }); + describe('Time range is between 0 - 24 hours', () => { + it('sets default values', () => { + const Wrapper = getWrapper({ + start: '2021-01-28T14:45:00.000Z', + end: '2021-01-28T15:00:00.000Z', + }); + render(, { + wrapper: Wrapper, + }); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + query: { + comparisonEnabled: 'true', + comparisonType: 'yesterday', + }, + }); + }); + it('selects yesterday and enables comparison', () => { + const Wrapper = getWrapper({ + start: '2021-01-28T14:45:00.000Z', + end: '2021-01-28T15:00:00.000Z', + comparisonEnabled: true, + comparisonType: 'yesterday', + }); + const component = render(, { + wrapper: Wrapper, + }); + expectTextsInDocument(component, ['Yesterday', 'A week ago']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); + }); + }); + + describe('Time range is between 24 hours - 1 week', () => { + it('sets default values', () => { + const Wrapper = getWrapper({ + start: '2021-01-26T15:00:00.000Z', + end: '2021-01-28T15:00:00.000Z', + }); + render(, { + wrapper: Wrapper, + }); + expect(spy).toHaveBeenCalledWith(expect.anything(), { + query: { + comparisonEnabled: 'true', + comparisonType: 'week', + }, + }); + }); + it('selects week and enables comparison', () => { + const Wrapper = getWrapper({ + start: '2021-01-26T15:00:00.000Z', + end: '2021-01-28T15:00:00.000Z', + comparisonEnabled: true, + comparisonType: 'week', + }); + const component = render(, { + wrapper: Wrapper, + }); + expectTextsNotInDocument(component, ['Yesterday']); + expectTextsInDocument(component, ['A week ago']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); + }); + }); + + describe('Time range is greater than 7 days', () => { + it('Shows absolute times without year when within the same year', () => { + const Wrapper = getWrapper({ + start: '2021-01-20T15:00:00.000Z', + end: '2021-01-28T15:00:00.000Z', + comparisonEnabled: true, + comparisonType: 'previousPeriod', + }); + const component = render(, { + wrapper: Wrapper, + }); + expect(spy).not.toHaveBeenCalled(); + expectTextsInDocument(component, ['20/01 - 28/01']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); + }); + + it('Shows absolute times with year when on different year', () => { + const Wrapper = getWrapper({ + start: '2020-12-20T15:00:00.000Z', + end: '2021-01-28T15:00:00.000Z', + comparisonEnabled: true, + comparisonType: 'previousPeriod', + }); + const component = render(, { + wrapper: Wrapper, + }); + expect(spy).not.toHaveBeenCalled(); + expectTextsInDocument(component, ['20/12/20 - 28/01/21']); + expect( + (component.getByTestId('comparisonSelect') as HTMLSelectElement) + .selectedIndex + ).toEqual(0); + }); + }); +}); 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 new file mode 100644 index 0000000000000..2195621167e83 --- /dev/null +++ b/x-pack/plugins/apm/public/components/shared/time_comparison/index.tsx @@ -0,0 +1,144 @@ +/* + * 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 { EuiCheckbox, EuiSelect } from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import moment from 'moment'; +import React from 'react'; +import { useHistory } from 'react-router-dom'; +import styled from 'styled-components'; +import { getDateDifference } from '../../../../common/utils/formatters'; +import { useUrlParams } from '../../../context/url_params_context/use_url_params'; +import { px, unit } from '../../../style/variables'; +import * as urlHelpers from '../../shared/Links/url_helpers'; +import { useBreakPoints } from '../../../hooks/use_break_points'; + +const PrependContainer = styled.div` + display: flex; + justify-content: center; + align-items: center; + background-color: ${({ theme }) => theme.eui.euiGradientMiddle}; + padding: 0 ${px(unit)}; +`; + +function formatPreviousPeriodDates({ + momentStart, + momentEnd, +}: { + momentStart: moment.Moment; + momentEnd: moment.Moment; +}) { + const isDifferentYears = momentStart.get('year') !== momentEnd.get('year'); + const dateFormat = isDifferentYears ? 'DD/MM/YY' : 'DD/MM'; + return `${momentStart.format(dateFormat)} - ${momentEnd.format(dateFormat)}`; +} + +function getSelectOptions({ start, end }: { start?: string; end?: string }) { + const momentStart = moment(start); + const momentEnd = moment(end); + const dateDiff = getDateDifference(momentStart, momentEnd, 'days'); + + const yesterdayOption = { + value: 'yesterday', + text: i18n.translate('xpack.apm.timeComparison.select.yesterday', { + defaultMessage: 'Yesterday', + }), + }; + + const aWeekAgoOption = { + value: 'week', + text: i18n.translate('xpack.apm.timeComparison.select.weekAgo', { + defaultMessage: 'A week ago', + }), + }; + + const prevPeriodOption = { + value: 'previousPeriod', + text: formatPreviousPeriodDates({ momentStart, momentEnd }), + }; + + // Less than one day + if (dateDiff < 1) { + return [yesterdayOption, aWeekAgoOption]; + } + + // Less than one week + if (dateDiff <= 7) { + return [aWeekAgoOption]; + } + + // above one week + return [prevPeriodOption]; +} + +export function TimeComparison() { + const history = useHistory(); + const { isMedium, isLarge } = useBreakPoints(); + const { + urlParams: { start, end, comparisonEnabled, comparisonType }, + } = useUrlParams(); + + const selectOptions = getSelectOptions({ start, end }); + + // Sets default values + if (comparisonEnabled === undefined || comparisonType === undefined) { + urlHelpers.replace(history, { + query: { + comparisonEnabled: comparisonEnabled === false ? 'false' : 'true', + comparisonType: comparisonType + ? comparisonType + : selectOptions[0].value, + }, + }); + return null; + } + + const isSelectedComparisonTypeAvailable = selectOptions.some( + ({ value }) => value === comparisonType + ); + + // Replaces type when current one is no longer available in the select options + if (selectOptions.length !== 0 && !isSelectedComparisonTypeAvailable) { + urlHelpers.replace(history, { + query: { comparisonType: selectOptions[0].value }, + }); + return null; + } + + return ( + + 0} + onChange={() => { + urlHelpers.push(history, { + query: { + comparisonEnabled: Boolean(!comparisonEnabled).toString(), + }, + }); + }} + /> + + } + onChange={(e) => { + urlHelpers.push(history, { + query: { + comparisonType: e.target.value, + }, + }); + }} + /> + ); +} diff --git a/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts b/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts index 0596d649116a0..1b8a131bd88a3 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/resolve_url_params.ts @@ -49,6 +49,8 @@ export function resolveUrlParams(location: Location, state: TimeUrlParams) { searchTerm, percentile, latencyAggregationType = LatencyAggregationType.avg, + comparisonEnabled, + comparisonType, } = query; const localUIFilters = pickKeys(query, ...localUIFilterNames); @@ -78,6 +80,10 @@ export function resolveUrlParams(location: Location, state: TimeUrlParams) { searchTerm: toString(searchTerm), percentile: toNumber(percentile), latencyAggregationType, + comparisonEnabled: comparisonEnabled + ? toBoolean(comparisonEnabled) + : undefined, + comparisonType, // ui filters environment, diff --git a/x-pack/plugins/apm/public/context/url_params_context/types.ts b/x-pack/plugins/apm/public/context/url_params_context/types.ts index d792c93b7d0dc..cd5fa55cd132f 100644 --- a/x-pack/plugins/apm/public/context/url_params_context/types.ts +++ b/x-pack/plugins/apm/public/context/url_params_context/types.ts @@ -29,4 +29,6 @@ export type IUrlParams = { searchTerm?: string; percentile?: number; latencyAggregationType?: string; + comparisonEnabled?: boolean; + comparisonType?: string; } & Partial>; diff --git a/x-pack/plugins/apm/public/components/app/RumDashboard/hooks/useBreakPoints.ts b/x-pack/plugins/apm/public/hooks/use_break_points.ts similarity index 100% rename from x-pack/plugins/apm/public/components/app/RumDashboard/hooks/useBreakPoints.ts rename to x-pack/plugins/apm/public/hooks/use_break_points.ts