[8.6] [Synthetics] Fix infinite loading of Monitor Details page (#146336) (#146383)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Synthetics] Fix infinite loading of Monitor Details page
(#146336)](https://github.com/elastic/kibana/pull/146336)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"Abdul Wahab
Zahid","email":"awahab07@yahoo.com"},"sourceCommit":{"committedDate":"2022-11-28T10:43:20Z","message":"[Synthetics]
Fix infinite loading of Monitor Details page (#146336)\n\nWith the
`SyntheticsRefreshContext` now refreshing itself periodically,\r\nhooks
which are called from multiple child components in parallel and\r\nwhich
load data when initialized, may cause multiple redundant, or based\r\non
the flow, unending back to back calls to server. This is because
a\r\nhook called multiple times in parallel, will dispatch the same
action\r\nthe number of times it is initialized.\r\n\r\nThe PR solves
one such case on Monitor details page, which was loading\r\nitself over
and over.\r\n\r\nThe PR also fixes the issue where **Last test run**
panel on Monitor\r\nDetails page stays out of date when data is auto
refreshed.","sha":"da5da4a31054a54a34cc1f6908ae934762238083","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","Team:uptime","release_note:skip","ci:build-cloud-image","v8.6.0","v8.7.0"],"number":146336,"url":"https://github.com/elastic/kibana/pull/146336","mergeCommit":{"message":"[Synthetics]
Fix infinite loading of Monitor Details page (#146336)\n\nWith the
`SyntheticsRefreshContext` now refreshing itself periodically,\r\nhooks
which are called from multiple child components in parallel and\r\nwhich
load data when initialized, may cause multiple redundant, or based\r\non
the flow, unending back to back calls to server. This is because
a\r\nhook called multiple times in parallel, will dispatch the same
action\r\nthe number of times it is initialized.\r\n\r\nThe PR solves
one such case on Monitor details page, which was loading\r\nitself over
and over.\r\n\r\nThe PR also fixes the issue where **Last test run**
panel on Monitor\r\nDetails page stays out of date when data is auto
refreshed.","sha":"da5da4a31054a54a34cc1f6908ae934762238083"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/146336","number":146336,"mergeCommit":{"message":"[Synthetics]
Fix infinite loading of Monitor Details page (#146336)\n\nWith the
`SyntheticsRefreshContext` now refreshing itself periodically,\r\nhooks
which are called from multiple child components in parallel and\r\nwhich
load data when initialized, may cause multiple redundant, or based\r\non
the flow, unending back to back calls to server. This is because
a\r\nhook called multiple times in parallel, will dispatch the same
action\r\nthe number of times it is initialized.\r\n\r\nThe PR solves
one such case on Monitor details page, which was loading\r\nitself over
and over.\r\n\r\nThe PR also fixes the issue where **Last test run**
panel on Monitor\r\nDetails page stays out of date when data is auto
refreshed.","sha":"da5da4a31054a54a34cc1f6908ae934762238083"}}]}]
BACKPORT-->
This commit is contained in:
Abdul Wahab Zahid 2022-11-29 15:57:23 +01:00 committed by GitHub
parent 6249c39c64
commit d1d2537aac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
13 changed files with 114 additions and 49 deletions

View file

@ -26,8 +26,9 @@ export const useSelectedMonitor = () => {
() => monitorsList.find((monitor) => monitor[ConfigKey.CONFIG_ID] === monitorId) ?? null,
[monitorId, monitorsList]
);
const { lastRefresh } = useSyntheticsRefreshContext();
const { syntheticsMonitor, syntheticsMonitorLoading } = useSelector(selectorMonitorDetailsState);
const { lastRefresh, refreshInterval } = useSyntheticsRefreshContext();
const { syntheticsMonitor, syntheticsMonitorLoading, syntheticsMonitorDispatchedAt } =
useSelector(selectorMonitorDetailsState);
const dispatch = useDispatch();
const isMonitorFromListValid =
@ -47,8 +48,23 @@ export const useSelectedMonitor = () => {
}, [dispatch, monitorId, availableMonitor, syntheticsMonitorLoading]);
useEffect(() => {
dispatch(getMonitorAction.get({ monitorId }));
}, [dispatch, monitorId, lastRefresh]);
// Only perform periodic refresh if the last dispatch was earlier enough
if (
!syntheticsMonitorLoading &&
!monitorListLoading &&
Date.now() - syntheticsMonitorDispatchedAt > refreshInterval
) {
dispatch(getMonitorAction.get({ monitorId }));
}
}, [
dispatch,
lastRefresh,
refreshInterval,
monitorId,
monitorListLoading,
syntheticsMonitorLoading,
syntheticsMonitorDispatchedAt,
]);
return {
monitor: availableMonitor,

View file

@ -6,15 +6,7 @@
*/
import React from 'react';
import {
EuiTitle,
EuiPanel,
EuiFlexGroup,
EuiFlexItem,
EuiText,
EuiSpacer,
EuiLoadingSpinner,
} from '@elastic/eui';
import { EuiTitle, EuiPanel, EuiFlexGroup, EuiFlexItem, EuiText, EuiSpacer } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { LoadWhenInView } from '@kbn/observability-plugin/public';
@ -34,15 +26,11 @@ import { MonitorErrorsCount } from './monitor_errors_count';
import { useAbsoluteDate } from '../../../hooks';
export const MonitorSummary = () => {
const { from: fromRelative, loading } = useEarliestStartDate();
const { from: fromRelative } = useEarliestStartDate();
const toRelative = 'now';
const { from, to } = useAbsoluteDate({ from: fromRelative, to: toRelative });
if (loading) {
return <LoadingState />;
}
const dateLabel = from === 'now-30d/d' ? LAST_30_DAYS_LABEL : TO_DATE_LABEL;
return (
@ -133,16 +121,6 @@ export const MonitorSummary = () => {
);
};
function LoadingState({ height }: { height?: string }) {
return (
<EuiFlexGroup alignItems="center" justifyContent="center" style={{ height: height ?? '100%' }}>
<EuiFlexItem grow={false}>
<EuiLoadingSpinner size="xl" />
</EuiFlexItem>
</EuiFlexGroup>
);
}
const SUMMARY_LABEL = i18n.translate('xpack.synthetics.detailsPanel.summary', {
defaultMessage: 'Summary',
});

View file

@ -51,6 +51,10 @@ describe('useMonitorList', () => {
};
});
afterEach(() => {
jest.useRealTimers();
});
it('returns expected initial state', () => {
const {
result: { current: hookResult },
@ -62,6 +66,8 @@ describe('useMonitorList', () => {
it('dispatches correct action for query url param', async () => {
const query = 'xyz';
const url = `/monitor/1?query=${query}`;
jest.useFakeTimers().setSystemTime(Date.now());
const WrapperWithState = ({ children }: { children: React.ReactElement }) => {
return (
<WrappedHelper url={url} path={MONITOR_ROUTE}>
@ -85,6 +91,8 @@ describe('useMonitorList', () => {
const url = `/monitor/1?tags=${JSON.stringify(tags)}&locations=${JSON.stringify(
locations
)}&monitorType=${JSON.stringify(monitorType)}`;
jest.useFakeTimers().setSystemTime(Date.now());
const WrapperWithState = ({ children }: { children: React.ReactElement }) => {
return (
<WrappedHelper url={url} path={MONITOR_ROUTE}>

View file

@ -9,11 +9,15 @@ import React, { createContext, useCallback, useContext, useEffect, useMemo, useS
interface SyntheticsRefreshContext {
lastRefresh: number;
refreshInterval: number;
refreshApp: () => void;
}
export const APP_DEFAULT_REFRESH_INTERVAL = 1000 * 30;
const defaultContext: SyntheticsRefreshContext = {
lastRefresh: 0,
refreshInterval: APP_DEFAULT_REFRESH_INTERVAL,
refreshApp: () => {
throw new Error('App refresh was not initialized, set it when you invoke the context');
},
@ -30,15 +34,15 @@ export const SyntheticsRefreshContextProvider: React.FC = ({ children }) => {
}, [setLastRefresh]);
const value = useMemo(() => {
return { lastRefresh, refreshApp };
return { lastRefresh, refreshApp, refreshInterval: APP_DEFAULT_REFRESH_INTERVAL };
}, [lastRefresh, refreshApp]);
useEffect(() => {
const interval = setInterval(() => {
refreshApp();
}, 1000 * 30);
}, value.refreshInterval);
return () => clearInterval(interval);
}, [refreshApp]);
}, [refreshApp, value.refreshInterval]);
return <SyntheticsRefreshContext.Provider value={value} children={children} />;
};

View file

@ -10,7 +10,7 @@ import userEvent from '@testing-library/user-event';
import { render } from '../utils/testing';
import React, { useState, Fragment } from 'react';
import { useUrlParams, SyntheticsUrlParamsHook } from './use_url_params';
import { SyntheticsRefreshContext } from '../contexts';
import { APP_DEFAULT_REFRESH_INTERVAL, SyntheticsRefreshContext } from '../contexts';
interface MockUrlParamsComponentProps {
hook: SyntheticsUrlParamsHook;
@ -53,7 +53,13 @@ describe('useUrlParams', () => {
it('accepts router props, updates URL params, and returns the current params', async () => {
const { findByText, history } = render(
<SyntheticsRefreshContext.Provider value={{ lastRefresh: 123, refreshApp: jest.fn() }}>
<SyntheticsRefreshContext.Provider
value={{
lastRefresh: 123,
refreshApp: jest.fn(),
refreshInterval: APP_DEFAULT_REFRESH_INTERVAL,
}}
>
<UseUrlParamsTestComponent hook={useUrlParams} />
</SyntheticsRefreshContext.Provider>
);
@ -71,7 +77,13 @@ describe('useUrlParams', () => {
it('clears search when null is passed to params', async () => {
const { findByText, history } = render(
<SyntheticsRefreshContext.Provider value={{ lastRefresh: 123, refreshApp: jest.fn() }}>
<SyntheticsRefreshContext.Provider
value={{
lastRefresh: 123,
refreshApp: jest.fn(),
refreshInterval: APP_DEFAULT_REFRESH_INTERVAL,
}}
>
<UseUrlParamsTestComponent hook={useUrlParams} updateParams={null} />
</SyntheticsRefreshContext.Provider>
);

View file

@ -11,15 +11,12 @@ import {
PingsResponse,
EncryptedSyntheticsSavedMonitor,
} from '../../../../../common/runtime_types';
import { QueryParams } from './api';
import { createAsyncAction } from '../utils/actions';
export const setMonitorDetailsLocationAction = createAction<string>(
'[MONITOR SUMMARY] SET LOCATION'
);
export const getMonitorStatusAction = createAsyncAction<QueryParams, Ping>('[MONITOR DETAILS] GET');
export const getMonitorAction = createAsyncAction<
{ monitorId: string },
EncryptedSyntheticsSavedMonitor
@ -30,6 +27,10 @@ export const getMonitorLastRunAction = createAsyncAction<
PingsResponse
>('[MONITOR DETAILS] GET LAST RUN');
export const updateMonitorLastRunAction = createAction<{ data: Ping }>(
'[MONITOR DETAILS] UPdATE LAST RUN'
);
export const getMonitorRecentPingsAction = createAsyncAction<
{
monitorId: string;

View file

@ -15,12 +15,6 @@ import {
} from '../../../../../common/runtime_types';
import { API_URLS, SYNTHETICS_API_URLS } from '../../../../../common/constants';
export interface QueryParams {
monitorId: string;
dateStart: string;
dateEnd: string;
}
export const fetchMonitorLastRun = async ({
monitorId,
locationId,

View file

@ -5,10 +5,19 @@
* 2.0.
*/
import { takeLeading } from 'redux-saga/effects';
import { PayloadAction } from '@reduxjs/toolkit';
import { takeLeading, takeEvery, select, put } from 'redux-saga/effects';
import { ConfigKey, Ping, PingsResponse } from '../../../../../common/runtime_types';
import { fetchEffectFactory } from '../utils/fetch_effect';
import { getMonitorLastRunAction, getMonitorRecentPingsAction, getMonitorAction } from './actions';
import {
getMonitorLastRunAction,
getMonitorRecentPingsAction,
getMonitorAction,
updateMonitorLastRunAction,
} from './actions';
import { fetchSyntheticsMonitor, fetchMonitorRecentPings, fetchMonitorLastRun } from './api';
import { selectLastRunMetadata } from './selectors';
export function* fetchSyntheticsMonitorEffect() {
yield takeLeading(
@ -28,6 +37,31 @@ export function* fetchSyntheticsMonitorEffect() {
getMonitorLastRunAction.fail
)
);
// Additional listener on `getMonitorRecentPingsAction.success` to possibly update the `lastRun` as well
yield takeEvery(
getMonitorRecentPingsAction.success,
function* (action: PayloadAction<PingsResponse>): Generator {
// If `lastRun` and pings from `getMonitorRecentPingsAction` are of the same monitor and location AND
// `getMonitorRecentPingsAction` fetched the latest pings than `lastRun`, update `lastRun` as well
const lastRun = yield select(selectLastRunMetadata);
const lastRunPing = (lastRun as { data?: Ping })?.data;
const recentPingFromList = action.payload?.pings[0];
if (
lastRunPing &&
recentPingFromList &&
lastRunPing?.[ConfigKey.CONFIG_ID] &&
recentPingFromList?.[ConfigKey.CONFIG_ID] &&
lastRunPing?.[ConfigKey.CONFIG_ID] === recentPingFromList?.[ConfigKey.CONFIG_ID] &&
lastRunPing?.observer?.geo?.name === recentPingFromList?.observer?.geo?.name &&
new Date(lastRunPing?.timestamp) < new Date(recentPingFromList?.timestamp)
) {
yield put(updateMonitorLastRunAction({ data: recentPingFromList }));
}
}
);
yield takeLeading(
getMonitorAction.get,
fetchEffectFactory(fetchSyntheticsMonitor, getMonitorAction.success, getMonitorAction.fail)

View file

@ -13,6 +13,7 @@ import { IHttpSerializedFetchError } from '../utils/http_error';
import {
getMonitorLastRunAction,
updateMonitorLastRunAction,
getMonitorRecentPingsAction,
setMonitorDetailsLocationAction,
getMonitorAction,
@ -30,6 +31,7 @@ export interface MonitorDetailsState {
};
syntheticsMonitorLoading: boolean;
syntheticsMonitor: EncryptedSyntheticsSavedMonitor | null;
syntheticsMonitorDispatchedAt: number;
error: IHttpSerializedFetchError | null;
selectedLocationId: string | null;
}
@ -39,6 +41,7 @@ const initialState: MonitorDetailsState = {
lastRun: { loading: false },
syntheticsMonitor: null,
syntheticsMonitorLoading: false,
syntheticsMonitorDispatchedAt: 0,
error: null,
selectedLocationId: null,
};
@ -62,6 +65,9 @@ export const monitorDetailsReducer = createReducer(initialState, (builder) => {
state.lastRun.loading = false;
state.error = action.payload;
})
.addCase(updateMonitorLastRunAction, (state, action) => {
state.lastRun.data = action.payload.data;
})
.addCase(getMonitorRecentPingsAction.get, (state, action) => {
state.pings.loading = true;
state.pings.data = state.pings.data.filter(
@ -78,7 +84,8 @@ export const monitorDetailsReducer = createReducer(initialState, (builder) => {
state.pings.loading = false;
})
.addCase(getMonitorAction.get, (state) => {
.addCase(getMonitorAction.get, (state, action) => {
state.syntheticsMonitorDispatchedAt = action.meta.dispatchedAt;
state.syntheticsMonitorLoading = true;
})
.addCase(getMonitorAction.success, (state, action) => {

View file

@ -34,6 +34,6 @@ export function* fetchOverviewStatusEffect() {
fetchOverviewStatus,
fetchOverviewStatusAction.success,
fetchOverviewStatusAction.fail
)
) as ReturnType<typeof fetchEffectFactory>
);
}

View file

@ -18,6 +18,6 @@ export function* fetchPingStatusesEffect() {
fetchMonitorPingStatuses,
getMonitorPingStatusesAction.success,
getMonitorPingStatusesAction.fail
)
) as ReturnType<typeof fetchEffectFactory>
);
}

View file

@ -10,10 +10,20 @@ import type { IHttpSerializedFetchError } from './http_error';
export function createAsyncAction<Payload, SuccessPayload>(actionStr: string) {
return {
get: createAction<Payload>(actionStr),
get: createAction(actionStr, (payload: Payload) => prepareForTimestamp(payload)),
success: createAction<SuccessPayload>(`${actionStr}_SUCCESS`),
fail: createAction<IHttpSerializedFetchError>(`${actionStr}_FAIL`),
};
}
export type Nullable<T> = T | null;
// Action prepare function which adds meta.dispatchedAt timestamp to the action
function prepareForTimestamp<Payload>(payload: Payload) {
return {
payload,
meta: {
dispatchedAt: Date.now(),
},
};
}

View file

@ -419,6 +419,7 @@ function getMonitorDetailsMockSlice() {
created_at: '2022-05-24T13:20:49.322Z',
},
syntheticsMonitorLoading: false,
syntheticsMonitorDispatchedAt: 0,
error: null,
selectedLocationId: 'us_central',
};