mirror of
https://github.com/supabase/supabase.git
synced 2026-10-18 15:49:09 +03:00
fix(logs): bound the OTEL single-log lookup by source and timestamp
The Logs Explorer's single-log lookup (`genSingleLogQueryOtel`, used by `useSingleLog` when a row is selected) matched on `id` alone and ran over the previewer's whole selected range. `id` is not part of the ClickHouse logs table's sort key, so every row click scanned the entire window while materializing `log_attributes` — the widest column in the table by a wide margin. Two bounds, both already available client-side: - `AND source = '...'`, derived from the table the previewer is showing. `source` is the second component of the sort key, so it narrows the sorted range before the time bound applies. - A ±1 minute `iso_timestamp_*` bracket around the selected row's own timestamp, read from the page already in memory. Falls back to the search range when unknown (a deep-linked `?log=<id>` that isn't in a loaded page). This applies the fix #47978 already made for the unified-logs inspection query to the legacy previewer, which was missed at the time — same reasoning, same ±1 minute window, same source-filter rationale. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
cf36ad9e52
commit
cef32c0cd2
5 files changed
+121
-10
No files matched your search
@@ -535,8 +535,8 @@ describe('genChartQueryOtel', () => {
|
||||
})
|
||||
|
||||
describe('genSingleLogQueryOtel', () => {
|
||||
it('fetches a single row by id with raw attributes', () => {
|
||||
expect(fmt(genSingleLogQueryOtel('123e4567-e89b-12d3-a456-426614174000')))
|
||||
it('fetches a single row by id with raw attributes, narrowed by source', () => {
|
||||
expect(fmt(genSingleLogQueryOtel(LogsTableName.EDGE, '123e4567-e89b-12d3-a456-426614174000')))
|
||||
.toMatchInlineSnapshot(`
|
||||
"-- Single Log Query (otel)
|
||||
select
|
||||
@@ -550,13 +550,25 @@ describe('genSingleLogQueryOtel', () => {
|
||||
logs
|
||||
where
|
||||
id = '123e4567-e89b-12d3-a456-426614174000'
|
||||
and source = 'edge_logs'
|
||||
limit
|
||||
1"
|
||||
`)
|
||||
})
|
||||
|
||||
// `id` isn't in the table's sort key, so the `source` predicate is what
|
||||
// narrows the scan — it must track the table, not be hardcoded.
|
||||
it('emits the source for the requested table', () => {
|
||||
expect(
|
||||
fmt(genSingleLogQueryOtel(LogsTableName.AUTH, '123e4567-e89b-12d3-a456-426614174000'))
|
||||
).toContain("source = 'auth_logs'")
|
||||
expect(
|
||||
fmt(genSingleLogQueryOtel(LogsTableName.PG_CRON, '123e4567-e89b-12d3-a456-426614174000'))
|
||||
).toContain("source = 'postgres_logs'")
|
||||
})
|
||||
|
||||
it('rejects a non-uuid id', () => {
|
||||
expect(() => genSingleLogQueryOtel("1' OR '1'='1")).toThrow('Invalid logId')
|
||||
expect(() => genSingleLogQueryOtel(LogsTableName.EDGE, "1' OR '1'='1")).toThrow('Invalid logId')
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -311,14 +311,21 @@ ORDER BY timestamp ASC`
|
||||
}
|
||||
|
||||
// Reject non-uuid ids before putting them in the query (injection guard).
|
||||
export const genSingleLogQueryOtel = (id: string): SafeLogSqlFragment => {
|
||||
//
|
||||
// `id` is not part of the table's sort key, so a lookup on it alone scans the
|
||||
// whole selected time window while materializing `log_attributes` — by far the
|
||||
// widest column. `source` is the second sort-key component, so filtering on it
|
||||
// narrows the sorted range before the time bound even applies. The caller pairs
|
||||
// this with a tight `iso_timestamp_*` bracket around the row's own timestamp
|
||||
// (see `useSingleLog`), mirroring `getUnifiedLogInspection`.
|
||||
export const genSingleLogQueryOtel = (table: LogsTableName, id: string): SafeLogSqlFragment => {
|
||||
if (!/^[0-9a-fA-F-]{1,64}$/.test(id)) {
|
||||
throw new Error('Invalid logId')
|
||||
}
|
||||
return safeSql`-- Single Log Query (otel)
|
||||
SELECT id, timestamp, event_message, source, severity_text, log_attributes
|
||||
FROM logs
|
||||
WHERE id = ${lit(id)}
|
||||
WHERE id = ${lit(id)} AND source = ${lit(OTEL_SOURCES[table].source)}
|
||||
LIMIT 1`
|
||||
}
|
||||
|
||||
|
||||
@@ -175,6 +175,12 @@ export const LogsPreviewer = ({
|
||||
refresh,
|
||||
} = useLogsPreview({ projectRef, table, filterOverride })
|
||||
|
||||
// The selected row is already in the loaded pages, so its timestamp is known
|
||||
// without another request — it bounds the single-log lookup to a tight window
|
||||
// instead of the whole selected range. Undefined for a deep-linked log that
|
||||
// isn't in a loaded page, which falls back to the search range.
|
||||
const selectedLogTimestamp = logData.find((x) => x.id === selectedLogId)?.timestamp
|
||||
|
||||
const {
|
||||
data: selectedLog,
|
||||
isLoading: isSelectedLogLoading,
|
||||
@@ -184,6 +190,7 @@ export const LogsPreviewer = ({
|
||||
id: selectedLogId ?? undefined,
|
||||
queryType,
|
||||
paramsToMerge: params,
|
||||
logTimestampMicros: selectedLogTimestamp,
|
||||
})
|
||||
|
||||
const { showUpgradePrompt, setShowUpgradePrompt } = useUpgradePrompt(timestampStart)
|
||||
|
||||
@@ -0,0 +1,40 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
import { resolveSingleLogWindow } from './useSingleLog'
|
||||
|
||||
// A single log is looked up by `id`, which is not part of the logs table's sort
|
||||
// key — the time bound is what stops the query scanning the whole selected
|
||||
// range, so these bounds are load-bearing for query cost, not just correctness.
|
||||
describe('resolveSingleLogWindow', () => {
|
||||
const searchRange = {
|
||||
iso_timestamp_start: '2026-08-01T00:00:00.000Z',
|
||||
iso_timestamp_end: '2026-08-08T00:00:00.000Z',
|
||||
}
|
||||
|
||||
it('brackets ±1 minute around the row timestamp, ignoring the wider search range', () => {
|
||||
const timestampMicros = Date.UTC(2026, 7, 5, 12, 30, 0) * 1000
|
||||
|
||||
expect(resolveSingleLogWindow(timestampMicros, searchRange)).toEqual({
|
||||
isoTimestampStart: '2026-08-05T12:29:00.000Z',
|
||||
isoTimestampEnd: '2026-08-05T12:31:00.000Z',
|
||||
})
|
||||
})
|
||||
|
||||
// A deep-linked log (?log=<id>) that isn't in a loaded page has no timestamp
|
||||
// client-side, so the previous whole-range behavior has to remain reachable.
|
||||
it('falls back to the search range when the timestamp is unknown', () => {
|
||||
for (const missing of [undefined, null, NaN]) {
|
||||
expect(resolveSingleLogWindow(missing, searchRange)).toEqual({
|
||||
isoTimestampStart: searchRange.iso_timestamp_start,
|
||||
isoTimestampEnd: searchRange.iso_timestamp_end,
|
||||
})
|
||||
}
|
||||
})
|
||||
|
||||
it('falls back to empty bounds when neither a timestamp nor a range is available', () => {
|
||||
expect(resolveSingleLogWindow(undefined, undefined)).toEqual({
|
||||
isoTimestampStart: '',
|
||||
isoTimestampEnd: '',
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -31,12 +31,50 @@ type SingleLogParams = {
|
||||
projectRef: string
|
||||
queryType?: QueryType
|
||||
paramsToMerge?: Partial<LogsEndpointParams>
|
||||
/**
|
||||
* The selected row's own timestamp (microseconds), when known. A single log is
|
||||
* always looked up by `id`, which is not part of the logs table's sort key, so
|
||||
* the query is bounded tightly around this timestamp instead of the
|
||||
* (potentially much wider) selected search range. Falls back to the search
|
||||
* range when unavailable — e.g. a deep-linked log that isn't in a loaded page.
|
||||
*/
|
||||
logTimestampMicros?: number | null
|
||||
}
|
||||
|
||||
// The row's timestamp is the exact stored value carried over from the row we
|
||||
// already fetched, not an approximate clock reading, so this only needs to
|
||||
// absorb rounding — not real clock skew. Mirrors `INSPECTION_WINDOW_MS` in
|
||||
// `data/logs/unified-log-inspection-query.ts`.
|
||||
const SINGLE_LOG_WINDOW_MS = 60 * 1000
|
||||
|
||||
/**
|
||||
* Resolves the time bounds for a single-log lookup: a tight window around the
|
||||
* row's own timestamp when it's known, otherwise the selected search range.
|
||||
*/
|
||||
export const resolveSingleLogWindow = (
|
||||
logTimestampMicros: number | null | undefined,
|
||||
paramsToMerge?: Partial<LogsEndpointParams>
|
||||
): { isoTimestampStart: string; isoTimestampEnd: string } => {
|
||||
if (typeof logTimestampMicros !== 'number' || !Number.isFinite(logTimestampMicros)) {
|
||||
return {
|
||||
isoTimestampStart: paramsToMerge?.iso_timestamp_start ?? '',
|
||||
isoTimestampEnd: paramsToMerge?.iso_timestamp_end ?? '',
|
||||
}
|
||||
}
|
||||
|
||||
const timestampMs = logTimestampMicros / 1000
|
||||
return {
|
||||
isoTimestampStart: new Date(timestampMs - SINGLE_LOG_WINDOW_MS).toISOString(),
|
||||
isoTimestampEnd: new Date(timestampMs + SINGLE_LOG_WINDOW_MS).toISOString(),
|
||||
}
|
||||
}
|
||||
|
||||
function useSingleLog({
|
||||
projectRef,
|
||||
id,
|
||||
queryType,
|
||||
paramsToMerge,
|
||||
logTimestampMicros,
|
||||
}: SingleLogParams): SingleLogHook {
|
||||
const table = queryType ? LOGS_TABLES[queryType] : undefined
|
||||
|
||||
@@ -48,7 +86,7 @@ function useSingleLog({
|
||||
if (!id || !table) return safeSql``
|
||||
if (useOtel) {
|
||||
try {
|
||||
return genSingleLogQueryOtel(id)
|
||||
return genSingleLogQueryOtel(table, id)
|
||||
} catch {
|
||||
// Malformed (non-uuid) id — emit nothing rather than throwing in render.
|
||||
return safeSql``
|
||||
@@ -57,6 +95,13 @@ function useSingleLog({
|
||||
return genSingleLogQuery(table, id)
|
||||
}, [id, table, useOtel])
|
||||
|
||||
// Bound the lookup to a tight window around the row's own timestamp, falling
|
||||
// back to the selected search range when it isn't known.
|
||||
const { isoTimestampStart, isoTimestampEnd } = useMemo(
|
||||
() => resolveSingleLogWindow(logTimestampMicros, paramsToMerge),
|
||||
[logTimestampMicros, paramsToMerge?.iso_timestamp_start, paramsToMerge?.iso_timestamp_end]
|
||||
)
|
||||
|
||||
const enabled = Boolean(id && table)
|
||||
|
||||
const { logsMetadata } = useIsFeatureEnabled(['logs:metadata'])
|
||||
@@ -77,8 +122,8 @@ function useSingleLog({
|
||||
'single-log',
|
||||
id,
|
||||
queryType,
|
||||
paramsToMerge?.iso_timestamp_start,
|
||||
paramsToMerge?.iso_timestamp_end,
|
||||
isoTimestampStart,
|
||||
isoTimestampEnd,
|
||||
{ otel: useOtel },
|
||||
],
|
||||
queryFn: async ({ signal }) => {
|
||||
@@ -86,8 +131,8 @@ function useSingleLog({
|
||||
projectRef,
|
||||
endpoint,
|
||||
sql,
|
||||
iso_timestamp_start: paramsToMerge?.iso_timestamp_start ?? '',
|
||||
iso_timestamp_end: paramsToMerge?.iso_timestamp_end ?? '',
|
||||
iso_timestamp_start: isoTimestampStart,
|
||||
iso_timestamp_end: isoTimestampEnd,
|
||||
method: 'get',
|
||||
signal,
|
||||
})
|
||||
|
||||
Reference in new issue
Block a user