Add log_upload config option to configure log upload separately from local app log level#681
Add log_upload config option to configure log upload separately from local app log level#681lmagyar wants to merge 4 commits into
Conversation
…local app log level
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThis PR adds a new ChangesTailscale log upload control
Estimated code review effort: 2 (Simple) | ~12 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tailscale/DOCS.md (1)
229-233: ⚡ Quick winAdd a one-line “when required” hint for
log_upload.Consider explicitly noting that users must set
log_upload: truewhen Tailscale Admin Console network logs are enabled, to avoid tailscaled startup refusal. This directly connects the option to the real failure mode users hit.Suggested doc tweak
### Option: `log_upload` Controls Tailscale's client log upload to log.tailscale.com. +Enable this if your tailnet policy/Admin Console requires client log upload; otherwise tailscaled can refuse to start. This option is disabled by default.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tailscale/DOCS.md` around lines 229 - 233, Add a one-line “when required” hint to the Option: `log_upload` section explaining that users must set log_upload: true when enabling Tailscale Admin Console network logs (otherwise tailscaled may refuse to start); update the `log_upload` docs to include this explicit note referencing the option name `log_upload` and the daemon `tailscaled` so users see the direct connection to the startup failure mode.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tailscale/DOCS.md`:
- Around line 229-233: Add a one-line “when required” hint to the Option:
`log_upload` section explaining that users must set log_upload: true when
enabling Tailscale Admin Console network logs (otherwise tailscaled may refuse
to start); update the `log_upload` docs to include this explicit note
referencing the option name `log_upload` and the daemon `tailscaled` so users
see the direct connection to the startup failure mode.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 95d4338a-88bc-46b5-ad3b-7b8c8d1aec2f
📒 Files selected for processing (5)
tailscale/DOCS.mdtailscale/config.yamltailscale/rootfs/etc/s6-overlay/s6-rc.d/init-magicdns-proxies-upstream-list/runtailscale/rootfs/etc/s6-overlay/s6-rc.d/tailscaled/runtailscale/translations/en.yaml
|
Another data point in favor of merging this: the control-plane rejection can strand a node even when network flow logs are off in the admin console. One of my nodes (Home Assistant OS, add-on v0.28.1) sat unreachable for 11 days with: The admin console's Logs → Network flow logs page showed "Not logging" the entire time I was debugging. Every add-on restart re-registered cleanly ( Because the flag is hardcoded, the local fix was patching it out of the image with Is there anything blocking this from landing in a release? Thank you for maintaining the add-on, and for this fix. |
Currently you can change |
Proposed Changes
It was previously an ugly solution: to enable log upload, users should also enable detailed debug level logging locally.
Related Issues
fixes #674
Summary by CodeRabbit
New Features
log.tailscale.com(disabled by default).Documentation
Behavior Updates