[APM] Make rangeFrom/rangeTo required (#107717)

This commit is contained in:
Dario Gieselaar 2021-08-10 11:56:15 +02:00 committed by GitHub
parent 778a1160f5
commit 78e2bd2788
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 282 additions and 49 deletions

View file

@ -27,6 +27,11 @@ describe('createRouter', () => {
rangeTo: t.string,
}),
}),
defaults: {
query: {
rangeFrom: 'now-30m',
},
},
children: [
{
path: '/services',
@ -164,6 +169,20 @@ describe('createRouter', () => {
router.getParams('/service-map', history.location, true);
}).not.toThrowError();
});
it('applies defaults', () => {
history.push('/services?rangeTo=now&transactionType=request');
const topLevelParams = router.getParams('/', history.location);
expect(topLevelParams).toEqual({
path: {},
query: {
rangeFrom: 'now-30m',
rangeTo: 'now',
},
});
});
});
describe('matchRoutes', () => {
@ -181,6 +200,19 @@ describe('createRouter', () => {
router.matchRoutes('/traces', history.location);
}).toThrowError('No matching route found for /traces');
});
it('applies defaults', () => {
history.push('/services?rangeTo=now&transactionType=request');
const matches = router.matchRoutes('/', history.location);
expect(matches[1]?.match.params).toEqual({
query: {
rangeFrom: 'now-30m',
rangeTo: 'now',
},
});
});
});
describe('link', () => {
@ -241,5 +273,17 @@ describe('createRouter', () => {
} as any);
}).toThrowError();
});
it('applies defaults', () => {
const href = router.link('/traces', {
// @ts-ignore
query: {
rangeTo: 'now',
aggregationType: 'avg',
},
});
expect(href).toEqual('/traces?aggregationType=avg&rangeFrom=now-30m&rangeTo=now');
});
});
});

View file

@ -76,10 +76,12 @@ export function createRouter<TRoutes extends Route[]>(routes: TRoutes): Router<T
const route = routesByReactRouterConfig.get(matchedRoute.route);
if (route?.params) {
const decoded = deepExactRt(route.params).decode({
path: matchedRoute.match.params,
query: qs.parse(location.search),
});
const decoded = deepExactRt(route.params).decode(
merge({}, route.defaults ?? {}, {
path: matchedRoute.match.params,
query: qs.parse(location.search),
})
);
if (isLeft(decoded)) {
throw new Error(PathReporter.report(decoded).join('\n'));
@ -110,12 +112,12 @@ export function createRouter<TRoutes extends Route[]>(routes: TRoutes): Router<T
const link = (path: string, ...args: any[]) => {
const params: { path?: Record<string, any>; query?: Record<string, any> } | undefined = args[0];
const paramsWithDefaults = merge({ path: {}, query: {} }, params);
const paramsWithBuiltInDefaults = merge({ path: {}, query: {} }, params);
path = path
.split('/')
.map((part) => {
return part.startsWith(':') ? paramsWithDefaults.path[part.split(':')[1]] : part;
return part.startsWith(':') ? paramsWithBuiltInDefaults.path[part.split(':')[1]] : part;
})
.join('/');
@ -125,15 +127,25 @@ export function createRouter<TRoutes extends Route[]>(routes: TRoutes): Router<T
throw new Error(`No matching route found for ${path}`);
}
const matchedRoutes = matches.map((match) => {
return routesByReactRouterConfig.get(match.route)!;
});
const validationType = mergeRt(
...(compact(
matches.map((match) => {
return routesByReactRouterConfig.get(match.route)?.params;
matchedRoutes.map((match) => {
return match.params;
})
) as [any, any])
);
const validation = validationType.decode(paramsWithDefaults);
const paramsWithRouteDefaults = merge(
{},
...matchedRoutes.map((route) => route.defaults ?? {}),
paramsWithBuiltInDefaults
);
const validation = validationType.decode(paramsWithRouteDefaults);
if (isLeft(validation)) {
throw new Error(PathReporter.report(validation).join('\n'));
@ -141,7 +153,7 @@ export function createRouter<TRoutes extends Route[]>(routes: TRoutes): Router<T
return qs.stringifyUrl({
url: path,
query: paramsWithDefaults.query,
query: paramsWithRouteDefaults.query,
});
};
@ -152,7 +164,10 @@ export function createRouter<TRoutes extends Route[]>(routes: TRoutes): Router<T
getParams: (...args: any[]) => {
const matches = matchRoutes(...args);
return matches.length
? merge({ path: {}, query: {} }, ...matches.map((match) => match.match.params))
? merge(
{ path: {}, query: {} },
...matches.map((match) => merge({}, match.route?.defaults ?? {}, match.match.params))
)
: undefined;
},
matchRoutes: (...args: any[]) => {

View file

@ -56,6 +56,7 @@ interface PlainRoute {
element: ReactElement;
children?: PlainRoute[];
params?: t.Type<any>;
defaults?: Record<string, Record<string, string>>;
}
interface ReadonlyPlainRoute {
@ -63,6 +64,7 @@ interface ReadonlyPlainRoute {
readonly element: ReactElement;
readonly children?: readonly ReadonlyPlainRoute[];
readonly params?: t.Type<any>;
readonly defaults?: Record<string, Record<string, string>>;
}
export type Route = PlainRoute | ReadonlyPlainRoute;

View file

@ -15,6 +15,7 @@ import { useTimeRange } from '../../../hooks/use_time_range';
import { Coordinate, TimeSeries } from '../../../../typings/timeseries';
import { TimeseriesChart } from '../../shared/charts/timeseries_chart';
import { useTheme } from '../../../hooks/use_theme';
import { useApmParams } from '../../../hooks/use_apm_params';
function yLabelFormat(y?: number | null) {
return asPercent(y || 0, 1);
@ -25,7 +26,11 @@ export function BackendErrorRateChart({ height }: { height: number }) {
const theme = useTheme();
const { start, end } = useTimeRange();
const {
query: { rangeFrom, rangeTo },
} = useApmParams('/backends/:backendName/overview');
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const {
urlParams: { kuery, environment },

View file

@ -19,13 +19,18 @@ import {
getMaxY,
getResponseTimeTickFormatter,
} from '../../shared/charts/transaction_charts/helper';
import { useApmParams } from '../../../hooks/use_apm_params';
export function BackendLatencyChart({ height }: { height: number }) {
const { backendName } = useApmBackendContext();
const theme = useTheme();
const { start, end } = useTimeRange();
const {
query: { rangeFrom, rangeTo },
} = useApmParams('/backends/:backendName/overview');
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const {
urlParams: { kuery, environment },

View file

@ -15,13 +15,18 @@ import { useTimeRange } from '../../../hooks/use_time_range';
import { Coordinate, TimeSeries } from '../../../../typings/timeseries';
import { TimeseriesChart } from '../../shared/charts/timeseries_chart';
import { useTheme } from '../../../hooks/use_theme';
import { useApmParams } from '../../../hooks/use_apm_params';
export function BackendThroughputChart({ height }: { height: number }) {
const { backendName } = useApmBackendContext();
const theme = useTheme();
const { start, end } = useTimeRange();
const {
query: { rangeFrom, rangeTo },
} = useApmParams('/backends/:backendName/overview');
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const {
urlParams: { kuery, environment },

View file

@ -27,7 +27,7 @@ import { BackendDetailTemplate } from '../../routing/templates/backend_detail_te
export function BackendDetailOverview() {
const {
path: { backendName },
query,
query: { rangeFrom, rangeTo },
} = useApmParams('/backends/:backendName/overview');
const apmRouter = useApmRouter();
@ -35,13 +35,16 @@ export function BackendDetailOverview() {
useBreadcrumb([
{
title: BackendInventoryTitle,
href: apmRouter.link('/backends'),
href: apmRouter.link('/backends', { query: { rangeFrom, rangeTo } }),
},
{
title: backendName,
href: apmRouter.link('/backends/:backendName/overview', {
path: { backendName },
query,
query: {
rangeFrom,
rangeTo,
},
}),
},
]);

View file

@ -101,6 +101,7 @@ export function ErrorGroupDetails() {
const {
path: { groupId },
query: { rangeFrom, rangeTo },
} = useApmParams('/services/:serviceName/errors/:groupId');
useBreadcrumb({
@ -110,6 +111,10 @@ export function ErrorGroupDetails() {
serviceName,
groupId,
},
query: {
rangeFrom,
rangeTo,
},
}),
});

View file

@ -18,13 +18,14 @@ export function ServiceDependenciesBreakdownChart({
}: {
height: number;
}) {
const { start, end } = useTimeRange();
const { serviceName } = useApmServiceContext();
const {
query: { kuery, environment },
query: { kuery, environment, rangeFrom, rangeTo },
} = useApmParams('/services/:serviceName/dependencies');
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const { data, status } = useFetcher(
(callApmApi) => {
return callApmApi({

View file

@ -13,6 +13,7 @@ import {
EuiToolTip,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { TypeOf } from '@kbn/typed-react-router-config';
import { orderBy } from 'lodash';
import React, { useMemo } from 'react';
import { ValuesType } from 'utility-types';
@ -31,6 +32,7 @@ import {
import { useApmParams } from '../../../../hooks/use_apm_params';
import { APIReturnType } from '../../../../services/rest/createCallApmApi';
import { unit } from '../../../../utils/style';
import { ApmRoutes } from '../../../routing/apm_route_config';
import { EnvironmentBadge } from '../../../shared/EnvironmentBadge';
import { ITableColumn, ManagedTable } from '../../../shared/managed_table';
import { ServiceLink } from '../../../shared/service_link';
@ -66,7 +68,7 @@ export function getServiceColumns({
showTransactionTypeColumn,
comparisonData,
}: {
query: Record<string, string | undefined>;
query: TypeOf<ApmRoutes, '/services'>['query'];
showTransactionTypeColumn: boolean;
comparisonData?: ServicesDetailedStatisticsAPIResponse;
}): Array<ITableColumn<ServiceListItem>> {

View file

@ -61,7 +61,10 @@ describe('ServiceList', () => {
environments: ['test'],
};
const renderedColumns = getServiceColumns({
query: {},
query: {
rangeFrom: 'now-15m',
rangeTo: 'now',
},
showTransactionTypeColumn: false,
}).map((c) => c.render!(service[c.field!], service));

View file

@ -10,6 +10,7 @@
import { EuiButton, EuiFlexItem, EuiHorizontalRule } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { useApmParams } from '../../../../hooks/use_apm_params';
import type { ContentsProps } from '.';
import { NodeStats } from '../../../../../common/service_map';
import { useUrlParams } from '../../../../context/url_params_context/use_url_params';
@ -17,14 +18,29 @@ import { useApmRouter } from '../../../../hooks/use_apm_router';
import { FETCH_STATUS, useFetcher } from '../../../../hooks/use_fetcher';
import { AnomalyDetection } from './anomaly_detection';
import { StatsList } from './stats_list';
import { useTimeRange } from '../../../../hooks/use_time_range';
export function ServiceContents({ onFocusClick, nodeData }: ContentsProps) {
const apmRouter = useApmRouter();
const {
urlParams: { environment, start, end },
urlParams: { environment },
} = useUrlParams();
const { query } = useApmParams('/*');
if (
!('rangeFrom' in query && 'rangeTo' in query) ||
!query.rangeFrom ||
!query.rangeTo
) {
throw new Error('Expected rangeFrom and rangeTo to be set');
}
const { rangeFrom, rangeTo } = query;
const { start, end } = useTimeRange({ rangeFrom, rangeTo });
const serviceName = nodeData.id!;
const { data = { transactionStats: {} } as NodeStats, status } = useFetcher(
@ -49,10 +65,12 @@ export function ServiceContents({ onFocusClick, nodeData }: ContentsProps) {
const detailsUrl = apmRouter.link('/services/:serviceName', {
path: { serviceName },
query: { rangeFrom, rangeTo },
});
const focusUrl = apmRouter.link('/services/:serviceName/service-map', {
path: { serviceName },
query: { rangeFrom, rangeTo },
});
const { serviceAnomalyStats } = nodeData;

View file

@ -53,14 +53,24 @@ export const BackendInventoryTitle = i18n.translate(
export const home = {
path: '/',
element: <Outlet />,
params: t.partial({
query: t.partial({
rangeFrom: t.string,
rangeTo: t.string,
environment: t.string,
kuery: t.string,
}),
params: t.type({
query: t.intersection([
t.partial({
environment: t.string,
kuery: t.string,
}),
t.type({
rangeFrom: t.string,
rangeTo: t.string,
}),
]),
}),
defaults: {
query: {
rangeFrom: 'now-15m',
rangeTo: 'now',
},
},
children: [
page({
path: '/services',

View file

@ -66,19 +66,29 @@ export const serviceDetail = {
serviceName: t.string,
}),
}),
t.partial({
query: t.partial({
environment: t.string,
rangeFrom: t.string,
rangeTo: t.string,
comparisonEnabled: t.string,
comparisonType: t.string,
latencyAggregationType: t.string,
transactionType: t.string,
kuery: t.string,
}),
t.type({
query: t.intersection([
t.type({
rangeFrom: t.string,
rangeTo: t.string,
}),
t.partial({
environment: t.string,
comparisonEnabled: t.string,
comparisonType: t.string,
latencyAggregationType: t.string,
transactionType: t.string,
kuery: t.string,
}),
]),
}),
]),
defaults: {
query: {
rangeFrom: 'now-15m',
rangeTo: 'now',
},
},
children: [
page({
path: '/overview',

View file

@ -18,7 +18,7 @@ const StyledLink = euiStyled(EuiLink)`${truncate('100%')};`;
interface BackendLinkProps {
backendName: string;
query?: TypeOf<ApmRoutes, '/backends/:backendName/overview'>['query'];
query: TypeOf<ApmRoutes, '/backends/:backendName/overview'>['query'];
subtype?: string;
type?: string;
}

View file

@ -19,7 +19,7 @@ const StyledLink = euiStyled(EuiLink)`${truncate('100%')};`;
interface ServiceLinkProps {
agentName?: AgentName;
query?: TypeOf<ApmRoutes, '/services/:serviceName/overview'>['query'];
query: TypeOf<ApmRoutes, '/services/:serviceName/overview'>['query'];
serviceName: string;
}

View file

@ -10,6 +10,7 @@ import {
getTimeRangeComparison,
} from '../components/shared/time_comparison/get_time_range_comparison';
import { useUrlParams } from '../context/url_params_context/use_url_params';
import { useApmParams } from './use_apm_params';
import { useTheme } from './use_theme';
import { useTimeRange } from './use_time_range';
@ -17,7 +18,16 @@ export function useComparison() {
const theme = useTheme();
const comparisonChartTheme = getComparisonChartTheme(theme);
const { start, end } = useTimeRange();
const { query } = useApmParams('/*');
if (!('rangeFrom' in query && 'rangeTo' in query)) {
throw new Error('rangeFrom or rangeTo not defined in query');
}
const { start, end } = useTimeRange({
rangeFrom: query.rangeFrom,
rangeTo: query.rangeTo,
});
const {
urlParams: { comparisonType, comparisonEnabled },

View file

@ -0,0 +1,68 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0; you may not use this file except in compliance with the Elastic License
* 2.0.
*/
import {
act,
renderHook,
RenderHookResult,
} from '@testing-library/react-hooks';
import { useTimeRange } from './use_time_range';
describe('useTimeRange', () => {
let hook: RenderHookResult<
Parameters<typeof useTimeRange>[0],
ReturnType<typeof useTimeRange>
>;
beforeEach(() => {
Date.now = jest.fn(() => new Date(Date.UTC(2021, 0, 1, 12)).valueOf());
hook = renderHook(
(props) => {
const { rangeFrom, rangeTo } = props;
return useTimeRange({ rangeFrom, rangeTo });
},
{ initialProps: { rangeFrom: 'now-15m', rangeTo: 'now' } }
);
});
afterEach(() => {});
it('returns the parsed range on first render', () => {
expect(hook.result.current.start).toEqual('2021-01-01T11:45:00.000Z');
expect(hook.result.current.end).toEqual('2021-01-01T12:00:00.000Z');
});
it('only changes the parsed range when rangeFrom/rangeTo change', () => {
Date.now = jest.fn(() => new Date(Date.UTC(2021, 0, 1, 13)).valueOf());
hook.rerender({ rangeFrom: 'now-15m', rangeTo: 'now' });
expect(hook.result.current.start).toEqual('2021-01-01T11:45:00.000Z');
expect(hook.result.current.end).toEqual('2021-01-01T12:00:00.000Z');
hook.rerender({ rangeFrom: 'now-30m', rangeTo: 'now' });
expect(hook.result.current.start).toEqual('2021-01-01T12:30:00.000Z');
expect(hook.result.current.end).toEqual('2021-01-01T13:00:00.000Z');
});
it('updates when refreshTimeRange is called', async () => {
Date.now = jest.fn(() => new Date(Date.UTC(2021, 0, 1, 13)).valueOf());
hook.rerender({ rangeFrom: 'now-15m', rangeTo: 'now' });
expect(hook.result.current.start).toEqual('2021-01-01T11:45:00.000Z');
expect(hook.result.current.end).toEqual('2021-01-01T12:00:00.000Z');
act(() => {
hook.result.current.refreshTimeRange();
});
expect(hook.result.current.start).toEqual('2021-01-01T12:45:00.000Z');
expect(hook.result.current.end).toEqual('2021-01-01T13:00:00.000Z');
});
});

View file

@ -5,19 +5,46 @@
* 2.0.
*/
import { useUrlParams } from '../context/url_params_context/use_url_params';
import { isEqual } from 'lodash';
import { useCallback, useRef, useState } from 'react';
import { getDateRange } from '../context/url_params_context/helpers';
export function useTimeRange() {
const {
urlParams: { start, end },
} = useUrlParams();
export function useTimeRange({
rangeFrom,
rangeTo,
}: {
rangeFrom: string;
rangeTo: string;
}) {
const rangeRef = useRef({ rangeFrom, rangeTo });
const [timeRangeId, setTimeRangeId] = useState(0);
const stateRef = useRef(getDateRange({ state: {}, rangeFrom, rangeTo }));
const updateParsedTime = useCallback(() => {
stateRef.current = getDateRange({ state: {}, rangeFrom, rangeTo });
}, [rangeFrom, rangeTo]);
if (!isEqual(rangeRef.current, { rangeFrom, rangeTo })) {
updateParsedTime();
}
const { start, end } = stateRef.current;
const refreshTimeRange = useCallback(() => {
updateParsedTime();
setTimeRangeId((id) => id + 1);
}, [setTimeRangeId, updateParsedTime]);
if (!start || !end) {
throw new Error('Time range not set');
throw new Error('start and/or end were unexpectedly not set');
}
return {
start,
end,
refreshTimeRange,
timeRangeId,
};
}