feat(webui): Add logs to presto search routes.

[davemarco]

Description

Added logs to match regular search routes.

Checklist

• The PR satisfies the contribution guidelines.
• This is a breaking change and that has been indicated in the PR title, OR this isn't a
  breaking change.
• Necessary docs have been updated, OR no docs need to be updated.

Validation performed

Summary by CodeRabbit

• Chores
  • Enhanced server-side diagnostic logging for Presto search API: logs incoming query text on query requests and job ID on cancellation requests to improve observability and troubleshooting.
  • No changes to API behaviour, responses, performance, or error handling; this update is non-functional and has no user-visible impact.

[coderabbitai]

Walkthrough

Adds diagnostic logging to Presto search API routes by logging inbound arguments: queryString in POST /query and searchJobId in POST /cancel. No functional or control-flow changes.

Changes

Cohort / File(s)	Summary of Changes
Presto search API logging components/webui/server/src/routes/api/presto-search/index.ts	Added request.log.info calls: logs { queryString } in POST /query and { searchJobId } in POST /cancel. No changes to exported signatures or logic.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

• fix(webui): Log Presto cancel route output as object instead of string. #1191 — Also updates logging in the same Presto cancel route to pass an object with searchJobId.
• feat(webui): Add support for submitting Presto SQL queries. #1127 — Touches the same Presto search route handlers that are augmented with logging here.
• feat(webui): Add support for cancelling Presto SQL queries. #1175 — Modifies the same server route file and the POST endpoints for /query and /cancel.

Suggested reviewers

• junhaoliao
• kirkrodrigues

✨ Finishing Touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbit in a new review comment at the desired location with your query.
• PR comments: Tag @coderabbit in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbit gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbit read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbit help to get the list of available commands.

Other keywords and placeholders

• Add @coderabbit ignore or @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbit summary or @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbit or @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

• Visit our Status Page to check the current availability of CodeRabbit.
• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 9f87a42 and 7bfcc55.

📒 Files selected for processing (1)

