Skip to content

Commit

Permalink
[Infra] Refactor useSearchSession to useReloadRequestTime for timesta…
Browse files Browse the repository at this point in the history
…mp based refresh (elastic#205338)

## Summary

Refactoring `useSearchSession` to `useReloadRequestTime` to decouple the
usage of search.session for refreshing, avoiding a re-rendering loop
issue. This change is then propagated to various Lens charts to make use
of that instead of relying on searchSessionId.

Closes elastic#203412

## How to test

- Go to Infrastructure Inventory page in Observability
- Enable Auto-Refresh
- Click on a infra item (host/service/etc) to open the fly-out
- Update the time range in the flyout
- Things should update without spurious refreshes.

---------

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit fe988fa)
  • Loading branch information
Bluefinger committed Jan 3, 2025
1 parent 98d935d commit 8c788b9
Show file tree
Hide file tree
Showing 22 changed files with 143 additions and 147 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { buildCombinedAssetFilter } from '../../../utils/filters/build';
import { type BrushEndArgs, LensChart, type OnFilterEvent, LensChartProps } from '../../lens';
import { useDatePickerContext } from '../hooks/use_date_picker';
import { extractRangeFromChartFilterEvent } from './chart_utils';
import { useSearchSessionContext } from '../../../hooks/use_search_session';
import { useReloadRequestTimeContext } from '../../../hooks/use_reload_request_time';

