Skip to content

Commit

Permalink
[ML] AIOps: Fix rerender issue with relative time ranges like "Last 1…
Browse files Browse the repository at this point in the history
…5 minutes" (elastic#176130)

## Summary

Fixes elastic#176066. Regression introduced in elastic#175289.

Fixes a rerendering issue with relative time ranges like "Last 15
minutes". The memoization didn't work correctly because for the relative
time range we would constantly get updated bounds.

### Checklist

Delete any items that are not applicable to this PR.

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] [Flaky Test
Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was
used on any tests changed
- [x] This was checked for breaking API changes and was [labeled
appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)
  • Loading branch information
walterra authored and fkanout committed Mar 4, 2024
1 parent 4668932 commit 8fe52d2
Show file tree
Hide file tree
Showing 6 changed files with 10 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import type * as estypes from '@elastic/elasticsearch/lib/api/typesWithBodyKey';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
import type { DataView } from '@kbn/data-views-plugin/public';
import type { Dictionary } from '@kbn/ml-url-state';
import {
LOG_RATE_ANALYSIS_TYPE,
type LogRateAnalysisType,
Expand Down Expand Up @@ -62,7 +61,6 @@ export function getDocumentCountStatsSplitLabel(
export interface LogRateAnalysisContentProps {
/** The data view to analyze. */
dataView: DataView;
setGlobalState?: (params: Dictionary<unknown>) => void;
/** Timestamp for the start of the range for initial analysis */
initialAnalysisStart?: number | WindowParameters;
timeRange?: { min: Moment; max: Moment };
Expand All @@ -84,7 +82,6 @@ export interface LogRateAnalysisContentProps {

export const LogRateAnalysisContent: FC<LogRateAnalysisContentProps> = ({
dataView,
setGlobalState,
initialAnalysisStart: incomingInitialAnalysisStart,
timeRange,
esSearchQuery = DEFAULT_SEARCH_QUERY,
Expand Down Expand Up @@ -150,7 +147,7 @@ export const LogRateAnalysisContent: FC<LogRateAnalysisContentProps> = ({
dataView,
'log_rate_analysis',
searchQuery,
setGlobalState,
undefined,
currentSelectedSignificantItem,
currentSelectedGroup,
undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ export interface LogRateAnalysisContentWrapperProps {
stickyHistogram?: boolean;
/** App dependencies */
appDependencies: AiopsAppDependencies;
/** On global timefilter update */
setGlobalState?: any;
/** Timestamp for start of initial analysis */
initialAnalysisStart?: number | WindowParameters;
/** Optional time range */
Expand All @@ -66,7 +64,6 @@ export interface LogRateAnalysisContentWrapperProps {
export const LogRateAnalysisContentWrapper: FC<LogRateAnalysisContentWrapperProps> = ({
dataView,
appDependencies,
setGlobalState,
initialAnalysisStart,
timeRange,
esSearchQuery,
Expand Down Expand Up @@ -100,7 +97,6 @@ export const LogRateAnalysisContentWrapper: FC<LogRateAnalysisContentWrapperProp
<DatePickerContextProvider {...datePickerDeps}>
<LogRateAnalysisContent
dataView={dataView}
setGlobalState={setGlobalState}
initialAnalysisStart={initialAnalysisStart}
timeRange={timeRange}
esSearchQuery={esSearchQuery}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,6 @@ export const LogRateAnalysisPage: FC<Props> = ({ stickyHistogram }) => {
embeddingOrigin={AIOPS_TELEMETRY_ID.AIOPS_DEFAULT_SOURCE}
esSearchQuery={searchQuery}
onWindowParametersChange={onWindowParametersHandler}
setGlobalState={setGlobalState}
stickyHistogram={stickyHistogram}
/>
</EuiFlexGroup>
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/aiops/public/hooks/use_data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const useData = (
const timeRangeMemoized = useMemo(
() => timefilter.getActiveBounds(),
// eslint-disable-next-line react-hooks/exhaustive-deps
[JSON.stringify(timefilter.getActiveBounds())]
[lastRefresh, JSON.stringify(timefilter.getTime())]
);

const fieldStatsRequest: DocumentStatsSearchStrategyParams | undefined = useMemo(() => {
Expand Down
12 changes: 7 additions & 5 deletions x-pack/plugins/aiops/public/hooks/use_document_count_stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,13 +77,13 @@ export function useDocumentCountStats<TParams extends DocumentStatsSearchStrateg

const [documentStatsCache, setDocumentStatsCache] = useState<Record<string, DocumentStats>>({});

const cacheKey = stringHash(
`${JSON.stringify(searchParams)}_${JSON.stringify(searchParamsCompare)}`
);

const fetchDocumentCountData = useCallback(async () => {
if (!searchParams) return;

const cacheKey = stringHash(
`${JSON.stringify(searchParams)}_${JSON.stringify(searchParamsCompare)}`
);

if (documentStatsCache[cacheKey]) {
setDocumentStats(documentStatsCache[cacheKey]);
return;
Expand Down Expand Up @@ -172,7 +172,9 @@ export function useDocumentCountStats<TParams extends DocumentStatsSearchStrateg
displayError(toasts, searchParams!.index, extractErrorProperties(error));
}
}
}, [data?.search, documentStatsCache, searchParams, searchParamsCompare, toasts]);
// custom comparison
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [data?.search, documentStatsCache, cacheKey]);

useEffect(
function getDocumentCountData() {
Expand Down
3 changes: 1 addition & 2 deletions x-pack/test/functional/apps/aiops/log_rate_analysis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,7 @@ export default function ({ getPageObjects, getService }: FtrProviderContext) {
});
}

// FLAKY: https://github.com/elastic/kibana/issues/176066
describe.skip('log rate analysis', async function () {
describe('log rate analysis', async function () {
for (const testData of logRateAnalysisTestData) {
describe(`with '${testData.sourceIndexOrSavedSearch}'`, function () {
before(async () => {
Expand Down

0 comments on commit 8fe52d2

Please sign in to comment.