Skip to content

feat(ourlogs): Switch needle in haystack to time based #111946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
k-fish wants to merge 2 commits into from
Closed

feat(ourlogs): Switch needle in haystack to time based #111946

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0b7d155
feat(ourlogs): Switch needle in haystack to time based
k-fish Mar 31, 2026
1662cb0
feat(logs): Emit metric for flex time pages fetched before data found
k-fish Mar 31, 2026
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
2 changes: 2 additions & 0 deletions static/app/views/explore/logs/constants.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ export const LOGS_FILTER_KEY_SECTIONS: FilterKeySection[] = [LOGS_FILTERS];
*/
export const LOGS_DRAWER_QUERY_PARAM = 'logsDrawer';

export const FLEX_TIME_INITIAL_SEARCH_DURATION_MS = 10_000;
export const FLEX_TIME_RESUME_SEARCH_DURATION_MS = 20_000;
export const VIRTUAL_STREAMED_INTERVAL_MS = 250;
export const MINIMUM_INFINITE_SCROLL_FETCH_COOLDOWN_MS = 1000;

Expand Down
22 changes: 6 additions & 16 deletions static/app/views/explore/logs/useLogsQuery.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import {PageFiltersFixture} from 'sentry-fixture/pageFilters';

import {makeTestQueryClient} from 'sentry-test/queryClient';
import {act, renderHookWithProviders, waitFor} from 'sentry-test/reactTestingLibrary';

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

View workflow job for this annotation

GitHub Actions / @typescript/native-preview 

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

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

View workflow job for this annotation

GitHub Actions / typescript 

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

import type {ApiResult} from 'sentry/api';
import {usePageFilters} from 'sentry/components/pageFilters/usePageFilters';
Expand Down Expand Up @@ -369,41 +369,31 @@
};
}

it('auto fetches only empty pages pages and end when signaled', async () => {
it('auto fetches empty pages until hasNext is false', async () => {
const mockFlextTimeRequests = [
makeMockEventsResponse({cursor: '', nextCursor: 'page2'}),
makeMockEventsResponse({cursor: 'page2', nextCursor: 'page3'}),
makeMockEventsResponse({cursor: 'page3', nextCursor: 'page4'}),
makeMockEventsResponse({cursor: 'page4', nextCursor: 'page5'}),
makeMockEventsResponse({cursor: 'page4', nextCursor: 'page5', hasNext: false}),
makeMockEventsResponse({cursor: 'page5', nextCursor: 'page6', hasNext: false}),
makeMockEventsResponse({cursor: 'page6', nextCursor: 'page7', hasNext: false}),
].map(response => MockApiClient.addMockResponse(response));

const {result} = renderHookWithProviders(
() => useInfiniteLogsQuery({highFidelity: true, maxAutoFetches: 3}),
() => useInfiniteLogsQuery({highFidelity: true}),
{
additionalWrapper: createWrapper(),
}
);

// the first 3 requests should have been called
await waitFor(() => expect(mockFlextTimeRequests[0]).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockFlextTimeRequests[1]).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockFlextTimeRequests[2]).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockFlextTimeRequests[3]).not.toHaveBeenCalled());

// should be allowed to resume autofetching
expect(result.current.canResumeAutoFetch).toBe(true);
act(() => result.current.resumeAutoFetch());

// the next 3 requests should have been called
await waitFor(() => expect(mockFlextTimeRequests[3]).toHaveBeenCalledTimes(1));
await waitFor(() => expect(mockFlextTimeRequests[4]).toHaveBeenCalledTimes(1));

// should not be allowed to resume autofetching
// should not be allowed to resume autofetching because hasNext is false
expect(result.current.canResumeAutoFetch).toBe(false);

await waitFor(() => expect(mockFlextTimeRequests[5]).not.toHaveBeenCalled());
await waitFor(() => expect(mockFlextTimeRequests[4]).not.toHaveBeenCalled());
});

