Skip to content

Commit f4b6260

Browse files
authored
fix(logs): Split flex time page params from existing page params (#101176)
Mixing the 2 into 1 type is confusing and this is in preparation to auto fetch the next page if the current page is empty which is possible with the flex time strategy.
1 parent 6a8714d commit f4b6260

File tree

3 files changed

+119
-48
lines changed

3 files changed

+119
-48
lines changed

static/app/views/explore/contexts/logs/logsAutoRefreshContext.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,10 @@ import {useQueryClient} from 'sentry/utils/queryClient';
66
import {decodeInteger, decodeScalar} from 'sentry/utils/queryString';
77
import {useLocation} from 'sentry/utils/useLocation';
88
import {useNavigate} from 'sentry/utils/useNavigate';
9-
import {useLogsQueryKeyWithInfinite} from 'sentry/views/explore/logs/useLogsQuery';
9+
import {
10+
useLogsQueryHighFidelity,
11+
useLogsQueryKeyWithInfinite,
12+
} from 'sentry/views/explore/logs/useLogsQuery';
1013

1114
export const LOGS_AUTO_REFRESH_KEY = 'live';
1215
export const LOGS_REFRESH_INTERVAL_KEY = 'refreshEvery';
@@ -121,9 +124,11 @@ function pausedAtAllowedToContinue(pausedAt: number | undefined) {
121124
export function useSetLogsAutoRefresh() {
122125
const location = useLocation();
123126
const navigate = useNavigate();
127+
const highFidelity = useLogsQueryHighFidelity();
124128
const {queryKey} = useLogsQueryKeyWithInfinite({
125129
referrer: 'api.explore.logs-table',
126130
autoRefresh: true,
131+
highFidelity,
127132
});
128133
const queryClient = useQueryClient();
129134
const {setPausedAt, pausedAt: currentPausedAt} = useLogsAutoRefresh();

static/app/views/explore/contexts/logs/logsPageData.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import {createDefinedContext} from 'sentry/utils/performance/contexts/utils';
44
import useOrganization from 'sentry/utils/useOrganization';
55
import {isLogsEnabled} from 'sentry/views/explore/logs/isLogsEnabled';
66
import type {UseInfiniteLogsQueryResult} from 'sentry/views/explore/logs/useLogsQuery';
7-
import {useInfiniteLogsQuery} from 'sentry/views/explore/logs/useLogsQuery';
7+
import {
8+
useInfiniteLogsQuery,
9+
useLogsQueryHighFidelity,
10+
} from 'sentry/views/explore/logs/useLogsQuery';
811

912
interface LogsPageData {
1013
infiniteLogsQueryResult: UseInfiniteLogsQueryResult;
@@ -24,7 +27,7 @@ export function LogsPageDataProvider({
2427
}) {
2528
const organization = useOrganization();
2629
const feature = isLogsEnabled(organization);
27-
const highFidelity = organization.features.includes('ourlogs-high-fidelity');
30+
const highFidelity = useLogsQueryHighFidelity();
2831
const infiniteLogsQueryResult = useInfiniteLogsQuery({
2932
disabled: !feature,
3033
highFidelity,

static/app/views/explore/logs/useLogsQuery.tsx

Lines changed: 108 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import {useCallback, useEffect, useMemo} from 'react';
1+
import {useCallback, useEffect, useMemo, useState} from 'react';
22
import {logger} from '@sentry/react';
33

44
import {type ApiResult} from 'sentry/api';
5+
import {defined} from 'sentry/utils';
56
import {encodeSort, type EventsMetaType} from 'sentry/utils/discover/eventView';
67
import type {Sort} from 'sentry/utils/discover/fields';
78
import {DiscoverDatasets} from 'sentry/utils/discover/types';
@@ -23,6 +24,7 @@ import {useTraceItemDetails} from 'sentry/views/explore/hooks/useTraceItemDetail
2324
import {
2425
AlwaysPresentLogFields,
2526
MAX_LOG_INGEST_DELAY,
27+
MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS,
2628
QUERY_PAGE_LIMIT,
2729
QUERY_PAGE_LIMIT_WITH_AUTO_REFRESH,
2830
} from 'sentry/views/explore/logs/constants';
@@ -211,9 +213,6 @@ function useLogsQueryKey({
211213
}
212214

213215
const orderby = eventViewPayload.sort;
214-
// we can only turn on high accuracy flex time sampling when
215-
// the order by is exactly timestamp descending,
216-
highFidelity = highFidelity && orderby === '-timestamp';
217216

218217
const params = {
219218
query: {
@@ -285,12 +284,39 @@ function getPageParam(
285284
pageParam: LogPageParam
286285
): LogPageParam => {
287286
const [pageData, _statusText, response] = result;
288-
289287
const sortBy = getTimeBasedSortBy(sortBys);
288+
290289
if (!sortBy) {
291290
// Only sort by timestamp precise is supported for infinite queries.
292291
return null;
293292
}
293+
294+
const isDescending = sortBy.kind === 'desc';
295+
// Previous pages have to have the sort order reversed in order to start at the limit from the initial page.
296+
const querySortDirection: Sort | undefined = isGetPreviousPage
297+
? {
298+
field: OurLogKnownFieldKey.TIMESTAMP,
299+
kind: isDescending ? 'asc' : 'desc',
300+
}
301+
: undefined;
302+
303+
if (highFidelity || isFlexTimePageParam(pageParam)) {
304+
const pageLinkHeader = response?.getResponseHeader('Link') ?? null;
305+
const links = parseLinkHeader(pageLinkHeader);
306+
const link = isGetPreviousPage ? links.previous : links.next;
307+
308+
if (!link?.results) {
309+
return undefined;
310+
}
311+
312+
return {
313+
querySortDirection,
314+
sortByDirection: sortBy.kind,
315+
autoRefresh,
316+
cursor: link.cursor ?? undefined,
317+
} as FlexTimePageParam;
318+
}
319+
294320
const firstRow = pageData.data?.[0];
295321
const lastRow = pageData.data?.[pageData.data.length - 1];
296322
if (!firstRow || !lastRow) {
@@ -319,45 +345,19 @@ function getPageParam(
319345
const logId = isGetPreviousPage
320346
? firstRow[OurLogKnownFieldKey.ID]
321347
: lastRow[OurLogKnownFieldKey.ID];
322-
const isDescending = sortBy.kind === 'desc';
323348
const timestampPrecise = isGetPreviousPage ? firstTimestamp : lastTimestamp;
324-
let querySortDirection: Sort | undefined = undefined;
325-
const reverseSortDirection = isDescending ? 'asc' : 'desc';
326-
327-
if (isGetPreviousPage) {
328-
// Previous pages have to have the sort order reversed in order to start at the limit from the initial page.
329-
querySortDirection = {
330-
field: OurLogKnownFieldKey.TIMESTAMP,
331-
kind: reverseSortDirection,
332-
};
333-
}
334349

335350
const indexFromInitialPage = isGetPreviousPage
336351
? (pageParam?.indexFromInitialPage ?? 0) - 1
337352
: (pageParam?.indexFromInitialPage ?? 0) + 1;
338353

339-
let cursor: PageParam['cursor'] = undefined;
340-
if (isDescending && !autoRefresh && highFidelity) {
341-
const pageLinkHeader = response?.getResponseHeader('Link') ?? null;
342-
const links = parseLinkHeader(pageLinkHeader);
343-
// the flex time sampling strategy has no previous page cursor and only
344-
// a next page cursor because it only works in timestamp desc order
345-
const link = isGetPreviousPage ? links.previous : links.next;
346-
// return `undefined` to indicate that there are no results in this direction
347-
if (!link?.results) {
348-
return undefined;
349-
}
350-
cursor = link.cursor ?? undefined;
351-
}
352-
353-
const pageParamResult: LogPageParam = {
354+
const pageParamResult: InfiniteScrollPageParam = {
354355
logId,
355356
timestampPrecise,
356357
querySortDirection,
357358
sortByDirection: sortBy.kind,
358359
indexFromInitialPage,
359360
autoRefresh,
360-
cursor,
361361
};
362362

363363
return pageParamResult;
@@ -417,7 +417,7 @@ function getParamBasedQuery(
417417
return query;
418418
}
419419

420-
if (pageParam.cursor) {
420+
if (isFlexTimePageParam(pageParam)) {
421421
return {
422422
...query,
423423
cursor: pageParam.cursor,
@@ -448,22 +448,32 @@ function getParamBasedQuery(
448448
};
449449
}
450450

451-
interface PageParam {
451+
interface BaseLogsPageParams {
452452
// Whether the page param is for auto refresh mode.
453453
autoRefresh: boolean;
454+
// The original sort direction of the query.
455+
sortByDirection: Sort['kind'];
456+
// When scrolling is happening towards current time, or during auto refresh, we flip the sort direction passed to the query to get X more rows in the future starting from the last seen row.
457+
querySortDirection?: Sort;
458+
}
459+
460+
interface FlexTimePageParam extends BaseLogsPageParams {
461+
cursor: string | undefined;
462+
}
463+
464+
interface InfiniteScrollPageParam extends BaseLogsPageParams {
454465
// The index of the page from the initial page. Useful for debugging and testing.
455466
indexFromInitialPage: number;
456467
// The id of the log row matching timestampPrecise. We use this to exclude the row from the query to avoid duplicates right on the nanosecond boundary.
457468
logId: string;
458-
// The original sort direction of the query.
459-
sortByDirection: Sort['kind'];
460469
timestampPrecise: bigint | null;
461-
cursor?: string;
462-
// When scrolling is happening towards current time, or during auto refresh, we flip the sort direction passed to the query to get X more rows in the future starting from the last seen row.
463-
querySortDirection?: Sort;
464470
}
465471

466-
export type LogPageParam = PageParam | null | undefined;
472+
export type LogPageParam = FlexTimePageParam | InfiniteScrollPageParam | null | undefined;
473+
474+
function isFlexTimePageParam(pageParam: LogPageParam): pageParam is FlexTimePageParam {
475+
return defined(pageParam) && 'cursor' in pageParam;
476+
}
467477

468478
type QueryKey = [url: string, endpointOptions: QueryKeyEndpointOptions, 'infinite'];
469479

@@ -579,6 +589,12 @@ export function useInfiniteLogsQuery({
579589
queryClient.setQueryData(
580590
queryKeyWithInfinite,
581591
(oldData: InfiniteData<ApiResult<EventsLogsResult>> | undefined) => {
592+
if (highFidelity) {
593+
// TODO: properly remove empty pages in high fidelity mode
594+
// to avoid reaching max pages
595+
return oldData;
596+
}
597+
582598
if (!oldData) {
583599
return oldData;
584600
}
@@ -601,7 +617,7 @@ export function useInfiniteLogsQuery({
601617
};
602618
}
603619
);
604-
}, [queryClient, queryKeyWithInfinite, sortBys]);
620+
}, [highFidelity, queryClient, queryKeyWithInfinite, sortBys]);
605621

606622
const {virtualStreamedTimestamp} = useVirtualStreaming({data, highFidelity});
607623

@@ -661,29 +677,76 @@ export function useInfiniteLogsQuery({
661677
[hasNextPage, fetchNextPage, isFetching, isError, nextPageHasData]
662678
);
663679

680+
const lastPageLength = data?.pages?.[data.pages.length - 1]?.[0]?.data?.length ?? 0;
681+
const limit = autoRefresh ? QUERY_PAGE_LIMIT_WITH_AUTO_REFRESH : QUERY_PAGE_LIMIT;
682+
const shouldAutoFetchNextPage =
683+
!!highFidelity &&
684+
hasNextPage &&
685+
nextPageHasData &&
686+
(lastPageLength === 0 || _data.length < limit);
687+
const [waitingToAutoFetch, setWaitingToAutoFetch] = useState<boolean>(false);
688+
689+
useEffect(() => {
690+
if (!shouldAutoFetchNextPage) {
691+
return () => {};
692+
}
693+
694+
setWaitingToAutoFetch(true);
695+
696+
const timeoutID = setTimeout(() => {
697+
setWaitingToAutoFetch(false);
698+
_fetchNextPage();
699+
}, MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS);
700+
701+
return () => clearTimeout(timeoutID);
702+
}, [shouldAutoFetchNextPage, _fetchNextPage]);
703+
664704
return {
665705
error,
666706
isError,
667707
isFetching,
668-
isPending: queryResult.isPending,
708+
isPending:
709+
// query is still pending
710+
queryResult.isPending ||
711+
// query finished but we're waiting to auto fetch the next page
712+
(waitingToAutoFetch && _data.length === 0) ||
713+
// started auto fetching the next page
714+
(shouldAutoFetchNextPage && _data.length === 0 && isFetchingNextPage),
669715
data: _data,
670716
meta: _meta,
671717
isRefetching: queryResult.isRefetching,
672718
isEmpty:
673719
!queryResult.isPending &&
674720
!queryResult.isRefetching &&
675721
!isError &&
676-
_data.length === 0,
722+
_data.length === 0 &&
723+
!shouldAutoFetchNextPage,
677724
fetchNextPage: _fetchNextPage,
678725
fetchPreviousPage: _fetchPreviousPage,
679726
refetch,
680727
hasNextPage,
681728
queryKey: queryKeyWithInfinite,
682729
hasPreviousPage,
683-
isFetchingNextPage,
730+
isFetchingNextPage:
731+
!shouldAutoFetchNextPage && _data.length > 0 && isFetchingNextPage,
684732
isFetchingPreviousPage,
685-
lastPageLength: data?.pages?.[data.pages.length - 1]?.[0]?.data?.length ?? 0,
733+
lastPageLength,
686734
};
687735
}
688736

689737
export type UseInfiniteLogsQueryResult = ReturnType<typeof useInfiniteLogsQuery>;
738+
739+
export function useLogsQueryHighFidelity() {
740+
const organization = useOrganization();
741+
const sortBys = useQueryParamsSortBys();
742+
const highFidelity = organization.features.includes('ourlogs-high-fidelity');
743+
744+
// we can only turn on high accuracy flex time sampling when
745+
// the order by is exactly timestamp descending,
746+
return (
747+
highFidelity &&
748+
sortBys.length === 1 &&
749+
sortBys[0]?.field === 'timestamp' &&
750+
sortBys[0]?.kind === 'desc'
751+
);
752+
}

0 commit comments

Comments
 (0)