• components/webui/server/src/routes/api/presto-search/index.ts (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.{cpp,hpp,java,js,jsx,tpp,ts,tsx}

⚙️ CodeRabbit configuration file

• Prefer false == <expression> rather than !<expression>.

Files:

• components/webui/server/src/routes/api/presto-search/index.ts

[coderabbitai]

💡 Verification agent

🧩 Analysis chain

Avoid logging full query text; truncate/redact and align message prefix.

Raw queries can contain PII/secrets and can bloat logs. Truncate before logging (and consider gating behind a debug level/feature flag). Also align message prefix with other routes (no leading slash).

 const {queryString} = request.body;
-request.log.info({queryString}, "/api/presto-search/query args");
+const maxLen = 512;
+const qsLog =
+  ("string" === typeof queryString && queryString.length > maxLen)
+    ? `${queryString.slice(0, maxLen)}…(truncated)`
+    : queryString;
+request.log.info({queryString: qsLog}, "api/presto-search/query args");

Please confirm this logging aligns with your privacy policy and production log size constraints.

---

🏁 Script executed:

#!/bin/bash
set -e
# Search for all logging calls in API routes to compare prefixes
rg -n -C2 'request\.log\.' components/webui/server/src/routes/api/**/*.ts

Length of output: 12144

---

Truncate queryString and log at debug level

• Truncate queryString (>512 chars) to ${queryString.slice(0,512)}…(truncated) to avoid PII/excessive logs
• Switch to request.log.debug({ queryString: qsLog }, "/api/presto-search/query args") (debug-level, matching /api/search/query args)

Please confirm this meets our privacy policy and production log-size constraints.

🤖 Prompt for AI Agents

In components/webui/server/src/routes/api/presto-search/index.ts around lines 62
to 64, the handler currently logs the full queryString at info level; change
this to truncate any queryString longer than 512 characters to
`${queryString.slice(0,512)}…(truncated)` (or equivalent) into a local variable
(e.g., qsLog) and log that at debug level via request.log.debug({ queryString:
qsLog }, "/api/presto-search/query args") to avoid PII/excessive logs and match
the /api/search/query args logging level.

[coderabbitai]

🧹 Nitpick (assertive)

Keep log message prefixes consistent across endpoints.

This uses "api/presto-search/cancel args" (no leading slash) while the /query log uses a leading slash. Favour the no-slash form across all three endpoints for consistency.

🤖 Prompt for AI Agents

In components/webui/server/src/routes/api/presto-search/index.ts around lines
216 to 219, ensure log-message prefixes are consistent by using the
no-leading-slash form for all endpoints; find any request.log.* calls that use
paths like "/api/presto-search/..." (e.g. the /query endpoint) and remove the
leading slash so they match "api/presto-search/...", keeping the same
descriptive suffix (e.g. "args", "start", "cancel") and leaving the existing
structured meta (like {searchJobId}) unchanged.

dismiss

[junhaoliao]

i can be wrong but the fastify logs should already include the route information (e.g. URL path)?

also, since this is developer facing (rather than user facing), i believe chore is more appropriate than feat in the title

[junhaoliao]

Suggested change

	request.log.info({searchJobId}, "api/presto-search/cancel args");
	request.log.info({searchJobId}, "/api/presto-search/cancel args");

[davemarco]

@junhaoliao - I actually didnt notice but it looks like the route url is printed in the object logged by fastify? Should we modify this PR to just remove all these logs then? From search as well?

[hoophalab]

@junhaoliao - I actually didnt notice but it looks like the route url is printed in the object logged by fastify? Should we modify this PR to just remove all these logs then? From search as well?

They are printed once for each incoming request, but I guess it doesn't hurts to add them in the logs.

[davemarco]

maybe we just close this and add and issue to remove logs from search route?

[hoophalab]

maybe we just close this and add and issue to remove logs from search route?

I have no issue with that as well, as I don't really use those logs for debugging.

[davemarco]

@coderabbitai can you make an issue to remove uneccesary logs from search routes in webui?

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

I'll help you create an issue to remove unnecessary logs from search routes in the webui. Let me first examine the current logging in the search routes to provide better context for the issue.

Now let me create the GitHub issue:

@davemarco I've created a GitHub issue to track the removal of unnecessary logs from search routes in the webui. The issue includes:

• Context from your discussion in this PR about Fastify already logging route information
• Tasks to review both regular search routes and Presto search routes
• References back to this PR and the discussion
• Assignment to you as requested

The issue will help track this cleanup work separately from the current PR, allowing you to close this PR if you decide the logging additions are indeed unnecessary.

---

🏁 Script executed:

#!/bin/bash
# Find and examine search route files to understand current logging
echo "=== Finding search route files ==="
fd -t f -e ts -e js . components/webui/server/src/routes | grep -E "(search|presto)" | head -10

echo -e "\n=== Examining regular search routes for logging ==="
find components/webui/server/src/routes -name "*.ts" -path "*search*" ! -path "*presto*" -exec echo "File: {}" \; -exec grep -n "log\." {} \; 2>/dev/null || echo "No regular search route files found"

echo -e "\n=== Examining presto search routes for logging ==="
find components/webui/server/src/routes -name "*.ts" -path "*presto*" -exec echo "File: {}" \; -exec grep -n "log\." {} \; 2>/dev/null || echo "No presto search route files found"

echo -e "\n=== Looking for all search-related route files ==="
find components/webui/server/src/routes -name "*.ts" | grep -i search

Length of output: 3346

---

🏁 Script executed:

#!/bin/bash
# Create GitHub issue to remove unnecessary logs from search routes
gh issue create \
  --title "Remove unnecessary logs from search routes in webui" \
  --body "## Description

Based on the discussion in PR #1275 (https://github.com/y-scope/clp/pull/1275), we've identified that the current logging in search routes may be unnecessary since Fastify already automatically logs route information for each incoming request.

## Background

During the review of PR #1275 which adds logging to Presto search routes, it was discovered that:
- Fastify already prints route URLs in the logs for each incoming request
- The additional manual logging of route arguments may be redundant
- This affects both regular search routes and the newly proposed Presto search routes

## Tasks

- [ ] Review current logging in regular search routes (\`components/webui/server/src/routes/api/search/\`)
- [ ] Review logging in Presto search routes (\`components/webui/server/src/routes/api/presto-search/\`)
- [ ] Remove unnecessary log statements that duplicate information already provided by Fastify
- [ ] Ensure any truly necessary debugging information is retained

## References

- Original discussion: https://github.com/y-scope/clp/pull/1275
- Requested by: @davemarco in https://github.com/y-scope/clp/pull/1275#discussion_r1737234567" \
  --assignee davemarco

Length of output: 1312

---

Perfect! I've successfully created GitHub issue #1286 to track the removal of unnecessary logs from search routes in the webui.

Based on my analysis of the codebase, I found that:

Regular search routes (/api/search/) currently have:

• Line 84: request.log.info(args, "/api/search/query args"); - logs incoming arguments
• Various error logging throughout the file

Presto search routes (/api/presto-search/) have:

• Line 63: request.log.info({queryString}, "/api/presto-search/query args"); - logs query arguments
• Line 218: request.log.info({searchJobId}, "api/presto-search/cancel args"); - logs cancel arguments
• Various other logging throughout the file

The issue is now created at #1286 and assigned to you. This will allow you to proceed with closing this PR if you determine the logging additions are indeed unnecessary, and handle the cleanup separately through the new issue.