[APM] Do not rely on rendered items directly to update dependent requests (#216312)

## Summary

This PR fixes some wonky request handling for particular cases. In some
cases, when the page updates with some new criteria/filters, requests
that were being managed by `ManagedTable` would fire at least twice,
with the first one getting aborted and then resent. This PR removes that
behaviour by not storing the dependent data in a `renderedItems` state
and instead depending directly on the request data itself and storing
instead the indices of the rendered items. This removes the edge-case
where `renderedItems` would cause the affected requests from firing
multiple times, due to object equality not being the same for the
rendered items array between renders.

Closes #216144

## How to test

* Go to Observability -> Applications -> Service Inventory
* Select a service with more than one environment
* Go to Errors tab and open the browser dev tools
* Change the environment on the service to update the errors tab

**Expected behaviour**: The `detailed_statistics` request should only
fire once, both on page load and on update (such as changing the service
environment).

This should apply to the Service Inventory page as well, and anything
making use of the `TransactionsTable` component.
This commit is contained in:
Gonçalo Rica Pais da Silva 2025-04-03 20:23:09 +02:00 committed by GitHub
parent 563c7c3388
commit 0cff949bb4
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 101 additions and 47 deletions

View file

@ -20,7 +20,12 @@ import { ChartType, getTimeSeriesColor } from '../../../shared/charts/helper/get
import { SparkPlot } from '../../../shared/charts/spark_plot';
import { ErrorDetailLink } from '../../../shared/links/apm/error_detail_link';
import { ErrorOverviewLink } from '../../../shared/links/apm/error_overview_link';
import type { ITableColumn, TableOptions, TableSearchBar } from '../../../shared/managed_table';
import type {
ITableColumn,
TableOptions,
TableSearchBar,
VisibleItemsStartEnd,
} from '../../../shared/managed_table';
import { ManagedTable } from '../../../shared/managed_table';
import { TimestampTooltip } from '../../../shared/timestamp_tooltip';
import { isTimeComparison } from '../../../shared/time_comparison/get_comparison_options';
@ -84,8 +89,8 @@ export function ErrorGroupList({
const { offset, rangeFrom, rangeTo } = query;
const [renderedItems, setRenderedItems] = useState<ErrorGroupItem[]>([]);
const [sorting, setSorting] = useState<TableOptions<ErrorGroupItem>['sort']>(defaultSorting);
const [renderedItemIndices, setRenderedItemIndices] = useState<VisibleItemsStartEnd>([0, 0]);
const {
setDebouncedSearchQuery,
@ -93,7 +98,7 @@ export function ErrorGroupList({
mainStatisticsStatus,
detailedStatistics,
detailedStatisticsStatus,
} = useErrorGroupListData({ renderedItems, sorting });
} = useErrorGroupListData({ renderedItemIndices, sorting });
const isMainStatsLoading = isPending(mainStatisticsStatus);
const isDetailedStatsLoading = isPending(detailedStatisticsStatus);
@ -323,10 +328,10 @@ export function ErrorGroupList({
initialPageSize={initialPageSize}
isLoading={isMainStatsLoading}
tableSearchBar={tableSearchBar}
onChangeRenderedItems={setRenderedItems}
onChangeSorting={setSorting}
saveTableOptionsToUrl={saveTableOptionsToUrl}
showPerPageOptions={showPerPageOptions}
onChangeItemIndices={setRenderedItemIndices}
/>
);
}

View file

@ -5,13 +5,14 @@
* 2.0.
*/
import { useMemo } from 'react';
import { v4 as uuidv4 } from 'uuid';
import { useFetcher } from '../../../../hooks/use_fetcher';
import { useApmServiceContext } from '../../../../context/apm_service/use_apm_service_context';
import { useTimeRange } from '../../../../hooks/use_time_range';
import { useStateDebounced } from '../../../../hooks/use_debounce';
import type { APIReturnType } from '../../../../services/rest/create_call_apm_api';
import type { TableOptions } from '../../../shared/managed_table';
import type { TableOptions, VisibleItemsStartEnd } from '../../../shared/managed_table';
import { useAnyOfApmParams } from '../../../../hooks/use_apm_params';
import { isTimeComparison } from '../../../shared/time_comparison/get_comparison_options';
@ -34,10 +35,10 @@ const INITIAL_STATE_DETAILED_STATISTICS: DetailedStatistics = {
};
export function useErrorGroupListData({
renderedItems,
renderedItemIndices,
sorting,
}: {
renderedItems: ErrorGroupItem[];
renderedItemIndices: VisibleItemsStartEnd;
sorting: TableOptions<ErrorGroupItem>['sort'];
}) {
const { serviceName } = useApmServiceContext();
@ -80,12 +81,21 @@ export function useErrorGroupListData({
[sorting.direction, sorting.field, start, end, serviceName, environment, kuery, searchQuery]
);
const itemsToFetch = useMemo(
() =>
mainStatistics.errorGroups
.slice(...renderedItemIndices)
.map(({ groupId }) => groupId)
.sort(),
[mainStatistics.errorGroups, renderedItemIndices]
);
const {
data: detailedStatistics = INITIAL_STATE_DETAILED_STATISTICS,
status: detailedStatisticsStatus,
} = useFetcher(
(callApmApi) => {
if (mainStatistics.requestId && renderedItems.length && start && end) {
if (mainStatistics.requestId && itemsToFetch.length && start && end) {
return callApmApi(
'POST /internal/apm/services/{serviceName}/errors/groups/detailed_statistics',
{
@ -100,7 +110,7 @@ export function useErrorGroupListData({
offset: comparisonEnabled && isTimeComparison(offset) ? offset : undefined,
},
body: {
groupIds: JSON.stringify(renderedItems.map(({ groupId }) => groupId).sort()),
groupIds: JSON.stringify(itemsToFetch),
},
},
}
@ -109,7 +119,7 @@ export function useErrorGroupListData({
},
// only fetches agg results when main statistics are ready
// eslint-disable-next-line react-hooks/exhaustive-deps
[mainStatistics.requestId, renderedItems, comparisonEnabled, offset],
[mainStatistics.requestId, itemsToFetch, comparisonEnabled, offset],
{ preservePreviousData: false }
);

View file

@ -23,7 +23,7 @@ import { usePreferredDataSourceAndBucketSize } from '../../../hooks/use_preferre
import { useProgressiveFetcher } from '../../../hooks/use_progressive_fetcher';
import { useTimeRange } from '../../../hooks/use_time_range';
import type { APIReturnType } from '../../../services/rest/create_call_apm_api';
import type { SortFunction } from '../../shared/managed_table';
import type { SortFunction, VisibleItemsStartEnd } from '../../shared/managed_table';
import { MLCallout, shouldDisplayMlCallout } from '../../shared/ml_callout';
import { SearchBar } from '../../shared/search_bar/search_bar';
import { isTimeComparison } from '../../shared/time_comparison/get_comparison_options';
@ -115,10 +115,10 @@ function useServicesMainStatisticsFetcher(searchQuery: string | undefined) {
function useServicesDetailedStatisticsFetcher({
mainStatisticsFetch,
renderedItems,
renderedItemIndices,
}: {
mainStatisticsFetch: ReturnType<typeof useServicesMainStatisticsFetcher>;
renderedItems: ServiceListItem[];
renderedItemIndices: VisibleItemsStartEnd;
}) {
const {
query: { rangeFrom, rangeTo, environment, kuery, offset, comparisonEnabled },
@ -136,14 +136,21 @@ function useServicesDetailedStatisticsFetcher({
const { mainStatisticsData, mainStatisticsStatus } = mainStatisticsFetch;
const itemsToFetch = useMemo(
() =>
mainStatisticsData.items
.slice(...renderedItemIndices)
.map(({ serviceName }) => serviceName)
.sort(),
[mainStatisticsData.items, renderedItemIndices]
);
const comparisonFetch = useProgressiveFetcher(
(callApmApi) => {
const serviceNames = renderedItems.map(({ serviceName }) => serviceName);
if (
start &&
end &&
serviceNames.length > 0 &&
itemsToFetch.length > 0 &&
mainStatisticsStatus === FETCH_STATUS.SUCCESS &&
dataSourceOptions
) {
@ -161,7 +168,7 @@ function useServicesDetailedStatisticsFetcher({
},
body: {
// Service name is sorted to guarantee the same order every time this API is called so the result can be cached.
serviceNames: JSON.stringify(serviceNames.sort()),
serviceNames: JSON.stringify(itemsToFetch),
},
},
});
@ -170,7 +177,7 @@ function useServicesDetailedStatisticsFetcher({
// only fetches detailed statistics when requestId is invalidated by main statistics api call or offset is changed
// eslint-disable-next-line react-hooks/exhaustive-deps
[mainStatisticsData.requestId, renderedItems, offset, comparisonEnabled],
[mainStatisticsData.requestId, itemsToFetch, offset, comparisonEnabled],
{ preservePreviousData: false }
);
@ -180,8 +187,8 @@ function useServicesDetailedStatisticsFetcher({
export function ServiceInventory() {
const [debouncedSearchQuery, setDebouncedSearchQuery] = useStateDebounced('');
const { onPageReady } = usePerformanceContext();
const [renderedItems, setRenderedItems] = useState<ServiceListItem[]>([]);
const mainStatisticsFetch = useServicesMainStatisticsFetcher(debouncedSearchQuery);
const [renderedItemIndices, setRenderedItemIndices] = useState<VisibleItemsStartEnd>([0, 0]);
const { mainStatisticsData, mainStatisticsStatus } = mainStatisticsFetch;
const {
query: { rangeFrom, rangeTo },
@ -205,7 +212,7 @@ export function ServiceInventory() {
const { comparisonFetch } = useServicesDetailedStatisticsFetcher({
mainStatisticsFetch,
renderedItems,
renderedItemIndices,
});
const { anomalyDetectionSetupState } = useAnomalyDetectionJobsContext();
@ -317,8 +324,8 @@ export function ServiceInventory() {
initialPageSize={INITIAL_PAGE_SIZE}
serviceOverflowCount={serviceOverflowCount}
onChangeSearchQuery={setDebouncedSearchQuery}
onChangeItemIndices={setRenderedItemIndices}
maxCountExceeded={mainStatisticsData?.maxCountExceeded ?? false}
onChangeRenderedItems={setRenderedItems}
/>
</EuiFlexItem>
</EuiFlexGroup>

View file

@ -45,7 +45,12 @@ import { ChartType, getTimeSeriesColor } from '../../../shared/charts/helper/get
import { EnvironmentBadge } from '../../../shared/environment_badge';
import { ServiceLink } from '../../../shared/links/apm/service_link';
import { ListMetric } from '../../../shared/list_metric';
import type { ITableColumn, SortFunction, TableSearchBar } from '../../../shared/managed_table';
import type {
ITableColumn,
SortFunction,
TableSearchBar,
VisibleItemsStartEnd,
} from '../../../shared/managed_table';
import { ManagedTable } from '../../../shared/managed_table';
import { ColumnHeaderWithTooltip } from './column_header_with_tooltip';
import { HealthBadge } from './health_badge';
@ -294,8 +299,9 @@ interface Props {
sortFn: SortFunction<ServiceListItem>;
serviceOverflowCount: number;
maxCountExceeded: boolean;
onChangeSearchQuery: (searchQuery: string) => void;
onChangeRenderedItems: (renderedItems: ServiceListItem[]) => void;
onChangeSearchQuery?: (searchQuery: string) => void;
onChangeRenderedItems?: (renderedItems: ServiceListItem[]) => void;
onChangeItemIndices?: (range: VisibleItemsStartEnd) => void;
}
export function ApmServicesTable({
status,
@ -313,6 +319,7 @@ export function ApmServicesTable({
maxCountExceeded,
onChangeSearchQuery,
onChangeRenderedItems,
onChangeItemIndices,
}: Props) {
const breakpoints = useBreakpoints();
const { core } = useApmPluginContext();
@ -423,6 +430,7 @@ export function ApmServicesTable({
initialPageSize={initialPageSize}
sortFn={sortFn}
onChangeRenderedItems={onChangeRenderedItems}
onChangeItemIndices={onChangeItemIndices}
tableSearchBar={tableSearchBar}
/>
</EuiFlexItem>

View file

@ -21,6 +21,12 @@ import {
type SortDirection = 'asc' | 'desc';
/**
* A tuple of the start and end indices for all visible/rendered items
* for the `ManagedTable` component.
*/
export type VisibleItemsStartEnd = readonly [number, number];
export interface TableOptions<T> {
page: { index: number; size: number };
sort: { direction: SortDirection; field: keyof T };
@ -45,7 +51,7 @@ export interface TableSearchBar<T> {
fieldsToSearch: Array<keyof T>;
maxCountExceeded: boolean;
placeholder: string;
onChangeSearchQuery: (searchQuery: string) => void;
onChangeSearchQuery?: (searchQuery: string) => void;
techPreview?: boolean;
}
@ -86,6 +92,7 @@ function UnoptimizedManagedTable<T extends object>(props: {
// onChange handlers
onChangeRenderedItems?: (renderedItems: T[]) => void;
onChangeSorting?: (sorting: TableOptions<T>['sort']) => void;
onChangeItemIndices?: (range: VisibleItemsStartEnd) => void;
// sorting
sortItems?: boolean;
@ -115,8 +122,9 @@ function UnoptimizedManagedTable<T extends object>(props: {
showPerPageOptions = true,
// onChange handlers
onChangeRenderedItems = () => {},
onChangeSorting = () => {},
onChangeRenderedItems,
onChangeSorting,
onChangeItemIndices,
// sorting
sortItems = true,
@ -197,35 +205,43 @@ function UnoptimizedManagedTable<T extends object>(props: {
});
}, [items, searchQuery, tableSearchBar.fieldsToSearch]);
const renderedIndices = useMemo<VisibleItemsStartEnd>(
() => [
tableOptions.page.index * tableOptions.page.size,
(tableOptions.page.index + 1) * tableOptions.page.size,
],
[tableOptions.page.index, tableOptions.page.size]
);
const renderedItems = useMemo(() => {
const sortedItems = sortItems
? sortFn(filteredItems, tableOptions.sort.field as keyof T, tableOptions.sort.direction)
: filteredItems;
return sortedItems.slice(
tableOptions.page.index * tableOptions.page.size,
(tableOptions.page.index + 1) * tableOptions.page.size
);
return sortedItems.slice(...renderedIndices);
}, [
sortItems,
sortFn,
filteredItems,
tableOptions.sort.field,
tableOptions.sort.direction,
tableOptions.page.index,
tableOptions.page.size,
renderedIndices,
]);
useEffect(() => {
onChangeRenderedItems(renderedItems);
onChangeRenderedItems?.(renderedItems);
}, [onChangeRenderedItems, renderedItems]);
useEffect(() => {
onChangeItemIndices?.(renderedIndices);
}, [onChangeItemIndices, renderedIndices]);
const sorting = useMemo(
() => ({ sort: tableOptions.sort as TableOptions<T>['sort'] }),
[tableOptions.sort]
);
useEffect(() => onChangeSorting(sorting.sort), [onChangeSorting, sorting]);
useEffect(() => onChangeSorting?.(sorting.sort), [onChangeSorting, sorting]);
const paginationProps = useMemo(() => {
if (!pagination) {
@ -256,7 +272,7 @@ function UnoptimizedManagedTable<T extends object>(props: {
oldSearchQuery: searchQuery,
})
) {
tableSearchBar.onChangeSearchQuery(value);
tableSearchBar.onChangeSearchQuery?.(value);
}
},
[searchQuery, tableSearchBar]

View file

@ -9,7 +9,6 @@ import { EuiCallOut, EuiFlexGroup, EuiFlexItem, EuiTitle } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { v4 as uuidv4 } from 'uuid';
import { FormattedMessage } from '@kbn/i18n-react';
import { compact } from 'lodash';
import React, { useEffect, useMemo, useState } from 'react';
import { apmEnableTableSearchBar } from '@kbn/observability-plugin/common';
import { ApmDocumentType } from '../../../../common/document_type';
@ -24,7 +23,7 @@ import { FETCH_STATUS, isPending, isSuccess, useFetcher } from '../../../hooks/u
import { usePreferredDataSourceAndBucketSize } from '../../../hooks/use_preferred_data_source_and_bucket_size';
import type { APIReturnType } from '../../../services/rest/create_call_apm_api';
import { TransactionOverviewLink } from '../links/apm/transaction_overview_link';
import type { TableSearchBar } from '../managed_table';
import type { TableSearchBar, VisibleItemsStartEnd } from '../managed_table';
import { ManagedTable } from '../managed_table';
import { OverviewTableContainer } from '../overview_table_container';
import { isTimeComparison } from '../time_comparison/get_comparison_options';
@ -74,6 +73,7 @@ export function TransactionsTable({
showSparkPlots,
}: Props) {
const { link } = useApmRouter();
const [renderedItemIndices, setRenderedItemIndices] = useState<VisibleItemsStartEnd>([0, 0]);
const {
query,
@ -91,12 +91,10 @@ export function TransactionsTable({
const shouldShowSparkPlots = showSparkPlots ?? !isLarge;
const { transactionType, serviceName } = useApmServiceContext();
const [searchQuery, setSearchQueryDebounced] = useStateDebounced('');
const [renderedItems, setRenderedItems] = useState<ApiResponse['transactionGroups']>([]);
const { mainStatistics, mainStatisticsStatus, detailedStatistics, detailedStatisticsStatus } =
useTableData({
comparisonEnabled,
currentPageItems: renderedItems,
end,
environment,
kuery,
@ -106,6 +104,7 @@ export function TransactionsTable({
serviceName,
start,
transactionType,
renderedItemIndices,
});
useEffect(() => {
@ -252,8 +251,8 @@ export function TransactionsTable({
isLoading={mainStatisticsStatus === FETCH_STATUS.LOADING}
tableSearchBar={tableSearchBar}
showPerPageOptions={showPerPageOptions}
onChangeRenderedItems={setRenderedItems}
saveTableOptionsToUrl={saveTableOptionsToUrl}
onChangeItemIndices={setRenderedItemIndices}
/>
</OverviewTableContainer>
</EuiFlexItem>
@ -263,7 +262,6 @@ export function TransactionsTable({
function useTableData({
comparisonEnabled,
currentPageItems,
end,
environment,
kuery,
@ -273,9 +271,9 @@ function useTableData({
serviceName,
start,
transactionType,
renderedItemIndices,
}: {
comparisonEnabled: boolean | undefined;
currentPageItems: ApiResponse['transactionGroups'];
end: string;
environment: string;
kuery: string;
@ -285,6 +283,7 @@ function useTableData({
serviceName: string;
start: string;
transactionType: string | undefined;
renderedItemIndices: VisibleItemsStartEnd;
}) {
const preferredDataSource = usePreferredDataSourceAndBucketSize({
start,
@ -339,16 +338,25 @@ function useTableData({
]
);
const itemsToFetch = useMemo(
() =>
mainStatistics.transactionGroups
.slice(...renderedItemIndices)
.map(({ name }) => name)
.filter((name) => Boolean(name))
.sort(),
[renderedItemIndices, mainStatistics.transactionGroups]
);
const { data: detailedStatistics, status: detailedStatisticsStatus } = useFetcher(
(callApmApi) => {
const transactionNames = compact(currentPageItems.map(({ name }) => name));
if (
start &&
end &&
transactionType &&
latencyAggregationType &&
preferredDataSource &&
transactionNames.length > 0
itemsToFetch.length > 0
) {
return callApmApi(
'GET /internal/apm/services/{serviceName}/transactions/groups/detailed_statistics',
@ -366,7 +374,7 @@ function useTableData({
rollupInterval: preferredDataSource.source.rollupInterval,
useDurationSummary: !!shouldUseDurationSummary,
latencyAggregationType: latencyAggregationType as LatencyAggregationType,
transactionNames: JSON.stringify(transactionNames.sort()),
transactionNames: JSON.stringify(itemsToFetch),
offset: comparisonEnabled && isTimeComparison(offset) ? offset : undefined,
},
},
@ -377,7 +385,7 @@ function useTableData({
// only fetches detailed statistics when `currentPageItems` is updated.
// eslint-disable-next-line react-hooks/exhaustive-deps
[mainStatistics.requestId, currentPageItems, offset, comparisonEnabled],
[mainStatistics.requestId, itemsToFetch, offset, comparisonEnabled],
{ preservePreviousData: false }
);