Skip to content

[management] cleanup logs#4933

Merged
pascal-fischer merged 3 commits intomainfrom
chore/cleanup-logs
Dec 10, 2025
Merged

[management] cleanup logs#4933
pascal-fischer merged 3 commits intomainfrom
chore/cleanup-logs

Conversation

@pascal-fischer
Copy link
Copy Markdown
Collaborator

@pascal-fischer pascal-fischer commented Dec 10, 2025

Describe your changes

Issue ticket number and link

Stack

Checklist

  • Is it a bug fix
  • Is a typo/documentation fix
  • Is a feature enhancement
  • It is a refactor
  • Created tests that fail without the change (if possible)

By submitting this pull request, you confirm that you have read and agree to the terms of the Contributor License Agreement.

Documentation

Select exactly one:

  • I added/updated documentation for this change
  • Documentation is not needed for this change (explain why)

Docs PR URL (required if "docs added" is checked)

Paste the PR link from https://github.com/netbirdio/docs here:

https://github.com/netbirdio/docs/pull/__

Summary by CodeRabbit

  • Chores
    • Reduced log verbosity and removed redundant timing instrumentation across server and service components, lowering noise from lifecycle and performance logs.
    • Simplified internal context/debugging instrumentation by removing extra debug-context wrappers and related timing/logging, preserving functional behavior.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 10, 2025

Walkthrough

This PR removes timing instrumentation and reduces logging verbosity across multiple management server files, downgrading several debug logs to trace level or eliminating them entirely, while preserving all functional logic and error handling.

Changes

Cohort / File(s) Summary
Timing instrumentation removal
management/internals/controllers/network_map/controller/controller.go, management/internals/shared/grpc/server.go, management/server/peer.go, management/server/settings/manager.go
Removes start-time captures and duration-logging statements from GetValidatedPeerWithMap, multiple gRPC server methods (GetServerKey, Sync, Login, sendInitialSync, etc.), LoginPeer, and GetSettings. Control flow and error handling unchanged.
Log level downgrades (Debug → Trace / Trace → Debug)
management/internals/shared/grpc/token_mgr.go, management/server/http/middleware/auth_middleware.go, management/server/posture/os_version.go, management/server/peer.go
Lowers verbosity by switching some Debugf to Tracef (and one Tracef to Debugf for peer disconnection). No logic or error-handling changes.
Context/debugging simplification
management/server/store/sql_store.go
Removes getDebuggingCtx and nbcontext usage; replaces timed debugging-context wrappers with direct use of the provided ctx (e.g., WithContext). Deletes the helper function and related cancellation/timeouts; adjusts a log level to Tracef.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Inspect management/server/store/sql_store.go for any lost context propagation (request/account IDs) that callers expect.
  • Verify gRPC methods in management/internals/shared/grpc/server.go still include required trace/debug information for production diagnostics where needed.
  • Confirm removal of timing logs does not affect any external monitoring/telemetry expectations.

Possibly related PRs

Suggested reviewers

  • crn4
  • pappz

Poem

🐰 Logs grow quiet, timers rest,

I hopped through code and did my best.
Trace the whispers, Debug steps light,
Fewer echoes in the night.
A tidy burrow—soft and bright. 🥕

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description follows the template structure but is largely incomplete: the 'Describe your changes' section is empty, no issue ticket is provided, and the documentation explanation is missing. Fill in the 'Describe your changes' section with details about log cleanup, provide an issue ticket number/link if applicable, and explain why documentation is not needed.
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title '[management] cleanup logs' is concise and clearly indicates the main change: removing or reducing logging statements across management-related files.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/cleanup-logs

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7682303 and 7923962.

📒 Files selected for processing (1)
  • management/server/settings/manager.go (0 hunks)
💤 Files with no reviewable changes (1)
  • management/server/settings/manager.go
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
  • GitHub Check: Client / Unit
  • GitHub Check: Windows
  • GitHub Check: Linux
  • GitHub Check: Darwin
  • GitHub Check: release_ui
  • GitHub Check: release
  • GitHub Check: release_ui_darwin
  • GitHub Check: Client / Unit
  • GitHub Check: Client / Unit
  • GitHub Check: Android / Build
  • GitHub Check: Build Cache
  • GitHub Check: JS / Lint

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.

crn4
crn4 previously approved these changes Dec 10, 2025
@sonarqubecloud
Copy link
Copy Markdown

@pascal-fischer pascal-fischer merged commit 44851e0 into main Dec 10, 2025
39 checks passed
@pascal-fischer pascal-fischer deleted the chore/cleanup-logs branch December 10, 2025 18:26
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.

2 participants