Skip to content

Commit d648cb3

Browse files
ref(logs): Unify pinned-log fetching into a single query+cache
The previous strategy paired per-id useQueries(queryFn: skipToken) with a separate driver query that imperatively seeded their caches via setQueryData. Those per-id entries had no queryFn, so once garbage-collected they could not be refetched, leaving rows permanently blank or stuck pending — a latent runtime invariant, and an anti-pattern of sourcing a query's data from a sibling query's side effects. Collapse it into one useQuery whose queryFn runs the same two-step fetch (selected range, then a UUIDv7-derived wide window) and returns its rows directly. placeholderData: keepPreviousData preserves the no-flicker-on-unpin behavior the per-id cache was hacked in to provide. The unpin effect now only removes ids that are still missing, so retained notFoundIds can't re-trigger a removal. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 24c4a94 commit d648cb3

1 file changed

Lines changed: 77 additions & 123 deletions

File tree

Lines changed: 77 additions & 123 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
import {useCallback, useEffect, useMemo} from 'react';
1+
import {useEffect, useMemo} from 'react';
22
import type {QueryClient, QueryFunctionContext} from '@tanstack/react-query';
3-
import {skipToken, useQueries, useQuery} from '@tanstack/react-query';
3+
import {keepPreviousData, useQuery} from '@tanstack/react-query';
44
import moment from 'moment-timezone';
55

66
import {normalizeDateTimeParams} from 'sentry/components/pageFilters/parse';
@@ -27,7 +27,7 @@ interface PinnedLogsOptions {
2727
logsPinning: LogsPinning | undefined;
2828
}
2929

30-
const DRIVER_QUERY_KEY = 'pinned-logs-driver';
30+
const PINNED_LOGS_QUERY_KEY = 'pinned-logs';
3131

3232
/**
3333
* Practically-infinite period so the wide step finds any log still in retention,
@@ -42,52 +42,22 @@ const WIDE_STATS_PERIOD = '9999d';
4242
*/
4343
const WINDOW_BUFFER_MS = 5 * 60 * 1000;
4444