export type ChartProps = Pick<LensChartProps, 'overrides'> & {
id: string;
Expand All @@ -34,7 +34,7 @@ export const Chart = ({
lensAttributes,
}: ChartProps) => {
const { setDateRange } = useDatePickerContext();
const { searchSessionId } = useSearchSessionContext();
const { reloadRequestTime } = useReloadRequestTimeContext();
const {
services: { dataViews },
} = useKibanaContextForPlugin();
Expand Down Expand Up @@ -86,7 +86,7 @@ export const Chart = ({
borderRadius="m"
dateRange={dateRange}
height={METRIC_CHART_HEIGHT}
searchSessionId={searchSessionId}
lastReloadRequestTime={reloadRequestTime}
filters={filters}
lensAttributes={lensAttributes}
overrides={overrides}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export interface ContainerKpiChartsProps {
dateRange: TimeRange;
query?: Query;
filters?: Filter[];
searchSessionId?: string;
lastReloadRequestTime?: number;
options?: {
getSubtitle?: (formulaValue: string) => string;
};
Expand All @@ -34,7 +34,7 @@ export const ContainerKpiCharts = ({
dataView,
filters,
query,
searchSessionId,
lastReloadRequestTime,
loading = false,
}: ContainerKpiChartsProps) => {
const isDockerContainer = useIntegrationCheck({ dependsOn: INTEGRATIONS.docker });
Expand All @@ -53,7 +53,7 @@ export const ContainerKpiCharts = ({
dataView={dataView}
filters={filters}
query={query}
searchSessionId={searchSessionId}
lastReloadRequestTime={lastReloadRequestTime}
loading={loading}
/>
)}
Expand All @@ -63,7 +63,7 @@ export const ContainerKpiCharts = ({
dataView={dataView}
filters={filters}
query={query}
searchSessionId={searchSessionId}
lastReloadRequestTime={lastReloadRequestTime}
loading={loading}
/>
)}
Expand All @@ -76,7 +76,7 @@ const DockerKpiCharts = ({
dataView,
filters,
query,
searchSessionId,
lastReloadRequestTime,
loading = false,
}: ContainerKpiChartsProps) => {
const charts = useDockerContainerKpiCharts({
Expand All @@ -92,7 +92,7 @@ const DockerKpiCharts = ({
dateRange={dateRange}
filters={filters}
query={query}
searchSessionId={searchSessionId}
lastReloadRequestTime={lastReloadRequestTime}
loading={loading}
/>
</EuiFlexItem>
Expand All @@ -107,7 +107,7 @@ const KubernetesKpiCharts = ({
filters,
options,
query,
searchSessionId,
lastReloadRequestTime,
loading = false,
}: ContainerKpiChartsProps) => {
const charts = useK8sContainerKpiCharts({
Expand All @@ -123,7 +123,7 @@ const KubernetesKpiCharts = ({
dateRange={dateRange}
filters={filters}
query={query}
searchSessionId={searchSessionId}
lastReloadRequestTime={lastReloadRequestTime}
loading={loading}
/>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ export interface HostKpiChartsProps {
dateRange: TimeRange;
query?: Query;
filters?: Filter[];
searchSessionId?: string;
lastReloadRequestTime?: number;
getSubtitle?: (formulaValue: string) => string;
loading?: boolean;
}
Expand All @@ -28,7 +28,7 @@ export const HostKpiCharts = ({
filters,
getSubtitle,
query,
searchSessionId,
lastReloadRequestTime,
loading = false,
}: HostKpiChartsProps) => {
const charts = useHostKpiCharts({
Expand All @@ -45,7 +45,7 @@ export const HostKpiCharts = ({
dateRange={dateRange}
filters={filters}
query={query}
searchSessionId={searchSessionId}
lastReloadRequestTime={lastReloadRequestTime}
loading={loading}
/>
</EuiFlexItem>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export const Kpi = ({
dateRange,
query,
filters,
searchSessionId,
lastReloadRequestTime,
loading,
...chartProps
}: LensConfig & {
Expand All @@ -25,6 +25,7 @@ export const Kpi = ({
filters?: Filter[];
searchSessionId?: string;
loading?: boolean;
lastReloadRequestTime?: number;
}) => {
const tooltipContent = useMemo(
() =>
Expand All @@ -44,7 +45,7 @@ export const Kpi = ({
query={query}
loading={loading}
toolTip={tooltipContent}
searchSessionId={searchSessionId}
lastReloadRequestTime={lastReloadRequestTime}
disableTriggers
hidePanelTitles
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import React from 'react';
import { SearchSessionProvider } from '../../hooks/use_search_session';
import { ReloadRequestTimeProvider } from '../../hooks/use_reload_request_time';
import { AssetDetailsRenderPropsProvider } from './hooks/use_asset_details_render_props';
import { DatePickerProvider } from './hooks/use_date_picker';
import { LoadingStateProvider } from './hooks/use_loading_state';
Expand All @@ -22,7 +22,7 @@ const RenderWithOptionalSearchSessionProvider = ({
}) => {
if (renderMode.mode === 'flyout') {
// flyout mode requires its own search session so that it doesn't interfere with the main page
return <SearchSessionProvider>{children}</SearchSessionProvider>;
return <ReloadRequestTimeProvider>{children}</ReloadRequestTimeProvider>;
}
return <>{children}</>;
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import createContainer from 'constate';
import { useCallback, useMemo, useState } from 'react';
import useEffectOnce from 'react-use/lib/useEffectOnce';
import { BehaviorSubject } from 'rxjs';
import { useSearchSessionContext } from '../../../hooks/use_search_session';
import { useReloadRequestTimeContext } from '../../../hooks/use_reload_request_time';
import { parseDateRange } from '../../../utils/datemath';
import { AssetDetailsProps } from '../types';
import { getDefaultDateRange, toTimestampRange } from '../utils';
Expand All @@ -22,7 +22,7 @@ export function useDatePicker({
dateRange = getDefaultDateRange(),
autoRefresh,
}: UseDateRangeProviderProps) {
const { updateSearchSessionId } = useSearchSessionContext();
const { updateReloadRequestTime } = useReloadRequestTimeContext();
const autoRefreshTick$ = useMemo(() => new BehaviorSubject(null), []);
const autoRefreshConfig$ = useMemo(
() => new BehaviorSubject<UseDateRangeProviderProps['autoRefresh'] | undefined>(undefined),
Expand Down Expand Up @@ -52,9 +52,9 @@ export function useDatePicker({
(newDateRange: TimeRange) => {
setUrlState({ dateRange: newDateRange });
setParsedDateRange(parseDateRange(newDateRange));
updateSearchSessionId();
updateReloadRequestTime();
},
[setUrlState, updateSearchSessionId]
[setUrlState, updateReloadRequestTime]
);

const onRefresh = useCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@ import { BehaviorSubject, EMPTY, of, Subject, Subscription, skip } from 'rxjs';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import { coreMock } from '@kbn/core/public/mocks';
import { SearchSessionState, waitUntilNextSessionCompletes$ } from '@kbn/data-plugin/public';
import { useSearchSessionContext } from '../../../hooks/use_search_session';
import { useReloadRequestTimeContext } from '../../../hooks/use_reload_request_time';
import { dataPluginMock } from '@kbn/data-plugin/public/mocks';

jest.mock('./use_date_picker');
jest.mock('../../../hooks/use_kibana');
jest.mock('../../../hooks/use_search_session');
jest.mock('../../../hooks/use_reload_request_time');

jest.mock('@kbn/data-plugin/public', () => ({
...jest.requireActual('@kbn/data-plugin/public'),
Expand All @@ -36,8 +36,8 @@ const waitUntilNextSessionCompletesMock$ = waitUntilNextSessionCompletes$ as jes
typeof waitUntilNextSessionCompletes$
>;

const useSearchSessionContextMock = useSearchSessionContext as jest.MockedFunction<
typeof useSearchSessionContext
const useRequestTimeContextMock = useReloadRequestTimeContext as jest.MockedFunction<
typeof useReloadRequestTimeContext
>;

describe('useLoadingState', () => {
Expand All @@ -50,12 +50,12 @@ describe('useLoadingState', () => {

const sessionState$ = new BehaviorSubject<SearchSessionState>(SearchSessionState.None);

const updateSearchSessionIdMock = jest.fn();
const updateReloadRequestTimeMock = jest.fn();

const mockSearchSessionContext = () => {
useSearchSessionContextMock.mockReturnValue({
updateSearchSessionId: updateSearchSessionIdMock,
searchSessionId: '',
const mockRequestTimeContext = () => {
useRequestTimeContextMock.mockReturnValue({
updateReloadRequestTime: updateReloadRequestTimeMock,
reloadRequestTime: 0,
});
};

Expand Down Expand Up @@ -89,7 +89,7 @@ describe('useLoadingState', () => {
subscription = new Subscription();
jest.useFakeTimers();
waitUntilNextSessionCompletesMock$.mockReturnValue(of(SearchSessionState.None));
mockSearchSessionContext();
mockRequestTimeContext();
mockUseKibana();
mockDatePickerContext();
});
Expand Down Expand Up @@ -152,7 +152,7 @@ describe('useLoadingState', () => {
unmount();
});

it('should not call updateSearchSessionId if waitUntilNextSessionCompletesMock$ returns empty', async () => {
it('should not call updateRequestTime if waitUntilNextSessionCompletesMock$ returns empty', async () => {
const { unmount } = renderHook(() => useLoadingState());

// waitUntilNextSessionCompletes$ returns EMPTY when the status is loading or none
Expand All @@ -165,12 +165,12 @@ describe('useLoadingState', () => {
});

// only the mount call must happen
await waitFor(() => expect(updateSearchSessionIdMock).toHaveBeenCalledTimes(1));
await waitFor(() => expect(updateReloadRequestTimeMock).toHaveBeenCalledTimes(1));

unmount();
});

it('should call updateSearchSessionId when waitUntilNextSessionCompletesMock$ returns', async () => {
it('should call updateRequestTime when waitUntilNextSessionCompletesMock$ returns', async () => {
const { unmount } = renderHook(() => useLoadingState());

// waitUntilNextSessionCompletes$ returns something when the status is Completed or BackgroundCompleted
Expand All @@ -182,7 +182,7 @@ describe('useLoadingState', () => {
jest.runOnlyPendingTimers();
});

await waitFor(() => expect(updateSearchSessionIdMock).toHaveBeenCalledTimes(2));
await waitFor(() => expect(updateReloadRequestTimeMock).toHaveBeenCalledTimes(2));

unmount();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { useCallback, useEffect, useMemo } from 'react';
import { SearchSessionState, waitUntilNextSessionCompletes$ } from '@kbn/data-plugin/public';
import { useKibanaContextForPlugin } from '../../../hooks/use_kibana';
import { useDatePickerContext } from './use_date_picker';
import { useSearchSessionContext } from '../../../hooks/use_search_session';
import { useReloadRequestTimeContext } from '../../../hooks/use_reload_request_time';

export type RequestState = 'running' | 'done' | 'error';
const WAIT_MS = 1000;
Expand All @@ -37,15 +37,15 @@ export const useLoadingState = () => {
const {
data: { search },
} = services;
const { updateSearchSessionId } = useSearchSessionContext();
const { updateReloadRequestTime } = useReloadRequestTimeContext();

const isAutoRefreshRequestPending$ = useMemo(() => new BehaviorSubject<boolean>(false), []);
const requestsCount$ = useMemo(() => new BehaviorSubject(0), []);
const requestState$ = useMemo(() => new BehaviorSubject<RequestState | null>(null), []);

useEffect(() => {
updateSearchSessionId();
}, [updateSearchSessionId]);
updateReloadRequestTime();
}, [updateReloadRequestTime]);

const waitUntilRequestsCompletes$ = useCallback(
() =>
Expand Down Expand Up @@ -140,7 +140,7 @@ export const useLoadingState = () => {
// This will only be called when Lens is used in the Asset Details page
return waitUntilNextSessionCompletes$(search.session).pipe(
tap(() => {
updateSearchSessionId();
updateReloadRequestTime();
})
);
}
Expand All @@ -149,7 +149,7 @@ export const useLoadingState = () => {
return of(null).pipe(
tap(() => {
if (!isAutoRefreshRequestPending) {
updateSearchSessionId();
updateReloadRequestTime();
}
})
);
Expand All @@ -169,7 +169,7 @@ export const useLoadingState = () => {
requestState$,
requestsCount$,
search.session,
updateSearchSessionId,
updateReloadRequestTime,
waitUntilRequestsCompletes$,
]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import { act, waitFor, renderHook } from '@testing-library/react';
import { useRequestObservable } from './use_request_observable';
import { type RequestState, useLoadingStateContext } from './use_loading_state';
import { useDatePickerContext, type UseDateRangeProviderProps } from './use_date_picker';
import { useSearchSessionContext } from '../../../hooks/use_search_session';
import { useReloadRequestTimeContext } from '../../../hooks/use_reload_request_time';
import { BehaviorSubject } from 'rxjs';

jest.mock('./use_loading_state');
jest.mock('./use_date_picker');
jest.mock('../../../hooks/use_search_session');
jest.mock('../../../hooks/use_reload_request_time');

const useLoadingStateContextMock = useLoadingStateContext as jest.MockedFunction<
typeof useLoadingStateContext
Expand All @@ -23,8 +23,8 @@ const useDatePickerContextMock = useDatePickerContext as jest.MockedFunction<
typeof useDatePickerContext
>;

const useSearchSessionMock = useSearchSessionContext as jest.MockedFunction<
typeof useSearchSessionContext
const useReloadRequestTimeMock = useReloadRequestTimeContext as jest.MockedFunction<
typeof useReloadRequestTimeContext
>;

describe('useRequestObservable', () => {
Expand All @@ -38,10 +38,10 @@ describe('useRequestObservable', () => {
// needed to spy on `next` function
requestStateMock$.next = jest.fn();

const mockUseSearchSessionMock = () => {
useSearchSessionMock.mockReturnValue({
updateSearchSessionId: jest.fn(() => {}),
searchSessionId: '',
const mockUseRequestTimeMock = () => {
useReloadRequestTimeMock.mockReturnValue({
updateReloadRequestTime: jest.fn(() => {}),
reloadRequestTime: 0,
});
};

Expand All @@ -59,12 +59,14 @@ describe('useRequestObservable', () => {
};

beforeEach(() => {
jest.useFakeTimers();
mockDatePickerContext();
mockUseSearchSessionMock();
mockUseRequestTimeMock();
mockUseLoadingStateContextMock();
});

afterEach(() => {
jest.useRealTimers();
jest.clearAllMocks();
});

Expand Down
Loading

0 comments on commit 8c788b9

Please sign in to comment.