Lokasi ngalangkungan proxy:   [ UP ]  
[Ngawartoskeun bug]   [Panyetelan cookie]                
Skip to content

chore(worker): Reduce logger verbosity#1179

Open
brendan-kellam wants to merge 3 commits intomainfrom
bkellam/improved-logging
Open

chore(worker): Reduce logger verbosity#1179
brendan-kellam wants to merge 3 commits intomainfrom
bkellam/improved-logging

Conversation

@brendan-kellam
Copy link
Copy Markdown
Contributor

@brendan-kellam brendan-kellam commented May 6, 2026

Summary by CodeRabbit

  • Changed
    • Reduced backend operational log verbosity by downgrading many non-critical messages to debug level across API startup, config updates, job scheduling, health checks, permission syncs, and repository indexing workflows.
    • Debug-level logs remain available for detailed troubleshooting while normal operation logs focus on significant events, reducing noise in standard logs.

@github-actions

This comment has been minimized.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 334e524d-5296-4a6c-9888-e0feceaa6d79

📥 Commits

Reviewing files that changed from the base of the PR and between 0a6a7c7 and 161d623.

📒 Files selected for processing (6)
  • packages/backend/src/azuredevops.ts
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/ee/repoPermissionSyncer.ts
  • packages/backend/src/ee/syncSearchContexts.ts
  • packages/backend/src/repoCompileUtils.ts
  • packages/backend/src/zoekt.ts
✅ Files skipped from review due to trivial changes (5)
  • packages/backend/src/ee/syncSearchContexts.ts
  • packages/backend/src/ee/accountPermissionSyncer.ts
  • packages/backend/src/azuredevops.ts
  • packages/backend/src/ee/repoPermissionSyncer.ts
  • packages/backend/src/repoCompileUtils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/backend/src/zoekt.ts

Walkthrough

Reduce logging verbosity by changing multiple backend and web informational logger.info(...) calls to logger.debug(...) across initialization, config/connection sync, job scheduling/execution, indexing, pruning, and health-check handlers; also add an Unreleased changelog entry documenting this change.

Changes

Log Level Downgrades

Layer / File(s) Summary
Documentation
CHANGELOG.md
Added [Unreleased]### Changed entry documenting the reduction of worker log verbosity (info → debug).
Initialization & Connectivity
packages/backend/src/api.ts, packages/backend/src/index.ts
API listen and Redis post-connection success logs changed from info to debug.
Configuration Management
packages/backend/src/configManager.ts
Config file update, connection sync job creation, and declarative connection deletion logs changed from info to debug.
Connection Scheduling & Execution
packages/backend/src/connectionManager.ts
Connection sync job scheduling, job start/run, repo upsert/delete summary logs changed from info to debug.
Permission & Search Sync Jobs
packages/backend/src/ee/accountPermissionSyncer.ts, packages/backend/src/ee/repoPermissionSyncer.ts, packages/backend/src/ee/syncSearchContexts.ts
Permission sync start/completion and search-context deletion logs downgraded from info to debug.
Audit Log Maintenance
packages/backend/src/ee/auditLogPruner.ts
Pruner scheduler startup, pruning cutoff announcement, and pruning outcome logs changed from info to debug.
Repository Indexing Workflows
packages/backend/src/repoIndexManager.ts, packages/backend/src/repoCompileUtils.ts
Indexing job start/completion, fetch/clone start and duration logs, indexing/cleanup logs, repo/shard deletions, and glob/repo-found summaries changed from info to debug; failure logs augmented with “Reason: …”.
Zoekt Subprocess & Cleanup
packages/backend/src/zoekt.ts
zoekt-git-index child-process stdout/stderr line logs and temp-shard cleanup logs changed from info to debug; cleanup warn-on-error behavior unchanged.
Azure DevOps Repo Fetching
packages/backend/src/azuredevops.ts
Repository fetch start and per-repo found logs downgraded from info to debug.
Health Check Endpoint
packages/web/src/app/api/(server)/health/route.ts
Health-check request log changed from info to debug.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • sourcebot-dev/sourcebot#973: Modifies repoIndexManager logging/cleanup behavior—overlaps with orphaned repo/index shard logging changes.
  • sourcebot-dev/sourcebot#860: Large worker/queue migration affecting connectionManager and repoIndexManager; potentially interacts with logging and job lifecycle code.
  • sourcebot-dev/sourcebot#600: Refactors permission syncer logic; touches the same account/permission sync files where logs were downgraded.

Suggested reviewers

  • msukkari
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main objective of the pull request—reducing logger verbosity by downgrading info-level logs to debug-level across multiple backend and worker components.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bkellam/improved-logging

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

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/backend/src/zoekt.ts`:
- Around line 43-47: The stderr output from the exec callback is being logged at
debug via logger.debug, which hides non-fatal warnings from zoekt-git-index;
change the stderr handling in the exec callback (the block that currently calls
stderr.split(...).forEach(line => logger.debug(line))) to log each non-empty
stderr line at warn (or info) level instead, include a short contextual prefix
like "zoekt-git-index:" or "zoekt index warning" in the message so it's clear in
logs, and otherwise keep the same splitting/filtering logic.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1ececb4a-c189-41e4-af07-5c8b4f25557e

📥 Commits

Reviewing files that changed from the base of the PR and between fdb05c0 and 0a6a7c7.

📒 Files selected for processing (9)
  • CHANGELOG.md
  • packages/backend/src/api.ts
  • packages/backend/src/configManager.ts
  • packages/backend/src/connectionManager.ts
  • packages/backend/src/ee/auditLogPruner.ts
  • packages/backend/src/index.ts
  • packages/backend/src/repoIndexManager.ts
  • packages/backend/src/zoekt.ts
  • packages/web/src/app/api/(server)/health/route.ts

Comment on lines 43 to 47
if (stderr) {
stderr.split('\n').filter(line => line.trim()).forEach(line => {
// TODO: logging as regular info here and not error because non error logs are being
// streamed in stderr and incorrectly being logged as errors at a high level
logger.info(line);
logger.debug(line);
});
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

stderr from zoekt-git-index should stay at warn (or info) to preserve observability of non-fatal indexing issues.

zoekt-git-index (like most CLI tools) emits non-fatal warnings — e.g., files skipped due to size, permission issues, branch resolution problems — to stderr while still exiting with code 0. The exec callback only rejects on a non-zero exit code (line 33), so the stderr log is the only production signal for these warnings. Downgrading it to debug makes them invisible at default log levels and will make diagnosing partial indexing failures harder.

🔧 Suggested fix: log stderr at `warn` level
         if (stderr) {
             stderr.split('\n').filter(line => line.trim()).forEach(line => {
-                logger.debug(line);
+                logger.warn(line);
             });
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (stderr) {
stderr.split('\n').filter(line => line.trim()).forEach(line => {
// TODO: logging as regular info here and not error because non error logs are being
// streamed in stderr and incorrectly being logged as errors at a high level
logger.info(line);
logger.debug(line);
});
}
if (stderr) {
stderr.split('\n').filter(line => line.trim()).forEach(line => {
logger.warn(line);
});
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/backend/src/zoekt.ts` around lines 43 - 47, The stderr output from
the exec callback is being logged at debug via logger.debug, which hides
non-fatal warnings from zoekt-git-index; change the stderr handling in the exec
callback (the block that currently calls stderr.split(...).forEach(line =>
logger.debug(line))) to log each non-empty stderr line at warn (or info) level
instead, include a short contextual prefix like "zoekt-git-index:" or "zoekt
index warning" in the message so it's clear in logs, and otherwise keep the same
splitting/filtering logic.

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