feat: add query_logs tool for custom log queries - #333
Conversation
Adds a query_logs debugging tool that runs a custom read-only ClickHouse SQL query against a project's unified logs stream, for cases where the get_logs service presets are too coarse. Reuses the existing analytics logs endpoint and validates that queries are SELECT/WITH only.
barryroodt
left a comment
There was a problem hiding this comment.
Really clean addition, Jordi. It mirrors the merged get_logs sibling almost exactly (same endpoint, same untrusted-data wrapping, same 24h defaulting), and I reproduced the checks against the PR head: tsc --noEmit, biome ci ., and the server.test.ts + logs.test.ts suites (111 tests) all pass. Nothing here blocks merge. A couple of small things worth a look:
The sql description is missing function_edge_logs. The source list guides the model on what to filter by, and right now it lists function_logs (edge-function runtime console output) but not function_edge_logs (the invocation/request logs the get_logs edge-function preset queries, see logs.ts:37). Since that description is effectively the contract the model writes SQL against, a model following it can't reach invocation logs and will filter on the wrong source. Everything else in the list looks right.
query_logs has no execution test yet. The test change adds it to the tool-listing assertion, which is exactly right for registration, but the new timestamp-defaulting / passthrough / wrapping logic in execute is the bug-prone part and it's currently untested (whereas get_logs has three). server.test.ts:1567 is a ready-made template and the existing /endpoints/logs mock already covers it, so no new mock needed. Happy to leave this as a fast-follow if you'd rather not expand scope here.
Tiny nit: sql: z.string() could take .min(1) to match executeSqlOptionsSchema.query, so an empty query gives a clear validation message instead of an opaque backend error. Very much a nice-to-have.
None of this is load-bearing for shipping. The function_edge_logs line is the one I'd genuinely want fixed before merge; the rest are optional.
- add function_edge_logs to the sql source-hint list so models can reach edge function invocation logs - require a non-empty sql query (.min(1)), matching execute_sql - add execution tests for query_logs: sql passthrough + timestamp defaulting, custom window forwarding, and empty-query rejection
|
Thanks for the thorough review! Addressed all three in f37cfc5:
🤖 Addressed by Claude Code |
barryroodt
left a comment
There was a problem hiding this comment.
Thanks for the quick turnaround @jordienr
function_edge_logsadded to the source list.min(1)on the schema- behavioral tests for
query_logs
LGTM
@mattrossman, @Rodriguespn - since I'm still the new guy, perhaps a quick scan and thumbs-up from either of you would be advisable 😁
commit: |
|
on further thought, let's leave both tools and potentially deprecate and remove getLogs in favour of queryLogs. |
- mark get_logs as deprecated on hosted projects in favour of query_logs, while keeping it as the path for CLI/self-hosted - document that query_logs (ClickHouse) is hosted-only and will not work on CLI/self-hosted yet
|
Addressed the deprecation + platform-availability guidance in a4d7228:
🤖 Addressed by Claude Code |
|
Following up with fresh numbers against the current head (ecc456d) Rerun of our eval A/B against the new head: passed all 3 eval checks on the first attempt, and the a4d7228 deprecation note fully flipped tool selection, 8 One finding: the 24 hour default still silently widens narrow questions. The eval asks about the last 15 minutes; all 8 Suggestion: same mechanism as a4d7228 - make the line directive, something like "when the user asks about a specific time range, always pass iso_timestamp_start/iso_timestamp_end to match it". Unrelated side effect: this exercise surfaced drift in our own evals fixture, fixed in supabase/evals#99. |
The permissive mention of iso_timestamp_start/iso_timestamp_end wasn't steering model behaviour, so narrow time-range questions silently inherited the 24h default and over-counted. Make it a directive instruction in both get_logs and query_logs, matching the mechanism that flipped tool selection.
|
Thanks for the fresh eval run — great to see the deprecation note flip selection cleanly, and good catch on the time-range default. Fixed in ea6d163: the timestamp guidance is now a directive rather than a permissive mention, using the same mechanism as the deprecation line. Both
Applied to 🤖 Addressed by Claude Code |
|
Nice! Reran the A/B against ea6d163 and the directive line does exactly what we hoped: the model now passes an explicit 15 minute window matching the question on every analytical |
The description promises iso_timestamp_start defaults to 24h before the end, but the handler always computed start from now(), so supplying only iso_timestamp_end produced an inverted/empty window. Derive the end first (supplied or now), then default start to end - 24h, shared by get_logs and query_logs.
…s split this committ assume we will have merged or should be merged to gether with cahnges on skills and MCP - supabase/agent-skills#112 - supabase/mcp#333 query_logs (hosted only, ClickHouse SQL) is now the preferred tool for production projects; get_logs remains the only option for local and self-hosted. Updates the MCP tools table, log-querying section, and skill step 3 in ai-agents.mdx, and the tip admonition in debugging.mdx. logs.mdx already had this right at line 321.
What
Adds a new
query_logstool to thedebuggingfeature group. It runs a custom ClickHouse SQL query against a project's unified logs stream, for cases where theget_logsservice presets are too coarse (filtering, aggregating, or joining across log fields).Tracks AI-701. Builds on the ClickHouse logs endpoint work in O11Y-1813 and the
get_logsClickHouse migration (#326).How it addresses the observability team's concerns
execute_sql), so the result is wrapped inwrapWithUntrustedDataBoundary, the same best-effort guardrailexecute_sqluses. Not foolproof, but consistent with the existing arbitrary-query tool.GET /v1/projects/{ref}/analytics/endpoints/logs(ClickHouse / logs.all.otel) and takes raw ClickHouse-dialect SQL. Does not touch the deprecated BigQuery-backedlogs.all.GETwithsqlas a query param. No new POST handler.get_logs(feat: support edge function runtime logs in get_logs #326) under the samedebugginggroup, so it inherits identical platform availability and introduces no new exposure on CLI/self-hosted.Details
query_logsparams:project_id,sql, optionaliso_timestamp_start/iso_timestamp_end. Defaults to the last 24h window (matchingget_logs).debugginggroup.Verification
tsc --noEmitclean,biome ciclean.logs.test.ts,server.test.ts). The 5 failing checks are|e2e|tests requiring liveSUPABASE_ACCESS_TOKEN/ANTHROPIC_API_KEY, which fork PRs don't receive — they pass when run in the upstream context with secrets.Note
Per CONTRIBUTING, feature PRs should track an accepted issue — this tracks AI-701.