Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,11 @@
import {decodeInteger, decodeScalar} from 'sentry/utils/queryString';
import {useLocation} from 'sentry/utils/useLocation';
import {useNavigate} from 'sentry/utils/useNavigate';
import {useLogsQueryKeyWithInfinite} from 'sentry/views/explore/logs/useLogsQuery';
import useOrganization from 'sentry/utils/useOrganization';
import {
useLogsQueryHighFidelity,
useLogsQueryKeyWithInfinite,
} from 'sentry/views/explore/logs/useLogsQuery';

export const LOGS_AUTO_REFRESH_KEY = 'live';
export const LOGS_REFRESH_INTERVAL_KEY = 'refreshEvery';
Expand Down Expand Up @@ -119,11 +123,14 @@
}

export function useSetLogsAutoRefresh() {
const organization = useOrganization();

Check failure on line 126 in static/app/views/explore/contexts/logs/logsAutoRefreshContext.tsx

View workflow job for this annotation

GitHub Actions / typescript

'organization' is declared but its value is never read.
const location = useLocation();
const navigate = useNavigate();
const highFidelity = useLogsQueryHighFidelity();
const {queryKey} = useLogsQueryKeyWithInfinite({
referrer: 'api.explore.logs-table',
autoRefresh: true,
highFidelity,
});
const queryClient = useQueryClient();
const {setPausedAt, pausedAt: currentPausedAt} = useLogsAutoRefresh();
Expand Down
7 changes: 5 additions & 2 deletions static/app/views/explore/contexts/logs/logsPageData.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import {createDefinedContext} from 'sentry/utils/performance/contexts/utils';
import useOrganization from 'sentry/utils/useOrganization';
import {isLogsEnabled} from 'sentry/views/explore/logs/isLogsEnabled';
import type {UseInfiniteLogsQueryResult} from 'sentry/views/explore/logs/useLogsQuery';
import {useInfiniteLogsQuery} from 'sentry/views/explore/logs/useLogsQuery';
import {
useInfiniteLogsQuery,
useLogsQueryHighFidelity,
} from 'sentry/views/explore/logs/useLogsQuery';

interface LogsPageData {
infiniteLogsQueryResult: UseInfiniteLogsQueryResult;
Expand All @@ -24,7 +27,7 @@ export function LogsPageDataProvider({
}) {
const organization = useOrganization();
const feature = isLogsEnabled(organization);
const highFidelity = organization.features.includes('ourlogs-high-fidelity');
const highFidelity = useLogsQueryHighFidelity();
const infiniteLogsQueryResult = useInfiniteLogsQuery({
disabled: !feature,
highFidelity,
Expand Down
174 changes: 101 additions & 73 deletions static/app/views/explore/logs/useLogsQuery.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import {useCallback, useEffect, useMemo} from 'react';
import {useCallback, useEffect, useMemo, useState} from 'react';
import {logger} from '@sentry/react';

import {type ApiResult} from 'sentry/api';
import {defined} from 'sentry/utils';
import {encodeSort, type EventsMetaType} from 'sentry/utils/discover/eventView';
import type {Sort} from 'sentry/utils/discover/fields';
import {DiscoverDatasets} from 'sentry/utils/discover/types';
Expand All @@ -23,6 +24,7 @@
import {
AlwaysPresentLogFields,
MAX_LOG_INGEST_DELAY,
MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS,
QUERY_PAGE_LIMIT,
QUERY_PAGE_LIMIT_WITH_AUTO_REFRESH,
} from 'sentry/views/explore/logs/constants';
Expand Down Expand Up @@ -211,9 +213,6 @@
}

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

const params = {
query: {
Expand Down Expand Up @@ -285,12 +284,39 @@
pageParam: LogPageParam
): LogPageParam => {
const [pageData, _statusText, response] = result;

const sortBy = getTimeBasedSortBy(sortBys);

if (!sortBy) {
// Only sort by timestamp precise is supported for infinite queries.
return null;
}

const isDescending = sortBy.kind === 'desc';
// Previous pages have to have the sort order reversed in order to start at the limit from the initial page.
const querySortDirection: Sort | undefined = isGetPreviousPage
? {
field: OurLogKnownFieldKey.TIMESTAMP,
kind: isDescending ? 'asc' : 'desc',
}
: undefined;

if (highFidelity || isFlexTimePageParam(pageParam)) {
const pageLinkHeader = response?.getResponseHeader('Link') ?? null;
const links = parseLinkHeader(pageLinkHeader);
const link = isGetPreviousPage ? links.previous : links.next;

if (!link?.results) {
return undefined;
}

return {
querySortDirection,
sortByDirection: sortBy.kind,
autoRefresh,
cursor: link.cursor ?? undefined,
} as FlexTimePageParam;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Flexible Pagination Overlaps With Cursor-Based

The getNextPageParam function's condition for flex time pagination (highFidelity || isFlexTimePageParam(pageParam)) is too broad. It now uses cursor-based pagination if highFidelity is true or if the previous page was flex time, even if isDescending or autoRefresh conditions don't warrant it. This can cause inconsistent pagination behavior.

Fix in Cursor Fix in Web


const firstRow = pageData.data?.[0];
const lastRow = pageData.data?.[pageData.data.length - 1];
if (!firstRow || !lastRow) {
Expand Down Expand Up @@ -319,45 +345,19 @@
const logId = isGetPreviousPage
? firstRow[OurLogKnownFieldKey.ID]
: lastRow[OurLogKnownFieldKey.ID];
const isDescending = sortBy.kind === 'desc';
const timestampPrecise = isGetPreviousPage ? firstTimestamp : lastTimestamp;
let querySortDirection: Sort | undefined = undefined;
const reverseSortDirection = isDescending ? 'asc' : 'desc';

if (isGetPreviousPage) {
// Previous pages have to have the sort order reversed in order to start at the limit from the initial page.
querySortDirection = {
field: OurLogKnownFieldKey.TIMESTAMP,
kind: reverseSortDirection,
};
}

const indexFromInitialPage = isGetPreviousPage
? (pageParam?.indexFromInitialPage ?? 0) - 1
: (pageParam?.indexFromInitialPage ?? 0) + 1;

let cursor: PageParam['cursor'] = undefined;
if (isDescending && !autoRefresh && highFidelity) {
const pageLinkHeader = response?.getResponseHeader('Link') ?? null;
const links = parseLinkHeader(pageLinkHeader);
// the flex time sampling strategy has no previous page cursor and only
// a next page cursor because it only works in timestamp desc order
const link = isGetPreviousPage ? links.previous : links.next;
// return `undefined` to indicate that there are no results in this direction
if (!link?.results) {
return undefined;
}
cursor = link.cursor ?? undefined;
}

const pageParamResult: LogPageParam = {
const pageParamResult: InfiniteScrollPageParam = {
logId,
timestampPrecise,
querySortDirection,
sortByDirection: sortBy.kind,
indexFromInitialPage,
autoRefresh,
cursor,
};

return pageParamResult;
Expand Down Expand Up @@ -417,7 +417,7 @@
return query;
}

if (pageParam.cursor) {
if (isFlexTimePageParam(pageParam)) {
return {
...query,
cursor: pageParam.cursor,
Expand Down Expand Up @@ -448,22 +448,32 @@
};
}

interface PageParam {
interface BaseLogsPageParams {
// Whether the page param is for auto refresh mode.
autoRefresh: boolean;
// The original sort direction of the query.
sortByDirection: Sort['kind'];
// 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.
querySortDirection?: Sort;
}

interface FlexTimePageParam extends BaseLogsPageParams {
cursor?: string;
}

interface InfiniteScrollPageParam extends BaseLogsPageParams {
// The index of the page from the initial page. Useful for debugging and testing.
indexFromInitialPage: number;
// 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.
logId: string;
// The original sort direction of the query.
sortByDirection: Sort['kind'];
timestampPrecise: bigint | null;
cursor?: string;
// 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.
querySortDirection?: Sort;
}

export type LogPageParam = PageParam | null | undefined;
export type LogPageParam = FlexTimePageParam | InfiniteScrollPageParam | null | undefined;

function isFlexTimePageParam(pageParam: LogPageParam): pageParam is FlexTimePageParam {
return defined(pageParam) && 'cursor' in pageParam;
}

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

Expand All @@ -483,7 +493,7 @@
autoRefresh,
highFidelity,
});
const queryClient = useQueryClient();

Check failure on line 496 in static/app/views/explore/logs/useLogsQuery.tsx

View workflow job for this annotation

GitHub Actions / typescript

'queryClient' is declared but its value is never read.

const sortBys = useQueryParamsSortBys();

Expand Down Expand Up @@ -574,35 +584,6 @@
refetch,
} = queryResult;

useEffect(() => {
// Remove empty pages from the query data. In the case of auto refresh it's possible that the most recent page in time is empty.
queryClient.setQueryData(
queryKeyWithInfinite,
(oldData: InfiniteData<ApiResult<EventsLogsResult>> | undefined) => {
if (!oldData) {
return oldData;
}
const pageIndexWithMostRecentTimestamp =
getTimeBasedSortBy(sortBys)?.kind === 'asc' ? 0 : oldData.pages.length - 1;

if (
(oldData.pages?.[pageIndexWithMostRecentTimestamp]?.[0]?.data?.length ?? 0) > 0
) {
return oldData;
}

return {
pages: oldData.pages.filter(
(_, index) => index !== pageIndexWithMostRecentTimestamp
),
pageParams: oldData.pageParams.filter(
(_, index) => index !== pageIndexWithMostRecentTimestamp
),
};
}
);
}, [queryClient, queryKeyWithInfinite, sortBys]);

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

const _data = useMemo(() => {
Expand Down Expand Up @@ -661,29 +642,76 @@
[hasNextPage, fetchNextPage, isFetching, isError, nextPageHasData]
);

const lastPageLength = data?.pages?.[data.pages.length - 1]?.[0]?.data?.length ?? 0;
const limit = autoRefresh ? QUERY_PAGE_LIMIT_WITH_AUTO_REFRESH : QUERY_PAGE_LIMIT;
const shouldAutoFetchNextPage =
highFidelity &&
hasNextPage &&
nextPageHasData &&
(lastPageLength === 0 || _data.length < limit);
const [waitingToAutoFetch, setWaitingToAutoFetch] = useState(false);

useEffect(() => {
if (!shouldAutoFetchNextPage) {
return () => {};
}

setWaitingToAutoFetch(true);

const timeoutID = setTimeout(() => {
setWaitingToAutoFetch(false);
_fetchNextPage();
}, MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS);

return () => clearTimeout(timeoutID);
}, [shouldAutoFetchNextPage, _fetchNextPage]);

return {
error,
isError,
isFetching,
isPending: queryResult.isPending,
isPending:
// query is still pending
queryResult.isPending ||
// query finished but we're waiting to auto fetch the next page
(waitingToAutoFetch && _data.length === 0) ||
// started auto fetching the next page
(shouldAutoFetchNextPage && _data.length === 0 && isFetchingNextPage),
data: _data,
meta: _meta,
isRefetching: queryResult.isRefetching,
isEmpty:
!queryResult.isPending &&
!queryResult.isRefetching &&
!isError &&
_data.length === 0,
_data.length === 0 &&
!shouldAutoFetchNextPage,
fetchNextPage: _fetchNextPage,
fetchPreviousPage: _fetchPreviousPage,
refetch,
hasNextPage,
queryKey: queryKeyWithInfinite,
hasPreviousPage,
isFetchingNextPage,
isFetchingNextPage:
!shouldAutoFetchNextPage && _data.length > 0 && isFetchingNextPage,
isFetchingPreviousPage,
lastPageLength: data?.pages?.[data.pages.length - 1]?.[0]?.data?.length ?? 0,
lastPageLength,
};
}

export type UseInfiniteLogsQueryResult = ReturnType<typeof useInfiniteLogsQuery>;

export function useLogsQueryHighFidelity() {
const organization = useOrganization();
const sortBys = useQueryParamsSortBys();
const highFidelity = organization.features.includes('ourlogs-high-fidelity');

// we can only turn on high accuracy flex time sampling when
// the order by is exactly timestamp descending,
return (
highFidelity &&
sortBys.length === 1 &&
sortBys[0]?.field === 'timestamp' &&
sortBys[0]?.kind === 'desc'
);
}
Loading