Skip to content

Add info-level logging to all MCP tools#250

Merged
RafaelPo merged 6 commits intomainfrom
chore/tool-logging
Feb 27, 2026
Merged

Add info-level logging to all MCP tools#250
RafaelPo merged 6 commits intomainfrom
chore/tool-logging

Conversation

@RafaelPo
Copy link
Contributor

Summary

  • Add info-level logging to all MCP tools that previously only logged on error
  • Add happy-path logging to upload proxy (upload_id, size, artifact_id)
  • Every tool now logs on entry (with key params) and on success (with outcomes)

Tools that gained logging:

Tool Entry log Success log
everyrow_browse_lists search, category result count
everyrow_use_list artifact_id artifact_id, rows
everyrow_progress task_id -
everyrow_results task_id, offset, page_size -
everyrow_list_sessions offset, limit -
everyrow_balance - balance amount
everyrow_cancel task_id -
upload request upload_id, filename -
upload proxy upload_id, size status, artifact_id

Test plan

  • All 401 tests pass
  • Ruff + pyright clean
  • Deploy to staging and verify logs appear in Cloud Logging

🤖 Generated with Claude Code

@RafaelPo
Copy link
Contributor Author

@claude code review

@github-actions
Copy link
Contributor

github-actions bot commented Feb 27, 2026

Claude finished @RafaelPo's task in 48s —— View job


Code Review: Add info-level logging to all MCP tools

  • Read changed files and gather context
  • Review tools.py changes
  • Review uploads.py changes
  • Post review summary

Verdict: Looks good — clean, consistent, and useful for observability. A few minor notes below.

tools.py

The new log lines follow the existing pattern (tool_name: key=value) established by the pre-existing tools (e.g. everyrow_rank, everyrow_single_agent). Good consistency.

  1. everyrow_progress uses debug while all others use info (line 1007) — this is intentional per the commit message ("Use debug-level logging for progress polling, info only on terminal"), and makes sense to avoid log spam from frequent polling. The terminal-state info log at line 1049 is a nice touch.

  2. everyrow_balance logs the dollar amount (line 1297: logger.info("everyrow_balance: $%.2f", ...)). This is borderline — balance info isn't a secret per se, but it is business-sensitive data showing up in structured logs. Worth considering whether this should be debug level instead, or if you're comfortable with it at info given your log access controls. Not a blocker.

  3. No entry log for everyrow_balance — the "called" log at line 1281 is fine, but it's the only tool that doesn't log any parameters on entry (since it takes none). Just noting for completeness; this is correct.

uploads.py

  1. Good refactor: extracting upload_id = data["upload_id"] to a local variable (line 122) before using it in both the result dict and the log line. Clean.

  2. resp.text[:200] in the warning log (line 240) — nice defensive truncation to avoid log bloat from large error bodies.

  3. Bare except Exception: pass (lines 248-249) when parsing artifact_id from the response — this is pragmatic since the log is purely informational. If the response isn't JSON or lacks the key, artifact_id stays None and the log still works. Acceptable for a non-critical logging path.

Overall

No bugs, no security issues, no behavioral changes. The logging is consistent with the existing patterns in the codebase and will meaningfully improve observability in production. LGTM.


RafaelPo and others added 6 commits February 27, 2026 22:54
- Log entry params for browse_lists, use_list, progress, results,
  list_sessions, balance, and cancel (previously only logged on error)
- Log success outcomes (result counts, artifact_ids, balance amounts)
- Log upload_id + filename on presigned URL request
- Log upload_id + size on proxy start, artifact_id on proxy completion
- Log proxy error responses with status and body

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Avoids log noise from the tight polling loop (~every 3s). Only logs
at INFO when the task reaches a terminal state (completed/failed/revoked).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Uses a module-level set to track which task_ids are being polled.
Logs at INFO on the first call ("polling started") and when the task
reaches a terminal state. Cleans up the set on terminal so re-polling
after a retry still logs correctly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The in-memory set doesn't work with multiple replicas — different
pods don't share state. Instead: log every poll at DEBUG level
(invisible at default INFO), log at INFO only on terminal state.
Task submission already logs when polling starts implicitly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
_fetch_task_result returns (df, session_id, artifact_id) but the call
site only unpacked 2 values, causing ValueError at runtime every time
everyrow_use_list was called.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
New tool added on main was missing info-level logging.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@RafaelPo RafaelPo merged commit 3a5b346 into main Feb 27, 2026
5 checks passed
@RafaelPo RafaelPo deleted the chore/tool-logging branch February 27, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant