Skip to content

Commit

Permalink
Move remaining uses of serviceName away from urlParams (#77248) (#77428)
Browse files Browse the repository at this point in the history
* Move remaining uses of serviceName away from urlParams

There were a few of these that were either missed or lost in merge conflict resolution.

I went through everything that's used as a path parameter and made sure it wasn't being used anywhere with `urlParams`.

Previously none of the charts were working, now they all are.

Looks like #67791 introduced a find/replace change that broke APM's e2e tests. This reverts that change.
  • Loading branch information
smith authored Sep 15, 2020
1 parent e88a4cd commit c758bea
Show file tree
Hide file tree
Showing 14 changed files with 44 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const DEFAULT_TIMEOUT = 60 * 1000;

Given(`a user browses the APM UI application`, () => {
// open service overview page
loginAndWaitForPage(`/app/apm#/services`, {
loginAndWaitForPage(`/app/apm/services`, {
from: '2020-06-01T14:59:32.686Z',
to: '2020-06-16T16:59:36.219Z',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ Given(`a user browses the APM UI application for RUM Data`, () => {
// open service overview page
const RANGE_FROM = 'now-24h';
const RANGE_TO = 'now';
loginAndWaitForPage(`/app/apm#/rum-preview`, {
loginAndWaitForPage(`/app/apm/rum-preview`, {
from: RANGE_FROM,
to: RANGE_TO,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,11 @@ const CoreVitalsThresholds = {
export function CoreVitals() {
const { urlParams, uiFilters } = useUrlParams();

const { start, end, serviceName } = urlParams;
const { start, end } = urlParams;

const { data, status } = useFetcher(
(callApmApi) => {
const { serviceName } = uiFilters;
if (start && end && serviceName) {
return callApmApi({
pathname: '/api/apm/rum-client/web-core-vitals',
Expand All @@ -34,7 +35,7 @@ export function CoreVitals() {
}
return Promise.resolve(null);
},
[start, end, serviceName, uiFilters]
[start, end, uiFilters]
);

const { lcp, lcpRanks, fid, fidRanks, cls, clsRanks } = data || {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ export function Buttons({
}: ButtonsProps) {
const { core } = useApmPluginContext();
const { basePath } = core.http;
// The params may contain the service name. We want to use the selected node's
// service name in the button URLs, so make a copy and set the
// `serviceName` property.
const urlParams = { ...useUrlParams().urlParams } as APMQueryParams;
urlParams.serviceName = selectedNodeServiceName;
const urlParams = useUrlParams().urlParams as APMQueryParams;

const detailsUrl = getAPMHref({
basePath,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,9 @@
import { EuiSuperDatePicker } from '@elastic/eui';
import { isEmpty, isEqual, pickBy } from 'lodash';
import React from 'react';
import { useHistory } from 'react-router-dom';
import { useHistory, useLocation } from 'react-router-dom';
import { UI_SETTINGS } from '../../../../../../../src/plugins/data/common';
import { useApmPluginContext } from '../../../hooks/useApmPluginContext';
import { useLocation } from '../../../hooks/useLocation';
import { useUrlParams } from '../../../hooks/useUrlParams';
import { clearCache } from '../../../services/rest/callApi';
import { fromQuery, toQuery } from '../Links/url_helpers';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,11 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/
import { EuiTitle } from '@elastic/eui';
import { EuiPanel, EuiSpacer, EuiTitle } from '@elastic/eui';
import theme from '@elastic/eui/dist/eui_theme_light.json';
import { i18n } from '@kbn/i18n';
import React, { useCallback } from 'react';
import { EuiPanel } from '@elastic/eui';
import { EuiSpacer } from '@elastic/eui';
import { useParams } from 'react-router-dom';
import { asPercent } from '../../../../../common/utils/formatters';
import { useChartsSync } from '../../../../hooks/useChartsSync';
import { useFetcher } from '../../../../hooks/useFetcher';
Expand All @@ -22,16 +21,11 @@ const tickFormatY = (y?: number) => {
};

export function ErroneousTransactionsRateChart() {
const { serviceName } = useParams<{ serviceName?: string }>();
const { urlParams, uiFilters } = useUrlParams();
const syncedChartsProps = useChartsSync();

const {
serviceName,
start,
end,
transactionType,
transactionName,
} = urlParams;
const { start, end, transactionType, transactionName } = urlParams;

const { data } = useFetcher(() => {
if (serviceName && start && end) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@ import { renderHook } from '@testing-library/react-hooks';
import theme from '@elastic/eui/dist/eui_theme_light.json';
import * as useFetcherModule from './useFetcher';
import { useAvgDurationByBrowser } from './useAvgDurationByBrowser';
import React, { ReactNode } from 'react';
import { MemoryRouter } from 'react-router-dom';

function Wrapper({ children }: { children?: ReactNode }) {
return <MemoryRouter>{children}</MemoryRouter>;
}

describe('useAvgDurationByBrowser', () => {
it('returns data', () => {
Expand All @@ -19,7 +25,9 @@ describe('useAvgDurationByBrowser', () => {
refetch: () => {},
status: 'success' as useFetcherModule.FETCH_STATUS,
});
const { result } = renderHook(() => useAvgDurationByBrowser());
const { result } = renderHook(() => useAvgDurationByBrowser(), {
wrapper: Wrapper,
});

expect(result.current.data).toEqual([
{
Expand Down
10 changes: 6 additions & 4 deletions x-pack/plugins/apm/public/hooks/useAvgDurationByBrowser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,13 @@
*/

import theme from '@elastic/eui/dist/eui_theme_light.json';
import { useFetcher } from './useFetcher';
import { useUrlParams } from './useUrlParams';
import { useParams } from 'react-router-dom';
import { getVizColorForIndex } from '../../common/viz_colors';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { AvgDurationByBrowserAPIResponse } from '../../server/lib/transactions/avg_duration_by_browser';
import { TimeSeries } from '../../typings/timeseries';
import { getVizColorForIndex } from '../../common/viz_colors';
import { useFetcher } from './useFetcher';
import { useUrlParams } from './useUrlParams';

function toTimeSeries(data?: AvgDurationByBrowserAPIResponse): TimeSeries[] {
if (!data) {
Expand All @@ -27,8 +28,9 @@ function toTimeSeries(data?: AvgDurationByBrowserAPIResponse): TimeSeries[] {
}

export function useAvgDurationByBrowser() {
const { serviceName } = useParams<{ serviceName?: string }>();
const {
urlParams: { serviceName, start, end, transactionName },
urlParams: { start, end, transactionName },
uiFilters,
} = useUrlParams();

Expand Down
5 changes: 3 additions & 2 deletions x-pack/plugins/apm/public/hooks/useAvgDurationByCountry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
*/

import { useParams } from 'react-router-dom';
import { useFetcher } from './useFetcher';
import { useUrlParams } from './useUrlParams';

export function useAvgDurationByCountry() {
const { serviceName } = useParams<{ serviceName?: string }>();
const {
urlParams: { serviceName, start, end, transactionName },
urlParams: { start, end, transactionName },
uiFilters,
} = useUrlParams();

Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/apm/public/hooks/useTransactionBreakdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { useParams } from 'react-router-dom';
import { useFetcher } from './useFetcher';
import { useUrlParams } from './useUrlParams';

export function useTransactionBreakdown() {
const { serviceName } = useParams<{ serviceName?: string }>();
const {
urlParams: { serviceName, start, end, transactionName, transactionType },
urlParams: { start, end, transactionName, transactionType },
uiFilters,
} = useUrlParams();

Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/apm/public/hooks/useTransactionCharts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@
*/

import { useMemo } from 'react';
import { useParams } from 'react-router-dom';
import { getTransactionCharts } from '../selectors/chartSelectors';
import { useFetcher } from './useFetcher';
import { useUrlParams } from './useUrlParams';

export function useTransactionCharts() {
const { serviceName } = useParams<{ serviceName?: string }>();
const {
urlParams: { serviceName, transactionType, start, end, transactionName },
urlParams: { transactionType, start, end, transactionName },
uiFilters,
} = useUrlParams();

Expand Down
9 changes: 5 additions & 4 deletions x-pack/plugins/apm/public/hooks/useTransactionDistribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
* you may not use this file except in compliance with the Elastic License.
*/

import { IUrlParams } from '../context/UrlParamsContext/types';
import { useFetcher } from './useFetcher';
import { useUiFilters } from '../context/UrlParamsContext';
import { useParams } from 'react-router-dom';
// eslint-disable-next-line @kbn/eslint/no-restricted-paths
import { TransactionDistributionAPIResponse } from '../../server/lib/transactions/distribution';
import { useUiFilters } from '../context/UrlParamsContext';
import { IUrlParams } from '../context/UrlParamsContext/types';
import { useFetcher } from './useFetcher';

const INITIAL_DATA = {
buckets: [] as TransactionDistributionAPIResponse['buckets'],
Expand All @@ -17,8 +18,8 @@ const INITIAL_DATA = {
};

export function useTransactionDistribution(urlParams: IUrlParams) {
const { serviceName } = useParams<{ serviceName?: string }>();
const {
serviceName,
start,
end,
transactionType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ describe('Observability dashboard data', () => {
);
const response = await fetchOverviewPageData(params);
expect(response).toEqual({
appLink: '/app/apm#/services?rangeFrom=now-15m&rangeTo=now',
appLink: '/app/apm/services?rangeFrom=now-15m&rangeTo=now',
stats: {
services: {
type: 'number',
Expand Down Expand Up @@ -82,7 +82,7 @@ describe('Observability dashboard data', () => {
);
const response = await fetchOverviewPageData(params);
expect(response).toEqual({
appLink: '/app/apm#/services?rangeFrom=now-15m&rangeTo=now',
appLink: '/app/apm/services?rangeFrom=now-15m&rangeTo=now',
stats: {
services: {
type: 'number',
Expand All @@ -109,7 +109,7 @@ describe('Observability dashboard data', () => {
);
const response = await fetchOverviewPageData(params);
expect(response).toEqual({
appLink: '/app/apm#/services?rangeFrom=now-15m&rangeTo=now',
appLink: '/app/apm/services?rangeFrom=now-15m&rangeTo=now',
stats: {
services: {
type: 'number',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const fetchOverviewPageData = async ({
const { serviceCount, transactionCoordinates } = data;

return {
appLink: `/app/apm#/services?rangeFrom=${relativeTime.start}&rangeTo=${relativeTime.end}`,
appLink: `/app/apm/services?rangeFrom=${relativeTime.start}&rangeTo=${relativeTime.end}`,
stats: {
services: {
type: 'number',
Expand Down

0 comments on commit c758bea

Please sign in to comment.