45-
function pinnedLogRowQueryKey(id: string, fields: string[]) {
46-
return ['pinned-log-row', id, fields] as const;
45+
interface PinnedLogsResult {
46+
notFoundIds: string[];
47+
rows: OurLogsResponseItem[];
4748
}
4849

4950
export function usePinnedLogsQuery({allRows, logsPinning}: PinnedLogsOptions) {
51+
const organization = useOrganization();
52+
const {selection, isReady: pageFiltersReady} = usePageFilters();
5053
const userFields = useQueryParamsFields();
54+
5155
const fields = useMemo(
5256
() => Array.from(new Set([...AlwaysPresentLogFields, ...userFields])),
5357
[userFields]
5458
);
5559

5660
const missingIds = useMissingPinnedLogIds(allRows, logsPinning);
57-
const {isFetching, isError, refetch} = usePinnedLogFetcher(
58-
missingIds,
59-
fields,
60-
logsPinning
61-
);
62-
const {rows, resolvedIds} = useCachedPinnedLogRows(missingIds, fields);
63-
64-
return {
65-
fetchedRows: rows,
66-
isPending: isFetching && missingIds.some(id => !resolvedIds.has(id)),
67-
isError,
68-
refetch,
69-
};
70-
}
71-
72-
/** Pinned ids that aren't already present in the loaded table rows. */
73-
function useMissingPinnedLogIds(
74-
allRows: LogTableRowItem[],
75-
logsPinning: LogsPinning | undefined
76-
) {
77-
return useMemo(() => {
78-
const allRowIds = new Set(allRows.map(row => row[OurLogKnownFieldKey.ID]));
79-
const pinnedIds = logsPinning?.getPinnedRowIds() ?? [];
80-
return pinnedIds.filter(id => !allRowIds.has(id));
81-
}, [logsPinning, allRows]);
82-
}
83-
84-
function usePinnedLogFetcher(
85-
missingIds: string[],
86-
fields: string[],
87-
logsPinning: LogsPinning | undefined
88-
) {
89-
const organization = useOrganization();
90-
const {selection, isReady: pageFiltersReady} = usePageFilters();
9161

9262
const baseQuery = useMemo(
9363
() => ({
@@ -105,100 +75,93 @@ function usePinnedLogFetcher(
10575
[selection.datetime]
10676
);
10777

108-
const driver = useQuery({
78+
const query = useQuery({
10979
queryKey: [
110-
DRIVER_QUERY_KEY,
80+
PINNED_LOGS_QUERY_KEY,
11181
{
11282
organizationSlug: organization.slug,
11383
ids: [...missingIds].sort(),
114-
fields,
11584
baseQuery,
11685
dateParams: inRangeDateParams,
11786
},
11887
],
11988
enabled: pageFiltersReady && !!logsPinning && missingIds.length > 0,
120-
staleTime: 0,
89+
staleTime: Infinity,
90+
placeholderData: keepPreviousData,
12191
queryFn: context =>
122-
fetchAndCachePinnedLogs(context, {
92+
fetchPinnedLogs(context, {
12393
ids: missingIds,
12494
organizationSlug: organization.slug,
12595
baseQuery,
12696
inRangeDateParams,
127-
fields,
12897
}),
12998
});
13099

131-
const notFoundIds = driver.data;
132-
const removePinnedRows = logsPinning?.removePinnedRows;
100+
const missingIdSet = useMemo(() => new Set(missingIds), [missingIds]);
101+
102+
const notFoundIds = query.data?.notFoundIds;
133103
useEffect(() => {
134-
if (removePinnedRows && notFoundIds?.length) {
135-
removePinnedRows(notFoundIds);
104+
if (!logsPinning?.removePinnedRows || !notFoundIds?.length) {
105+
return;
106+
}
107+
const idsToRemove = notFoundIds.filter(id => missingIdSet.has(id));
108+
if (idsToRemove.length) {
109+
logsPinning?.removePinnedRows(idsToRemove);
136110
}
137-
}, [notFoundIds, removePinnedRows]);
111+
}, [logsPinning, missingIdSet, notFoundIds]);
112+
113+
const fetchedRows = useMemo(
114+
() =>
115+
(query.data?.rows ?? []).filter(row =>
116+
missingIdSet.has(row[OurLogKnownFieldKey.ID])
117+
),
118+
[query.data, missingIdSet]
119+
);
120+
const resolvedIds = useMemo(
121+
() => new Set(fetchedRows.map(row => row[OurLogKnownFieldKey.ID])),
122+
[fetchedRows]
123+
);
138124

139125
return {
140-
isFetching: driver.fetchStatus === 'fetching',
141-
isError: driver.isError,
142-
refetch: driver.refetch,
126+
fetchedRows,
127+
isPending: query.isFetching && missingIds.some(id => !resolvedIds.has(id)),
128+
isError: query.isError,
129+
refetch: query.refetch,
143130
};
144131
}
145132

146-
function useCachedPinnedLogRows(missingIds: string[], fields: string[]) {
147-
const combine = useCallback(
148-
(results: Array<{data: unknown}>) => {
149-
const rows: OurLogsResponseItem[] = [];
150-
const resolvedIds = new Set<string>();
151-
results.forEach((result, index) => {
152-
const row = result.data as OurLogsResponseItem | undefined;
153-
if (row) {
154-
rows.push(row);
155-
resolvedIds.add(missingIds[index]!);
156-
}
157-
});
158-
return {rows, resolvedIds};
159-
},
160-
[missingIds]
161-
);
162-
163-
return useQueries({
164-
queries: missingIds.map(id => ({
165-
queryKey: pinnedLogRowQueryKey(id, fields),
166-
queryFn: skipToken,
167-
staleTime: Infinity,
168-
})),
169-
combine,
170-
});
133+
function useMissingPinnedLogIds(
134+
allRows: LogTableRowItem[],
135+
logsPinning: LogsPinning | undefined
136+
) {
137+
return useMemo(() => {
138+
const allRowIds = new Set(allRows.map(row => row[OurLogKnownFieldKey.ID]));
139+
const pinnedIds = logsPinning?.getPinnedRowIds() ?? [];
140+
return pinnedIds.filter(id => !allRowIds.has(id));
141+
}, [logsPinning, allRows]);
171142
}
172143

173-
type FetchAndCacheContext = Pick<QueryFunctionContext, 'signal' | 'meta'> & {
144+
type FetchContext = Pick<QueryFunctionContext, 'signal' | 'meta'> & {
174145
client: QueryClient;
175146
};
176147

177-
interface FetchAndCacheOptions {
148+
interface FetchOptions {
178149
baseQuery: Record<string, unknown>;
179-
fields: string[];
180150
ids: string[];
181151
inRangeDateParams: Record<string, unknown>;
182152
organizationSlug: string;
183153
}
184154

185-
async function fetchAndCachePinnedLogs(
186-
{client, signal, meta}: FetchAndCacheContext,
187-
{ids, organizationSlug, baseQuery, inRangeDateParams, fields}: FetchAndCacheOptions
188-
): Promise<string[]> {
189-
const idsToFetch = ids.filter(
190-
id => !client.getQueryData(pinnedLogRowQueryKey(id, fields))
191-
);
192-
if (idsToFetch.length === 0) {
193-
return [];
194-
}
195-
155+
async function fetchPinnedLogs(
156+
{client, signal, meta}: FetchContext,
157+
{ids, organizationSlug, baseQuery, inRangeDateParams}: FetchOptions
158+
): Promise<PinnedLogsResult> {
196159
const url = getApiUrl('/organizations/$organizationIdOrSlug/events/', {
197160
path: {organizationIdOrSlug: organizationSlug},
198161
});
199162

200-
const fetchByIds = (idsForFetch: string[], dateParams: Record<string, unknown>) => {
201-
return apiFetch<EventsLogsResult>({
163+
const fetchByIds = (idsForFetch: string[], dateParams: Record<string, unknown>) =>
164+
apiFetch<EventsLogsResult>({
202165
client,
203166
signal,
204167
meta,
@@ -215,36 +178,43 @@ async function fetchAndCachePinnedLogs(
215178
{infinite: false},
216179
],
217180
});
181+
182+
const rowsById = new Map<string, OurLogsResponseItem>();
183+
const collect = (result: EventsLogsResult) => {
184+
const found = new Set<string>();
185+
for (const row of result.data) {
186+
const id = row[OurLogKnownFieldKey.ID];
187+
rowsById.set(id, row);
188+
found.add(id);
189+
}
190+
return found;
218191
};
219192

220193
// Step 1: Search in the parent selected range for pins that are not loaded yet.
221194
// Start with this smaller range so we don't have to scan the org's full retention period.
222195
let foundInRange = new Set<string>();
223196
try {
224-
foundInRange = seedAndCollect(
225-
client,
226-
(await fetchByIds(idsToFetch, inRangeDateParams)).json,
227-
fields
228-
);
197+
foundInRange = collect((await fetchByIds(ids, inRangeDateParams)).json);
229198
} catch {
230199
// The selected range failed; let the wide window resolve everything instead.
231200
}
232201

233-
const stillMissing = idsToFetch.filter(id => !foundInRange.has(id));
202+
const stillMissing = ids.filter(id => !foundInRange.has(id));
234203
if (stillMissing.length === 0) {
235-
return [];
204+
return {rows: [...rowsById.values()], notFoundIds: []};
236205
}
237206

238207
// Step 2: Any IDs not found in the parent selected range escalate to a wider window.
239208
const wide = await fetchByIds(stillMissing, wideDateParams(stillMissing));
240-
const foundWide = seedAndCollect(client, wide.json, fields);
209+
const foundWide = collect(wide.json);
241210

242-
// A partial scan didn't prove the unfound ids absent, so don't unpin them.
243-
if (wide.json.meta?.dataScanned === 'partial') {
244-
return [];
245-
}
211+
const notFoundIds =
212+
wide.json.meta?.dataScanned === 'partial'
213+
? // A partial scan didn't prove the unfound ids absent, so don't unpin them.
214+
[]
215+
: stillMissing.filter(id => !foundWide.has(id));
246216

247-
return stillMissing.filter(id => !foundWide.has(id));
217+
return {rows: [...rowsById.values()], notFoundIds};
248218
}
249219

250220
/**
@@ -264,19 +234,3 @@ function wideDateParams(ids: string[]): Record<string, unknown> {
264234
end: getUtcDateString(moment(Math.max(...decoded) + WINDOW_BUFFER_MS)),
265235
};
266236
}
267-
268-
const seedAndCollect = (
269-
client: QueryClient,
270-
result: EventsLogsResult,
271-
fields: string[]
272-
) => {
273-
const foundIds = new Set<string>();
274-
275-
for (const row of result.data) {
276-
const id = row[OurLogKnownFieldKey.ID];
277-
client.setQueryData(pinnedLogRowQueryKey(id, fields), row);
278-
foundIds.add(id);
279-
}
280-
281-
return foundIds;
282-
};

0 commit comments

Comments
 (0)