From 77beed076c27fc4de431b97b9c10c2784d1d3668 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Mon, 18 Mar 2019 11:29:04 +0100 Subject: [PATCH 01/16] [APM] Add useFetcher hook Fix test Address feedback [APM] Add useFetcher to ServiceOverview, and add `react-testing-library` Fix tests Make spec descriptions more clear [APM] Add useFetcher to serviceDetailsView Fix tests Add redirect Fix snapshot Fix redirect to correct bucket Convert TransactionDetails [APM] Fix Datepicker double loading and move date parsing to urlParams Convert more to hooks Revert "[APM] Fix Datepicker double loading and move date parsing to urlParams" This reverts commit ed41e17d57daf0f6053a6e7bd901a500d2c64f92. --- x-pack/package.json | 1 + .../DetailView/index.test.tsx | 72 ++--- .../ErrorGroupDetails/DetailView/index.tsx | 9 +- .../components/app/ErrorGroupDetails/view.tsx | 226 +++++++------ .../app/ErrorGroupOverview/index.tsx | 68 ++-- .../app/ServiceDetails/CPUUsageChart.tsx | 4 +- .../app/ServiceDetails/MemoryUsageChart.tsx | 4 +- .../app/ServiceDetails/ServiceMetrics.tsx | 105 ++++--- .../components/app/ServiceDetails/view.tsx | 83 ++--- .../app/ServiceOverview/ServiceList/index.tsx | 2 +- .../__test__/ServiceOverview.test.js | 77 ----- .../__test__/ServiceOverview.test.tsx | 132 ++++++++ .../ServiceOverview.test.js.snap | 43 --- .../ServiceOverview.test.tsx.snap | 296 ++++++++++++++++++ .../components/app/ServiceOverview/index.ts | 2 - .../components/app/ServiceOverview/view.tsx | 66 ++-- .../TransactionDetails/Distribution/index.tsx | 49 ++- .../app/TransactionDetails/view.tsx | 98 +++--- ...w.test.js => TransactionOverview.test.tsx} | 45 ++- ...snap => TransactionOverview.test.tsx.snap} | 8 +- .../app/TransactionOverview/index.tsx | 157 ++++++---- .../app/TransactionOverview/useRedirect.ts | 17 + .../shared/charts/TransactionCharts/index.tsx | 4 +- .../shared/charts/TransactionCharts/view.tsx | 19 +- .../plugins/apm/public/hooks/useFetcher.tsx | 69 ++++ .../public/hooks/useServiceMetricCharts.ts | 84 +++++ .../hooks/useTransactionDetailsCharts.ts | 41 +++ .../hooks/useTransactionDistribution.ts | 40 +++ .../apm/public/hooks/useTransactionList.ts | 52 +++ .../hooks/useTransactionOverviewCharts.ts | 40 +++ .../plugins/apm/public/hooks/useWaterfall.ts | 30 ++ .../public/services/rest/apm/error_groups.ts | 19 +- .../apm/public/services/rest/apm/metrics.ts | 3 +- .../apm/public/services/rest/apm/services.ts | 9 + .../public/services/rest/apm/status_check.ts | 4 +- .../apm/public/services/rest/apm/traces.ts | 9 + .../services/rest/apm/transaction_groups.ts | 55 ++-- .../__jest__/serviceList.test.js | 71 ----- .../reactReduxRequest/errorDistribution.tsx | 49 --- .../store/reactReduxRequest/errorGroup.tsx | 45 --- .../reactReduxRequest/errorGroupList.tsx | 52 --- .../reactReduxRequest/serviceDetails.tsx | 51 --- .../store/reactReduxRequest/serviceList.tsx | 49 --- .../serviceMetricsCharts.tsx | 92 ------ .../transactionDetailsCharts.tsx | 73 ----- .../transactionDistribution.tsx | 81 ----- .../reactReduxRequest/transactionList.tsx | 85 ----- .../transactionOverviewCharts.tsx | 97 ------ .../store/reactReduxRequest/waterfall.tsx | 51 --- .../public/store/selectors/chartSelectors.ts | 46 +-- x-pack/plugins/apm/public/store/urlParams.ts | 34 +- yarn.lock | 35 +++ 52 files changed, 1468 insertions(+), 1485 deletions(-) delete mode 100644 x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.js create mode 100644 x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.tsx delete mode 100644 x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.js.snap create mode 100644 x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.tsx.snap rename x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/{TransactionOverview.test.js => TransactionOverview.test.tsx} (60%) rename x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/{TransactionOverview.test.js.snap => TransactionOverview.test.tsx.snap} (93%) create mode 100644 x-pack/plugins/apm/public/components/app/TransactionOverview/useRedirect.ts create mode 100644 x-pack/plugins/apm/public/hooks/useFetcher.tsx create mode 100644 x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts create mode 100644 x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts create mode 100644 x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts create mode 100644 x-pack/plugins/apm/public/hooks/useTransactionList.ts create mode 100644 x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts create mode 100644 x-pack/plugins/apm/public/hooks/useWaterfall.ts delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/serviceList.test.js delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/errorDistribution.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/errorGroup.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/errorGroupList.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/serviceDetails.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/serviceList.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/serviceMetricsCharts.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/transactionDetailsCharts.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/transactionDistribution.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/transactionList.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/waterfall.tsx diff --git a/x-pack/package.json b/x-pack/package.json index f33a8da23137c..bd24005712339 100644 --- a/x-pack/package.json +++ b/x-pack/package.json @@ -254,6 +254,7 @@ "react-shortcuts": "^2.0.0", "react-sticky": "^6.0.3", "react-syntax-highlighter": "^5.7.0", + "react-testing-library": "^6.0.0", "react-vis": "^1.8.1", "recompose": "^0.26.0", "reduce-reducers": "^0.4.3", diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.test.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.test.tsx index b265efb818f3f..f0ffb072c8338 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.test.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/DetailView/index.test.tsx @@ -7,8 +7,6 @@ import { shallow } from 'enzyme'; import { Location } from 'history'; import React from 'react'; -import { RRRRenderResponse } from 'react-redux-request'; -import { ErrorGroupAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/get_error_group'; import { APMError } from 'x-pack/plugins/apm/typings/es_schemas/ui/APMError'; import { mockMoment } from '../../../../utils/testHelpers'; import { DetailView } from './index'; @@ -31,21 +29,17 @@ describe('DetailView', () => { }); it('should render Discover button', () => { - const errorGroup: RRRRenderResponse = { - args: [], - status: 'SUCCESS', - data: { - occurrencesCount: 10, - error: ({ - '@timestamp': 'myTimestamp', - http: { request: { method: 'GET' } }, - url: { full: 'myUrl' }, - service: { name: 'myService' }, - user: { id: 'myUserId' }, - error: { exception: { handled: true } }, - transaction: { id: 'myTransactionId', sampled: true } - } as unknown) as APMError - } + const errorGroup = { + occurrencesCount: 10, + error: ({ + '@timestamp': 'myTimestamp', + http: { request: { method: 'GET' } }, + url: { full: 'myUrl' }, + service: { name: 'myService' }, + user: { id: 'myUserId' }, + error: { exception: { handled: true } }, + transaction: { id: 'myTransactionId', sampled: true } + } as unknown) as APMError }; const wrapper = shallow( { }); it('should render StickyProperties', () => { - const errorGroup: RRRRenderResponse = { - args: [], - status: 'SUCCESS', - data: { - occurrencesCount: 10, - error: {} as APMError - } + const errorGroup = { + occurrencesCount: 10, + error: {} as APMError }; const wrapper = shallow( { }); it('should render tabs', () => { - const errorGroup: RRRRenderResponse = { - args: [], - status: 'SUCCESS', - data: { - occurrencesCount: 10, - error: ({ - '@timestamp': 'myTimestamp', - service: {}, - user: {} - } as unknown) as APMError - } + const errorGroup = { + occurrencesCount: 10, + error: ({ + '@timestamp': 'myTimestamp', + service: {}, + user: {} + } as unknown) as APMError }; const wrapper = shallow( { }); it('should render TabContent', () => { - const errorGroup: RRRRenderResponse = { - args: [], - status: 'SUCCESS', - data: { - occurrencesCount: 10, - error: ({ - '@timestamp': 'myTimestamp', - context: {} - } as unknown) as APMError - } + const errorGroup = { + occurrencesCount: 10, + error: ({ + '@timestamp': 'myTimestamp', + context: {} + } as unknown) as APMError }; const wrapper = shallow( ; + errorGroup: ErrorGroupAPIResponse; urlParams: IUrlParams; location: Location; } export function DetailView({ errorGroup, urlParams, location }: Props) { - if (errorGroup.status !== STATUS.SUCCESS) { - return null; - } - const { transaction, error, occurrencesCount } = errorGroup.data; + const { transaction, error, occurrencesCount } = errorGroup; if (!error) { return null; diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx index 170ac689239e2..e15236dae8a2a 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx @@ -20,8 +20,11 @@ import React, { Fragment } from 'react'; import styled from 'styled-components'; import { NOT_AVAILABLE_LABEL } from 'x-pack/plugins/apm/common/i18n'; import { idx } from 'x-pack/plugins/apm/common/idx'; -import { ErrorDistributionRequest } from '../../../store/reactReduxRequest/errorDistribution'; -import { ErrorGroupDetailsRequest } from '../../../store/reactReduxRequest/errorGroup'; +import { useFetcher } from '../../../hooks/useFetcher'; +import { + loadErrorDistribution, + loadErrorGroupDetails +} from '../../../services/rest/apm/error_groups'; import { IUrlParams } from '../../../store/urlParams'; import { fontFamilyCode, fontSizes, px, units } from '../../../style/variables'; // @ts-ignore @@ -64,124 +67,119 @@ interface Props { } export function ErrorGroupDetailsView({ urlParams, location }: Props) { + const { serviceName, start, end, errorGroupId } = urlParams; + const { data: errorGroupData } = useFetcher(loadErrorGroupDetails, { + serviceName, + start, + end, + errorGroupId + }); + + const { data: errorDistributionData } = useFetcher(loadErrorDistribution, { + serviceName, + start, + end + }); + + if (!errorGroupData || !errorDistributionData) { + return null; + } + + // If there are 0 occurrences, show only distribution chart w. empty message + const showDetails = errorGroupData.occurrencesCount !== 0; + const logMessage = idx(errorGroupData, _ => _.error.error.log.message); + const excMessage = idx( + errorGroupData, + _ => _.error.error.exception[0].message + ); + const culprit = idx(errorGroupData, _ => _.error.error.culprit); + const isUnhandled = + idx(errorGroupData, _ => _.error.error.exception[0].handled) === false; + return ( - { - // If there are 0 occurrences, show only distribution chart w. empty message - const showDetails = errorGroup.data.occurrencesCount !== 0; - const logMessage = idx(errorGroup, _ => _.data.error.error.log.message); - const excMessage = idx( - errorGroup, - _ => _.data.error.error.exception[0].message - ); - const culprit = idx(errorGroup, _ => _.data.error.error.culprit); - const isUnhandled = - idx(errorGroup, _ => _.data.error.error.exception[0].handled) === - false; - - return ( -
- - - -

- {i18n.translate( - 'xpack.apm.errorGroupDetails.errorGroupTitle', - { - defaultMessage: 'Error group {errorGroupId}', - values: { - errorGroupId: getShortGroupId(urlParams.errorGroupId) - } - } - )} -

- - - {isUnhandled && ( - - +
+ + + +

+ {i18n.translate('xpack.apm.errorGroupDetails.errorGroupTitle', { + defaultMessage: 'Error group {errorGroupId}', + values: { + errorGroupId: getShortGroupId(urlParams.errorGroupId) + } + })} +

+ + + {isUnhandled && ( + + + {i18n.translate('xpack.apm.errorGroupDetails.unhandledLabel', { + defaultMessage: 'Unhandled' + })} + + + )} + + + + + + + + {showDetails && ( + + + {logMessage && ( + + + {logMessage} + )} - ( - + - - {showDetails && ( - - )} -
- ); - }} - /> + + {excMessage || NOT_AVAILABLE_LABEL} + + {culprit || NOT_AVAILABLE_LABEL} + + + )} + + + + + {showDetails && ( + + )} +
); } diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx index 9e6c40a978741..11306d30fa937 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx @@ -15,9 +15,12 @@ import { i18n } from '@kbn/i18n'; import { Location } from 'history'; import React from 'react'; import { ErrorDistribution } from 'x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution'; -import { ErrorDistributionRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/errorDistribution'; import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; -import { ErrorGroupOverviewRequest } from '../../../store/reactReduxRequest/errorGroupList'; +import { useFetcher } from '../../../hooks/useFetcher'; +import { + loadErrorDistribution, + loadErrorGroupList +} from '../../../services/rest/apm/error_groups'; import { ErrorGroupList } from './List'; interface ErrorGroupOverviewProps { @@ -29,23 +32,48 @@ const ErrorGroupOverview: React.SFC = ({ urlParams, location }) => { + const { + serviceName, + start, + end, + errorGroupId, + kuery, + sortField, + sortDirection + } = urlParams; + const { data: errorDistributionData } = useFetcher(loadErrorDistribution, { + serviceName, + start, + end, + errorGroupId, + kuery + }); + + const { data: errorGroupListData } = useFetcher(loadErrorGroupList, { + serviceName, + start, + end, + sortField, + sortDirection, + kuery + }); + + if (!errorDistributionData || !errorGroupListData) { + return null; + } + return ( - ( - + @@ -59,15 +87,11 @@ const ErrorGroupOverview: React.SFC = ({

Errors

- ( - - )} + items={errorGroupListData} + location={location} /> diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/CPUUsageChart.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/CPUUsageChart.tsx index 46b3243e8649d..264aae21da89e 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/CPUUsageChart.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/CPUUsageChart.tsx @@ -11,11 +11,11 @@ import React from 'react'; import CustomPlot from 'x-pack/plugins/apm/public/components/shared/charts/CustomPlot'; import { HoverXHandlers } from 'x-pack/plugins/apm/public/components/shared/charts/SyncChartGroup'; import { asPercent } from 'x-pack/plugins/apm/public/utils/formatters'; -import { CPUChartAPIResponse } from 'x-pack/plugins/apm/server/lib/metrics/get_cpu_chart_data/transformer'; import { Coordinate } from 'x-pack/plugins/apm/typings/timeseries'; +import { CPUMetricSeries } from '../../../store/selectors/chartSelectors'; interface Props { - data: CPUChartAPIResponse; + data: CPUMetricSeries; hoverXHandlers: HoverXHandlers; } diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/MemoryUsageChart.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/MemoryUsageChart.tsx index 4f620be40ec88..4356142add4c6 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/MemoryUsageChart.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/MemoryUsageChart.tsx @@ -11,11 +11,11 @@ import React from 'react'; import CustomPlot from 'x-pack/plugins/apm/public/components/shared/charts/CustomPlot'; import { HoverXHandlers } from 'x-pack/plugins/apm/public/components/shared/charts/SyncChartGroup'; import { asPercent } from 'x-pack/plugins/apm/public/utils/formatters'; -import { MemoryChartAPIResponse } from 'x-pack/plugins/apm/server/lib/metrics/get_memory_chart_data/transformer'; import { Coordinate } from 'x-pack/plugins/apm/typings/timeseries'; +import { MemoryMetricSeries } from '../../../store/selectors/chartSelectors'; interface Props { - data: MemoryChartAPIResponse; + data: MemoryMetricSeries; hoverXHandlers: HoverXHandlers; } diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx index 1ae81b44af12c..b368f9eebc333 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx @@ -17,10 +17,11 @@ import React from 'react'; import { ErrorDistribution } from 'x-pack/plugins/apm/public/components/app/ErrorGroupDetails/Distribution'; import { SyncChartGroup } from 'x-pack/plugins/apm/public/components/shared/charts/SyncChartGroup'; import { TransactionCharts } from 'x-pack/plugins/apm/public/components/shared/charts/TransactionCharts'; -import { ErrorDistributionRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/errorDistribution'; -import { MetricsChartDataRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/serviceMetricsCharts'; -import { TransactionOverviewChartsRequestForAllTypes } from 'x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts'; import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { useFetcher } from '../../../hooks/useFetcher'; +import { useServiceMetricCharts } from '../../../hooks/useServiceMetricCharts'; +import { useTransactionOverviewCharts } from '../../../hooks/useTransactionOverviewCharts'; +import { loadErrorDistribution } from '../../../services/rest/apm/error_groups'; import { CPUUsageChart } from './CPUUsageChart'; import { MemoryUsageChart } from './MemoryUsageChart'; @@ -30,17 +31,31 @@ interface ServiceMetricsProps { } export function ServiceMetrics({ urlParams, location }: ServiceMetricsProps) { + const { serviceName, start, end, errorGroupId, kuery } = urlParams; + const { data: errorDistributionData } = useFetcher(loadErrorDistribution, { + serviceName, + start, + end, + errorGroupId, + kuery + }); + + const { data: transactionOverviewChartsData } = useTransactionOverviewCharts( + urlParams + ); + + const { data: serviceMetricChartData } = useServiceMetricCharts(urlParams); + + if (!errorDistributionData) { + return null; + } + return ( - ( - - )} + location={location} /> @@ -48,18 +63,13 @@ export function ServiceMetrics({ urlParams, location }: ServiceMetricsProps) { - ( - + @@ -68,34 +78,27 @@ export function ServiceMetrics({ urlParams, location }: ServiceMetricsProps) { - { - return ( - ( - - - - - - - - - - - - - )} - /> - ); - }} + ( + + + + + + + + + + + + + )} /> diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx index 5ae7e3b105633..2e17197eebd49 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx @@ -6,56 +6,59 @@ import { EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiTitle } from '@elastic/eui'; import { Location } from 'history'; -import React, { Fragment } from 'react'; -import { ServiceDetailsRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/serviceDetails'; +import React from 'react'; import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { useFetcher } from '../../../hooks/useFetcher'; +import { loadServiceDetails } from '../../../services/rest/apm/services'; // @ts-ignore import { FilterBar } from '../../shared/FilterBar'; import { ServiceDetailTabs } from './ServiceDetailTabs'; import { ServiceIntegrations } from './ServiceIntegrations'; -interface ServiceDetailsProps { +interface Props { urlParams: IUrlParams; location: Location; } -export class ServiceDetailsView extends React.Component { - public render() { - const { urlParams, location } = this.props; - return ( - + + + +

{urlParams.serviceName}

+ + + + + + + + + + + + { - return ( - - - - -

{urlParams.serviceName}

- - - - - - - - - - - - - - ); - }} + transactionTypes={serviceDetailsData.types} /> - ); - } + + ); } diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/ServiceList/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceOverview/ServiceList/index.tsx index 0d9dd1ed1381f..8bb29b285e097 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceOverview/ServiceList/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceOverview/ServiceList/index.tsx @@ -16,7 +16,7 @@ import { asDecimal, asMillis } from '../../../../utils/formatters'; import { ITableColumn, ManagedTable } from '../../../shared/ManagedTable'; interface Props { - items: IServiceListItem[]; + items?: IServiceListItem[]; noItemsMessage?: React.ReactNode; } diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.js b/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.js deleted file mode 100644 index 2a6d3f8e5fd2e..0000000000000 --- a/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.js +++ /dev/null @@ -1,77 +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 React from 'react'; -import { shallow } from 'enzyme'; -import { ServiceOverview } from '../view'; -import { STATUS } from '../../../../constants'; -import * as apmRestServices from '../../../../services/rest/apm/status_check'; -jest.mock('../../../../services/rest/apm/status_check'); - -describe('Service Overview -> View', () => { - let mockAgentStatus; - let wrapper; - let instance; - - beforeEach(() => { - mockAgentStatus = { - dataFound: true - }; - - apmRestServices.loadAgentStatus = jest.fn(() => - Promise.resolve(mockAgentStatus) - ); - - wrapper = shallow(); - instance = wrapper.instance(); - }); - - it('should render when historical data is found', () => { - expect(wrapper).toMatchSnapshot(); - const List = wrapper - .find('ServiceListRequest') - .props() - .render({}); - expect(List.props).toMatchSnapshot(); - }); - - it('should render when historical data is not found', () => { - wrapper.setState({ historicalDataFound: false }); - expect(wrapper).toMatchSnapshot(); - const List = wrapper - .find('ServiceListRequest') - .props() - .render({}); - expect(List.props).toMatchSnapshot(); - }); - - it('should check for historical data once', () => {}); - - describe('checking for historical data', () => { - it('should set historical data to true if data is found', async () => { - const props = { - serviceList: { - status: STATUS.SUCCESS, - data: [] - } - }; - await instance.checkForHistoricalData(props); - expect(wrapper.state('historicalDataFound')).toEqual(true); - }); - - it('should set historical data state to false if data is NOT found', async () => { - const props = { - serviceList: { - status: STATUS.SUCCESS, - data: [] - } - }; - mockAgentStatus.dataFound = false; - await instance.checkForHistoricalData(props); - expect(wrapper.state('historicalDataFound')).toEqual(false); - }); - }); -}); diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.tsx b/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.tsx new file mode 100644 index 0000000000000..a82be56866e39 --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/ServiceOverview.test.tsx @@ -0,0 +1,132 @@ +/* + * 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 from 'react'; +import { Provider } from 'react-redux'; +import { render, wait, waitForElement } from 'react-testing-library'; +import 'react-testing-library/cleanup-after-each'; +import * as apmRestServices from 'x-pack/plugins/apm/public/services/rest/apm/services'; +// @ts-ignore +import configureStore from 'x-pack/plugins/apm/public/store/config/configureStore'; +import * as statusCheck from '../../../../services/rest/apm/status_check'; +import { ServiceOverview } from '../view'; + +function Comp() { + const store = configureStore(); + + return ( + + + + ); +} + +describe('Service Overview -> View', () => { + afterEach(() => { + jest.resetAllMocks(); + }); + + // Suppress warnings about "act" until async/await syntax is supported: https://github.com/facebook/react/issues/14769 + /* tslint:disable:no-console */ + const originalError = console.error; + beforeAll(() => { + console.error = jest.fn(); + }); + afterAll(() => { + console.error = originalError; + }); + + it('should render services, when list is not empty', async () => { + // mock rest requests + const spy1 = jest + .spyOn(statusCheck, 'loadAgentStatus') + .mockResolvedValue(true); + const spy2 = jest + .spyOn(apmRestServices, 'loadServiceList') + .mockResolvedValue([ + { + serviceName: 'My Python Service', + agentName: 'python', + transactionsPerMinute: 100, + errorsPerMinute: 200, + avgResponseTime: 300 + }, + { + serviceName: 'My Go Service', + agentName: 'go', + transactionsPerMinute: 400, + errorsPerMinute: 500, + avgResponseTime: 600 + } + ]); + + const { container, getByText } = render(); + + // wait for requests to be made + await wait( + () => + expect(spy1).toHaveBeenCalledTimes(1) && + expect(spy2).toHaveBeenCalledTimes(1) + ); + + await waitForElement(() => getByText('My Python Service')); + + expect(container.querySelectorAll('.euiTableRow')).toMatchSnapshot(); + }); + + it('should render getting started message, when list is empty and no historical data is found', async () => { + // mock rest requests + const spy1 = jest + .spyOn(statusCheck, 'loadAgentStatus') + .mockResolvedValue(false); + + const spy2 = jest + .spyOn(apmRestServices, 'loadServiceList') + .mockResolvedValue([]); + + const { container, getByText } = render(); + + // wait for requests to be made + await wait( + () => + expect(spy1).toHaveBeenCalledTimes(1) && + expect(spy2).toHaveBeenCalledTimes(1) + ); + + // wait for elements to be rendered + await waitForElement(() => + getByText( + "Looks like you don't have any APM services installed. Let's add some!" + ) + ); + + expect(container.querySelectorAll('.euiTableRow')).toMatchSnapshot(); + }); + + it('should render empty message, when list is empty and historical data is found', async () => { + // mock rest requests + const spy1 = jest + .spyOn(statusCheck, 'loadAgentStatus') + .mockResolvedValue(true); + const spy2 = jest + .spyOn(apmRestServices, 'loadServiceList') + .mockResolvedValue([]); + + const { container, getByText } = render(); + + // wait for requests to be made + await wait( + () => + expect(spy1).toHaveBeenCalledTimes(1) && + expect(spy2).toHaveBeenCalledTimes(1) + ); + + // wait for elements to be rendered + await waitForElement(() => getByText('No services found')); + + expect(container.querySelectorAll('.euiTableRow')).toMatchSnapshot(); + }); +}); diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.js.snap b/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.js.snap deleted file mode 100644 index a08ba5312ba5c..0000000000000 --- a/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.js.snap +++ /dev/null @@ -1,43 +0,0 @@ -// Jest Snapshot v1, https://goo.gl/fbAQLP - -exports[`Service Overview -> View should render when historical data is found 1`] = ` - - - -`; - -exports[`Service Overview -> View should render when historical data is found 2`] = ` -Object { - "items": Array [], - "noItemsMessage": , -} -`; - -exports[`Service Overview -> View should render when historical data is not found 1`] = ` - - - -`; - -exports[`Service Overview -> View should render when historical data is not found 2`] = ` -Object { - "items": Array [], - "noItemsMessage": , -} -`; diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.tsx.snap b/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.tsx.snap new file mode 100644 index 0000000000000..b7f431249daf8 --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/ServiceOverview.test.tsx.snap @@ -0,0 +1,296 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`Service Overview -> View should render empty message, when list is empty and historical data is found 1`] = ` +NodeList [ + + +
+ +
+ +
+ No services found +
+
+ +
+ +
+ + , +] +`; + +exports[`Service Overview -> View should render getting started message, when list is empty and no historical data is found 1`] = ` +NodeList [ + + +
+ +
+ +
+ Looks like you don't have any APM services installed. Let's add some! +
+
+
+

+ Upgrading from a pre-7.x version? Make sure you've also upgraded + your APM server instance(s) to at least 7.0. +

+

+ You may also have old data that needs to be migrated. + + + Learn more by visiting the Kibana Upgrade Assistant + + . +

+
+ +
+
+ + + +
+ +
+ + , +] +`; + +exports[`Service Overview -> View should render services, when list is not empty 1`] = ` +NodeList [ + + +
+ Name +
+
+ + + My Go Service + + +
+ + +
+ Agent +
+
+ go +
+ + +
+ Avg. response time +
+
+ 1 ms +
+ + +
+ Trans. per minute +
+
+ 400.0 tpm +
+ + +
+ Errors per minute +
+
+ 500.0 err. +
+ + , + + +
+ Name +
+
+ + + My Python Service + + +
+ + +
+ Agent +
+
+ python +
+ + +
+ Avg. response time +
+
+ 0 ms +
+ + +
+ Trans. per minute +
+
+ 100.0 tpm +
+ + +
+ Errors per minute +
+
+ 200.0 err. +
+ + , +] +`; diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/index.ts b/x-pack/plugins/apm/public/components/app/ServiceOverview/index.ts index 7e2ec20677c3d..378bf3118886a 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceOverview/index.ts +++ b/x-pack/plugins/apm/public/components/app/ServiceOverview/index.ts @@ -5,14 +5,12 @@ */ import { connect } from 'react-redux'; -import { getServiceList } from 'x-pack/plugins/apm/public/store/reactReduxRequest/serviceList'; import { IReduxState } from 'x-pack/plugins/apm/public/store/rootReducer'; import { getUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; import { ServiceOverview as View } from './view'; function mapStateToProps(state = {} as IReduxState) { return { - serviceList: getServiceList(state), urlParams: getUrlParams(state) }; } diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx index 1c11b5deff8b4..bdedf87384edb 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx @@ -5,59 +5,33 @@ */ import { EuiPanel } from '@elastic/eui'; -import React, { Component } from 'react'; -import { RRRRenderResponse } from 'react-redux-request'; +import React from 'react'; import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; -import { IServiceListItem } from 'x-pack/plugins/apm/server/lib/services/get_services'; +import { useFetcher } from '../../../hooks/useFetcher'; +import { loadServiceList } from '../../../services/rest/apm/services'; import { loadAgentStatus } from '../../../services/rest/apm/status_check'; -import { ServiceListRequest } from '../../../store/reactReduxRequest/serviceList'; import { NoServicesMessage } from './NoServicesMessage'; import { ServiceList } from './ServiceList'; interface Props { urlParams: IUrlParams; - serviceList: RRRRenderResponse; } -interface State { - // any data submitted from APM agents found (not just in the given time range) - historicalDataFound: boolean; -} - -export class ServiceOverview extends Component { - public state = { historicalDataFound: true }; - - public async checkForHistoricalData() { - const result = await loadAgentStatus(); - this.setState({ historicalDataFound: result.dataFound }); - } - - public componentDidMount() { - this.checkForHistoricalData(); - } - - public render() { - const { urlParams } = this.props; - - // Render method here uses this.props.serviceList instead of received "data" from RRR - // to make it easier to test -- mapStateToProps uses the RRR selector so the data - // is the same either way - return ( - - ( - - } - /> - )} - /> - - ); - } +export function ServiceOverview({ urlParams }: Props) { + const { start, end, kuery } = urlParams; + const { data: agentStatus = true } = useFetcher(loadAgentStatus, []); + const { data: serviceListData } = useFetcher(loadServiceList, { + start, + end, + kuery + }); + + return ( + + } + /> + + ); } diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx index 5a5e4977458b8..e53ad7358004f 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx @@ -52,7 +52,7 @@ export function getFormattedBuckets(buckets: IBucket[], bucketSize: number) { interface Props { location: Location; - distribution: ITransactionDistributionAPIResponse; + distribution?: ITransactionDistributionAPIResponse; urlParams: IUrlParams; } @@ -95,9 +95,54 @@ export class TransactionDistribution extends Component { ); }; + public redirectToTransactionType() { + const { urlParams, location, distribution } = this.props; + + if (!distribution || !distribution.defaultSample) { + return; + } + + const { traceId, transactionId } = distribution.defaultSample; + const hasIncorrectSample = + traceId !== urlParams.traceId || + transactionId !== urlParams.transactionId; + + if (hasIncorrectSample) { + history.replace({ + ...location, + search: fromQuery({ + ...toQuery(location.search), + traceId, + transactionId + }) + }); + } + } + + public componentDidMount() { + this.redirectToTransactionType(); + } + + public componentDidUpdate() { + this.redirectToTransactionType(); + } + public render() { const { location, distribution, urlParams } = this.props; + if (!distribution) { + return null; + } + + // delay rendering until after redirect + const hasIncorrectSample = + distribution.defaultSample && + (distribution.defaultSample.traceId !== urlParams.traceId || + distribution.defaultSample.transactionId !== urlParams.transactionId); + if (hasIncorrectSample) { + return null; + } + const buckets = getFormattedBuckets( distribution.buckets, distribution.bucketSize @@ -163,7 +208,7 @@ export class TransactionDistribution extends Component { bucketIndex={bucketIndex} onClick={(bucket: IChartPoint) => { if (bucket.sample && bucket.y > 0) { - history.replace({ + history.push({ ...location, search: fromQuery({ ...toQuery(location.search), diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx index 7b9eff0cccf3f..daf5c4e3e0695 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx @@ -7,10 +7,11 @@ import { EuiPanel, EuiSpacer, EuiTitle } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { Location } from 'history'; +import _ from 'lodash'; import React from 'react'; -import { TransactionDetailsChartsRequest } from '../../../store/reactReduxRequest/transactionDetailsCharts'; -import { TransactionDistributionRequest } from '../../../store/reactReduxRequest/transactionDistribution'; -import { WaterfallRequest } from '../../../store/reactReduxRequest/waterfall'; +import { useTransactionDetailsCharts } from '../../../hooks/useTransactionDetailsCharts'; +import { useTransactionDistribution } from '../../../hooks/useTransactionDistribution'; +import { useWaterfall } from '../../../hooks/useWaterfall'; import { IUrlParams } from '../../../store/urlParams'; import { TransactionCharts } from '../../shared/charts/TransactionCharts'; import { EmptyMessage } from '../../shared/EmptyMessage'; @@ -25,6 +26,13 @@ interface Props { } export function TransactionDetailsView({ urlParams, location }: Props) { + const { data: distributionData } = useTransactionDistribution(urlParams); + const { data: transactionDetailsChartsData } = useTransactionDetailsCharts( + urlParams + ); + const { data: waterfall } = useWaterfall(urlParams); + const transaction = waterfall.getTransactionById(urlParams.transactionId); + return (
@@ -32,75 +40,51 @@ export function TransactionDetailsView({ urlParams, location }: Props) { - - - ( - - )} + location={location} /> - ( - - )} + location={location} /> - { - const transaction = waterfall.getTransactionById( - urlParams.transactionId - ); - if (!transaction) { - return ( - - ); - } - return ( - - ); - }} - /> + {!transaction ? ( + + ) : ( + + )}
); } diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.js b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx similarity index 60% rename from x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.js rename to x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx index b68d06d162a59..e6fd0343a2235 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.js +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx @@ -4,15 +4,17 @@ * you may not use this file except in compliance with the Elastic License. */ -import React from 'react'; import { shallow } from 'enzyme'; +import { History } from 'history'; +import React from 'react'; +import { RouteComponentProps } from 'react-router'; import { TransactionOverviewView } from '..'; jest.mock( 'ui/chrome', () => ({ getBasePath: () => `/some/base/path`, - getInjected: key => { + getInjected: (key: string) => { if (key === 'mlEnabled') { return true; } @@ -20,7 +22,7 @@ jest.mock( }, getUiSettingsClient: () => { return { - get: key => { + get: (key: string) => { switch (key) { case 'timepicker:timeDefaults': return { from: 'now-15m', to: 'now', mode: 'quick' }; @@ -37,14 +39,19 @@ jest.mock( ); const setup = () => { + const routerProps = { + location: { search: '' }, + history: { + push: jest.fn() as History['push'], + replace: jest.fn() as History['replace'] + } + } as RouteComponentProps; + const props = { + ...routerProps, agentName: 'test-agent', serviceName: 'test-service', - serviceTransactionTypes: ['a', 'b'], - location: {}, - history: { - push: jest.fn() - }, + serviceTransactionTypes: ['firstType', 'secondType'], urlParams: { transactionType: 'test-type', serviceName: 'MyServiceName' } }; @@ -53,10 +60,21 @@ const setup = () => { }; describe('TransactionOverviewView', () => { - it('should render null if there is no transaction type in the search string', () => { - const { wrapper } = setup(); - wrapper.setProps({ urlParams: { serviceName: 'MyServiceName' } }); - expect(wrapper).toMatchInlineSnapshot(`""`); + describe('when no transaction type is given', () => { + it('should render null', () => { + const { wrapper } = setup(); + wrapper.setProps({ urlParams: { serviceName: 'MyServiceName' } }); + expect(wrapper).toMatchInlineSnapshot(`""`); + }); + + it('should redirect to first type', () => { + const { wrapper, props } = setup(); + wrapper.setProps({ urlParams: { serviceName: 'MyServiceName' } }); + expect(props.history.replace).toHaveBeenCalledWith({ + pathname: '/MyServiceName/transactions/firstType', + search: '' + }); + }); }); it('should render with type filter controls', () => { @@ -67,8 +85,9 @@ describe('TransactionOverviewView', () => { it('should render without type filter controls if there is just a single type', () => { const { wrapper } = setup(); wrapper.setProps({ - serviceTransactionTypes: ['a'] + serviceTransactionTypes: ['singleType'] }); + expect(wrapper.find('EuiFormRow').exists()).toBe(false); expect(wrapper).toMatchSnapshot(); }); }); diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.js.snap b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap similarity index 93% rename from x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.js.snap rename to x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap index d6a37bfaa2078..3e889206410f6 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.js.snap +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap @@ -18,12 +18,12 @@ exports[`TransactionOverviewView should render with type filter controls 1`] = ` options={ Array [ Object { - "text": "a", - "value": "a", + "text": "firstType", + "value": "firstType", }, Object { - "text": "b", - "value": "b", + "text": "secondType", + "value": "secondType", }, ] } diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx index eb462a9f917c6..b862e8b885d64 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx @@ -12,91 +12,118 @@ import { EuiTitle } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; +import { Location } from 'history'; +import { first } from 'lodash'; import React from 'react'; import { RouteComponentProps, withRouter } from 'react-router-dom'; import { TransactionCharts } from 'x-pack/plugins/apm/public/components/shared/charts/TransactionCharts'; import { legacyEncodeURIComponent } from 'x-pack/plugins/apm/public/components/shared/Links/url_helpers'; -import { TransactionListRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/transactionList'; -import { TransactionOverviewChartsRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts'; import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { useTransactionList } from '../../../hooks/useTransactionList'; +import { useTransactionOverviewCharts } from '../../../hooks/useTransactionOverviewCharts'; import { TransactionList } from './List'; +import { useRedirect } from './useRedirect'; interface TransactionOverviewProps extends RouteComponentProps { urlParams: IUrlParams; serviceTransactionTypes: string[]; } -export class TransactionOverviewView extends React.Component< - TransactionOverviewProps -> { - public handleTypeChange = (event: React.ChangeEvent) => { - const { urlParams, history, location } = this.props; - const type = legacyEncodeURIComponent(event.target.value); - history.push({ +function getRedirectLocation({ + urlParams, + location, + serviceTransactionTypes +}: { + location: Location; + urlParams: IUrlParams; + serviceTransactionTypes: string[]; +}) { + const { serviceName, transactionType } = urlParams; + const firstTransactionType = first(serviceTransactionTypes); + if (!transactionType && firstTransactionType) { + return { ...location, - pathname: `/${urlParams.serviceName}/transactions/${type}` - }); - }; + pathname: `/${serviceName}/transactions/${firstTransactionType}` + }; + } +} - public render() { - const { urlParams, serviceTransactionTypes, location } = this.props; - const { serviceName, transactionType } = urlParams; +export function TransactionOverviewView({ + urlParams, + serviceTransactionTypes, + location, + history +}: TransactionOverviewProps) { + const { serviceName, transactionType } = urlParams; - // filtering by type is currently required - if (!serviceName || !transactionType) { - return null; - } + // redirect to first transaction type + useRedirect( + getRedirectLocation({ + urlParams, + location, + serviceTransactionTypes + }) + ); - return ( - - {serviceTransactionTypes.length > 1 ? ( - - ({ - text: `${type}`, - value: type - }))} - value={transactionType} - onChange={this.handleTypeChange} - /> - - ) : null} + const { data: transactionOverviewCharts } = useTransactionOverviewCharts( + urlParams + ); - ( - - )} - /> + const { data: transactionListData } = useTransactionList(urlParams); - + // filtering by type is currently required + if (!serviceName || !transactionType) { + return null; + } - - -

Transactions

- - - ( - - )} + return ( + + {serviceTransactionTypes.length > 1 ? ( + + ({ + text: `${type}`, + value: type + }))} + value={transactionType} + onChange={event => { + const type = legacyEncodeURIComponent(event.target.value); + history.push({ + ...location, + pathname: `/${urlParams.serviceName}/transactions/${type}` + }); + }} /> - - - ); - } + + ) : null} + + + + + + + +

Transactions

+ + + + + + ); } export const TransactionOverview = withRouter(TransactionOverviewView); diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/useRedirect.ts b/x-pack/plugins/apm/public/components/app/TransactionOverview/useRedirect.ts new file mode 100644 index 0000000000000..cc0a2b7cb0e9d --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/useRedirect.ts @@ -0,0 +1,17 @@ +/* + * 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 { Location } from 'history'; +import { useEffect } from 'react'; +import { history } from '../../shared/Links/url_helpers'; + +export function useRedirect(redirectLocation?: Location) { + useEffect(() => { + if (redirectLocation) { + history.replace(redirectLocation); + } + }); +} diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index d62122b9137de..64ebcef365c67 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -5,14 +5,12 @@ */ import { connect } from 'react-redux'; -import { selectHasMLJob } from 'x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts'; import { IReduxState } from 'x-pack/plugins/apm/public/store/rootReducer'; import { selectIsMLAvailable } from 'x-pack/plugins/apm/public/store/selectors/license'; import { TransactionChartsView } from './view'; const mapStateToProps = (state: IReduxState) => ({ - mlAvailable: selectIsMLAvailable(state), - hasMLJob: selectHasMLJob(state) + mlAvailable: selectIsMLAvailable(state) }); export const TransactionCharts = connect(mapStateToProps)( diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx index 1c4d79ed1e990..f7f7c64283e72 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx @@ -28,11 +28,9 @@ import { SyncChartGroup } from '../SyncChartGroup'; interface TransactionChartProps { charts: ITransactionChartData; - chartWrapper?: React.ComponentClass | React.SFC; location: Location; urlParams: IUrlParams; mlAvailable: boolean; - hasMLJob: boolean; } const ShiftedIconWrapper = styled.span` @@ -78,7 +76,8 @@ export class TransactionChartsView extends Component { }; public renderMLHeader() { - if (!this.props.mlAvailable || !this.props.hasMLJob) { + const hasMLJob = this.props.charts.hasMLJob; + if (!this.props.mlAvailable || !hasMLJob) { return null; } @@ -123,11 +122,7 @@ export class TransactionChartsView extends Component { } public render() { - const { - charts, - urlParams, - chartWrapper: ChartWrapper = React.Fragment - } = this.props; + const { charts, urlParams } = this.props; const { noHits, responseTimeSeries, tpmSeries } = charts; const { transactionType } = urlParams; @@ -137,7 +132,7 @@ export class TransactionChartsView extends Component { - + @@ -153,13 +148,13 @@ export class TransactionChartsView extends Component { tickFormatY={this.getResponseTimeTickFormatter} formatTooltipValue={this.getResponseTimeTooltipFormatter} /> - + - + {tpmLabel(transactionType)} @@ -171,7 +166,7 @@ export class TransactionChartsView extends Component { formatTooltipValue={this.getTPMTooltipFormatter} truncateLegends /> - + diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.tsx new file mode 100644 index 0000000000000..e9f39cc7463fa --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useFetcher.tsx @@ -0,0 +1,69 @@ +/* + * 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 { useEffect, useState } from 'react'; + +export enum FETCH_STATUS { + LOADING = 'loading', + SUCCESS = 'success', + FAILURE = 'failure' +} + +// use this in request methods to signal to `useFetch` that all arguments are not yet available +export class MissingArgumentsError extends Error {} + +export function useFetcher( + fn: (options: Opts) => Promise, + options: Opts +) { + const [result, setResult] = useState<{ + data?: Response; + status?: FETCH_STATUS; + error?: Error; + }>({}); + + useEffect( + () => { + let didCancel = false; + + setResult({ + data: result.data, // preserve data from previous state while loading next state + status: FETCH_STATUS.LOADING, + error: undefined + }); + + fn(options) + .then(data => { + if (!didCancel) { + setResult({ + data, + status: FETCH_STATUS.SUCCESS, + error: undefined + }); + } + }) + .catch(e => { + if (e instanceof MissingArgumentsError) { + return; + } + if (!didCancel) { + setResult({ + data: undefined, + status: FETCH_STATUS.FAILURE, + error: e + }); + } + }); + + return () => { + didCancel = true; + }; + }, + [...Object.keys(options), ...Object.values(options)] + ); + + return result; +} diff --git a/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts b/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts new file mode 100644 index 0000000000000..53a667953bfe7 --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts @@ -0,0 +1,84 @@ +/* + * 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 { useMemo } from 'react'; +import { MetricsChartAPIResponse } from '../../server/lib/metrics/get_all_metrics_chart_data'; +import { MemoryChartAPIResponse } from '../../server/lib/metrics/get_memory_chart_data'; +import { loadMetricsChartDataForService } from '../services/rest/apm/metrics'; +import { + getCPUSeries, + getMemorySeries +} from '../store/selectors/chartSelectors'; +import { IUrlParams } from '../store/urlParams'; +import { useFetcher } from './useFetcher'; + +const memory: MemoryChartAPIResponse = { + series: { + memoryUsedAvg: [], + memoryUsedMax: [] + }, + overallValues: { + memoryUsedAvg: null, + memoryUsedMax: null + }, + totalHits: 0 +}; + +const INITIAL_DATA: MetricsChartAPIResponse = { + memory, + cpu: { + series: { + systemCPUAverage: [], + systemCPUMax: [], + processCPUAverage: [], + processCPUMax: [] + }, + overallValues: { + systemCPUAverage: null, + systemCPUMax: null, + processCPUAverage: null, + processCPUMax: null + }, + totalHits: 0 + } +}; + +export function useServiceMetricCharts(urlParams: IUrlParams) { + const { + serviceName, + transactionType, + start, + end, + transactionName, + kuery + } = urlParams; + + const { data = INITIAL_DATA, error, status } = useFetcher( + loadMetricsChartDataForService, + { + serviceName, + transactionName, + transactionType, + start, + end, + kuery + } + ); + + const memoizedData = useMemo( + () => ({ + memory: getMemorySeries(urlParams, data.memory), + cpu: getCPUSeries(data.cpu) + }), + [data] + ); + + return { + data: memoizedData, + status, + error + }; +} diff --git a/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts b/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts new file mode 100644 index 0000000000000..44e34059c09ca --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts @@ -0,0 +1,41 @@ +/* + * 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 { useMemo } from 'react'; +import { loadTransactionDetailsCharts } from '../services/rest/apm/transaction_groups'; +import { getTransactionCharts } from '../store/selectors/chartSelectors'; +import { IUrlParams } from '../store/urlParams'; +import { useFetcher } from './useFetcher'; + +export function useTransactionDetailsCharts(urlParams: IUrlParams) { + const { + serviceName, + transactionType, + start, + end, + transactionName, + kuery + } = urlParams; + + const { data, error, status } = useFetcher(loadTransactionDetailsCharts, { + serviceName, + transactionName, + transactionType, + start, + end, + kuery + }); + + const memoizedData = useMemo(() => getTransactionCharts(urlParams, data), [ + data + ]); + + return { + data: memoizedData, + status, + error + }; +} diff --git a/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts b/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts new file mode 100644 index 0000000000000..6c9780b8a84a1 --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts @@ -0,0 +1,40 @@ +/* + * 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 { loadTransactionDistribution } from '../services/rest/apm/transaction_groups'; +import { IUrlParams } from '../store/urlParams'; +import { useFetcher } from './useFetcher'; + +const INITIAL_DATA = { buckets: [], totalHits: 0, bucketSize: 0 }; + +export function useTransactionDistribution(urlParams: IUrlParams) { + const { + serviceName, + transactionType, + transactionId, + traceId, + start, + end, + transactionName, + kuery + } = urlParams; + + const { data = INITIAL_DATA, status, error } = useFetcher( + loadTransactionDistribution, + { + serviceName, + transactionType, + transactionId, + traceId, + start, + end, + transactionName, + kuery + } + ); + + return { data, status, error }; +} diff --git a/x-pack/plugins/apm/public/hooks/useTransactionList.ts b/x-pack/plugins/apm/public/hooks/useTransactionList.ts new file mode 100644 index 0000000000000..7b01573baefbf --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useTransactionList.ts @@ -0,0 +1,52 @@ +/* + * 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 { useMemo } from 'react'; +import { TransactionListAPIResponse } from '../../server/lib/transactions/get_top_transactions'; +import { loadTransactionList } from '../services/rest/apm/transaction_groups'; +import { IUrlParams } from '../store/urlParams'; +import { useFetcher } from './useFetcher'; + +const getRelativeImpact = ( + impact: number, + impactMin: number, + impactMax: number +) => + Math.max( + ((impact - impactMin) / Math.max(impactMax - impactMin, 1)) * 100, + 1 + ); + +function getWithRelativeImpact(items: TransactionListAPIResponse) { + const impacts = items.map(({ impact }) => impact); + const impactMin = Math.min(...impacts); + const impactMax = Math.max(...impacts); + + return items.map(item => { + return { + ...item, + impactRelative: getRelativeImpact(item.impact, impactMin, impactMax) + }; + }); +} + +export function useTransactionList(urlParams: IUrlParams) { + const { serviceName, transactionType, start, end, kuery } = urlParams; + const { data = [], error, status } = useFetcher(loadTransactionList, { + serviceName, + start, + end, + transactionType, + kuery + }); + + const memoizedData = useMemo(() => getWithRelativeImpact(data), [data]); + return { + data: memoizedData, + status, + error + }; +} diff --git a/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts b/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts new file mode 100644 index 0000000000000..e21fb514c1ede --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts @@ -0,0 +1,40 @@ +/* + * 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 { useMemo } from 'react'; +import { loadTransactionOverviewCharts } from '../services/rest/apm/transaction_groups'; +import { getTransactionCharts } from '../store/selectors/chartSelectors'; +import { IUrlParams } from '../store/urlParams'; +import { useFetcher } from './useFetcher'; + +export function useTransactionOverviewCharts(urlParams: IUrlParams) { + const { + serviceName, + transactionType, + start, + end, + + kuery + } = urlParams; + + const { data, error, status } = useFetcher(loadTransactionOverviewCharts, { + serviceName, + start, + end, + transactionType, + kuery + }); + + const memoizedData = useMemo(() => getTransactionCharts(urlParams, data), [ + data + ]); + + return { + data: memoizedData, + status, + error + }; +} diff --git a/x-pack/plugins/apm/public/hooks/useWaterfall.ts b/x-pack/plugins/apm/public/hooks/useWaterfall.ts new file mode 100644 index 0000000000000..24b4a0bd5ac18 --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useWaterfall.ts @@ -0,0 +1,30 @@ +/* + * 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 { getWaterfall } from '../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers'; +import { loadTrace } from '../services/rest/apm/traces'; +import { IUrlParams } from '../store/urlParams'; +import { useFetcher } from './useFetcher'; + +const INITIAL_DATA = { trace: [], errorsPerTransaction: {} }; + +export function useWaterfall(urlParams: IUrlParams) { + const { traceId, start, end } = urlParams; + const { data = INITIAL_DATA, status, error } = useFetcher(loadTrace, { + traceId, + start, + end + }); + + // TODO consider wrapping in `useMemo` + const waterfall = getWaterfall( + data.trace, + data.errorsPerTransaction, + urlParams.transactionId + ); + + return { data: waterfall, status, error }; +} diff --git a/x-pack/plugins/apm/public/services/rest/apm/error_groups.ts b/x-pack/plugins/apm/public/services/rest/apm/error_groups.ts index 0edfaf3495f12..02008a3ceac95 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/error_groups.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/error_groups.ts @@ -7,29 +7,27 @@ import { ErrorDistributionAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/distribution/get_distribution'; import { ErrorGroupAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/get_error_group'; import { ErrorGroupListAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/get_error_groups'; +import { MissingArgumentsError } from '../../../hooks/useFetcher'; import { IUrlParams } from '../../../store/urlParams'; import { callApi } from '../callApi'; import { getEncodedEsQuery } from './apm'; -interface ErrorGroupListParams extends IUrlParams { - size: number; -} - export async function loadErrorGroupList({ serviceName, start, end, kuery, - size, sortField, sortDirection -}: ErrorGroupListParams) { +}: IUrlParams) { + if (!(serviceName && start && end)) { + throw new MissingArgumentsError(); + } return callApi({ pathname: `/api/apm/services/${serviceName}/errors`, query: { start, end, - size, sortField, sortDirection, esFilterQuery: await getEncodedEsQuery(kuery) @@ -44,6 +42,9 @@ export async function loadErrorGroupDetails({ kuery, errorGroupId }: IUrlParams) { + if (!(serviceName && start && end && errorGroupId)) { + throw new MissingArgumentsError(); + } return callApi({ pathname: `/api/apm/services/${serviceName}/errors/${errorGroupId}`, query: { @@ -61,6 +62,10 @@ export async function loadErrorDistribution({ kuery, errorGroupId }: IUrlParams) { + if (!(serviceName && start && end)) { + throw new MissingArgumentsError(); + } + const pathname = errorGroupId ? `/api/apm/services/${serviceName}/errors/${errorGroupId}/distribution` : `/api/apm/services/${serviceName}/errors/distribution`; diff --git a/x-pack/plugins/apm/public/services/rest/apm/metrics.ts b/x-pack/plugins/apm/public/services/rest/apm/metrics.ts index 37c8cf0e918b9..bfeee4c1f289a 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/metrics.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/metrics.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { MetricsChartAPIResponse } from 'x-pack/plugins/apm/server/lib/metrics/get_all_metrics_chart_data'; import { IUrlParams } from '../../../store/urlParams'; import { callApi } from '../callApi'; import { getEncodedEsQuery } from './apm'; @@ -14,7 +15,7 @@ export async function loadMetricsChartDataForService({ end, kuery }: IUrlParams) { - return callApi({ + return callApi({ pathname: `/api/apm/services/${serviceName}/metrics/charts`, query: { start, diff --git a/x-pack/plugins/apm/public/services/rest/apm/services.ts b/x-pack/plugins/apm/public/services/rest/apm/services.ts index 430d2fa21770b..0af17d58813ad 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/services.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/services.ts @@ -6,11 +6,16 @@ import { ServiceAPIResponse } from 'x-pack/plugins/apm/server/lib/services/get_service'; import { ServiceListAPIResponse } from 'x-pack/plugins/apm/server/lib/services/get_services'; +import { MissingArgumentsError } from '../../../hooks/useFetcher'; import { IUrlParams } from '../../../store/urlParams'; import { callApi } from '../callApi'; import { getEncodedEsQuery } from './apm'; export async function loadServiceList({ start, end, kuery }: IUrlParams) { + if (!(start && end)) { + throw new MissingArgumentsError(); + } + return callApi({ pathname: `/api/apm/services`, query: { @@ -27,6 +32,10 @@ export async function loadServiceDetails({ end, kuery }: IUrlParams) { + if (!(serviceName && start && end)) { + throw new MissingArgumentsError(); + } + return callApi({ pathname: `/api/apm/services/${serviceName}`, query: { diff --git a/x-pack/plugins/apm/public/services/rest/apm/status_check.ts b/x-pack/plugins/apm/public/services/rest/apm/status_check.ts index 30fe628d8e7f4..0c6b660930b41 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/status_check.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/status_check.ts @@ -13,7 +13,9 @@ export async function loadServerStatus() { } export async function loadAgentStatus() { - return callApi<{ dataFound: boolean }>({ + const res = await callApi<{ dataFound: boolean }>({ pathname: `/api/apm/status/agent` }); + + return res.dataFound; } diff --git a/x-pack/plugins/apm/public/services/rest/apm/traces.ts b/x-pack/plugins/apm/public/services/rest/apm/traces.ts index 1a681ecc3ade4..75d2f3af9308b 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/traces.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/traces.ts @@ -6,11 +6,16 @@ import { TraceListAPIResponse } from 'x-pack/plugins/apm/server/lib/traces/get_top_traces'; import { TraceAPIResponse } from 'x-pack/plugins/apm/server/lib/traces/get_trace'; +import { MissingArgumentsError } from '../../../hooks/useFetcher'; import { IUrlParams } from '../../../store/urlParams'; import { callApi } from '../callApi'; import { getEncodedEsQuery } from './apm'; export async function loadTrace({ traceId, start, end }: IUrlParams) { + if (!(traceId && start && end)) { + throw new MissingArgumentsError(); + } + return callApi({ pathname: `/api/apm/traces/${traceId}`, query: { @@ -21,6 +26,10 @@ export async function loadTrace({ traceId, start, end }: IUrlParams) { } export async function loadTraceList({ start, end, kuery }: IUrlParams) { + if (!(start && end)) { + throw new MissingArgumentsError(); + } + return callApi({ pathname: '/api/apm/traces', query: { diff --git a/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts b/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts index 53025a0daf685..1b8e76754d84f 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts @@ -7,6 +7,7 @@ import { TimeSeriesAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/charts'; import { ITransactionDistributionAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/distribution'; import { TransactionListAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/get_top_transactions'; +import { MissingArgumentsError } from '../../../hooks/useFetcher'; import { IUrlParams } from '../../../store/urlParams'; import { callApi } from '../callApi'; import { getEncodedEsQuery } from './apm'; @@ -16,8 +17,12 @@ export async function loadTransactionList({ start, end, kuery, - transactionType = 'request' + transactionType }: IUrlParams) { + if (!(serviceName && transactionType && start && end)) { + throw new MissingArgumentsError(); + } + return await callApi({ pathname: `/api/apm/services/${serviceName}/transaction_groups/${transactionType}`, query: { @@ -33,11 +38,15 @@ export async function loadTransactionDistribution({ start, end, transactionName, - transactionType = 'request', + transactionType, transactionId, traceId, kuery -}: Required) { +}: IUrlParams) { + if (!(serviceName && transactionName && transactionType && start && end)) { + throw new MissingArgumentsError(); + } + return callApi({ pathname: `/api/apm/services/${serviceName}/transaction_groups/${transactionType}/${encodeURIComponent( transactionName @@ -52,17 +61,21 @@ export async function loadTransactionDistribution({ }); } -export async function loadDetailsCharts({ +export async function loadTransactionDetailsCharts({ serviceName, start, end, kuery, - transactionType = 'request', + transactionType, transactionName -}: Required) { +}: IUrlParams) { + if (!(serviceName && transactionName && transactionType && start && end)) { + throw new MissingArgumentsError(); + } + return callApi({ pathname: `/api/apm/services/${serviceName}/transaction_groups/${transactionType}/${encodeURIComponent( - transactionName + transactionName as string )}/charts`, query: { start, @@ -72,31 +85,23 @@ export async function loadDetailsCharts({ }); } -export async function loadOverviewCharts({ +export async function loadTransactionOverviewCharts({ serviceName, start, end, kuery, - transactionType = 'request' + transactionType }: IUrlParams) { - return callApi({ - pathname: `/api/apm/services/${serviceName}/transaction_groups/${transactionType}/charts`, - query: { - start, - end, - esFilterQuery: await getEncodedEsQuery(kuery) - } - }); -} + if (!(serviceName && start && end)) { + throw new MissingArgumentsError(); + } + + const pathname = transactionType + ? `/api/apm/services/${serviceName}/transaction_groups/${transactionType}/charts` + : `/api/apm/services/${serviceName}/transaction_groups/charts`; -export async function loadOverviewChartsForAllTypes({ - serviceName, - start, - end, - kuery -}: IUrlParams) { return callApi({ - pathname: `/api/apm/services/${serviceName}/transaction_groups/charts`, + pathname, query: { start, end, diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/serviceList.test.js b/x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/serviceList.test.js deleted file mode 100644 index b45d3bbc76b1c..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/serviceList.test.js +++ /dev/null @@ -1,71 +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 React from 'react'; -import * as rest from '../../../services/rest/apm/services'; -import { getServiceList, ServiceListRequest } from '../serviceList'; -import { mountWithStore } from '../../../utils/testHelpers'; - -describe('serviceList', () => { - describe('getServiceList', () => { - it('should return default value when empty', () => { - const state = { reactReduxRequest: {}, sorting: { service: {} } }; - expect(getServiceList(state)).toEqual({ data: [] }); - }); - - it('should return serviceList when not empty', () => { - const state = { - reactReduxRequest: { serviceList: { data: [{ foo: 'bar' }] } }, - sorting: { service: {} } - }; - expect(getServiceList(state)).toEqual({ data: [{ foo: 'bar' }] }); - }); - }); - - describe('ServiceListRequest', () => { - let loadSpy; - let renderSpy; - let wrapper; - - beforeEach(() => { - const state = { - reactReduxRequest: { - serviceList: { status: 'my-status', data: [{ foo: 'bar' }] } - }, - sorting: { service: {} } - }; - - loadSpy = jest.spyOn(rest, 'loadServiceList').mockReturnValue(); - renderSpy = jest.fn().mockReturnValue(
rendered
); - - wrapper = mountWithStore( - , - state - ); - }); - - it('should call render method', () => { - expect(renderSpy).toHaveBeenCalledWith({ - data: [{ foo: 'bar' }], - status: 'my-status' - }); - }); - - it('should call "loadServiceList"', () => { - expect(loadSpy).toHaveBeenCalledWith({ - start: 'myStart', - end: 'myEnd' - }); - }); - - it('should render component', () => { - expect(wrapper.html()).toEqual('
rendered
'); - }); - }); -}); diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/errorDistribution.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/errorDistribution.tsx deleted file mode 100644 index ef91d06bfad6e..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/errorDistribution.tsx +++ /dev/null @@ -1,49 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { ErrorDistributionAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/distribution/get_distribution'; -import { loadErrorDistribution } from '../../services/rest/apm/error_groups'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'errorDistribution'; -const INITIAL_DATA: ErrorDistributionAPIResponse = { - buckets: [], - totalHits: 0, - bucketSize: 0 -}; -const withInitialData = createInitialDataSelector(INITIAL_DATA); - -export function getErrorDistribution(state: IReduxState) { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function ErrorDistributionRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { serviceName, start, end, errorGroupId, kuery } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/errorGroup.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/errorGroup.tsx deleted file mode 100644 index 37e1983c0a2a8..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/errorGroup.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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { ErrorGroupAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/get_error_group'; -import { loadErrorGroupDetails } from '../../services/rest/apm/error_groups'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'errorGroupDetails'; -const INITIAL_DATA: ErrorGroupAPIResponse = { occurrencesCount: 0 }; -const withInitialData = createInitialDataSelector(INITIAL_DATA); - -export function getErrorGroupDetails(state: IReduxState) { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function ErrorGroupDetailsRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { serviceName, errorGroupId, start, end, kuery } = urlParams; - - if (!(serviceName && start && end && errorGroupId)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/errorGroupList.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/errorGroupList.tsx deleted file mode 100644 index b7c3daf2c38b1..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/errorGroupList.tsx +++ /dev/null @@ -1,52 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { ErrorGroupListAPIResponse } from 'x-pack/plugins/apm/server/lib/errors/get_error_groups'; -import { loadErrorGroupList } from '../../services/rest/apm/error_groups'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'errorGroupList'; -const INITIAL_DATA: ErrorGroupListAPIResponse = []; -const withInitialData = createInitialDataSelector(INITIAL_DATA); - -export function getErrorGroupList(state: IReduxState) { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function ErrorGroupOverviewRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { - serviceName, - start, - end, - sortField, - sortDirection, - kuery - } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/serviceDetails.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/serviceDetails.tsx deleted file mode 100644 index 274f620c7f933..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/serviceDetails.tsx +++ /dev/null @@ -1,51 +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 { first, get } from 'lodash'; -import React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; -import { ServiceAPIResponse } from 'x-pack/plugins/apm/server/lib/services/get_service'; -import { loadServiceDetails } from '../../services/rest/apm/services'; -import { IReduxState } from '../rootReducer'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'serviceDetails'; -const INITIAL_DATA = { types: [] }; -const withInitialData = createInitialDataSelector(INITIAL_DATA); - -export function getServiceDetails(state: IReduxState) { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function getDefaultTransactionType(state: IReduxState) { - const types: string[] = get(state.reactReduxRequest[ID], 'data.types'); - return first(types); -} - -export function ServiceDetailsRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { serviceName, start, end, kuery } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/serviceList.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/serviceList.tsx deleted file mode 100644 index 22ea86e906502..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/serviceList.tsx +++ /dev/null @@ -1,49 +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 React from 'react'; -import { Request, RRRRender, RRRRenderResponse } from 'react-redux-request'; -import { ServiceListAPIResponse } from 'x-pack/plugins/apm/server/lib/services/get_services'; -import { loadServiceList } from '../../services/rest/apm/services'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'serviceList'; -const INITIAL_DATA: ServiceListAPIResponse = []; -const withInitialData = createInitialDataSelector( - INITIAL_DATA -); - -export function getServiceList( - state: IReduxState -): RRRRenderResponse { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function ServiceListRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { start, end, kuery } = urlParams; - - if (!(start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/serviceMetricsCharts.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/serviceMetricsCharts.tsx deleted file mode 100644 index 19a0dbdc0327e..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/serviceMetricsCharts.tsx +++ /dev/null @@ -1,92 +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 React from 'react'; -import { Request, RRRRender, RRRRenderResponse } from 'react-redux-request'; -import { createSelector } from 'reselect'; -import { loadMetricsChartDataForService } from 'x-pack/plugins/apm/public/services/rest/apm/metrics'; -import { IMemoryChartData } from 'x-pack/plugins/apm/public/store/selectors/chartSelectors'; -import { MetricsChartAPIResponse } from 'x-pack/plugins/apm/server/lib/metrics/get_all_metrics_chart_data'; -import { IReduxState } from '../rootReducer'; -import { getCPUSeries, getMemorySeries } from '../selectors/chartSelectors'; -import { getUrlParams, IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'metricsChartData'; -const INITIAL_DATA: MetricsChartAPIResponse = { - memory: { - series: { - memoryUsedAvg: [], - memoryUsedMax: [] - }, - overallValues: { - memoryUsedAvg: null, - memoryUsedMax: null - }, - totalHits: 0 - }, - cpu: { - series: { - systemCPUAverage: [], - systemCPUMax: [], - processCPUAverage: [], - processCPUMax: [] - }, - overallValues: { - systemCPUAverage: null, - systemCPUMax: null, - processCPUAverage: null, - processCPUMax: null - }, - totalHits: 0 - } -}; - -type MetricsChartDataSelector = ( - state: IReduxState -) => RRRRenderResponse; - -const withInitialData = createInitialDataSelector( - INITIAL_DATA -); - -const selectMetricsChartData: MetricsChartDataSelector = state => - withInitialData(state.reactReduxRequest[ID]); - -export const selectTransformedMetricsChartData = createSelector( - [getUrlParams, selectMetricsChartData], - (urlParams, response) => ({ - ...response, - data: { - ...response.data, - memory: getMemorySeries(urlParams, response.data.memory), - cpu: getCPUSeries(response.data.cpu) - } - }) -); - -interface Props { - urlParams: IUrlParams; - render: RRRRender; -} - -export function MetricsChartDataRequest({ urlParams, render }: Props) { - const { serviceName, start, end } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionDetailsCharts.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/transactionDetailsCharts.tsx deleted file mode 100644 index 50b631d25ecfa..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionDetailsCharts.tsx +++ /dev/null @@ -1,73 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { createSelector } from 'reselect'; -import { ITransactionChartData } from 'x-pack/plugins/apm/public/store/selectors/chartSelectors'; -import { TimeSeriesAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/charts'; -import { loadDetailsCharts } from '../../services/rest/apm/transaction_groups'; -import { IReduxState } from '../rootReducer'; -import { getTransactionCharts } from '../selectors/chartSelectors'; -import { getUrlParams, IUrlParams } from '../urlParams'; - -const ID = 'transactionDetailsCharts'; -const INITIAL_DATA: TimeSeriesAPIResponse = { - apmTimeseries: { - totalHits: 0, - responseTimes: { - avg: [], - p95: [], - p99: [] - }, - tpmBuckets: [], - overallAvgDuration: undefined - }, - anomalyTimeseries: undefined -}; - -export const getTransactionDetailsCharts = createSelector( - getUrlParams, - (state: IReduxState) => state.reactReduxRequest[ID], - (urlParams, detailCharts = {}) => { - return { - ...detailCharts, - data: getTransactionCharts(urlParams, detailCharts.data || INITIAL_DATA) - }; - } -); - -interface Props { - urlParams: IUrlParams; - render: RRRRender; -} - -export function TransactionDetailsChartsRequest({ urlParams, render }: Props) { - const { - serviceName, - start, - end, - transactionType, - transactionName, - kuery - } = urlParams; - - if (!(serviceName && start && end && transactionType && transactionName)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionDistribution.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/transactionDistribution.tsx deleted file mode 100644 index b4232de6a9db2..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionDistribution.tsx +++ /dev/null @@ -1,81 +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 React from 'react'; -import { Request, RRRRender, RRRRenderResponse } from 'react-redux-request'; -import { createSelector } from 'reselect'; -import { ITransactionDistributionAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/distribution'; -import { loadTransactionDistribution } from '../../services/rest/apm/transaction_groups'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'transactionDistribution'; -const INITIAL_DATA = { buckets: [], totalHits: 0, bucketSize: 0 }; -const withInitialData = createInitialDataSelector< - ITransactionDistributionAPIResponse ->(INITIAL_DATA); - -export function getTransactionDistribution( - state: IReduxState -): RRRRenderResponse { - return withInitialData(state.reactReduxRequest[ID]); -} - -export const getDefaultDistributionSample = createSelector( - getTransactionDistribution, - distribution => { - const { defaultSample = {} } = distribution.data; - return { - traceId: defaultSample.traceId, - transactionId: defaultSample.transactionId - }; - } -); - -export function TransactionDistributionRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { - serviceName, - transactionType, - transactionId, - traceId, - start, - end, - transactionName, - kuery - } = urlParams; - - if (!(serviceName && transactionType && start && end && transactionName)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionList.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/transactionList.tsx deleted file mode 100644 index 246af040be84b..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionList.tsx +++ /dev/null @@ -1,85 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { createSelector } from 'reselect'; -import { TransactionListAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/get_top_transactions'; -import { loadTransactionList } from '../../services/rest/apm/transaction_groups'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'transactionList'; -const INITIAL_DATA: TransactionListAPIResponse = []; -const withInitialData = createInitialDataSelector( - INITIAL_DATA -); - -const getRelativeImpact = ( - impact: number, - impactMin: number, - impactMax: number -) => - Math.max( - ((impact - impactMin) / Math.max(impactMax - impactMin, 1)) * 100, - 1 - ); - -function getWithRelativeImpact(items: TransactionListAPIResponse) { - const impacts = items.map(({ impact }) => impact); - const impactMin = Math.min(...impacts); - const impactMax = Math.max(...impacts); - - return items.map(item => { - return { - ...item, - impactRelative: getRelativeImpact(item.impact, impactMin, impactMax) - }; - }); -} - -export const getTransactionList = createSelector( - (state: IReduxState) => withInitialData(state.reactReduxRequest[ID]), - transactionList => { - return { - ...transactionList, - data: getWithRelativeImpact(transactionList.data) - }; - } -); - -export function TransactionListRequest({ - urlParams, - render -}: { - urlParams: IUrlParams; - render: RRRRender; -}) { - const { serviceName, start, end, transactionType, kuery } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts.tsx deleted file mode 100644 index 520ba31e3229b..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/transactionOverviewCharts.tsx +++ /dev/null @@ -1,97 +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 { get } from 'lodash'; -import React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { createSelector } from 'reselect'; -import { ITransactionChartData } from 'x-pack/plugins/apm/public/store/selectors/chartSelectors'; -import { TimeSeriesAPIResponse } from 'x-pack/plugins/apm/server/lib/transactions/charts'; -import { - loadOverviewCharts, - loadOverviewChartsForAllTypes -} from '../../services/rest/apm/transaction_groups'; -import { IReduxState } from '../rootReducer'; -import { getTransactionCharts } from '../selectors/chartSelectors'; -import { getUrlParams, IUrlParams } from '../urlParams'; - -const ID = 'transactionOverviewCharts'; -const INITIAL_DATA: TimeSeriesAPIResponse = { - apmTimeseries: { - totalHits: 0, - responseTimes: { - avg: [], - p95: [], - p99: [] - }, - tpmBuckets: [], - overallAvgDuration: undefined - }, - anomalyTimeseries: undefined -}; - -const selectChartData = (state: IReduxState) => state.reactReduxRequest[ID]; - -export const getTransactionOverviewCharts = createSelector( - [getUrlParams, selectChartData], - (urlParams, overviewCharts = {}) => { - return { - ...overviewCharts, - data: getTransactionCharts(urlParams, overviewCharts.data || INITIAL_DATA) - }; - } -); - -export const selectHasMLJob = createSelector( - [selectChartData], - chartData => get(chartData, 'data.anomalyTimeseries') !== undefined -); - -interface Props { - urlParams: IUrlParams; - render: RRRRender; -} - -export function TransactionOverviewChartsRequest({ urlParams, render }: Props) { - const { serviceName, start, end, transactionType, kuery } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} - -// Ignores transaction type from urlParams and requests charts -// for ALL transaction types within this service -export function TransactionOverviewChartsRequestForAllTypes({ - urlParams, - render -}: Props) { - const { serviceName, start, end, kuery } = urlParams; - - if (!(serviceName && start && end)) { - return null; - } - - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/waterfall.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/waterfall.tsx deleted file mode 100644 index abd7e73702a3d..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/waterfall.tsx +++ /dev/null @@ -1,51 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { TraceAPIResponse } from 'x-pack/plugins/apm/server/lib/traces/get_trace'; -import { - getWaterfall, - IWaterfall -} from '../../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers'; -import { loadTrace } from '../../services/rest/apm/traces'; -import { IUrlParams } from '../urlParams'; - -export const ID = 'waterfall'; - -interface Props { - urlParams: IUrlParams; - traceId?: string; - render: RRRRender; -} - -export function WaterfallRequest({ urlParams, render, traceId }: Props) { - const { start, end } = urlParams; - - if (!(traceId && start && end)) { - return null; - } - - return ( - - id={ID} - fn={loadTrace} - args={[{ traceId, start, end }]} - render={({ - args, - data = { trace: [], errorsPerTransaction: {} }, - status - }) => { - const waterfall = getWaterfall( - data.trace, - data.errorsPerTransaction, - urlParams.transactionId - ); - return render({ args, data: waterfall, status }); - }} - /> - ); -} diff --git a/x-pack/plugins/apm/public/store/selectors/chartSelectors.ts b/x-pack/plugins/apm/public/store/selectors/chartSelectors.ts index 424fe1515819f..7eedff75a1c28 100644 --- a/x-pack/plugins/apm/public/store/selectors/chartSelectors.ts +++ b/x-pack/plugins/apm/public/store/selectors/chartSelectors.ts @@ -65,13 +65,27 @@ export interface ITransactionChartData { noHits: boolean; tpmSeries: ITpmBucket[] | IEmptySeries[]; responseTimeSeries: TimeSerie[] | IEmptySeries[]; + hasMLJob: boolean; } +const INITIAL_DATA = { + apmTimeseries: { + totalHits: 0, + responseTimes: { + avg: [], + p95: [], + p99: [] + }, + tpmBuckets: [], + overallAvgDuration: undefined + }, + anomalyTimeseries: undefined +}; + export function getTransactionCharts( - urlParams: IUrlParams, - timeseriesResponse: TimeSeriesAPIResponse -) { - const { start, end, transactionType } = urlParams; + { start, end, transactionType }: IUrlParams, + timeseriesResponse: TimeSeriesAPIResponse = INITIAL_DATA +): ITransactionChartData { const { apmTimeseries, anomalyTimeseries } = timeseriesResponse; const noHits = apmTimeseries.totalHits === 0; const tpmSeries = noHits @@ -82,26 +96,22 @@ export function getTransactionCharts( ? getEmptySerie(start, end) : getResponseTimeSeries(apmTimeseries, anomalyTimeseries); - const chartsResult: ITransactionChartData = { + return { noHits, tpmSeries, - responseTimeSeries + responseTimeSeries, + hasMLJob: timeseriesResponse.anomalyTimeseries !== undefined }; - - return chartsResult; } -export interface IMemoryChartData extends MetricsChartAPIResponse { - series: TimeSerie[] | IEmptySeries[]; -} +export type MemoryMetricSeries = ReturnType; export function getMemorySeries( - urlParams: IUrlParams, + { start, end }: IUrlParams, memoryChartResponse: MetricsChartAPIResponse['memory'] ) { - const { start, end } = urlParams; const { series, overallValues, totalHits } = memoryChartResponse; - const seriesList: IMemoryChartData['series'] = + const seriesList = totalHits === 0 ? getEmptySerie(start, end) : [ @@ -132,14 +142,12 @@ export function getMemorySeries( ]; return { - ...memoryChartResponse, + totalHits: memoryChartResponse.totalHits, series: seriesList }; } -export interface ICPUChartData extends MetricsChartAPIResponse { - series: TimeSerie[]; -} +export type CPUMetricSeries = ReturnType; export function getCPUSeries(CPUChartResponse: MetricsChartAPIResponse['cpu']) { const { series, overallValues } = CPUChartResponse; @@ -183,7 +191,7 @@ export function getCPUSeries(CPUChartResponse: MetricsChartAPIResponse['cpu']) { } ]; - return { ...CPUChartResponse, series: seriesList }; + return { totalHits: CPUChartResponse.totalHits, series: seriesList }; } interface TimeSerie { diff --git a/x-pack/plugins/apm/public/store/urlParams.ts b/x-pack/plugins/apm/public/store/urlParams.ts index 270f3c690624b..c9bfeef2ee638 100644 --- a/x-pack/plugins/apm/public/store/urlParams.ts +++ b/x-pack/plugins/apm/public/store/urlParams.ts @@ -7,14 +7,11 @@ import datemath from '@elastic/datemath'; import { Location } from 'history'; import { compact, pick } from 'lodash'; -import { createSelector } from 'reselect'; import { legacyDecodeURIComponent, toQuery } from '../components/shared/Links/url_helpers'; import { LOCATION_UPDATE } from './location'; -import { getDefaultTransactionType } from './reactReduxRequest/serviceDetails'; -import { getDefaultDistributionSample } from './reactReduxRequest/transactionDistribution'; import { IReduxState } from './rootReducer'; // ACTION TYPES @@ -153,16 +150,11 @@ export function toNumber(value?: string) { } } -function toString(str?: string | string[]) { - if ( - str === '' || - str === 'null' || - str === 'undefined' || - Array.isArray(str) - ) { +function toString(value?: string) { + if (value === '' || value === 'null' || value === 'undefined') { return; } - return str; + return value; } export function toBoolean(value?: string) { @@ -211,23 +203,9 @@ export function refreshTimeRange(time: TimeRange): TimeRangeRefreshAction { } // Selectors -export const getUrlParams = createSelector( - (state: IReduxState) => state.urlParams, - getDefaultTransactionType, - getDefaultDistributionSample, - ( - urlParams, - transactionType: string, - { traceId, transactionId } - ): IUrlParams => { - return { - transactionType, - transactionId, - traceId, - ...urlParams - }; - } -); +export function getUrlParams(state: IReduxState) { + return state.urlParams; +} export interface IUrlParams { detailTab?: string; diff --git a/yarn.lock b/yarn.lock index a09253ff177c7..372b115894ebc 100644 --- a/yarn.lock +++ b/yarn.lock @@ -779,6 +779,13 @@ dependencies: regenerator-runtime "^0.12.0" +"@babel/runtime@^7.3.1", "@babel/runtime@^7.3.4": + version "7.3.4" + resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.3.4.tgz#73d12ba819e365fcf7fd152aed56d6df97d21c83" + integrity sha512-IvfvnMdSaLBateu0jfsYIpZTxAc2cKEXEMiezGGN75QcBcecDUKd3PgLAncT0oOgxKy8dd8hrJKj9MfzgfZd6g== + dependencies: + regenerator-runtime "^0.12.0" + "@babel/template@^7.0.0", "@babel/template@^7.1.0", "@babel/template@^7.1.2", "@babel/template@^7.2.2": version "7.2.2" resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.2.2.tgz#005b3fdf0ed96e88041330379e0da9a708eb2907" @@ -1143,6 +1150,11 @@ dependencies: url-pattern "^1.0.3" +"@sheerun/mutationobserver-shim@^0.3.2": + version "0.3.2" + resolved "https://registry.yarnpkg.com/@sheerun/mutationobserver-shim/-/mutationobserver-shim-0.3.2.tgz#8013f2af54a2b7d735f71560ff360d3a8176a87b" + integrity sha512-vTCdPp/T/Q3oSqwHmZ5Kpa9oI7iLtGl3RQaA/NyLHikvcrPxACkkKVr/XzkSPJWXHRhKGzVvb0urJsbMlRxi1Q== + "@sindresorhus/is@^0.7.0": version "0.7.0" resolved "https://registry.yarnpkg.com/@sindresorhus/is/-/is-0.7.0.tgz#9a06f4f137ee84d7df0460c1fdb1135ffa6c50fd" @@ -7686,6 +7698,16 @@ dom-serializer@0, dom-serializer@~0.1.0: domelementtype "^1.3.0" entities "^1.1.1" +dom-testing-library@^3.13.1: + version "3.17.1" + resolved "https://registry.yarnpkg.com/dom-testing-library/-/dom-testing-library-3.17.1.tgz#3291bc3cf68c555ba5e663697ee77d604aaa122b" + integrity sha512-SbkaRfQvuLjnv+xFgSo/cmKoN9tjBL6Rh1f3nQH9jnjUe5q+keRwacYSi3uSpcB4D1K768iavCayKH3ZN9ea+g== + dependencies: + "@babel/runtime" "^7.3.4" + "@sheerun/mutationobserver-shim" "^0.3.2" + pretty-format "^24.0.0" + wait-for-expect "^1.1.0" + dom-walk@^0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/dom-walk/-/dom-walk-0.1.1.tgz#672226dc74c8f799ad35307df936aba11acd6018" @@ -18770,6 +18792,14 @@ react-test-renderer@^16.0.0-0, react-test-renderer@^16.8.0: react-is "^16.8.2" scheduler "^0.13.2" +react-testing-library@^6.0.0: + version "6.0.0" + resolved "https://registry.yarnpkg.com/react-testing-library/-/react-testing-library-6.0.0.tgz#81edfcfae8a795525f48685be9bf561df45bb35d" + integrity sha512-h0h+YLe4KWptK6HxOMnoNN4ngu3W8isrwDmHjPC5gxc+nOZOCurOvbKVYCvvuAw91jdO7VZSm/5KR7TxKnz0qA== + dependencies: + "@babel/runtime" "^7.3.1" + dom-testing-library "^3.13.1" + react-textarea-autosize@^7.0.4: version "7.0.4" resolved "https://registry.yarnpkg.com/react-textarea-autosize/-/react-textarea-autosize-7.0.4.tgz#4e4be649b544a88713e7b5043f76950f35d3d503" @@ -23911,6 +23941,11 @@ w3c-hr-time@^1.0.1: dependencies: browser-process-hrtime "^0.1.2" +wait-for-expect@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/wait-for-expect/-/wait-for-expect-1.1.0.tgz#6607375c3f79d32add35cd2c87ce13f351a3d453" + integrity sha512-vQDokqxyMyknfX3luCDn16bSaRcOyH6gGuUXMIbxBLeTo6nWuEWYqMTT9a+44FmW8c2m6TRWBdNvBBjA1hwEKg== + walk@2.3.x: version "2.3.9" resolved "https://registry.yarnpkg.com/walk/-/walk-2.3.9.tgz#31b4db6678f2ae01c39ea9fb8725a9031e558a7b" From d889b5f959e7539c90cff7498bd2b2a52e7d440e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Sun, 24 Mar 2019 16:59:17 +0100 Subject: [PATCH 02/16] Convert enzyme test to RTL test --- x-pack/plugins/__mocks__/ui/chrome.js | 9 +- .../__jest__/TransactionOverview.test.tsx | 137 +++++++++--------- .../TransactionOverview.test.tsx.snap | 124 ++-------------- .../app/TransactionOverview/index.tsx | 2 + .../app/TransactionOverview/useRedirect.ts | 5 +- 5 files changed, 94 insertions(+), 183 deletions(-) diff --git a/x-pack/plugins/__mocks__/ui/chrome.js b/x-pack/plugins/__mocks__/ui/chrome.js index e56124bede6e1..b85d345a68ce3 100644 --- a/x-pack/plugins/__mocks__/ui/chrome.js +++ b/x-pack/plugins/__mocks__/ui/chrome.js @@ -15,10 +15,14 @@ function getUiSettingsClient() { default: throw new Error(`Unexpected config key: ${key}`); } - } + }, }; } +function getBasePath() { + return '/some/base/path'; +} + function addBasePath(path) { return path; } @@ -43,6 +47,7 @@ function getXsrfToken() { export default { getInjected, addBasePath, + getBasePath, getUiSettingsClient, - getXsrfToken + getXsrfToken, }; diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx index e6fd0343a2235..5c702658dbce3 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/TransactionOverview.test.tsx @@ -4,90 +4,91 @@ * you may not use this file except in compliance with the Elastic License. */ -import { shallow } from 'enzyme'; -import { History } from 'history'; +import createHistory from 'history/createHashHistory'; import React from 'react'; -import { RouteComponentProps } from 'react-router'; -import { TransactionOverviewView } from '..'; +import { Provider } from 'react-redux'; +import { Router } from 'react-router-dom'; +import { queryByLabelText, render } from 'react-testing-library'; +// @ts-ignore +import configureStore from 'x-pack/plugins/apm/public/store/config/configureStore'; +import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { TransactionOverview } from '..'; -jest.mock( - 'ui/chrome', - () => ({ - getBasePath: () => `/some/base/path`, - getInjected: (key: string) => { - if (key === 'mlEnabled') { - return true; - } - throw new Error(`inexpected key ${key}`); - }, - getUiSettingsClient: () => { - return { - get: (key: string) => { - switch (key) { - case 'timepicker:timeDefaults': - return { from: 'now-15m', to: 'now', mode: 'quick' }; - case 'timepicker:refreshIntervalDefaults': - return { display: 'Off', pause: false, value: 0 }; - default: - throw new Error(`Unexpected config key: ${key}`); - } - } - }; - } - }), - { virtual: true } -); - -const setup = () => { - const routerProps = { - location: { search: '' }, - history: { - push: jest.fn() as History['push'], - replace: jest.fn() as History['replace'] - } - } as RouteComponentProps; +function setup(props: { + urlParams: IUrlParams; + serviceTransactionTypes: string[]; +}) { + const store = configureStore(); + const history = createHistory(); + history.replace = jest.fn(); - const props = { - ...routerProps, - agentName: 'test-agent', - serviceName: 'test-service', - serviceTransactionTypes: ['firstType', 'secondType'], - urlParams: { transactionType: 'test-type', serviceName: 'MyServiceName' } - }; + const { container } = render( + + + + + + ); - const wrapper = shallow(); - return { props, wrapper }; -}; + return { container, history }; +} describe('TransactionOverviewView', () => { - describe('when no transaction type is given', () => { + describe('when no transaction type is given', () => { it('should render null', () => { - const { wrapper } = setup(); - wrapper.setProps({ urlParams: { serviceName: 'MyServiceName' } }); - expect(wrapper).toMatchInlineSnapshot(`""`); + const { container } = setup({ + serviceTransactionTypes: ['firstType', 'secondType'], + urlParams: { + serviceName: 'MyServiceName' + } + }); + expect(container).toMatchInlineSnapshot(`
`); }); it('should redirect to first type', () => { - const { wrapper, props } = setup(); - wrapper.setProps({ urlParams: { serviceName: 'MyServiceName' } }); - expect(props.history.replace).toHaveBeenCalledWith({ - pathname: '/MyServiceName/transactions/firstType', - search: '' + const { history } = setup({ + serviceTransactionTypes: ['firstType', 'secondType'], + urlParams: { + serviceName: 'MyServiceName' + } }); + expect(history.replace).toHaveBeenCalledWith( + expect.objectContaining({ + pathname: '/MyServiceName/transactions/firstType' + }) + ); }); }); - it('should render with type filter controls', () => { - const { wrapper } = setup(); - expect(wrapper).toMatchSnapshot(); + const FILTER_BY_TYPE_LABEL = 'Filter by type'; + + describe('when transactionType is selected and multiple transaction types are given', () => { + it('should render dropdown with transaction types', () => { + const { container } = setup({ + serviceTransactionTypes: ['firstType', 'secondType'], + urlParams: { + transactionType: 'secondType', + serviceName: 'MyServiceName' + } + }); + + expect( + queryByLabelText(container, FILTER_BY_TYPE_LABEL) + ).toMatchSnapshot(); + }); }); - it('should render without type filter controls if there is just a single type', () => { - const { wrapper } = setup(); - wrapper.setProps({ - serviceTransactionTypes: ['singleType'] + describe('when a transaction type is selected, and there are no other transaction types', () => { + it('should not render a dropdown with transaction types', () => { + const { container } = setup({ + serviceTransactionTypes: ['firstType'], + urlParams: { + transactionType: 'firstType', + serviceName: 'MyServiceName' + } + }); + + expect(queryByLabelText(container, FILTER_BY_TYPE_LABEL)).toBeNull(); }); - expect(wrapper.find('EuiFormRow').exists()).toBe(false); - expect(wrapper).toMatchSnapshot(); }); }); diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap index 3e889206410f6..41374c9f8fff8 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/__jest__/__snapshots__/TransactionOverview.test.tsx.snap @@ -1,115 +1,19 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`TransactionOverviewView should render with type filter controls 1`] = ` - - + - - - + - -`; - -exports[`TransactionOverviewView should render without type filter controls if there is just a single type 1`] = ` - - - - - -

- Transactions -

- - - - - + secondType + + `; diff --git a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx index b862e8b885d64..1aa497fb6e9d3 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionOverview/index.tsx @@ -58,6 +58,7 @@ export function TransactionOverviewView({ // redirect to first transaction type useRedirect( + history, getRedirectLocation({ urlParams, location, @@ -80,6 +81,7 @@ export function TransactionOverviewView({ {serviceTransactionTypes.length > 1 ? ( { if (redirectLocation) { history.replace(redirectLocation); From dabe03906a263b8382267f2f33bc66eba1a52f11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Sun, 24 Mar 2019 23:34:40 +0100 Subject: [PATCH 03/16] Fix issue with distribution sample --- .../TransactionDetails/Distribution/index.tsx | 41 ++++++++----------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx index e53ad7358004f..353332e76e4fe 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Distribution/index.tsx @@ -98,25 +98,25 @@ export class TransactionDistribution extends Component { public redirectToTransactionType() { const { urlParams, location, distribution } = this.props; - if (!distribution || !distribution.defaultSample) { + if ( + !distribution || + !distribution.defaultSample || + urlParams.traceId || + urlParams.transactionId + ) { return; } const { traceId, transactionId } = distribution.defaultSample; - const hasIncorrectSample = - traceId !== urlParams.traceId || - transactionId !== urlParams.transactionId; - - if (hasIncorrectSample) { - history.replace({ - ...location, - search: fromQuery({ - ...toQuery(location.search), - traceId, - transactionId - }) - }); - } + + history.replace({ + ...location, + search: fromQuery({ + ...toQuery(location.search), + traceId, + transactionId + }) + }); } public componentDidMount() { @@ -130,16 +130,7 @@ export class TransactionDistribution extends Component { public render() { const { location, distribution, urlParams } = this.props; - if (!distribution) { - return null; - } - - // delay rendering until after redirect - const hasIncorrectSample = - distribution.defaultSample && - (distribution.defaultSample.traceId !== urlParams.traceId || - distribution.defaultSample.transactionId !== urlParams.transactionId); - if (hasIncorrectSample) { + if (!distribution || !urlParams.traceId || !urlParams.transactionId) { return null; } From 0e2e070d29fa0a629898574b7cb910726bcbe9c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Sun, 24 Mar 2019 23:36:07 +0100 Subject: [PATCH 04/16] Make license check context based --- .../app/Main/LicenseCheck/index.tsx | 38 +-- .../ServiceIntegrations/index.tsx | 187 ++++++++++++++- .../ServiceIntegrations/view.tsx | 183 -------------- .../app/TransactionDetails/view.tsx | 1 - .../shared/charts/TransactionCharts/index.tsx | 223 +++++++++++++++++- .../shared/charts/TransactionCharts/view.tsx | 219 ----------------- .../store/reactReduxRequest/license.tsx | 35 --- .../apm/public/store/selectors/license.ts | 17 -- 8 files changed, 414 insertions(+), 489 deletions(-) delete mode 100644 x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/view.tsx delete mode 100644 x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/license.tsx delete mode 100644 x-pack/plugins/apm/public/store/selectors/license.ts diff --git a/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx b/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx index 8db0127fb4a2b..e9416d3d2e0e6 100644 --- a/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx +++ b/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx @@ -4,21 +4,31 @@ * you may not use this file except in compliance with the Elastic License. */ import React from 'react'; -import { STATUS } from '../../../../constants/index'; -import { LicenceRequest } from '../../../../store/reactReduxRequest/license'; +import { + FETCH_STATUS, + useFetcher +} from 'x-pack/plugins/apm/public/hooks/useFetcher'; +import { loadLicense } from 'x-pack/plugins/apm/public/services/rest/xpack'; import { InvalidLicenseNotification } from './InvalidLicenseNotification'; -export const LicenseCheck: React.FunctionComponent = ({ children }) => { - return ( - { - const hasValidLicense = licenseData.license.is_active; - if (licenseStatus === STATUS.SUCCESS && !hasValidLicense) { - return ; - } +const initialLicense = { + features: { + watcher: { is_available: false }, + ml: { is_available: false } + }, + license: { is_active: false } +}; +export const LicenseContext = React.createContext(initialLicense); + +export const LicenseCheck: React.FC = ({ children }) => { + const { data = initialLicense, status } = useFetcher(loadLicense, {}); + const hasValidLicense = data.license.is_active; + + // if license is invalid show an error message + if (status === FETCH_STATUS.SUCCESS && !hasValidLicense) { + return ; + } - return children; - }} - /> - ); + // render rest of application and pass down license via context + return ; }; diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/index.tsx index 3e6b8a6f2bac3..b740c0507b10d 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/index.tsx @@ -4,16 +4,181 @@ * you may not use this file except in compliance with the Elastic License. */ -import { connect } from 'react-redux'; -import { IReduxState } from 'x-pack/plugins/apm/public/store/rootReducer'; -import { selectIsMLAvailable } from 'x-pack/plugins/apm/public/store/selectors/license'; -import { ServiceIntegrationsView } from './view'; - -function mapStateToProps(state = {} as IReduxState) { - return { - mlAvailable: selectIsMLAvailable(state) - }; +import { + EuiButton, + EuiContextMenu, + EuiContextMenuPanelItemDescriptor, + EuiPopover +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { Location } from 'history'; +import { memoize } from 'lodash'; +import React, { Fragment } from 'react'; +import chrome from 'ui/chrome'; +import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { LicenseContext } from '../../Main/LicenseCheck'; +import { MachineLearningFlyout } from './MachineLearningFlyout'; +import { WatcherFlyout } from './WatcherFlyout'; + +interface Props { + location: Location; + transactionTypes: string[]; + urlParams: IUrlParams; } +interface State { + isPopoverOpen: boolean; + activeFlyout: FlyoutName; +} +type FlyoutName = null | 'ML' | 'Watcher'; + +export class ServiceIntegrations extends React.Component { + public state: State = { isPopoverOpen: false, activeFlyout: null }; -const ServiceIntegrations = connect(mapStateToProps)(ServiceIntegrationsView); -export { ServiceIntegrations }; + public getPanelItems = memoize((mlAvailable: boolean) => { + let panelItems: EuiContextMenuPanelItemDescriptor[] = []; + if (mlAvailable) { + panelItems = panelItems.concat(this.getMLPanelItems()); + } + return panelItems.concat(this.getWatcherPanelItems()); + }); + + public getMLPanelItems = () => { + return [ + { + name: i18n.translate( + 'xpack.apm.serviceDetails.integrationsMenu.enableMLAnomalyDetectionButtonLabel', + { + defaultMessage: 'Enable ML anomaly detection' + } + ), + icon: 'machineLearningApp', + toolTipContent: i18n.translate( + 'xpack.apm.serviceDetails.integrationsMenu.enableMLAnomalyDetectionButtonTooltip', + { + defaultMessage: 'Set up a machine learning job for this service' + } + ), + onClick: () => { + this.closePopover(); + this.openFlyout('ML'); + } + }, + { + name: i18n.translate( + 'xpack.apm.serviceDetails.integrationsMenu.viewMLJobsButtonLabel', + { + defaultMessage: 'View existing ML jobs' + } + ), + icon: 'machineLearningApp', + href: chrome.addBasePath('/app/ml'), + target: '_blank', + onClick: () => this.closePopover() + } + ]; + }; + + public getWatcherPanelItems = () => { + return [ + { + name: i18n.translate( + 'xpack.apm.serviceDetails.integrationsMenu.enableWatcherErrorReportsButtonLabel', + { + defaultMessage: 'Enable watcher error reports' + } + ), + icon: 'watchesApp', + onClick: () => { + this.closePopover(); + this.openFlyout('Watcher'); + } + }, + { + name: i18n.translate( + 'xpack.apm.serviceDetails.integrationsMenu.viewWatchesButtonLabel', + { + defaultMessage: 'View existing watches' + } + ), + icon: 'watchesApp', + href: chrome.addBasePath( + '/app/kibana#/management/elasticsearch/watcher' + ), + target: '_blank', + onClick: () => this.closePopover() + } + ]; + }; + + public openPopover = () => + this.setState({ + isPopoverOpen: true + }); + + public closePopover = () => + this.setState({ + isPopoverOpen: false + }); + + public openFlyout = (name: FlyoutName) => + this.setState({ activeFlyout: name }); + + public closeFlyouts = () => this.setState({ activeFlyout: null }); + + public render() { + const button = ( + + {i18n.translate( + 'xpack.apm.serviceDetails.integrationsMenu.integrationsButtonLabel', + { + defaultMessage: 'Integrations' + } + )} + + ); + + return ( + + {license => ( + + + + + + + + )} + + ); + } +} diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/view.tsx deleted file mode 100644 index c7e60934ddcd1..0000000000000 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/view.tsx +++ /dev/null @@ -1,183 +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 { - EuiButton, - EuiContextMenu, - EuiContextMenuPanelItemDescriptor, - EuiPopover -} from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import { Location } from 'history'; -import { memoize } from 'lodash'; -import React from 'react'; -import chrome from 'ui/chrome'; -import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; -import { MachineLearningFlyout } from './MachineLearningFlyout'; -import { WatcherFlyout } from './WatcherFlyout'; - -export interface ServiceIntegrationProps { - mlAvailable: boolean; - location: Location; - transactionTypes: string[]; - urlParams: IUrlParams; -} -interface ServiceIntegrationState { - isPopoverOpen: boolean; - activeFlyout: FlyoutName; -} -type FlyoutName = null | 'ML' | 'Watcher'; - -export class ServiceIntegrationsView extends React.Component< - ServiceIntegrationProps, - ServiceIntegrationState -> { - public state = { isPopoverOpen: false, activeFlyout: null }; - - public getPanelItems = memoize((mlAvailable: boolean) => { - let panelItems: EuiContextMenuPanelItemDescriptor[] = []; - if (mlAvailable) { - panelItems = panelItems.concat(this.getMLPanelItems()); - } - return panelItems.concat(this.getWatcherPanelItems()); - }); - - public getMLPanelItems = () => { - return [ - { - name: i18n.translate( - 'xpack.apm.serviceDetails.integrationsMenu.enableMLAnomalyDetectionButtonLabel', - { - defaultMessage: 'Enable ML anomaly detection' - } - ), - icon: 'machineLearningApp', - toolTipContent: i18n.translate( - 'xpack.apm.serviceDetails.integrationsMenu.enableMLAnomalyDetectionButtonTooltip', - { - defaultMessage: 'Set up a machine learning job for this service' - } - ), - onClick: () => { - this.closePopover(); - this.openFlyout('ML'); - } - }, - { - name: i18n.translate( - 'xpack.apm.serviceDetails.integrationsMenu.viewMLJobsButtonLabel', - { - defaultMessage: 'View existing ML jobs' - } - ), - icon: 'machineLearningApp', - href: chrome.addBasePath('/app/ml'), - target: '_blank', - onClick: () => this.closePopover() - } - ]; - }; - - public getWatcherPanelItems = () => { - return [ - { - name: i18n.translate( - 'xpack.apm.serviceDetails.integrationsMenu.enableWatcherErrorReportsButtonLabel', - { - defaultMessage: 'Enable watcher error reports' - } - ), - icon: 'watchesApp', - onClick: () => { - this.closePopover(); - this.openFlyout('Watcher'); - } - }, - { - name: i18n.translate( - 'xpack.apm.serviceDetails.integrationsMenu.viewWatchesButtonLabel', - { - defaultMessage: 'View existing watches' - } - ), - icon: 'watchesApp', - href: chrome.addBasePath( - '/app/kibana#/management/elasticsearch/watcher' - ), - target: '_blank', - onClick: () => this.closePopover() - } - ]; - }; - - public openPopover = () => - this.setState({ - isPopoverOpen: true - }); - - public closePopover = () => - this.setState({ - isPopoverOpen: false - }); - - public openFlyout = (name: FlyoutName) => - this.setState({ activeFlyout: name }); - - public closeFlyouts = () => this.setState({ activeFlyout: null }); - - public render() { - const button = ( - - {i18n.translate( - 'xpack.apm.serviceDetails.integrationsMenu.integrationsButtonLabel', - { - defaultMessage: 'Integrations' - } - )} - - ); - - return ( - - - - - - - - ); - } -} diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx b/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx index daf5c4e3e0695..aa5f0ef31224b 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/view.tsx @@ -20,7 +20,6 @@ import { TransactionDistribution } from './Distribution'; import { Transaction } from './Transaction'; interface Props { - mlAvailable: boolean; urlParams: IUrlParams; location: Location; } diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx index 64ebcef365c67..74ad7fb66c442 100644 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/index.tsx @@ -4,15 +4,220 @@ * you may not use this file except in compliance with the Elastic License. */ -import { connect } from 'react-redux'; -import { IReduxState } from 'x-pack/plugins/apm/public/store/rootReducer'; -import { selectIsMLAvailable } from 'x-pack/plugins/apm/public/store/selectors/license'; -import { TransactionChartsView } from './view'; +import { + EuiFlexGrid, + EuiFlexGroup, + EuiFlexItem, + EuiIconTip, + EuiPanel, + EuiText, + EuiTitle +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { Location } from 'history'; +import React, { Component } from 'react'; +import styled from 'styled-components'; +import { MLJobLink } from 'x-pack/plugins/apm/public/components/shared/Links/MLJobLink'; +import { ITransactionChartData } from 'x-pack/plugins/apm/public/store/selectors/chartSelectors'; +import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { Coordinate } from 'x-pack/plugins/apm/typings/timeseries'; +import { asInteger, asMillis, tpmUnit } from '../../../../utils/formatters'; +import { LicenseContext } from '../../../app/Main/LicenseCheck'; +// @ts-ignore +import CustomPlot from '../CustomPlot'; +import { SyncChartGroup } from '../SyncChartGroup'; -const mapStateToProps = (state: IReduxState) => ({ - mlAvailable: selectIsMLAvailable(state) -}); +interface TransactionChartProps { + charts: ITransactionChartData; + location: Location; + urlParams: IUrlParams; +} -export const TransactionCharts = connect(mapStateToProps)( - TransactionChartsView +const ShiftedIconWrapper = styled.span` + padding-right: 5px; + position: relative; + top: -1px; + display: inline-block; +`; + +const ShiftedEuiText = styled(EuiText)` + position: relative; + top: 5px; +`; + +const msTimeUnitLabel = i18n.translate( + 'xpack.apm.metrics.transactionChart.msTimeUnitLabel', + { + defaultMessage: 'ms' + } ); + +export class TransactionCharts extends Component { + public getResponseTimeTickFormatter = (t: number) => { + return this.props.charts.noHits ? `- ${msTimeUnitLabel}` : asMillis(t); + }; + + public getResponseTimeTooltipFormatter = (p: Coordinate) => { + return this.props.charts.noHits || !p + ? `- ${msTimeUnitLabel}` + : asMillis(p.y); + }; + + public getTPMFormatter = (t: number | null) => { + const { urlParams, charts } = this.props; + const unit = tpmUnit(urlParams.transactionType); + return charts.noHits || t === null + ? `- ${unit}` + : `${asInteger(t)} ${unit}`; + }; + + public getTPMTooltipFormatter = (p: Coordinate) => { + return this.getTPMFormatter(p.y); + }; + + public renderMLHeader(mlAvailable: boolean) { + const hasMLJob = this.props.charts.hasMLJob; + if (!mlAvailable || !hasMLJob) { + return null; + } + + const { serviceName, transactionType } = this.props.urlParams; + + if (!serviceName) { + return null; + } + + return ( + + + + = 75.' + } + )} + /> + + + {i18n.translate( + 'xpack.apm.metrics.transactionChart.machineLearningLabel', + { + defaultMessage: 'Machine learning:' + } + )}{' '} + + + View Job + + + + ); + } + + public render() { + const { charts, urlParams } = this.props; + const { noHits, responseTimeSeries, tpmSeries } = charts; + const { transactionType } = urlParams; + + return ( + ( + + + + + + + + {responseTimeLabel(transactionType)} + + + + {license => + this.renderMLHeader(license.features.ml.is_available) + } + + + + + + + + + + + + {tpmLabel(transactionType)} + + + + + + + )} + /> + ); + } +} + +function tpmLabel(type?: string) { + return type === 'request' + ? i18n.translate( + 'xpack.apm.metrics.transactionChart.requestsPerMinuteLabel', + { + defaultMessage: 'Requests per minute' + } + ) + : i18n.translate( + 'xpack.apm.metrics.transactionChart.transactionsPerMinuteLabel', + { + defaultMessage: 'Transactions per minute' + } + ); +} + +function responseTimeLabel(type?: string) { + switch (type) { + case 'page-load': + return i18n.translate( + 'xpack.apm.metrics.transactionChart.pageLoadTimesLabel', + { + defaultMessage: 'Page load times' + } + ); + case 'route-change': + return i18n.translate( + 'xpack.apm.metrics.transactionChart.routeChangeTimesLabel', + { + defaultMessage: 'Route change times' + } + ); + default: + return i18n.translate( + 'xpack.apm.metrics.transactionChart.transactionDurationLabel', + { + defaultMessage: 'Transaction duration' + } + ); + } +} diff --git a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx b/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx deleted file mode 100644 index f7f7c64283e72..0000000000000 --- a/x-pack/plugins/apm/public/components/shared/charts/TransactionCharts/view.tsx +++ /dev/null @@ -1,219 +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 { - EuiFlexGrid, - EuiFlexGroup, - EuiFlexItem, - EuiIconTip, - EuiPanel, - EuiText, - EuiTitle -} from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import { Location } from 'history'; -import React, { Component } from 'react'; -import styled from 'styled-components'; -import { MLJobLink } from 'x-pack/plugins/apm/public/components/shared/Links/MLJobLink'; -import { ITransactionChartData } from 'x-pack/plugins/apm/public/store/selectors/chartSelectors'; -import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; -import { Coordinate } from 'x-pack/plugins/apm/typings/timeseries'; -import { asInteger, asMillis, tpmUnit } from '../../../../utils/formatters'; -// @ts-ignore -import CustomPlot from '../CustomPlot'; -import { SyncChartGroup } from '../SyncChartGroup'; - -interface TransactionChartProps { - charts: ITransactionChartData; - location: Location; - urlParams: IUrlParams; - mlAvailable: boolean; -} - -const ShiftedIconWrapper = styled.span` - padding-right: 5px; - position: relative; - top: -1px; - display: inline-block; -`; - -const ShiftedEuiText = styled(EuiText)` - position: relative; - top: 5px; -`; - -const msTimeUnitLabel = i18n.translate( - 'xpack.apm.metrics.transactionChart.msTimeUnitLabel', - { - defaultMessage: 'ms' - } -); - -export class TransactionChartsView extends Component { - public getResponseTimeTickFormatter = (t: number) => { - return this.props.charts.noHits ? `- ${msTimeUnitLabel}` : asMillis(t); - }; - - public getResponseTimeTooltipFormatter = (p: Coordinate) => { - return this.props.charts.noHits || !p - ? `- ${msTimeUnitLabel}` - : asMillis(p.y); - }; - - public getTPMFormatter = (t: number | null) => { - const { urlParams, charts } = this.props; - const unit = tpmUnit(urlParams.transactionType); - return charts.noHits || t === null - ? `- ${unit}` - : `${asInteger(t)} ${unit}`; - }; - - public getTPMTooltipFormatter = (p: Coordinate) => { - return this.getTPMFormatter(p.y); - }; - - public renderMLHeader() { - const hasMLJob = this.props.charts.hasMLJob; - if (!this.props.mlAvailable || !hasMLJob) { - return null; - } - - const { serviceName, transactionType } = this.props.urlParams; - - if (!serviceName) { - return null; - } - - return ( - - - - = 75.' - } - )} - /> - - - {i18n.translate( - 'xpack.apm.metrics.transactionChart.machineLearningLabel', - { - defaultMessage: 'Machine learning:' - } - )}{' '} - - - View Job - - - - ); - } - - public render() { - const { charts, urlParams } = this.props; - const { noHits, responseTimeSeries, tpmSeries } = charts; - const { transactionType } = urlParams; - - return ( - ( - - - - - - - - {responseTimeLabel(transactionType)} - - - {this.renderMLHeader()} - - - - - - - - - - - {tpmLabel(transactionType)} - - - - - - - )} - /> - ); - } -} - -function tpmLabel(type?: string) { - return type === 'request' - ? i18n.translate( - 'xpack.apm.metrics.transactionChart.requestsPerMinuteLabel', - { - defaultMessage: 'Requests per minute' - } - ) - : i18n.translate( - 'xpack.apm.metrics.transactionChart.transactionsPerMinuteLabel', - { - defaultMessage: 'Transactions per minute' - } - ); -} - -function responseTimeLabel(type?: string) { - switch (type) { - case 'page-load': - return i18n.translate( - 'xpack.apm.metrics.transactionChart.pageLoadTimesLabel', - { - defaultMessage: 'Page load times' - } - ); - case 'route-change': - return i18n.translate( - 'xpack.apm.metrics.transactionChart.routeChangeTimesLabel', - { - defaultMessage: 'Route change times' - } - ); - default: - return i18n.translate( - 'xpack.apm.metrics.transactionChart.transactionDurationLabel', - { - defaultMessage: 'Transaction duration' - } - ); - } -} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/license.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/license.tsx deleted file mode 100644 index a8bc527c50af1..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/license.tsx +++ /dev/null @@ -1,35 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { LicenseApiResponse, loadLicense } from '../../services/rest/xpack'; -import { IReduxState } from '../rootReducer'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'license'; -const INITIAL_DATA = { - features: { - watcher: { is_available: false }, - ml: { is_available: false } - }, - license: { is_active: false } -}; - -const withInitialData = createInitialDataSelector(INITIAL_DATA); - -export function getLicense(state: IReduxState) { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function LicenceRequest({ - render -}: { - render: RRRRender; -}) { - return ( - - ); -} diff --git a/x-pack/plugins/apm/public/store/selectors/license.ts b/x-pack/plugins/apm/public/store/selectors/license.ts deleted file mode 100644 index 24ce0a4953b93..0000000000000 --- a/x-pack/plugins/apm/public/store/selectors/license.ts +++ /dev/null @@ -1,17 +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 { createSelector } from 'reselect'; -import { getLicense } from 'x-pack/plugins/apm/public/store/reactReduxRequest/license'; - -export const selectIsMLAvailable = createSelector( - [getLicense], - license => - license.data && - license.data.features && - license.data.features.ml && - license.data.features.ml.is_available -); From a77423fa9c76862932e0d5dd4e2836fdf012f31a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Sun, 24 Mar 2019 23:59:02 +0100 Subject: [PATCH 05/16] Convert TraceList --- .../app/TraceOverview/TraceList.tsx | 12 +++- .../components/app/TraceOverview/view.tsx | 34 ++++------- .../store/reactReduxRequest/traceList.tsx | 56 ------------------- 3 files changed, 21 insertions(+), 81 deletions(-) delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/traceList.tsx diff --git a/x-pack/plugins/apm/public/components/app/TraceOverview/TraceList.tsx b/x-pack/plugins/apm/public/components/app/TraceOverview/TraceList.tsx index 06d548ea8661d..b7ac9f63f61cc 100644 --- a/x-pack/plugins/apm/public/components/app/TraceOverview/TraceList.tsx +++ b/x-pack/plugins/apm/public/components/app/TraceOverview/TraceList.tsx @@ -11,6 +11,7 @@ import styled from 'styled-components'; import { ITransactionGroup } from 'x-pack/plugins/apm/server/lib/transaction_groups/transform'; import { fontSizes, truncate } from '../../../style/variables'; import { asMillis } from '../../../utils/formatters'; +import { EmptyMessage } from '../../shared/EmptyMessage'; import { ImpactBar } from '../../shared/ImpactBar'; import { TransactionLink } from '../../shared/Links/TransactionLink'; import { ITableColumn, ManagedTable } from '../../shared/ManagedTable'; @@ -22,7 +23,6 @@ const StyledTransactionLink = styled(TransactionLink)` interface Props { items: ITransactionGroup[]; - noItemsMessage: React.ReactNode; isLoading: boolean; } @@ -88,7 +88,15 @@ const traceListColumns: Array> = [ } ]; -export function TraceList({ items = [], noItemsMessage, isLoading }: Props) { +const noItemsMessage = ( + +); + +export function TraceList({ items = [], isLoading }: Props) { const noItems = isLoading ? null : noItemsMessage; return ( - ) => ( - - } - /> - )} - /> + ); } diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/traceList.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/traceList.tsx deleted file mode 100644 index 6bb98a9a1866a..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/traceList.tsx +++ /dev/null @@ -1,56 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { createSelector } from 'reselect'; -import { TraceListAPIResponse } from 'x-pack/plugins/apm/server/lib/traces/get_top_traces'; -import { loadTraceList } from '../../services/rest/apm/traces'; -import { IReduxState } from '../rootReducer'; -import { IUrlParams } from '../urlParams'; -import { createInitialDataSelector } from './helpers'; - -const ID = 'traceList'; -const INITIAL_DATA: TraceListAPIResponse = []; -const withInitialData = createInitialDataSelector(INITIAL_DATA); - -const selectRRR = (state = {} as IReduxState) => state.reactReduxRequest; - -export const selectTraceList = createSelector( - [selectRRR], - reactReduxRequest => { - return withInitialData(reactReduxRequest[ID]); - } -); - -interface Props { - urlParams: IUrlParams; - render: RRRRender; -} - -export function TraceListRequest({ urlParams, render }: Props) { - const { start, end, kuery } = urlParams; - - if (!start || !end) { - return null; - } - - return ( - - ); -} From 913f6a76acd768dca385d0ac6e5483c65b0a0990 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Mon, 25 Mar 2019 00:57:56 +0100 Subject: [PATCH 06/16] Convert MachineLearningFlyout to hooks --- .../MachineLearningFlyout.tsx | 391 ------------------ .../TransactionSelect.tsx | 0 .../MachineLearningFlyout/index.tsx | 203 +++++++++ .../MachineLearningFlyout/view.tsx | 252 +++++++++++ .../reactReduxRequest/machineLearningJobs.tsx | 39 -- 5 files changed, 455 insertions(+), 430 deletions(-) delete mode 100644 x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout.tsx rename x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/{ => MachineLearningFlyout}/TransactionSelect.tsx (100%) create mode 100644 x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/index.tsx create mode 100644 x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/machineLearningJobs.tsx diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout.tsx deleted file mode 100644 index aea1ae113c532..0000000000000 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout.tsx +++ /dev/null @@ -1,391 +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 { - EuiButton, - EuiCallOut, - EuiFlexGroup, - EuiFlexItem, - EuiFlyout, - EuiFlyoutBody, - EuiFlyoutFooter, - EuiFlyoutHeader, - EuiFormRow, - EuiSpacer, - EuiText, - EuiTitle -} from '@elastic/eui'; -import { i18n } from '@kbn/i18n'; -import { FormattedMessage } from '@kbn/i18n/react'; -import { Location } from 'history'; -import React, { Component } from 'react'; -import { toastNotifications } from 'ui/notify'; -import { getMlJobId } from 'x-pack/plugins/apm/common/ml_job_constants'; -import { KibanaLink } from 'x-pack/plugins/apm/public/components/shared/Links/KibanaLink'; -import { MLJobLink } from 'x-pack/plugins/apm/public/components/shared/Links/MLJobLink'; -import { startMLJob } from 'x-pack/plugins/apm/public/services/rest/ml'; -import { getAPMIndexPattern } from 'x-pack/plugins/apm/public/services/rest/savedObjects'; -import { MLJobsRequest } from 'x-pack/plugins/apm/public/store/reactReduxRequest/machineLearningJobs'; -import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; -import { TransactionSelect } from './TransactionSelect'; - -interface FlyoutProps { - isOpen: boolean; - onClose: () => void; - urlParams: IUrlParams; - location: Location; - serviceTransactionTypes: string[]; -} - -interface FlyoutState { - isLoading: boolean; - hasMLJob: boolean; - hasIndexPattern: boolean; - selectedTransactionType?: string; -} - -export class MachineLearningFlyout extends Component { - public state = { - isLoading: false, - hasIndexPattern: false, - hasMLJob: false, - selectedTransactionType: this.props.urlParams.transactionType - }; - public willUnmount = false; - - public componentWillUnmount() { - this.willUnmount = true; - } - - public async componentDidMount() { - const indexPattern = await getAPMIndexPattern(); - if (!this.willUnmount) { - this.setState({ hasIndexPattern: !!indexPattern }); - } - } - - // TODO: This should use `getDerivedStateFromProps` - public componentDidUpdate(prevProps: FlyoutProps) { - if ( - prevProps.urlParams.transactionType !== - this.props.urlParams.transactionType - ) { - this.setState({ - selectedTransactionType: this.props.urlParams.transactionType - }); - } - } - - public createJob = async () => { - this.setState({ isLoading: true }); - try { - const { serviceName, transactionType } = this.props.urlParams; - if (!serviceName || !transactionType) { - throw new Error( - 'Service name and transaction type are required to create this ML job' - ); - } - const res = await startMLJob({ serviceName, transactionType }); - const didSucceed = res.datafeeds[0].success && res.jobs[0].success; - if (!didSucceed) { - throw new Error('Creating ML job failed'); - } - this.addSuccessToast(); - } catch (e) { - this.addErrorToast(); - } - - this.setState({ isLoading: false }); - this.props.onClose(); - }; - - public addErrorToast = () => { - const { urlParams } = this.props; - const { serviceName } = urlParams; - - if (!serviceName) { - return; - } - - toastNotifications.addWarning({ - title: i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreationFailedNotificationTitle', - { - defaultMessage: 'Job creation failed' - } - ), - text: ( -

- {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreationFailedNotificationText', - { - defaultMessage: - 'Your current license may not allow for creating machine learning jobs, or this job may already exist.' - } - )} -

- ) - }); - }; - - public addSuccessToast = () => { - const { location, urlParams } = this.props; - const { serviceName, transactionType } = urlParams; - - if (!serviceName) { - return; - } - - toastNotifications.addSuccess({ - title: i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreatedNotificationTitle', - { - defaultMessage: 'Job successfully created' - } - ), - text: ( -

- {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreatedNotificationText', - { - defaultMessage: - 'The analysis is now running for {serviceName} ({transactionType}). It might take a while before results are added to the response times graph.', - values: { - serviceName, - transactionType - } - } - )}{' '} - - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreatedNotificationText.viewJobLinkText', - { - defaultMessage: 'View job' - } - )} - -

- ) - }); - }; - - public render() { - const { isOpen, onClose, urlParams, location } = this.props; - const { serviceName, transactionType } = urlParams; - const { isLoading, hasIndexPattern, selectedTransactionType } = this.state; - - if (!isOpen || !serviceName) { - return null; - } - - return ( - { - if (status === 'LOADING') { - return null; - } - - const hasMLJob = data.jobs.some( - job => - job.job_id === getMlJobId(serviceName, selectedTransactionType) - ); - - return ( - - - -

- {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.enableAnomalyDetectionTitle', - { - defaultMessage: 'Enable anomaly detection' - } - )} -

- - - - - {hasMLJob && ( -
- -

- {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.callout.jobExistsDescription', - { - defaultMessage: - 'There is currently a job running for {serviceName} ({transactionType}).', - values: { - serviceName, - transactionType - } - } - )}{' '} - - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.callout.jobExistsDescription.viewJobLinkText', - { - defaultMessage: 'View existing job' - } - )} - -

- - -
- )} - - {!hasIndexPattern && ( -
- - - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.callout.noPatternTitle.setupInstructionLinkText', - { - defaultMessage: 'Setup Instructions' - } - )} - - ) - }} - /> - - } - color="warning" - iconType="alert" - /> - -
- )} - - -

- - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.createMLJobDescription.transactionDurationGraphText', - { - defaultMessage: 'the transaction duration graph' - } - )} - - ) - }} - /> -

-

- - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.manageMLJobDescription.mlJobsPageLinkText', - { - defaultMessage: - 'Machine Learning jobs management page' - } - )} - - ) - }} - />{' '} - - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.manageMLJobDescription.noteText', - { - defaultMessage: - 'Note: It might take a few minutes for the job to begin calculating results.' - } - )} - -

- - - - - - - - {this.props.serviceTransactionTypes.length > 1 ? ( - - this.setState({ - selectedTransactionType: value - }) - } - /> - ) : null} - - - - - {i18n.translate( - 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.createNewJobButtonLabel', - { - defaultMessage: 'Create new job' - } - )} - - - - - - - ); - }} - /> - ); - } -} diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/TransactionSelect.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/TransactionSelect.tsx similarity index 100% rename from x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/TransactionSelect.tsx rename to x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/TransactionSelect.tsx diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/index.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/index.tsx new file mode 100644 index 0000000000000..0f32fb9c91359 --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/index.tsx @@ -0,0 +1,203 @@ +/* + * 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 { i18n } from '@kbn/i18n'; +import { Location } from 'history'; +import React, { Component } from 'react'; +import { toastNotifications } from 'ui/notify'; +import { MLJobLink } from 'x-pack/plugins/apm/public/components/shared/Links/MLJobLink'; +import { startMLJob } from 'x-pack/plugins/apm/public/services/rest/ml'; +import { getAPMIndexPattern } from 'x-pack/plugins/apm/public/services/rest/savedObjects'; +import { IUrlParams } from 'x-pack/plugins/apm/public/store/urlParams'; +import { MachineLearningFlyoutView } from './view'; + +interface Props { + isOpen: boolean; + onClose: () => void; + urlParams: IUrlParams; + location: Location; + serviceTransactionTypes: string[]; +} + +interface State { + isCreatingJob: boolean; + hasMLJob: boolean; + hasIndexPattern: boolean; + selectedTransactionType?: string; +} + +export class MachineLearningFlyout extends Component { + public state: State = { + isCreatingJob: false, + hasIndexPattern: false, + hasMLJob: false, + selectedTransactionType: this.props.urlParams.transactionType + }; + public willUnmount = false; + + public componentWillUnmount() { + this.willUnmount = true; + } + + public async componentDidMount() { + const indexPattern = await getAPMIndexPattern(); + if (!this.willUnmount) { + // TODO: this is causing warning from react because setState happens after + // the component has been unmounted - dispite of the checks + this.setState({ hasIndexPattern: !!indexPattern }); + } + } + + // TODO: This should use `getDerivedStateFromProps` + public componentDidUpdate(prevProps: Props) { + if ( + prevProps.urlParams.transactionType !== + this.props.urlParams.transactionType + ) { + this.setState({ + selectedTransactionType: this.props.urlParams.transactionType + }); + } + } + + public onClickCreate = async () => { + this.setState({ isCreatingJob: true }); + try { + const { serviceName, transactionType } = this.props.urlParams; + if (!serviceName || !transactionType) { + throw new Error( + 'Service name and transaction type are required to create this ML job' + ); + } + const res = await startMLJob({ serviceName, transactionType }); + const didSucceed = res.datafeeds[0].success && res.jobs[0].success; + if (!didSucceed) { + throw new Error('Creating ML job failed'); + } + this.addSuccessToast(); + } catch (e) { + this.addErrorToast(); + } + + this.setState({ isCreatingJob: false }); + this.props.onClose(); + }; + + public addErrorToast = () => { + const { urlParams } = this.props; + const { serviceName } = urlParams; + + if (!serviceName) { + return; + } + + toastNotifications.addWarning({ + title: i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreationFailedNotificationTitle', + { + defaultMessage: 'Job creation failed' + } + ), + text: ( +

+ {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreationFailedNotificationText', + { + defaultMessage: + 'Your current license may not allow for creating machine learning jobs, or this job may already exist.' + } + )} +

+ ) + }); + }; + + public addSuccessToast = () => { + const { location, urlParams } = this.props; + const { serviceName, transactionType } = urlParams; + + if (!serviceName) { + return; + } + + toastNotifications.addSuccess({ + title: i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreatedNotificationTitle', + { + defaultMessage: 'Job successfully created' + } + ), + text: ( +

+ {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreatedNotificationText', + { + defaultMessage: + 'The analysis is now running for {serviceName} ({transactionType}). It might take a while before results are added to the response times graph.', + values: { + serviceName, + transactionType + } + } + )}{' '} + + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.jobCreatedNotificationText.viewJobLinkText', + { + defaultMessage: 'View job' + } + )} + +

+ ) + }); + }; + + public onChangeTransaction(value: string) { + this.setState({ + selectedTransactionType: value + }); + } + + public render() { + const { + isOpen, + onClose, + urlParams, + location, + serviceTransactionTypes + } = this.props; + const { serviceName, transactionType } = urlParams; + const { + isCreatingJob, + hasIndexPattern, + selectedTransactionType + } = this.state; + + if (!isOpen || !serviceName) { + return null; + } + + return ( + + ); + } +} diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx new file mode 100644 index 0000000000000..6ba86c8876bde --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx @@ -0,0 +1,252 @@ +/* + * 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 { + EuiButton, + EuiCallOut, + EuiFlexGroup, + EuiFlexItem, + EuiFlyout, + EuiFlyoutBody, + EuiFlyoutFooter, + EuiFlyoutHeader, + EuiFormRow, + EuiSpacer, + EuiText, + EuiTitle +} from '@elastic/eui'; +import { i18n } from '@kbn/i18n'; +import { FormattedMessage } from '@kbn/i18n/react'; +import { Location } from 'history'; +import React from 'react'; +import { getMlJobId } from 'x-pack/plugins/apm/common/ml_job_constants'; +import { KibanaLink } from 'x-pack/plugins/apm/public/components/shared/Links/KibanaLink'; +import { MLJobLink } from 'x-pack/plugins/apm/public/components/shared/Links/MLJobLink'; +import { + FETCH_STATUS, + useFetcher +} from 'x-pack/plugins/apm/public/hooks/useFetcher'; +import { getMLJob } from 'x-pack/plugins/apm/public/services/rest/ml'; +import { TransactionSelect } from './TransactionSelect'; + +interface Props { + hasIndexPattern: boolean; + isCreatingJob: boolean; + location: Location; + onChangeTransaction: (value: string) => void; + onClickCreate: () => void; + onClose: () => void; + selectedTransactionType?: string; + serviceName: string; + serviceTransactionTypes: string[]; + transactionType?: string; +} + +const INITIAL_DATA = { count: 0, jobs: [] }; +export function MachineLearningFlyoutView({ + hasIndexPattern, + isCreatingJob, + location, + onChangeTransaction, + onClickCreate, + onClose, + selectedTransactionType, + serviceName, + serviceTransactionTypes, + transactionType +}: Props) { + const { data = INITIAL_DATA, status } = useFetcher(getMLJob, { + serviceName, + transactionType + }); + + if (status === FETCH_STATUS.LOADING) { + return null; + } + + const hasMLJob = data.jobs.some( + job => job.job_id === getMlJobId(serviceName, selectedTransactionType) + ); + + return ( + + + +

+ {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.enableAnomalyDetectionTitle', + { + defaultMessage: 'Enable anomaly detection' + } + )} +

+ + + + + {hasMLJob && ( +
+ +

+ {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.callout.jobExistsDescription', + { + defaultMessage: + 'There is currently a job running for {serviceName} ({transactionType}).', + values: { + serviceName, + transactionType + } + } + )}{' '} + + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.callout.jobExistsDescription.viewJobLinkText', + { + defaultMessage: 'View existing job' + } + )} + +

+ + +
+ )} + + {!hasIndexPattern && ( +
+ + + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.callout.noPatternTitle.setupInstructionLinkText', + { + defaultMessage: 'Setup Instructions' + } + )} + + ) + }} + /> + + } + color="warning" + iconType="alert" + /> + +
+ )} + + +

+ + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.createMLJobDescription.transactionDurationGraphText', + { + defaultMessage: 'the transaction duration graph' + } + )} + + ) + }} + /> +

+

+ + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.manageMLJobDescription.mlJobsPageLinkText', + { + defaultMessage: 'Machine Learning jobs management page' + } + )} + + ) + }} + />{' '} + + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.manageMLJobDescription.noteText', + { + defaultMessage: + 'Note: It might take a few minutes for the job to begin calculating results.' + } + )} + +

+ + + + + + + + {serviceTransactionTypes.length > 1 ? ( + + ) : null} + + + + + {i18n.translate( + 'xpack.apm.serviceDetails.enableAnomalyDetectionPanel.createNewJobButtonLabel', + { + defaultMessage: 'Create new job' + } + )} + + + + + + + ); +} diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/machineLearningJobs.tsx b/x-pack/plugins/apm/public/store/reactReduxRequest/machineLearningJobs.tsx deleted file mode 100644 index 5fd06b3a56804..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/machineLearningJobs.tsx +++ /dev/null @@ -1,39 +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 React from 'react'; -import { Request, RRRRender } from 'react-redux-request'; -import { getMLJob, MLJobApiResponse } from '../../services/rest/ml'; -import { IReduxState } from '../rootReducer'; -import { createInitialDataSelector } from './helpers'; - -const INITIAL_DATA = { count: 0, jobs: [] }; -const withInitialData = createInitialDataSelector(INITIAL_DATA); -const ID = 'MLJobs'; - -function selectMlJobs(state: IReduxState) { - return withInitialData(state.reactReduxRequest[ID]); -} - -export function MLJobsRequest({ - serviceName, - transactionType = '*', - render -}: { - serviceName: string; - transactionType?: string; - render: RRRRender; -}) { - return ( - - ); -} From eab4369ce40286efb7d1ada266fe849122ab862a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Mon, 25 Mar 2019 02:48:44 +0100 Subject: [PATCH 07/16] Convert GlobalProgress loading indicator to use context --- .../components/app/Main/FetchStatus.tsx | 53 ++++++++++++ .../app/Main/GlobalProgress/index.ts | 26 ------ .../app/Main/GlobalProgress/view.tsx | 26 ------ .../apm/public/components/app/Main/index.tsx | 27 +++--- .../plugins/apm/public/hooks/useFetcher.tsx | 82 ++++++++++--------- x-pack/plugins/apm/public/index.tsx | 12 +-- .../public/store/__jest__/rootReducer.test.ts | 1 - .../__jest__/helpers.test.js | 31 ------- .../public/store/reactReduxRequest/helpers.ts | 18 ---- .../plugins/apm/public/store/rootReducer.ts | 6 +- .../apm/typings/react-redux-request.d.ts | 36 -------- 11 files changed, 117 insertions(+), 201 deletions(-) create mode 100644 x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx delete mode 100644 x-pack/plugins/apm/public/components/app/Main/GlobalProgress/index.ts delete mode 100644 x-pack/plugins/apm/public/components/app/Main/GlobalProgress/view.tsx delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/helpers.test.js delete mode 100644 x-pack/plugins/apm/public/store/reactReduxRequest/helpers.ts delete mode 100644 x-pack/plugins/apm/typings/react-redux-request.d.ts diff --git a/x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx b/x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx new file mode 100644 index 0000000000000..a002c8d5826a5 --- /dev/null +++ b/x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx @@ -0,0 +1,53 @@ +/* + * 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 { EuiDelayHide, EuiPortal, EuiProgress } from '@elastic/eui'; +import React, { Fragment, useMemo, useReducer } from 'react'; + +export const FetchStatusContext = React.createContext({ + statuses: {}, + dispatchStatus: (action: Action) => undefined as void +}); + +interface State { + [key: string]: boolean; +} + +interface Action { + isLoading: boolean; + name: string; +} + +function reducer(statuses: State, action: Action) { + return { ...statuses, [action.name]: action.isLoading }; +} + +function getIsAnyLoading(statuses: State) { + return Object.entries(statuses).some(([name, isLoading]) => isLoading); +} + +export function FetchStatus({ children }: { children: React.ReactNode }) { + const [statuses, dispatchStatus] = useReducer(reducer, {}); + const isLoading = useMemo(() => getIsAnyLoading(statuses), [statuses]); + + return ( + + ( + + + + )} + /> + + + + ); +} diff --git a/x-pack/plugins/apm/public/components/app/Main/GlobalProgress/index.ts b/x-pack/plugins/apm/public/components/app/Main/GlobalProgress/index.ts deleted file mode 100644 index f9a8b4ecb2dd3..0000000000000 --- a/x-pack/plugins/apm/public/components/app/Main/GlobalProgress/index.ts +++ /dev/null @@ -1,26 +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 { get, some } from 'lodash'; -import { connect } from 'react-redux'; -import { IReduxState } from 'x-pack/plugins/apm/public/store/rootReducer'; -import { STATUS } from '../../../../constants/index'; -import { GlobalProgressView } from './view'; - -function getIsLoading(state: IReduxState) { - return some( - state.reactReduxRequest, - subState => get(subState, 'status') === STATUS.LOADING - ); -} - -function mapStateToProps(state = {} as IReduxState) { - return { - isLoading: getIsLoading(state) - }; -} - -export const GlobalProgress = connect(mapStateToProps)(GlobalProgressView); diff --git a/x-pack/plugins/apm/public/components/app/Main/GlobalProgress/view.tsx b/x-pack/plugins/apm/public/components/app/Main/GlobalProgress/view.tsx deleted file mode 100644 index 77995714eff42..0000000000000 --- a/x-pack/plugins/apm/public/components/app/Main/GlobalProgress/view.tsx +++ /dev/null @@ -1,26 +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 { EuiDelayHide, EuiPortal, EuiProgress } from '@elastic/eui'; -import React from 'react'; - -interface Props { - isLoading: boolean; -} - -export function GlobalProgressView({ isLoading }: Props) { - return ( - ( - - - - )} - /> - ); -} diff --git a/x-pack/plugins/apm/public/components/app/Main/index.tsx b/x-pack/plugins/apm/public/components/app/Main/index.tsx index 407d1394f81f8..4ad0cd10d20b9 100644 --- a/x-pack/plugins/apm/public/components/app/Main/index.tsx +++ b/x-pack/plugins/apm/public/components/app/Main/index.tsx @@ -10,6 +10,7 @@ import styled from 'styled-components'; import { px, topNavHeight, unit, units } from '../../../style/variables'; // @ts-ignore import ConnectRouterToRedux from '../../shared/ConnectRouterToRedux'; +import { FetchStatus } from './FetchStatus'; import { LicenseCheck } from './LicenseCheck'; import { routes } from './routeConfig'; import { ScrollToTopOnPathChange } from './ScrollToTopOnPathChange'; @@ -23,17 +24,19 @@ const MainContainer = styled.div` export function Main() { return ( - - - - - - - {routes.map((route, i) => ( - - ))} - - - + + + + + + + + {routes.map((route, i) => ( + + ))} + + + + ); } diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.tsx index e9f39cc7463fa..ef71e54b2f7a8 100644 --- a/x-pack/plugins/apm/public/hooks/useFetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/useFetcher.tsx @@ -4,7 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { useEffect, useState } from 'react'; +import { useContext, useEffect, useState } from 'react'; +import { FetchStatusContext } from '../components/app/Main/FetchStatus'; export enum FETCH_STATUS { LOADING = 'loading', @@ -19,51 +20,56 @@ export function useFetcher( fn: (options: Opts) => Promise, options: Opts ) { + const { dispatchStatus } = useContext(FetchStatusContext); const [result, setResult] = useState<{ data?: Response; status?: FETCH_STATUS; error?: Error; }>({}); - useEffect( - () => { - let didCancel = false; + const useEffectKey = [...Object.keys(options), ...Object.values(options)]; - setResult({ - data: result.data, // preserve data from previous state while loading next state - status: FETCH_STATUS.LOADING, - error: undefined - }); + useEffect(() => { + let didCancel = false; + + dispatchStatus({ name: fn.name, isLoading: true }); - fn(options) - .then(data => { - if (!didCancel) { - setResult({ - data, - status: FETCH_STATUS.SUCCESS, - error: undefined - }); - } - }) - .catch(e => { - if (e instanceof MissingArgumentsError) { - return; - } - if (!didCancel) { - setResult({ - data: undefined, - status: FETCH_STATUS.FAILURE, - error: e - }); - } - }); + setResult({ + data: result.data, // preserve data from previous state while loading next state + status: FETCH_STATUS.LOADING, + error: undefined + }); + + fn(options) + .then(data => { + if (!didCancel) { + dispatchStatus({ name: fn.name, isLoading: false }); + setResult({ + data, + status: FETCH_STATUS.SUCCESS, + error: undefined + }); + } + }) + .catch(e => { + if (e instanceof MissingArgumentsError) { + return; + } + if (!didCancel) { + dispatchStatus({ name: fn.name, isLoading: false }); + setResult({ + data: undefined, + status: FETCH_STATUS.FAILURE, + error: e + }); + } + }); - return () => { - didCancel = true; - }; - }, - [...Object.keys(options), ...Object.values(options)] - ); + return () => { + dispatchStatus({ name: fn.name, isLoading: false }); + didCancel = true; + }; + }, useEffectKey); - return result; + return result || {}; } diff --git a/x-pack/plugins/apm/public/index.tsx b/x-pack/plugins/apm/public/index.tsx index 1c7a154ee7f10..3779776c54e0c 100644 --- a/x-pack/plugins/apm/public/index.tsx +++ b/x-pack/plugins/apm/public/index.tsx @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import React, { Fragment } from 'react'; +import React from 'react'; import ReactDOM from 'react-dom'; import { Provider } from 'react-redux'; import { Router } from 'react-router-dom'; @@ -18,7 +18,6 @@ import { uiModules } from 'ui/modules'; import 'uiExports/autocompleteProviders'; import { GlobalHelpExtension } from './components/app/GlobalHelpExtension'; import { Main } from './components/app/Main'; -import { GlobalProgress } from './components/app/Main/GlobalProgress'; import { history } from './components/shared/Links/url_helpers'; // @ts-ignore import configureStore from './store/config/configureStore'; @@ -53,12 +52,9 @@ waitForRoot.then(() => { ReactDOM.render( - - - -
- - + +
+ , document.getElementById(REACT_APP_ROOT_ID) diff --git a/x-pack/plugins/apm/public/store/__jest__/rootReducer.test.ts b/x-pack/plugins/apm/public/store/__jest__/rootReducer.test.ts index 0eb91fc267b47..17c0f941079a9 100644 --- a/x-pack/plugins/apm/public/store/__jest__/rootReducer.test.ts +++ b/x-pack/plugins/apm/public/store/__jest__/rootReducer.test.ts @@ -12,7 +12,6 @@ describe('root reducer', () => { expect(state).toEqual({ location: { hash: '', pathname: '', search: '' }, - reactReduxRequest: {}, urlParams: {} }); }); diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/helpers.test.js b/x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/helpers.test.js deleted file mode 100644 index 5ed1bb04651b1..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/__jest__/helpers.test.js +++ /dev/null @@ -1,31 +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 { createInitialDataSelector } from '../helpers'; - -describe('createInitialDataSelector', () => { - it('should use initialData when data is missing from state', () => { - const state = {}; - const initialData = { foo: 'bar' }; - const withInitialData = createInitialDataSelector(initialData); - - expect(withInitialData(state)).toBe(withInitialData(state)); - expect(withInitialData(state, initialData)).toEqual({ - data: { foo: 'bar' } - }); - }); - - it('should use data when available', () => { - const state = { data: 'hello' }; - const initialData = { foo: 'bar' }; - const withInitialData = createInitialDataSelector(initialData); - - expect(withInitialData(state)).toBe(withInitialData(state)); - expect(withInitialData(state, initialData)).toEqual({ - data: 'hello' - }); - }); -}); diff --git a/x-pack/plugins/apm/public/store/reactReduxRequest/helpers.ts b/x-pack/plugins/apm/public/store/reactReduxRequest/helpers.ts deleted file mode 100644 index 7575120cfc45b..0000000000000 --- a/x-pack/plugins/apm/public/store/reactReduxRequest/helpers.ts +++ /dev/null @@ -1,18 +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 { get } from 'lodash'; -import { createSelector } from 'reselect'; - -export function createInitialDataSelector(initialData: T) { - return createSelector( - state => state, - state => { - const data: T = get(state, 'data') || initialData; - return { ...state, data }; - } - ); -} diff --git a/x-pack/plugins/apm/public/store/rootReducer.ts b/x-pack/plugins/apm/public/store/rootReducer.ts index 2395c3f0a6af2..0104e26d15416 100644 --- a/x-pack/plugins/apm/public/store/rootReducer.ts +++ b/x-pack/plugins/apm/public/store/rootReducer.ts @@ -5,20 +5,16 @@ */ import { Location } from 'history'; -import { reducer } from 'react-redux-request'; import { combineReducers } from 'redux'; -import { StringMap } from '../../typings/common'; import { locationReducer } from './location'; import { IUrlParams, urlParamsReducer } from './urlParams'; export interface IReduxState { location: Location; urlParams: IUrlParams; - reactReduxRequest: StringMap; } export const rootReducer = combineReducers({ location: locationReducer, - urlParams: urlParamsReducer, - reactReduxRequest: reducer + urlParams: urlParamsReducer }); diff --git a/x-pack/plugins/apm/typings/react-redux-request.d.ts b/x-pack/plugins/apm/typings/react-redux-request.d.ts deleted file mode 100644 index de49e9dc648a5..0000000000000 --- a/x-pack/plugins/apm/typings/react-redux-request.d.ts +++ /dev/null @@ -1,36 +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. - */ - -// Everything in here should be moved to http://github.com/sqren/react-redux-request - -declare module 'react-redux-request' { - import React from 'react'; - - // status and args are optional, especially for places that use initial data in a reducer - export interface RRRRenderResponse { - status?: 'SUCCESS' | 'LOADING' | 'FAILURE'; - data: T; - args?: P; - } - - export type RRRRender = ( - res: RRRRenderResponse - ) => React.ReactNode; - - export interface RequestProps { - id: string; - fn: (args: any) => Promise; - selector?: (state: any) => any; - args?: any[]; - render?: RRRRender; - } - - export function reducer(state: any): any; - - export class Request extends React.Component< - RequestProps - > {} -} From 03d2b5318c2ed69f0939379984cc039752a6119f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Mon, 25 Mar 2019 15:24:38 +0100 Subject: [PATCH 08/16] Add caching to network requests --- .../plugins/apm/public/hooks/useFetcher.tsx | 28 ++++-- .../public/services/__test__/callApi.test.ts | 94 +++++++++++++------ .../apm/public/services/rest/callApi.ts | 30 +++++- 3 files changed, 113 insertions(+), 39 deletions(-) diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.tsx index ef71e54b2f7a8..6c35d8e04b785 100644 --- a/x-pack/plugins/apm/public/hooks/useFetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/useFetcher.tsx @@ -17,8 +17,8 @@ export enum FETCH_STATUS { export class MissingArgumentsError extends Error {} export function useFetcher( - fn: (options: Opts) => Promise, - options: Opts + fn: (fnOptions: Opts) => Promise, + fnOptions: Opts ) { const { dispatchStatus } = useContext(FetchStatusContext); const [result, setResult] = useState<{ @@ -27,21 +27,28 @@ export function useFetcher( error?: Error; }>({}); - const useEffectKey = [...Object.keys(options), ...Object.values(options)]; + const useEffectKey = [...Object.keys(fnOptions), ...Object.values(fnOptions)]; useEffect(() => { let didCancel = false; + let didFinish = false; - dispatchStatus({ name: fn.name, isLoading: true }); - - setResult({ - data: result.data, // preserve data from previous state while loading next state - status: FETCH_STATUS.LOADING, - error: undefined + // only apply the loading indicator if the promise did not resolve immediately + // the promise will resolve immediately if the value was found in cache + requestAnimationFrame(() => { + if (!didFinish && !didCancel) { + dispatchStatus({ name: fn.name, isLoading: true }); + setResult({ + data: result.data, // preserve data from previous state while loading next state + status: FETCH_STATUS.LOADING, + error: undefined + }); + } }); - fn(options) + fn(fnOptions) .then(data => { + didFinish = true; if (!didCancel) { dispatchStatus({ name: fn.name, isLoading: false }); setResult({ @@ -52,6 +59,7 @@ export function useFetcher( } }) .catch(e => { + didFinish = true; if (e instanceof MissingArgumentsError) { return; } diff --git a/x-pack/plugins/apm/public/services/__test__/callApi.test.ts b/x-pack/plugins/apm/public/services/__test__/callApi.test.ts index 0d2183b469821..6d3a5b4e89972 100644 --- a/x-pack/plugins/apm/public/services/__test__/callApi.test.ts +++ b/x-pack/plugins/apm/public/services/__test__/callApi.test.ts @@ -5,7 +5,8 @@ */ import * as kfetchModule from 'ui/kfetch'; -import { callApi } from '../rest/callApi'; +import { mockNow } from '../../utils/testHelpers'; +import { _clearCache, callApi } from '../rest/callApi'; import { SessionStorageMock } from './SessionStorageMock'; describe('callApi', () => { @@ -21,42 +22,79 @@ describe('callApi', () => { afterEach(() => { kfetchSpy.mockClear(); + _clearCache(); }); - describe('callApi', () => { - describe('apm_debug', () => { - beforeEach(() => { - sessionStorage.setItem('apm_debug', 'true'); - }); + describe('apm_debug', () => { + beforeEach(() => { + sessionStorage.setItem('apm_debug', 'true'); + }); + + it('should add debug param for APM endpoints', async () => { + await callApi({ pathname: `/api/apm/status/server` }); + + expect(kfetchSpy).toHaveBeenCalledWith( + { pathname: '/api/apm/status/server', query: { _debug: true } }, + undefined + ); + }); + + it('should not add debug param for non-APM endpoints', async () => { + await callApi({ pathname: `/api/kibana` }); + + expect(kfetchSpy).toHaveBeenCalledWith( + { pathname: '/api/kibana' }, + undefined + ); + }); + }); - it('should add debug param for APM endpoints', async () => { - await callApi({ pathname: `/api/apm/status/server` }); + describe('prependBasePath', () => { + it('should be passed on to kFetch', async () => { + await callApi({ pathname: `/api/kibana` }, { prependBasePath: false }); - expect(kfetchSpy).toHaveBeenCalledWith( - { pathname: '/api/apm/status/server', query: { _debug: true } }, - undefined - ); - }); + expect(kfetchSpy).toHaveBeenCalledWith( + { pathname: '/api/kibana' }, + { prependBasePath: false } + ); + }); + }); + + describe('cache', () => { + it('should return cached response for subsequent calls with identical arguments', async () => { + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); + + expect(kfetchSpy).toHaveBeenCalledTimes(1); + }); + + it('should not return cached response for subsequent calls if arguments change', async () => { + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar1' } }); + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar2' } }); + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar3' } }); + + expect(kfetchSpy).toHaveBeenCalledTimes(3); + }); - it('should not add debug param for non-APM endpoints', async () => { - await callApi({ pathname: `/api/kibana` }); + it('should not return cached response if calls contain `end` param in the future', async () => { + const nowSpy = mockNow('2019-01-10'); + await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-11' } }); + await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-11' } }); + await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-11' } }); - expect(kfetchSpy).toHaveBeenCalledWith( - { pathname: '/api/kibana' }, - undefined - ); - }); + expect(kfetchSpy).toHaveBeenCalledTimes(3); + nowSpy.mockRestore(); }); - describe('prependBasePath', () => { - it('should be passed on to kFetch', async () => { - await callApi({ pathname: `/api/kibana` }, { prependBasePath: false }); + it('should return cached response if calls contain `end` param in the past', async () => { + const nowSpy = mockNow('2019-01-10'); + await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-09' } }); + await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-09' } }); + await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-09' } }); - expect(kfetchSpy).toHaveBeenCalledWith( - { pathname: '/api/kibana' }, - { prependBasePath: false } - ); - }); + expect(kfetchSpy).toHaveBeenCalledTimes(1); + nowSpy.mockRestore(); }); }); }); diff --git a/x-pack/plugins/apm/public/services/rest/callApi.ts b/x-pack/plugins/apm/public/services/rest/callApi.ts index 26fea4f17e9cb..a13d71df2c410 100644 --- a/x-pack/plugins/apm/public/services/rest/callApi.ts +++ b/x-pack/plugins/apm/public/services/rest/callApi.ts @@ -26,10 +26,38 @@ function fetchOptionsWithDebug(fetchOptions: KFetchOptions) { }; } +const cache = new Map(); + +export function _clearCache() { + cache.clear(); +} + export async function callApi( fetchOptions: KFetchOptions, options?: KFetchKibanaOptions ): Promise { + const cacheKey = JSON.stringify(fetchOptions); + const cacheResponse = cache.get(cacheKey); + if (cacheResponse) { + return cacheResponse; + } + const combinedFetchOptions = fetchOptionsWithDebug(fetchOptions); - return await kfetch(combinedFetchOptions, options); + const res = await kfetch(combinedFetchOptions, options); + + if (isCachable(fetchOptions)) { + cache.set(cacheKey, res); + } + + return res; +} + +function isCachable(fetchOptions: KFetchOptions) { + const end = fetchOptions.query && fetchOptions.query.end; + + // do not cache items where the `end` param is in the future + return ( + end === undefined || + (typeof end === 'string' && new Date(end).getTime() < Date.now()) + ); } From 382fc7cc9c56c824181686d53602f2dfddf12b96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Tue, 26 Mar 2019 18:20:54 +0100 Subject: [PATCH 09/16] Address feedback --- .../components/app/ErrorGroupDetails/view.tsx | 22 ++++++------- .../app/ErrorGroupOverview/index.tsx | 32 ++++++++++--------- ...tchStatus.tsx => GlobalFetchIndicator.tsx} | 12 ++++--- .../app/Main/LicenseCheck/index.tsx | 2 +- .../apm/public/components/app/Main/index.tsx | 6 ++-- .../MachineLearningFlyout/view.tsx | 8 ++--- .../app/ServiceDetails/ServiceMetrics.tsx | 12 +++---- .../components/app/ServiceDetails/view.tsx | 10 +++--- .../components/app/ServiceOverview/view.tsx | 11 +++---- .../components/app/TraceOverview/view.tsx | 9 +++--- .../waterfall_helpers.test.ts | 15 ++++----- .../waterfall_helpers/waterfall_helpers.ts | 3 +- .../plugins/apm/public/hooks/useFetcher.tsx | 25 ++++++++------- .../public/hooks/useServiceMetricCharts.ts | 19 +++++------ .../hooks/useTransactionDetailsCharts.ts | 20 +++++++----- .../hooks/useTransactionDistribution.ts | 16 ++++++++-- .../apm/public/hooks/useTransactionList.ts | 12 +++---- .../hooks/useTransactionOverviewCharts.ts | 18 +++++++---- .../plugins/apm/public/hooks/useWaterfall.ts | 22 ++++++------- 19 files changed, 142 insertions(+), 132 deletions(-) rename x-pack/plugins/apm/public/components/app/Main/{FetchStatus.tsx => GlobalFetchIndicator.tsx} (83%) diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx index e15236dae8a2a..8b1130a605f11 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupDetails/view.tsx @@ -68,18 +68,16 @@ interface Props { export function ErrorGroupDetailsView({ urlParams, location }: Props) { const { serviceName, start, end, errorGroupId } = urlParams; - const { data: errorGroupData } = useFetcher(loadErrorGroupDetails, { - serviceName, - start, - end, - errorGroupId - }); - - const { data: errorDistributionData } = useFetcher(loadErrorDistribution, { - serviceName, - start, - end - }); + + const { data: errorGroupData } = useFetcher( + () => loadErrorGroupDetails({ serviceName, start, end, errorGroupId }), + [serviceName, start, end, errorGroupId] + ); + + const { data: errorDistributionData } = useFetcher( + () => loadErrorDistribution({ serviceName, start, end }), + [serviceName, start, end] + ); if (!errorGroupData || !errorDistributionData) { return null; diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx index 11306d30fa937..e4cf7b71db52b 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/index.tsx @@ -41,22 +41,24 @@ const ErrorGroupOverview: React.SFC = ({ sortField, sortDirection } = urlParams; - const { data: errorDistributionData } = useFetcher(loadErrorDistribution, { - serviceName, - start, - end, - errorGroupId, - kuery - }); + const { data: errorDistributionData } = useFetcher( + () => + loadErrorDistribution({ serviceName, start, end, errorGroupId, kuery }), + [serviceName, start, end, errorGroupId, kuery] + ); - const { data: errorGroupListData } = useFetcher(loadErrorGroupList, { - serviceName, - start, - end, - sortField, - sortDirection, - kuery - }); + const { data: errorGroupListData } = useFetcher( + () => + loadErrorGroupList({ + serviceName, + start, + end, + sortField, + sortDirection, + kuery + }), + [serviceName, start, end, sortField, sortDirection, kuery] + ); if (!errorDistributionData || !errorGroupListData) { return null; diff --git a/x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx b/x-pack/plugins/apm/public/components/app/Main/GlobalFetchIndicator.tsx similarity index 83% rename from x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx rename to x-pack/plugins/apm/public/components/app/Main/GlobalFetchIndicator.tsx index a002c8d5826a5..0cd13009685ad 100644 --- a/x-pack/plugins/apm/public/components/app/Main/FetchStatus.tsx +++ b/x-pack/plugins/apm/public/components/app/Main/GlobalFetchIndicator.tsx @@ -6,7 +6,7 @@ import { EuiDelayHide, EuiPortal, EuiProgress } from '@elastic/eui'; import React, { Fragment, useMemo, useReducer } from 'react'; -export const FetchStatusContext = React.createContext({ +export const GlobalFetchContext = React.createContext({ statuses: {}, dispatchStatus: (action: Action) => undefined as void }); @@ -25,10 +25,14 @@ function reducer(statuses: State, action: Action) { } function getIsAnyLoading(statuses: State) { - return Object.entries(statuses).some(([name, isLoading]) => isLoading); + return Object.values(statuses).some(isLoading => isLoading); } -export function FetchStatus({ children }: { children: React.ReactNode }) { +export function GlobalFetchIndicator({ + children +}: { + children: React.ReactNode; +}) { const [statuses, dispatchStatus] = useReducer(reducer, {}); const isLoading = useMemo(() => getIsAnyLoading(statuses), [statuses]); @@ -44,7 +48,7 @@ export function FetchStatus({ children }: { children: React.ReactNode }) { )} /> - diff --git a/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx b/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx index e9416d3d2e0e6..1385096babdef 100644 --- a/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx +++ b/x-pack/plugins/apm/public/components/app/Main/LicenseCheck/index.tsx @@ -21,7 +21,7 @@ const initialLicense = { export const LicenseContext = React.createContext(initialLicense); export const LicenseCheck: React.FC = ({ children }) => { - const { data = initialLicense, status } = useFetcher(loadLicense, {}); + const { data = initialLicense, status } = useFetcher(() => loadLicense(), []); const hasValidLicense = data.license.is_active; // if license is invalid show an error message diff --git a/x-pack/plugins/apm/public/components/app/Main/index.tsx b/x-pack/plugins/apm/public/components/app/Main/index.tsx index 4ad0cd10d20b9..908a0f0211645 100644 --- a/x-pack/plugins/apm/public/components/app/Main/index.tsx +++ b/x-pack/plugins/apm/public/components/app/Main/index.tsx @@ -10,7 +10,7 @@ import styled from 'styled-components'; import { px, topNavHeight, unit, units } from '../../../style/variables'; // @ts-ignore import ConnectRouterToRedux from '../../shared/ConnectRouterToRedux'; -import { FetchStatus } from './FetchStatus'; +import { GlobalFetchIndicator } from './GlobalFetchIndicator'; import { LicenseCheck } from './LicenseCheck'; import { routes } from './routeConfig'; import { ScrollToTopOnPathChange } from './ScrollToTopOnPathChange'; @@ -24,7 +24,7 @@ const MainContainer = styled.div` export function Main() { return ( - + @@ -37,6 +37,6 @@ export function Main() { - + ); } diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx index 6ba86c8876bde..9a146ae71e548 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceIntegrations/MachineLearningFlyout/view.tsx @@ -58,10 +58,10 @@ export function MachineLearningFlyoutView({ serviceTransactionTypes, transactionType }: Props) { - const { data = INITIAL_DATA, status } = useFetcher(getMLJob, { - serviceName, - transactionType - }); + const { data = INITIAL_DATA, status } = useFetcher( + () => getMLJob({ serviceName, transactionType }), + [serviceName, transactionType] + ); if (status === FETCH_STATUS.LOADING) { return null; diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx index b368f9eebc333..8d5a99fdd90a3 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/ServiceMetrics.tsx @@ -32,13 +32,11 @@ interface ServiceMetricsProps { export function ServiceMetrics({ urlParams, location }: ServiceMetricsProps) { const { serviceName, start, end, errorGroupId, kuery } = urlParams; - const { data: errorDistributionData } = useFetcher(loadErrorDistribution, { - serviceName, - start, - end, - errorGroupId, - kuery - }); + const { data: errorDistributionData } = useFetcher( + () => + loadErrorDistribution({ serviceName, start, end, errorGroupId, kuery }), + [serviceName, start, end, errorGroupId, kuery] + ); const { data: transactionOverviewChartsData } = useTransactionOverviewCharts( urlParams diff --git a/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx index 2e17197eebd49..36789ed3b67fa 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceDetails/view.tsx @@ -22,12 +22,10 @@ interface Props { export function ServiceDetailsView({ urlParams, location }: Props) { const { serviceName, start, end, kuery } = urlParams; - const { data: serviceDetailsData } = useFetcher(loadServiceDetails, { - serviceName, - start, - end, - kuery - }); + const { data: serviceDetailsData } = useFetcher( + () => loadServiceDetails({ serviceName, start, end, kuery }), + [serviceName, start, end, kuery] + ); if (!serviceDetailsData) { return null; diff --git a/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx b/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx index bdedf87384edb..3480aae74d5c1 100644 --- a/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx +++ b/x-pack/plugins/apm/public/components/app/ServiceOverview/view.tsx @@ -19,12 +19,11 @@ interface Props { export function ServiceOverview({ urlParams }: Props) { const { start, end, kuery } = urlParams; - const { data: agentStatus = true } = useFetcher(loadAgentStatus, []); - const { data: serviceListData } = useFetcher(loadServiceList, { - start, - end, - kuery - }); + const { data: agentStatus = true } = useFetcher(() => loadAgentStatus(), []); + const { data: serviceListData } = useFetcher( + () => loadServiceList({ start, end, kuery }), + [start, end, kuery] + ); return ( diff --git a/x-pack/plugins/apm/public/components/app/TraceOverview/view.tsx b/x-pack/plugins/apm/public/components/app/TraceOverview/view.tsx index c8e232eb5451a..d371f1531d2f2 100644 --- a/x-pack/plugins/apm/public/components/app/TraceOverview/view.tsx +++ b/x-pack/plugins/apm/public/components/app/TraceOverview/view.tsx @@ -17,11 +17,10 @@ interface Props { export function TraceOverview(props: Props) { const { start, end, kuery } = props.urlParams; - const { status, data = [] } = useFetcher(loadTraceList, { - start, - end, - kuery - }); + const { status, data = [] } = useFetcher( + () => loadTraceList({ start, end, kuery }), + [start, end, kuery] + ); return ( diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts index e8196261a2a73..067bfd628764b 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.test.ts @@ -96,13 +96,12 @@ describe('waterfall_helpers', () => { it('should return full waterfall', () => { const entryTransactionId = 'myTransactionId1'; - const errorCountsByTransactionId = { + const errorsPerTransaction = { myTransactionId1: 2, myTransactionId2: 3 }; const waterfall = getWaterfall( - hits, - errorCountsByTransactionId, + { trace: hits, errorsPerTransaction }, entryTransactionId ); expect(waterfall.orderedItems.length).toBe(6); @@ -112,13 +111,12 @@ describe('waterfall_helpers', () => { it('should return partial waterfall', () => { const entryTransactionId = 'myTransactionId2'; - const errorCountsByTransactionId = { + const errorsPerTransaction = { myTransactionId1: 2, myTransactionId2: 3 }; const waterfall = getWaterfall( - hits, - errorCountsByTransactionId, + { trace: hits, errorsPerTransaction }, entryTransactionId ); expect(waterfall.orderedItems.length).toBe(4); @@ -128,13 +126,12 @@ describe('waterfall_helpers', () => { it('getTransactionById', () => { const entryTransactionId = 'myTransactionId1'; - const errorCountsByTransactionId = { + const errorsPerTransaction = { myTransactionId1: 2, myTransactionId2: 3 }; const waterfall = getWaterfall( - hits, - errorCountsByTransactionId, + { trace: hits, errorsPerTransaction }, entryTransactionId ); const transaction = waterfall.getTransactionById('myTransactionId2'); diff --git a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts index 6056551d41df1..5e54300427f88 100644 --- a/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts +++ b/x-pack/plugins/apm/public/components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers.ts @@ -239,8 +239,7 @@ function createGetTransactionById(itemsById: IWaterfallIndex) { } export function getWaterfall( - trace: TraceAPIResponse['trace'], - errorsPerTransaction: TraceAPIResponse['errorsPerTransaction'], + { trace, errorsPerTransaction }: TraceAPIResponse, entryTransactionId?: Transaction['transaction']['id'] ): IWaterfall { if (isEmpty(trace) || !entryTransactionId) { diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.tsx index 6c35d8e04b785..38dfbb4c2602e 100644 --- a/x-pack/plugins/apm/public/hooks/useFetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/useFetcher.tsx @@ -5,7 +5,7 @@ */ import { useContext, useEffect, useState } from 'react'; -import { FetchStatusContext } from '../components/app/Main/FetchStatus'; +import { GlobalFetchContext } from '../components/app/Main/GlobalFetchIndicator'; export enum FETCH_STATUS { LOADING = 'loading', @@ -16,19 +16,17 @@ export enum FETCH_STATUS { // use this in request methods to signal to `useFetch` that all arguments are not yet available export class MissingArgumentsError extends Error {} -export function useFetcher( - fn: (fnOptions: Opts) => Promise, - fnOptions: Opts +export function useFetcher( + fn: () => Promise, + useEffectKey: Array ) { - const { dispatchStatus } = useContext(FetchStatusContext); + const { dispatchStatus } = useContext(GlobalFetchContext); const [result, setResult] = useState<{ data?: Response; status?: FETCH_STATUS; error?: Error; }>({}); - const useEffectKey = [...Object.keys(fnOptions), ...Object.values(fnOptions)]; - useEffect(() => { let didCancel = false; let didFinish = false; @@ -46,8 +44,9 @@ export function useFetcher( } }); - fn(fnOptions) - .then(data => { + async function doFetch() { + const data = await fn(); + try { didFinish = true; if (!didCancel) { dispatchStatus({ name: fn.name, isLoading: false }); @@ -57,8 +56,7 @@ export function useFetcher( error: undefined }); } - }) - .catch(e => { + } catch (e) { didFinish = true; if (e instanceof MissingArgumentsError) { return; @@ -71,7 +69,10 @@ export function useFetcher( error: e }); } - }); + } + } + + doFetch(); return () => { dispatchStatus({ name: fn.name, isLoading: false }); diff --git a/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts b/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts index 53a667953bfe7..040ac6efa59d8 100644 --- a/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts +++ b/x-pack/plugins/apm/public/hooks/useServiceMetricCharts.ts @@ -57,15 +57,16 @@ export function useServiceMetricCharts(urlParams: IUrlParams) { } = urlParams; const { data = INITIAL_DATA, error, status } = useFetcher( - loadMetricsChartDataForService, - { - serviceName, - transactionName, - transactionType, - start, - end, - kuery - } + () => + loadMetricsChartDataForService({ + serviceName, + transactionName, + transactionType, + start, + end, + kuery + }), + [serviceName, transactionName, transactionType, start, end, kuery] ); const memoizedData = useMemo( diff --git a/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts b/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts index 44e34059c09ca..7a9adfc55035a 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionDetailsCharts.ts @@ -20,14 +20,18 @@ export function useTransactionDetailsCharts(urlParams: IUrlParams) { kuery } = urlParams; - const { data, error, status } = useFetcher(loadTransactionDetailsCharts, { - serviceName, - transactionName, - transactionType, - start, - end, - kuery - }); + const { data, error, status } = useFetcher( + () => + loadTransactionDetailsCharts({ + serviceName, + transactionName, + transactionType, + start, + end, + kuery + }), + [serviceName, transactionName, transactionType, start, end, kuery] + ); const memoizedData = useMemo(() => getTransactionCharts(urlParams, data), [ data diff --git a/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts b/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts index 6c9780b8a84a1..815d709ebfdb2 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts @@ -23,8 +23,18 @@ export function useTransactionDistribution(urlParams: IUrlParams) { } = urlParams; const { data = INITIAL_DATA, status, error } = useFetcher( - loadTransactionDistribution, - { + () => + loadTransactionDistribution({ + serviceName, + transactionType, + transactionId, + traceId, + start, + end, + transactionName, + kuery + }), + [ serviceName, transactionType, transactionId, @@ -33,7 +43,7 @@ export function useTransactionDistribution(urlParams: IUrlParams) { end, transactionName, kuery - } + ] ); return { data, status, error }; diff --git a/x-pack/plugins/apm/public/hooks/useTransactionList.ts b/x-pack/plugins/apm/public/hooks/useTransactionList.ts index 7b01573baefbf..fa04aca4280e0 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionList.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionList.ts @@ -35,13 +35,11 @@ function getWithRelativeImpact(items: TransactionListAPIResponse) { export function useTransactionList(urlParams: IUrlParams) { const { serviceName, transactionType, start, end, kuery } = urlParams; - const { data = [], error, status } = useFetcher(loadTransactionList, { - serviceName, - start, - end, - transactionType, - kuery - }); + const { data = [], error, status } = useFetcher( + () => + loadTransactionList({ serviceName, start, end, transactionType, kuery }), + [serviceName, start, end, transactionType, kuery] + ); const memoizedData = useMemo(() => getWithRelativeImpact(data), [data]); return { diff --git a/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts b/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts index e21fb514c1ede..20779069680cb 100644 --- a/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts +++ b/x-pack/plugins/apm/public/hooks/useTransactionOverviewCharts.ts @@ -20,13 +20,17 @@ export function useTransactionOverviewCharts(urlParams: IUrlParams) { kuery } = urlParams; - const { data, error, status } = useFetcher(loadTransactionOverviewCharts, { - serviceName, - start, - end, - transactionType, - kuery - }); + const { data, error, status } = useFetcher( + () => + loadTransactionOverviewCharts({ + serviceName, + start, + end, + transactionType, + kuery + }), + [serviceName, start, end, transactionType, kuery] + ); const memoizedData = useMemo(() => getTransactionCharts(urlParams, data), [ data diff --git a/x-pack/plugins/apm/public/hooks/useWaterfall.ts b/x-pack/plugins/apm/public/hooks/useWaterfall.ts index 24b4a0bd5ac18..d3b23fa2f156b 100644 --- a/x-pack/plugins/apm/public/hooks/useWaterfall.ts +++ b/x-pack/plugins/apm/public/hooks/useWaterfall.ts @@ -4,6 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ +import { useMemo } from 'react'; import { getWaterfall } from '../components/app/TransactionDetails/Transaction/WaterfallContainer/Waterfall/waterfall_helpers/waterfall_helpers'; import { loadTrace } from '../services/rest/apm/traces'; import { IUrlParams } from '../store/urlParams'; @@ -12,19 +13,16 @@ import { useFetcher } from './useFetcher'; const INITIAL_DATA = { trace: [], errorsPerTransaction: {} }; export function useWaterfall(urlParams: IUrlParams) { - const { traceId, start, end } = urlParams; - const { data = INITIAL_DATA, status, error } = useFetcher(loadTrace, { - traceId, - start, - end - }); - - // TODO consider wrapping in `useMemo` - const waterfall = getWaterfall( - data.trace, - data.errorsPerTransaction, - urlParams.transactionId + const { traceId, start, end, transactionId } = urlParams; + const { data = INITIAL_DATA, status, error } = useFetcher( + () => loadTrace({ traceId, start, end }), + [traceId, start, end] ); + const waterfall = useMemo(() => getWaterfall(data, transactionId), [ + data, + transactionId + ]); + return { data: waterfall, status, error }; } From 8327b6ff1a1d7202e19b5e59856d906a6c090db2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Tue, 26 Mar 2019 18:40:35 +0100 Subject: [PATCH 10/16] Add object-hash to get stable cache key --- x-pack/package.json | 2 + .../public/services/__test__/callApi.test.ts | 124 ++++++++++++++---- .../services/rest/apm/transaction_groups.ts | 2 +- .../apm/public/services/rest/callApi.ts | 25 +++- yarn.lock | 10 ++ 5 files changed, 129 insertions(+), 34 deletions(-) diff --git a/x-pack/package.json b/x-pack/package.json index bd24005712339..de22821cf2fcb 100644 --- a/x-pack/package.json +++ b/x-pack/package.json @@ -151,6 +151,7 @@ "@scant/router": "^0.1.0", "@slack/client": "^4.8.0", "@turf/boolean-contains": "6.0.1", + "@types/object-hash": "^1.2.0", "angular-resource": "1.4.9", "angular-sanitize": "1.6.5", "angular-ui-ace": "0.2.3", @@ -226,6 +227,7 @@ "ngreact": "^0.5.1", "node-fetch": "^2.1.2", "nodemailer": "^4.6.4", + "object-hash": "^1.3.1", "object-path-immutable": "^0.5.3", "oppsy": "^2.0.0", "papaparse": "^4.6.0", diff --git a/x-pack/plugins/apm/public/services/__test__/callApi.test.ts b/x-pack/plugins/apm/public/services/__test__/callApi.test.ts index 6d3a5b4e89972..b28287ab9d630 100644 --- a/x-pack/plugins/apm/public/services/__test__/callApi.test.ts +++ b/x-pack/plugins/apm/public/services/__test__/callApi.test.ts @@ -61,40 +61,110 @@ describe('callApi', () => { }); describe('cache', () => { - it('should return cached response for subsequent calls with identical arguments', async () => { - await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); - await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); - await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); - - expect(kfetchSpy).toHaveBeenCalledTimes(1); + let nowSpy: jest.Mock; + beforeEach(() => { + nowSpy = mockNow('2019'); }); - it('should not return cached response for subsequent calls if arguments change', async () => { - await callApi({ pathname: `/api/kibana`, query: { foo: 'bar1' } }); - await callApi({ pathname: `/api/kibana`, query: { foo: 'bar2' } }); - await callApi({ pathname: `/api/kibana`, query: { foo: 'bar3' } }); - - expect(kfetchSpy).toHaveBeenCalledTimes(3); + beforeEach(() => { + nowSpy.mockRestore(); }); - it('should not return cached response if calls contain `end` param in the future', async () => { - const nowSpy = mockNow('2019-01-10'); - await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-11' } }); - await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-11' } }); - await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-11' } }); + describe('when the call does not contain start/end params', () => { + it('should not return cached response for identical calls', async () => { + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); + await callApi({ pathname: `/api/kibana`, query: { foo: 'bar' } }); - expect(kfetchSpy).toHaveBeenCalledTimes(3); - nowSpy.mockRestore(); + expect(kfetchSpy).toHaveBeenCalledTimes(3); + }); }); - it('should return cached response if calls contain `end` param in the past', async () => { - const nowSpy = mockNow('2019-01-10'); - await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-09' } }); - await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-09' } }); - await callApi({ pathname: `/api/kibana`, query: { end: '2019-01-09' } }); - - expect(kfetchSpy).toHaveBeenCalledTimes(1); - nowSpy.mockRestore(); + describe('when the call contains start/end params', () => { + it('should return cached response for identical calls', async () => { + await callApi({ + pathname: `/api/kibana`, + query: { start: '2010', end: '2011' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2010', end: '2011' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2010', end: '2011' } + }); + + expect(kfetchSpy).toHaveBeenCalledTimes(1); + }); + + it('should not return cached response for subsequent calls if arguments change', async () => { + await callApi({ + pathname: `/api/kibana`, + query: { start: '2010', end: '2011', foo: 'bar1' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2010', end: '2011', foo: 'bar2' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2010', end: '2011', foo: 'bar3' } + }); + + expect(kfetchSpy).toHaveBeenCalledTimes(3); + }); + + it('should not return cached response if `end` is a future timestamp', async () => { + await callApi({ + pathname: `/api/kibana`, + query: { end: '2030' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { end: '2030' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { end: '2030' } + }); + + expect(kfetchSpy).toHaveBeenCalledTimes(3); + }); + + it('should return cached response if calls contain `end` param in the past', async () => { + await callApi({ + pathname: `/api/kibana`, + query: { start: '2009', end: '2010' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2009', end: '2010' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2009', end: '2010' } + }); + + expect(kfetchSpy).toHaveBeenCalledTimes(1); + }); + + it('should return cached response even if order of properties change', async () => { + await callApi({ + pathname: `/api/kibana`, + query: { end: '2010', start: '2009' } + }); + await callApi({ + pathname: `/api/kibana`, + query: { start: '2009', end: '2010' } + }); + await callApi({ + query: { start: '2009', end: '2010' }, + pathname: `/api/kibana` + }); + + expect(kfetchSpy).toHaveBeenCalledTimes(1); + }); }); }); }); diff --git a/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts b/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts index 1b8e76754d84f..a5be968852334 100644 --- a/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts +++ b/x-pack/plugins/apm/public/services/rest/apm/transaction_groups.ts @@ -75,7 +75,7 @@ export async function loadTransactionDetailsCharts({ return callApi({ pathname: `/api/apm/services/${serviceName}/transaction_groups/${transactionType}/${encodeURIComponent( - transactionName as string + transactionName )}/charts`, query: { start, diff --git a/x-pack/plugins/apm/public/services/rest/callApi.ts b/x-pack/plugins/apm/public/services/rest/callApi.ts index a13d71df2c410..d0cc182095916 100644 --- a/x-pack/plugins/apm/public/services/rest/callApi.ts +++ b/x-pack/plugins/apm/public/services/rest/callApi.ts @@ -4,7 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { startsWith } from 'lodash'; +import { FetchOptions } from 'apollo-link-http'; +import { isString, startsWith } from 'lodash'; +import hash from 'object-hash'; import { kfetch, KFetchOptions } from 'ui/kfetch'; import { KFetchKibanaOptions } from 'ui/kfetch/kfetch'; @@ -36,7 +38,7 @@ export async function callApi( fetchOptions: KFetchOptions, options?: KFetchKibanaOptions ): Promise { - const cacheKey = JSON.stringify(fetchOptions); + const cacheKey = getCacheKey(fetchOptions); const cacheResponse = cache.get(cacheKey); if (cacheResponse) { return cacheResponse; @@ -52,12 +54,23 @@ export async function callApi( return res; } +// only cache items that has a time range with `start` and `end` params, +// and where `end` is not a timestamp in the future function isCachable(fetchOptions: KFetchOptions) { - const end = fetchOptions.query && fetchOptions.query.end; + if ( + !(fetchOptions.query && fetchOptions.query.start && fetchOptions.query.end) + ) { + return false; + } - // do not cache items where the `end` param is in the future return ( - end === undefined || - (typeof end === 'string' && new Date(end).getTime() < Date.now()) + isString(fetchOptions.query.end) && + new Date(fetchOptions.query.end).getTime() < Date.now() ); } + +// order the options object to make sure that two objects with the same arguments, produce produce the +// same cache key regardless of the order of properties +function getCacheKey(options: FetchOptions) { + return hash(options); +} diff --git a/yarn.lock b/yarn.lock index 372b115894ebc..b2ea84c9cbfaa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2126,6 +2126,11 @@ resolved "https://registry.yarnpkg.com/@types/numeral/-/numeral-0.0.25.tgz#b6f55062827a4787fe4ab151cf3412a468e65271" integrity sha512-ShHzHkYD+Ldw3eyttptCpUhF1/mkInWwasQkCNXZHOsJMJ/UMa8wXrxSrTJaVk0r4pLK/VnESVM0wFsfQzNEKQ== +"@types/object-hash@^1.2.0": + version "1.2.0" + resolved "https://registry.yarnpkg.com/@types/object-hash/-/object-hash-1.2.0.tgz#d65904331bd0b05c7d5ece75f9ddfdbe82affd30" + integrity sha512-0JKYQRatHdzijO/ni7JV5eHUJWaMRpGvwiABk8U5iAk5Corm0yLNEfYGNkZWYc+wCyCKKpg0+TsZIvP8AymIYA== + "@types/opn@^5.1.0": version "5.1.0" resolved "https://registry.yarnpkg.com/@types/opn/-/opn-5.1.0.tgz#bff7bc371677f4bdbb37884400e03fd81f743927" @@ -16411,6 +16416,11 @@ object-copy@^0.1.0: define-property "^0.2.5" kind-of "^3.0.3" +object-hash@^1.3.1: + version "1.3.1" + resolved "https://registry.yarnpkg.com/object-hash/-/object-hash-1.3.1.tgz#fde452098a951cb145f039bb7d455449ddc126df" + integrity sha512-OSuu/pU4ENM9kmREg0BdNrUDIl1heYa4mBZacJc+vVWz4GtAwu7jO8s4AIt2aGRUTqxykpWzI3Oqnsm13tTMDA== + object-inspect@^1.6.0: version "1.6.0" resolved "https://registry.yarnpkg.com/object-inspect/-/object-inspect-1.6.0.tgz#c70b6cbf72f274aab4c34c0c82f5167bf82cf15b" From 0da33c88d299c289135d33b4d2889f05668e758e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Wed, 27 Mar 2019 10:30:00 +0100 Subject: [PATCH 11/16] Fix try block --- x-pack/plugins/apm/public/hooks/useFetcher.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.tsx index 38dfbb4c2602e..6022f8bb75853 100644 --- a/x-pack/plugins/apm/public/hooks/useFetcher.tsx +++ b/x-pack/plugins/apm/public/hooks/useFetcher.tsx @@ -45,9 +45,8 @@ export function useFetcher( }); async function doFetch() { - const data = await fn(); try { - didFinish = true; + const data = await fn(); if (!didCancel) { dispatchStatus({ name: fn.name, isLoading: false }); setResult({ @@ -57,7 +56,6 @@ export function useFetcher( }); } } catch (e) { - didFinish = true; if (e instanceof MissingArgumentsError) { return; } @@ -70,6 +68,7 @@ export function useFetcher( }); } } + didFinish = true; } doFetch(); From 106b0d75bb77102274128bb04d5adde2e3d15f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Wed, 27 Mar 2019 12:50:20 +0100 Subject: [PATCH 12/16] Add LRU cache --- package.json | 1 + x-pack/plugins/apm/public/services/rest/callApi.ts | 5 +++-- yarn.lock | 12 +++++------- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/package.json b/package.json index 3514f8777e3ce..be801f6878855 100644 --- a/package.json +++ b/package.json @@ -114,6 +114,7 @@ "@kbn/ui-framework": "1.0.0", "@types/json-stable-stringify": "^1.0.32", "@types/lodash.clonedeep": "^4.5.4", + "@types/lru-cache": "^5.1.0", "@types/react-grid-layout": "^0.16.7", "@types/recompose": "^0.30.5", "JSONStream": "1.1.1", diff --git a/x-pack/plugins/apm/public/services/rest/callApi.ts b/x-pack/plugins/apm/public/services/rest/callApi.ts index d0cc182095916..27842a19a322a 100644 --- a/x-pack/plugins/apm/public/services/rest/callApi.ts +++ b/x-pack/plugins/apm/public/services/rest/callApi.ts @@ -6,6 +6,7 @@ import { FetchOptions } from 'apollo-link-http'; import { isString, startsWith } from 'lodash'; +import LRU from 'lru-cache'; import hash from 'object-hash'; import { kfetch, KFetchOptions } from 'ui/kfetch'; import { KFetchKibanaOptions } from 'ui/kfetch/kfetch'; @@ -28,10 +29,10 @@ function fetchOptionsWithDebug(fetchOptions: KFetchOptions) { }; } -const cache = new Map(); +const cache = new LRU({ max: 100, maxAge: 1000 * 60 * 60 }); export function _clearCache() { - cache.clear(); + cache.reset(); } export async function callApi( diff --git a/yarn.lock b/yarn.lock index b2ea84c9cbfaa..0e5d281a73a51 100644 --- a/yarn.lock +++ b/yarn.lock @@ -772,13 +772,6 @@ dependencies: regenerator-runtime "^0.12.0" -"@babel/runtime@^7.3.4": - version "7.3.4" - resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.3.4.tgz#73d12ba819e365fcf7fd152aed56d6df97d21c83" - integrity sha512-IvfvnMdSaLBateu0jfsYIpZTxAc2cKEXEMiezGGN75QcBcecDUKd3PgLAncT0oOgxKy8dd8hrJKj9MfzgfZd6g== - dependencies: - regenerator-runtime "^0.12.0" - "@babel/runtime@^7.3.1", "@babel/runtime@^7.3.4": version "7.3.4" resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.3.4.tgz#73d12ba819e365fcf7fd152aed56d6df97d21c83" @@ -2058,6 +2051,11 @@ resolved "https://registry.yarnpkg.com/@types/loglevel/-/loglevel-1.5.3.tgz#adfce55383edc5998a2170ad581b3e23d6adb5b8" integrity sha512-TzzIZihV+y9kxSg5xJMkyIkaoGkXi50isZTtGHObNHRqAAwjGNjSCNPI7AUAv0tZUKTq9f2cdkCUd/2JVZUTrA== +"@types/lru-cache@^5.1.0": + version "5.1.0" + resolved "https://registry.yarnpkg.com/@types/lru-cache/-/lru-cache-5.1.0.tgz#57f228f2b80c046b4a1bd5cac031f81f207f4f03" + integrity sha512-RaE0B+14ToE4l6UqdarKPnXwVDuigfFv+5j9Dze/Nqr23yyuqdNvzcZi3xB+3Agvi5R4EOgAksfv3lXX4vBt9w== + "@types/mime-db@*": version "1.27.0" resolved "https://registry.yarnpkg.com/@types/mime-db/-/mime-db-1.27.0.tgz#9bc014a1fd1fdf47649c1a54c6dd7966b8284792" From 6deda74330a9cbb1d4e587c2cae098b83c917b24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Wed, 27 Mar 2019 18:17:52 +0100 Subject: [PATCH 13/16] Add test for hooks and minor cleanup --- x-pack/package.json | 1 + .../List/__test__/List.test.tsx | 37 +++++- .../FilterBar/__test__/DatePicker.test.tsx | 4 +- .../hooks/useFetcher.integration.test.tsx | 121 ++++++++++++++++++ .../apm/public/hooks/useFetcher.test.tsx | 61 +++++++++ .../plugins/apm/public/utils/testHelpers.tsx | 61 ++------- yarn.lock | 71 ++++++++++ 7 files changed, 295 insertions(+), 61 deletions(-) create mode 100644 x-pack/plugins/apm/public/hooks/useFetcher.integration.test.tsx create mode 100644 x-pack/plugins/apm/public/hooks/useFetcher.test.tsx diff --git a/x-pack/package.json b/x-pack/package.json index de22821cf2fcb..498ddec427223 100644 --- a/x-pack/package.json +++ b/x-pack/package.json @@ -247,6 +247,7 @@ "react-dom": "^16.8.0", "react-dropzone": "^4.2.9", "react-fast-compare": "^2.0.4", + "react-hooks-testing-library": "^0.3.8", "react-markdown-renderer": "^1.4.0", "react-portal": "^3.2.0", "react-redux": "^5.0.7", diff --git a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/List/__test__/List.test.tsx b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/List/__test__/List.test.tsx index 229ce73fe225b..f850a981923da 100644 --- a/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/List/__test__/List.test.tsx +++ b/x-pack/plugins/apm/public/components/app/ErrorGroupOverview/List/__test__/List.test.tsx @@ -6,13 +6,13 @@ import { mount } from 'enzyme'; import { Location } from 'history'; +import createHistory from 'history/createHashHistory'; +import PropTypes from 'prop-types'; import React from 'react'; import { MemoryRouter } from 'react-router-dom'; -import { - mockMoment, - mountWithRouterAndStore, - toJson -} from '../../../../../utils/testHelpers'; +// @ts-ignore +import { createMockStore } from 'redux-test-utils'; +import { mockMoment, toJson } from '../../../../../utils/testHelpers'; import { ErrorGroupList } from '../index'; import props from './props.json'; @@ -47,3 +47,30 @@ describe('ErrorGroupOverview -> List', () => { expect(toJson(wrapper)).toMatchSnapshot(); }); }); + +export function mountWithRouterAndStore( + Component: React.ReactElement, + storeState = {} +) { + const store = createMockStore(storeState); + const history = createHistory(); + + const options = { + context: { + store, + router: { + history, + route: { + match: { path: '/', url: '/', params: {}, isExact: true }, + location: { pathname: '/', search: '', hash: '', key: '4yyjf5' } + } + } + }, + childContextTypes: { + store: PropTypes.object.isRequired, + router: PropTypes.object.isRequired + } + }; + + return mount(Component, options); +} diff --git a/x-pack/plugins/apm/public/components/shared/FilterBar/__test__/DatePicker.test.tsx b/x-pack/plugins/apm/public/components/shared/FilterBar/__test__/DatePicker.test.tsx index 2ff953e1fe536..5508bc0b0148a 100644 --- a/x-pack/plugins/apm/public/components/shared/FilterBar/__test__/DatePicker.test.tsx +++ b/x-pack/plugins/apm/public/components/shared/FilterBar/__test__/DatePicker.test.tsx @@ -11,7 +11,7 @@ import { MemoryRouter } from 'react-router-dom'; import { Store } from 'redux'; // @ts-ignore import configureStore from 'x-pack/plugins/apm/public/store/config/configureStore'; -import { mockNow } from 'x-pack/plugins/apm/public/utils/testHelpers'; +import { mockNow, tick } from 'x-pack/plugins/apm/public/utils/testHelpers'; import { DatePicker, DatePickerComponent } from '../DatePicker'; function mountPicker(initialState = {}) { @@ -54,8 +54,6 @@ describe('DatePicker', () => { }); }); - const tick = () => new Promise(resolve => setImmediate(resolve, 0)); - describe('refresh cycle', () => { let nowSpy: jest.Mock; beforeEach(() => { diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.integration.test.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.integration.test.tsx new file mode 100644 index 0000000000000..6d4d0557bc27a --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useFetcher.integration.test.tsx @@ -0,0 +1,121 @@ +/* + * 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 from 'react'; +import { render } from 'react-testing-library'; +import { delay, tick } from '../utils/testHelpers'; +import { useFetcher } from './useFetcher'; + +// Suppress warnings about "act" until async/await syntax is supported: https://github.com/facebook/react/issues/14769 +/* tslint:disable:no-console */ +const originalError = console.error; +beforeAll(() => { + console.error = jest.fn(); +}); +afterAll(() => { + console.error = originalError; +}); + +async function asyncFn(name: string, ms: number) { + await delay(ms); + return `Hello from ${name}`; +} + +describe('when simulating race condition', () => { + let requestCallOrder: Array<[string, string, number]>; + let renderSpy: jest.Mock; + + beforeEach(async () => { + jest.useFakeTimers(); + jest + .spyOn(window, 'requestAnimationFrame') + .mockImplementation(cb => cb(0) as any); + + renderSpy = jest.fn(); + requestCallOrder = []; + + function MyComponent({ + name, + ms, + renderFn + }: { + name: string; + ms: number; + renderFn: any; + }) { + const { data, status, error } = useFetcher( + async () => { + requestCallOrder.push(['request', name, ms]); + const res = await asyncFn(name, ms); + requestCallOrder.push(['response', name, ms]); + return res; + }, + [name, ms] + ); + renderFn({ data, status, error }); + return null; + } + + const { rerender } = render( + + ); + + rerender(); + }); + + it('should render initially render loading state', async () => { + expect(renderSpy).lastCalledWith({ + data: undefined, + error: undefined, + status: 'loading' + }); + }); + + it('should render "Hello from Peter" after 200ms', async () => { + jest.advanceTimersByTime(200); + await tick(); + + expect(renderSpy).lastCalledWith({ + data: 'Hello from Peter', + error: undefined, + status: 'success' + }); + }); + + it('should render "Hello from Peter" after 600ms', async () => { + jest.advanceTimersByTime(600); + await tick(); + + expect(renderSpy).lastCalledWith({ + data: 'Hello from Peter', + error: undefined, + status: 'success' + }); + }); + + it('should should NOT have rendered "Hello from John" at any point', async () => { + jest.advanceTimersByTime(600); + await tick(); + + expect(renderSpy).not.toHaveBeenCalledWith({ + data: 'Hello from John', + error: undefined, + status: 'success' + }); + }); + + it('should send and receive calls in the right order', async () => { + jest.advanceTimersByTime(600); + await tick(); + + expect(requestCallOrder).toEqual([ + ['request', 'John', 500], + ['request', 'Peter', 100], + ['response', 'Peter', 100], + ['response', 'John', 500] + ]); + }); +}); diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx new file mode 100644 index 0000000000000..03b3fcec04b6c --- /dev/null +++ b/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx @@ -0,0 +1,61 @@ +/* + * 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 { cleanup, renderHook } from 'react-hooks-testing-library'; +import { delay } from '../utils/testHelpers'; +import { useFetcher } from './useFetcher'; + +afterEach(cleanup); + +// Suppress warnings about "act" until async/await syntax is supported: https://github.com/facebook/react/issues/14769 +/* tslint:disable:no-console */ +const originalError = console.error; +beforeAll(() => { + console.error = jest.fn(); +}); +afterAll(() => { + console.error = originalError; +}); + +describe('useFetcher', () => { + let output: ReturnType; + beforeEach(() => { + jest.useFakeTimers(); + async function fn() { + await delay(500); + return 'response from hook'; + } + output = renderHook(() => useFetcher(() => fn(), [])); + }); + + it('should initially be empty', async () => { + expect(output.result.current).toEqual({ + data: undefined, + error: undefined, + status: undefined + }); + }); + + it('should show loading spinner', async () => { + await output.waitForNextUpdate(); + expect(output.result.current).toEqual({ + data: undefined, + error: undefined, + status: 'loading' + }); + }); + + it('should show success after 1 second', async () => { + jest.advanceTimersByTime(1000); + await output.waitForNextUpdate(); + + expect(output.result.current).toEqual({ + data: 'response from hook', + error: undefined, + status: 'success' + }); + }); +}); diff --git a/x-pack/plugins/apm/public/utils/testHelpers.tsx b/x-pack/plugins/apm/public/utils/testHelpers.tsx index 845dc7b19dd14..3b720cba6e03d 100644 --- a/x-pack/plugins/apm/public/utils/testHelpers.tsx +++ b/x-pack/plugins/apm/public/utils/testHelpers.tsx @@ -8,11 +8,9 @@ import { mount, ReactWrapper } from 'enzyme'; import enzymeToJson from 'enzyme-to-json'; -import createHistory from 'history/createHashHistory'; import 'jest-styled-components'; import moment from 'moment'; import { Moment } from 'moment-timezone'; -import PropTypes from 'prop-types'; import React from 'react'; import { Provider } from 'react-redux'; import { MemoryRouter } from 'react-router-dom'; @@ -29,51 +27,6 @@ export function toJson(wrapper: ReactWrapper) { }); } -const defaultRoute = { - match: { path: '/', url: '/', params: {}, isExact: true }, - location: { pathname: '/', search: '', hash: '', key: '4yyjf5' } -}; - -export function mountWithRouterAndStore( - Component: React.ReactElement, - storeState = {}, - route = defaultRoute -) { - const store = createMockStore(storeState); - const history = createHistory(); - - const options = { - context: { - store, - router: { - history, - route - } - }, - childContextTypes: { - store: PropTypes.object.isRequired, - router: PropTypes.object.isRequired - } - }; - - return mount(Component, options); -} - -export function mountWithStore(Component: React.ReactElement, storeState = {}) { - const store = createMockStore(storeState); - - const options = { - context: { - store - }, - childContextTypes: { - store: PropTypes.object.isRequired - } - }; - - return mount(Component, options); -} - export function mockMoment() { // avoid timezone issues jest @@ -90,11 +43,6 @@ export function mockMoment() { }); } -// Await this when you need to "flush" promises to immediately resolve or throw in tests -export async function asyncFlush() { - return new Promise(resolve => setTimeout(resolve, 0)); -} - // Useful for getting the rendered href from any kind of link component export async function getRenderedHref( Component: React.FunctionComponent<{}>, @@ -109,7 +57,7 @@ export async function getRenderedHref( ); - await asyncFlush(); + await tick(); return mounted.render().attr('href'); } @@ -118,3 +66,10 @@ export function mockNow(date: string) { const fakeNow = new Date(date).getTime(); return jest.spyOn(Date, 'now').mockReturnValue(fakeNow); } + +export function delay(ms: number) { + return new Promise(resolve => setTimeout(resolve, ms)); +} + +// Await this when you need to "flush" promises to immediately resolve or throw in tests +export const tick = () => new Promise(resolve => setImmediate(resolve, 0)); diff --git a/yarn.lock b/yarn.lock index 0e5d281a73a51..4bd503343a588 100644 --- a/yarn.lock +++ b/yarn.lock @@ -779,6 +779,13 @@ dependencies: regenerator-runtime "^0.12.0" +"@babel/runtime@^7.4.2": + version "7.4.2" + resolved "https://registry.yarnpkg.com/@babel/runtime/-/runtime-7.4.2.tgz#f5ab6897320f16decd855eed70b705908a313fe8" + integrity sha512-7Bl2rALb7HpvXFL7TETNzKSAeBVCPHELzc0C//9FCxN8nsiueWSJBqaF+2oIJScyILStASR/Cx5WMkXGYTiJFA== + dependencies: + regenerator-runtime "^0.13.2" + "@babel/template@^7.0.0", "@babel/template@^7.1.0", "@babel/template@^7.1.2", "@babel/template@^7.2.2": version "7.2.2" resolved "https://registry.yarnpkg.com/@babel/template/-/template-7.2.2.tgz#005b3fdf0ed96e88041330379e0da9a708eb2907" @@ -1057,6 +1064,14 @@ normalize-path "^2.0.1" through2 "^2.0.3" +"@jest/types@^24.5.0": + version "24.5.0" + resolved "https://registry.yarnpkg.com/@jest/types/-/types-24.5.0.tgz#feee214a4d0167b0ca447284e95a57aa10b3ee95" + integrity sha512-kN7RFzNMf2R8UDadPOl6ReyI+MT8xfqRuAnuVL+i4gwjv/zubdDK+EDeLHYwq1j0CSSR2W/MmgaRlMZJzXdmVA== + dependencies: + "@types/istanbul-lib-coverage" "^1.1.0" + "@types/yargs" "^12.0.9" + "@mapbox/geojson-area@0.2.2": version "0.2.2" resolved "https://registry.yarnpkg.com/@mapbox/geojson-area/-/geojson-area-0.2.2.tgz#18d7814aa36bf23fbbcc379f8e26a22927debf10" @@ -1951,6 +1966,11 @@ dependencies: "@types/node" "*" +"@types/istanbul-lib-coverage@^1.1.0": + version "1.1.0" + resolved "https://registry.yarnpkg.com/@types/istanbul-lib-coverage/-/istanbul-lib-coverage-1.1.0.tgz#2cc2ca41051498382b43157c8227fea60363f94a" + integrity sha512-ohkhb9LehJy+PA40rDtGAji61NCgdtKLAlFoYp4cnuuQEswwdK3vz9SOIkkyc3wrk8dzjphQApNs56yyXLStaQ== + "@types/jest-diff@*": version "20.0.1" resolved "https://registry.yarnpkg.com/@types/jest-diff/-/jest-diff-20.0.1.tgz#35cc15b9c4f30a18ef21852e255fdb02f6d59b89" @@ -2436,6 +2456,11 @@ "@types/events" "*" "@types/node" "*" +"@types/yargs@^12.0.9": + version "12.0.10" + resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-12.0.10.tgz#17a8ec65cd8e88f51b418ceb271af18d3137df67" + integrity sha512-WsVzTPshvCSbHThUduGGxbmnwcpkgSctHGHTqzWyFg4lYAuV5qXlyFPOsP3OWqCINfmg/8VXP+zJaa4OxEsBQQ== + "@types/zen-observable@^0.8.0": version "0.8.0" resolved "https://registry.yarnpkg.com/@types/zen-observable/-/zen-observable-0.8.0.tgz#8b63ab7f1aa5321248aad5ac890a485656dcea4d" @@ -7711,6 +7736,16 @@ dom-testing-library@^3.13.1: pretty-format "^24.0.0" wait-for-expect "^1.1.0" +dom-testing-library@^3.18.2: + version "3.18.2" + resolved "https://registry.yarnpkg.com/dom-testing-library/-/dom-testing-library-3.18.2.tgz#07d65166743ad3299b7bee5b488e9622c31241bc" + integrity sha512-+nYUgGhHarrCY8kLVmyHlgM+IGwBXXrYsWIJB6vpAx2ne9WFgKfwMGcOkkTKQhuAro0sP6RIuRGfm5NF3+ccmQ== + dependencies: + "@babel/runtime" "^7.3.4" + "@sheerun/mutationobserver-shim" "^0.3.2" + pretty-format "^24.5.0" + wait-for-expect "^1.1.0" + dom-walk@^0.1.0: version "0.1.1" resolved "https://registry.yarnpkg.com/dom-walk/-/dom-walk-0.1.1.tgz#672226dc74c8f799ad35307df936aba11acd6018" @@ -17702,6 +17737,16 @@ pretty-format@^24.0.0: ansi-regex "^4.0.0" ansi-styles "^3.2.0" +pretty-format@^24.5.0: + version "24.5.0" + resolved "https://registry.yarnpkg.com/pretty-format/-/pretty-format-24.5.0.tgz#cc69a0281a62cd7242633fc135d6930cd889822d" + integrity sha512-/3RuSghukCf8Riu5Ncve0iI+BzVkbRU5EeUoArKARZobREycuH5O4waxvaNIloEXdb0qwgmEAed5vTpX1HNROQ== + dependencies: + "@jest/types" "^24.5.0" + ansi-regex "^4.0.0" + ansi-styles "^3.2.0" + react-is "^16.8.4" + pretty-hrtime@^1.0.0: version "1.0.3" resolved "https://registry.yarnpkg.com/pretty-hrtime/-/pretty-hrtime-1.0.3.tgz#b7e3ea42435a4c9b2759d99e0f201eb195802ee1" @@ -18513,6 +18558,14 @@ react-grid-layout@^0.16.2: react-draggable "3.x" react-resizable "1.x" +react-hooks-testing-library@^0.3.8: + version "0.3.8" + resolved "https://registry.yarnpkg.com/react-hooks-testing-library/-/react-hooks-testing-library-0.3.8.tgz#717595ed7be500023963dd502f188aa932bf70f0" + integrity sha512-YFnyd2jH2voikSBGufqhprnxMTHgosOHlO5EXhuQycWxfeTCIiw/17aiYbpvRRDRB/0j8QvI/jHXMNVBKw7WzA== + dependencies: + "@babel/runtime" "^7.4.2" + react-testing-library "^6.0.2" + react-input-autosize@^2.1.2, react-input-autosize@^2.2.1: version "2.2.1" resolved "https://registry.yarnpkg.com/react-input-autosize/-/react-input-autosize-2.2.1.tgz#ec428fa15b1592994fb5f9aa15bb1eb6baf420f8" @@ -18554,6 +18607,11 @@ react-is@^16.8.1, react-is@^16.8.2: resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.8.2.tgz#09891d324cad1cb0c1f2d91f70a71a4bee34df0f" integrity sha512-D+NxhSR2HUCjYky1q1DwpNUD44cDpUXzSmmFyC3ug1bClcU/iDNy0YNn1iwme28fn+NFhpA13IndOd42CrFb+Q== +react-is@^16.8.4: + version "16.8.5" + resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.8.5.tgz#c54ac229dd66b5afe0de5acbe47647c3da692ff8" + integrity sha512-sudt2uq5P/2TznPV4Wtdi+Lnq3yaYW8LfvPKLM9BKD8jJNBkxMVyB0C9/GmVhLw7Jbdmndk/73n7XQGeN9A3QQ== + react-is@~16.3.0: version "16.3.2" resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.3.2.tgz#f4d3d0e2f5fbb6ac46450641eb2e25bf05d36b22" @@ -18808,6 +18866,14 @@ react-testing-library@^6.0.0: "@babel/runtime" "^7.3.1" dom-testing-library "^3.13.1" +react-testing-library@^6.0.2: + version "6.0.3" + resolved "https://registry.yarnpkg.com/react-testing-library/-/react-testing-library-6.0.3.tgz#8b5d276a353c17ce4f7486015bb7a1c8827c442c" + integrity sha512-tN0A6nywSOoL8kriqru3rSdw31PxuquL7xnW6xBI0aTNw0VO3kZQtaEa0npUH9dX0MIsSunB0nbElRrc4VtAzw== + dependencies: + "@babel/runtime" "^7.4.2" + dom-testing-library "^3.18.2" + react-textarea-autosize@^7.0.4: version "7.0.4" resolved "https://registry.yarnpkg.com/react-textarea-autosize/-/react-textarea-autosize-7.0.4.tgz#4e4be649b544a88713e7b5043f76950f35d3d503" @@ -19276,6 +19342,11 @@ regenerator-runtime@^0.12.0, regenerator-runtime@^0.12.1: resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.12.1.tgz#fa1a71544764c036f8c49b13a08b2594c9f8a0de" integrity sha512-odxIc1/vDlo4iZcfXqRYFj0vpXFNoGdKMAUieAlFYO6m/nl5e9KR/beGf41z4a1FI+aQgtjhuaSlDxQ0hmkrHg== +regenerator-runtime@^0.13.2: + version "0.13.2" + resolved "https://registry.yarnpkg.com/regenerator-runtime/-/regenerator-runtime-0.13.2.tgz#32e59c9a6fb9b1a4aff09b4930ca2d4477343447" + integrity sha512-S/TQAZJO+D3m9xeN1WTI8dLKBBiRgXBlTJvbWjCThHWZj9EvHK70Ff50/tYj2J/fvBY6JtFVwRuazHN2E7M9BA== + regenerator-transform@^0.13.3: version "0.13.3" resolved "https://registry.yarnpkg.com/regenerator-transform/-/regenerator-transform-0.13.3.tgz#264bd9ff38a8ce24b06e0636496b2c856b57bcbb" From 5612dc759dea888fc90a675888740c6f54a3e4c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 28 Mar 2019 11:07:59 +0100 Subject: [PATCH 14/16] Move deps to devDeps --- package.json | 2 +- x-pack/package.json | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/package.json b/package.json index be801f6878855..20c260c35596e 100644 --- a/package.json +++ b/package.json @@ -114,7 +114,6 @@ "@kbn/ui-framework": "1.0.0", "@types/json-stable-stringify": "^1.0.32", "@types/lodash.clonedeep": "^4.5.4", - "@types/lru-cache": "^5.1.0", "@types/react-grid-layout": "^0.16.7", "@types/recompose": "^0.30.5", "JSONStream": "1.1.1", @@ -294,6 +293,7 @@ "@types/json5": "^0.0.30", "@types/listr": "^0.13.0", "@types/lodash": "^3.10.1", + "@types/lru-cache": "^5.1.0", "@types/minimatch": "^2.0.29", "@types/mocha": "^5.2.6", "@types/moment-timezone": "^0.5.8", diff --git a/x-pack/package.json b/x-pack/package.json index 498ddec427223..60f4f85122c84 100644 --- a/x-pack/package.json +++ b/x-pack/package.json @@ -44,8 +44,8 @@ "@types/d3-array": "^1.2.1", "@types/d3-scale": "^2.0.0", "@types/d3-shape": "^1.3.1", - "@types/d3-time": "^1.0.7", "@types/d3-time-format": "^2.1.0", + "@types/d3-time": "^1.0.7", "@types/elasticsearch": "^5.0.30", "@types/graphql": "^0.13.1", "@types/history": "^4.6.2", @@ -55,12 +55,13 @@ "@types/jsonwebtoken": "^7.2.7", "@types/lodash": "^3.10.1", "@types/mocha": "^5.2.6", + "@types/object-hash": "^1.2.0", "@types/pngjs": "^3.3.1", "@types/prop-types": "^15.5.3", - "@types/react": "^16.8.0", "@types/react-dom": "^16.8.0", "@types/react-redux": "^6.0.6", "@types/react-router-dom": "^4.3.1", + "@types/react": "^16.8.0", "@types/recompose": "^0.30.2", "@types/reduce-reducers": "^0.1.3", "@types/sinon": "^5.0.1", @@ -115,7 +116,9 @@ "proxyquire": "1.7.11", "react-docgen-typescript-loader": "^3.0.0", "react-docgen-typescript-webpack-plugin": "^1.1.0", + "react-hooks-testing-library": "^0.3.8", "react-test-renderer": "^16.8.0", + "react-testing-library": "^6.0.0", "redux-test-utils": "0.2.2", "rsync": "0.4.0", "run-sequence": "^2.2.1", @@ -151,7 +154,6 @@ "@scant/router": "^0.1.0", "@slack/client": "^4.8.0", "@turf/boolean-contains": "6.0.1", - "@types/object-hash": "^1.2.0", "angular-resource": "1.4.9", "angular-sanitize": "1.6.5", "angular-ui-ace": "0.2.3", @@ -247,7 +249,6 @@ "react-dom": "^16.8.0", "react-dropzone": "^4.2.9", "react-fast-compare": "^2.0.4", - "react-hooks-testing-library": "^0.3.8", "react-markdown-renderer": "^1.4.0", "react-portal": "^3.2.0", "react-redux": "^5.0.7", @@ -257,7 +258,6 @@ "react-shortcuts": "^2.0.0", "react-sticky": "^6.0.3", "react-syntax-highlighter": "^5.7.0", - "react-testing-library": "^6.0.0", "react-vis": "^1.8.1", "recompose": "^0.26.0", "reduce-reducers": "^0.4.3", From 4f9b37377f386076dcfd62d6dcc4ac7ff6158acc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 28 Mar 2019 12:46:24 +0100 Subject: [PATCH 15/16] Add test for error case --- .../apm/public/hooks/useFetcher.test.tsx | 102 +++++++++++++----- 1 file changed, 74 insertions(+), 28 deletions(-) diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx index 03b3fcec04b6c..710cbcbde25a9 100644 --- a/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx +++ b/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx @@ -21,41 +21,87 @@ afterAll(() => { }); describe('useFetcher', () => { - let output: ReturnType; - beforeEach(() => { - jest.useFakeTimers(); - async function fn() { - await delay(500); - return 'response from hook'; - } - output = renderHook(() => useFetcher(() => fn(), [])); - }); + describe('when resolving after 500ms', () => { + let output: ReturnType; + beforeEach(() => { + jest.useFakeTimers(); + async function fn() { + await delay(500); + return 'response from hook'; + } + output = renderHook(() => useFetcher(() => fn(), [])); + }); - it('should initially be empty', async () => { - expect(output.result.current).toEqual({ - data: undefined, - error: undefined, - status: undefined + it('should initially be empty', async () => { + expect(output.result.current).toEqual({ + data: undefined, + error: undefined, + status: undefined + }); }); - }); - it('should show loading spinner', async () => { - await output.waitForNextUpdate(); - expect(output.result.current).toEqual({ - data: undefined, - error: undefined, - status: 'loading' + it('should show loading spinner after 100ms', async () => { + jest.advanceTimersByTime(100); + await output.waitForNextUpdate(); + + expect(output.result.current).toEqual({ + data: undefined, + error: undefined, + status: 'loading' + }); + }); + + it('should show success after 1 second', async () => { + jest.advanceTimersByTime(1000); + await output.waitForNextUpdate(); + + expect(output.result.current).toEqual({ + data: 'response from hook', + error: undefined, + status: 'success' + }); }); }); - it('should show success after 1 second', async () => { - jest.advanceTimersByTime(1000); - await output.waitForNextUpdate(); + describe('when throwing after 500ms', () => { + let output: ReturnType; + beforeEach(() => { + jest.useFakeTimers(); + async function fn() { + await delay(500); + throw new Error('Something went wrong'); + } + output = renderHook(() => useFetcher(() => fn(), [])); + }); + + it('should initially be empty', async () => { + expect(output.result.current).toEqual({ + data: undefined, + error: undefined, + status: undefined + }); + }); + + it('should show loading spinner after 100ms', async () => { + jest.advanceTimersByTime(100); + await output.waitForNextUpdate(); + + expect(output.result.current).toEqual({ + data: undefined, + error: undefined, + status: 'loading' + }); + }); + + it('should show error after 1 second', async () => { + jest.advanceTimersByTime(1000); + await output.waitForNextUpdate(); - expect(output.result.current).toEqual({ - data: 'response from hook', - error: undefined, - status: 'success' + expect(output.result.current).toEqual({ + data: undefined, + error: expect.any(Error), + status: 'failure' + }); }); }); }); From 81edcaa46556288bbfce575568edfdd363a24ba0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Thu, 28 Mar 2019 14:00:45 +0100 Subject: [PATCH 16/16] At integration-like test to validate the steps of the hook --- .../apm/public/hooks/useFetcher.test.tsx | 80 +++++++++++++++---- 1 file changed, 66 insertions(+), 14 deletions(-) diff --git a/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx b/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx index 710cbcbde25a9..87722ce3c0275 100644 --- a/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx +++ b/x-pack/plugins/apm/public/hooks/useFetcher.test.tsx @@ -22,18 +22,18 @@ afterAll(() => { describe('useFetcher', () => { describe('when resolving after 500ms', () => { - let output: ReturnType; + let hook: ReturnType; beforeEach(() => { jest.useFakeTimers(); async function fn() { await delay(500); return 'response from hook'; } - output = renderHook(() => useFetcher(() => fn(), [])); + hook = renderHook(() => useFetcher(() => fn(), [])); }); it('should initially be empty', async () => { - expect(output.result.current).toEqual({ + expect(hook.result.current).toEqual({ data: undefined, error: undefined, status: undefined @@ -42,9 +42,9 @@ describe('useFetcher', () => { it('should show loading spinner after 100ms', async () => { jest.advanceTimersByTime(100); - await output.waitForNextUpdate(); + await hook.waitForNextUpdate(); - expect(output.result.current).toEqual({ + expect(hook.result.current).toEqual({ data: undefined, error: undefined, status: 'loading' @@ -53,9 +53,9 @@ describe('useFetcher', () => { it('should show success after 1 second', async () => { jest.advanceTimersByTime(1000); - await output.waitForNextUpdate(); + await hook.waitForNextUpdate(); - expect(output.result.current).toEqual({ + expect(hook.result.current).toEqual({ data: 'response from hook', error: undefined, status: 'success' @@ -64,18 +64,18 @@ describe('useFetcher', () => { }); describe('when throwing after 500ms', () => { - let output: ReturnType; + let hook: ReturnType; beforeEach(() => { jest.useFakeTimers(); async function fn() { await delay(500); throw new Error('Something went wrong'); } - output = renderHook(() => useFetcher(() => fn(), [])); + hook = renderHook(() => useFetcher(() => fn(), [])); }); it('should initially be empty', async () => { - expect(output.result.current).toEqual({ + expect(hook.result.current).toEqual({ data: undefined, error: undefined, status: undefined @@ -84,9 +84,9 @@ describe('useFetcher', () => { it('should show loading spinner after 100ms', async () => { jest.advanceTimersByTime(100); - await output.waitForNextUpdate(); + await hook.waitForNextUpdate(); - expect(output.result.current).toEqual({ + expect(hook.result.current).toEqual({ data: undefined, error: undefined, status: 'loading' @@ -95,13 +95,65 @@ describe('useFetcher', () => { it('should show error after 1 second', async () => { jest.advanceTimersByTime(1000); - await output.waitForNextUpdate(); + await hook.waitForNextUpdate(); - expect(output.result.current).toEqual({ + expect(hook.result.current).toEqual({ data: undefined, error: expect.any(Error), status: 'failure' }); }); }); + + describe('when a hook already has data', () => { + it('should show "first response" while loading "second response"', async () => { + jest.useFakeTimers(); + const hook = renderHook( + ({ callback, args }) => useFetcher(callback, args), + { + initialProps: { + callback: async () => 'first response', + args: ['a'] + } + } + ); + await hook.waitForNextUpdate(); + + // assert: first response has loaded and should be rendered + expect(hook.result.current).toEqual({ + data: 'first response', + error: undefined, + status: 'success' + }); + + // act: re-render hook with async callback + hook.rerender({ + callback: async () => { + await delay(500); + return 'second response'; + }, + args: ['b'] + }); + + jest.advanceTimersByTime(100); + await hook.waitForNextUpdate(); + + // assert: while loading new data the previous data should still be rendered + expect(hook.result.current).toEqual({ + data: 'first response', + error: undefined, + status: 'loading' + }); + + jest.advanceTimersByTime(500); + await hook.waitForNextUpdate(); + + // assert: "second response" has loaded and should be rendered + expect(hook.result.current).toEqual({ + data: 'second response', + error: undefined, + status: 'success' + }); + }); + }); });