it('auto fetches until limit', async () => {
Expand Down Expand Up @@ -433,7 +423,7 @@
].map(response => MockApiClient.addMockResponse(response));

const {result} = renderHookWithProviders(
() => useInfiniteLogsQuery({highFidelity: true, maxAutoFetches: 3}),
() => useInfiniteLogsQuery({highFidelity: true}),
{
additionalWrapper: createWrapper(),
}
Expand Down
47 changes: 38 additions & 9 deletions static/app/views/explore/logs/useLogsQuery.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {useCallback, useEffect, useMemo, useState} from 'react';
import {useCallback, useEffect, useMemo, useRef, useState} from 'react';
import * as Sentry from '@sentry/react';
import {logger} from '@sentry/react';

import {type ApiResult} from 'sentry/api';
Expand Down Expand Up @@ -28,6 +29,8 @@ import {SAMPLING_MODE} from 'sentry/views/explore/hooks/useProgressiveQuery';
import {useTraceItemDetails} from 'sentry/views/explore/hooks/useTraceItemDetails';
import {
AlwaysPresentLogFields,
FLEX_TIME_INITIAL_SEARCH_DURATION_MS,
FLEX_TIME_RESUME_SEARCH_DURATION_MS,
MAX_LOG_INGEST_DELAY,
MAX_LOGS_INFINITE_QUERY_PAGES,
QUERY_PAGE_LIMIT,
Expand Down Expand Up @@ -405,12 +408,10 @@ type QueryKey = [
export function useInfiniteLogsQuery({
disabled,
highFidelity,
maxAutoFetches = 5,
referrer,
}: {
disabled?: boolean;
highFidelity?: boolean;
maxAutoFetches?: number;
referrer?: string;
} = {}) {
const _referrer = referrer ?? 'api.explore.logs-table';
Expand Down Expand Up @@ -689,15 +690,40 @@ export function useInfiniteLogsQuery({
const lastPageLength = data?.pages?.[data.pages.length - 1]?.[0]?.data?.length ?? 0;
const limit = autoRefresh ? QUERY_PAGE_LIMIT_WITH_AUTO_REFRESH : QUERY_PAGE_LIMIT;

// the original state starts at -1 because we have to count
// the 1 query made by default outside of the auto fetches
const [autoFetchesRemaining, setAutoFetchesRemaining] = useState(maxAutoFetches - 1);
const [autoFetchStartTime, setAutoFetchStartTime] = useState<number>(() => Date.now());
Copy link
Copy Markdown
Contributor

@sentry sentry bot Mar 31, 2026

Choose a reason for hiding this comment

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

Bug: The auto-fetch timer state (autoFetchStartTime, autoFetchDuration) is not reset when the user changes query parameters, preventing or delaying auto-fetching on subsequent searches.
Severity: HIGH

Suggested Fix

Use a useEffect hook that triggers when the query key (queryKeyWithInfinite) changes. Inside this effect, reset the autoFetchStartTime state to Date.now() and the autoFetchDuration state to the initial value (FLEX_TIME_INITIAL_SEARCH_DURATION_MS). This will ensure every new, distinct query starts with a fresh auto-fetch timer.

Prompt for AI Agent 
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/explore/logs/useLogsQuery.tsx#L693

Potential issue: The state variables `autoFetchStartTime` and `autoFetchDuration` are
initialized when the `useInfiniteLogsQuery` hook mounts but are not reset when query
parameters change. If a user waits more than 10 seconds after the page loads and then
modifies their query (e.g., changes a filter or search term), the new query will not
auto-fetch new logs because the condition `Date.now() - autoFetchStartTime <
autoFetchDuration` will be false. Additionally, if a user clicks "Continue Scanning"
(which sets the duration to 20s) and then runs a new query, the initial auto-fetch for
that new query will incorrectly use a 20-second duration instead of the intended 10
seconds.

Did we get this right? 👍 / 👎 to inform future reviews.

const [autoFetchDuration, setAutoFetchDuration] = useState(
FLEX_TIME_INITIAL_SEARCH_DURATION_MS
);
Copy link
Copy Markdown
Contributor

@cursor cursor bot Mar 31, 2026

Choose a reason for hiding this comment

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

Autofetch timer never resets per new query

Medium Severity

autoFetchStartTime is initialized once and only refreshed by resumeAutoFetch, so the 10-second autofetch window is measured from hook mount instead of each new search. After the page has been open long enough, new queries in useLogsQuery.tsx stop auto-fetching immediately and require manual Continue Scanning clicks.

Additional Locations (1)
Fix in Cursor Fix in Web

const canAutoFetchNextPage =
!!highFidelity &&
hasNextPage &&
nextPageHasData &&
(lastPageLength === 0 || _data.length < limit);
const shouldAutoFetchNextPage = canAutoFetchNextPage && autoFetchesRemaining > 0;
const shouldAutoFetchNextPage =
canAutoFetchNextPage && Date.now() - autoFetchStartTime < autoFetchDuration;
Copy link
Copy Markdown
Contributor

@cursor cursor bot Mar 31, 2026

Choose a reason for hiding this comment

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

Time-only autofetch can flood API requests

Medium Severity

Replacing maxAutoFetches with only a wall-clock check in useLogsQuery.tsx removes the request-count guard. When empty pages return quickly, shouldAutoFetchNextPage stays true and the loop can issue many sequential _fetchNextPage calls within 10–20 seconds, creating significantly higher backend load than before.

Additional Locations (1)
Fix in Cursor Fix in Web


const autoFetchPageCount = useRef(0);
const prevShouldAutoFetch = useRef(false);

useEffect(() => {
if (shouldAutoFetchNextPage && !prevShouldAutoFetch.current) {
autoFetchPageCount.current = 0;
}

if (!shouldAutoFetchNextPage && prevShouldAutoFetch.current && highFidelity) {
Sentry.metrics.distribution(
'explore.logs.flex_time_pages_before_data',
autoFetchPageCount.current,
{
attributes: {
found_data: _data.length > 0 ? 'true' : 'false',
},
}
);
}

prevShouldAutoFetch.current = shouldAutoFetchNextPage;
}, [shouldAutoFetchNextPage, highFidelity, _data.length]);

useEffect(() => {
if (!shouldAutoFetchNextPage) {
Expand All @@ -708,7 +734,7 @@ export function useInfiniteLogsQuery({
return;
}

setAutoFetchesRemaining(remaining => remaining - 1);
autoFetchPageCount.current += 1;
_fetchNextPage();
}, [shouldAutoFetchNextPage, isFetchingNextPage, _fetchNextPage, nextPageCursor]);

Expand Down Expand Up @@ -741,7 +767,10 @@ export function useInfiniteLogsQuery({
isFetchingPreviousPage,
lastPageLength,
canResumeAutoFetch: canAutoFetchNextPage,
resumeAutoFetch: () => setAutoFetchesRemaining(maxAutoFetches),
resumeAutoFetch: () => {
setAutoFetchStartTime(Date.now());
setAutoFetchDuration(FLEX_TIME_RESUME_SEARCH_DURATION_MS);
},
Copy link
Copy Markdown
Contributor

@cursor cursor bot Mar 31, 2026

Choose a reason for hiding this comment

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

Resume duration leaks into later searches

Low Severity

resumeAutoFetch sets autoFetchDuration to FLEX_TIME_RESUME_SEARCH_DURATION_MS, but that value is never restored for later queries. After one resume, subsequent “initial” searches in useLogsQuery.tsx inherit the longer 20s window, so the shorter first-pass behavior no longer applies.

Additional Locations (1)
Fix in Cursor Fix in Web

dataScanned,
bytesScanned: totalBytesScanned,
};
Expand Down
Loading