Drive logging policy from startup env and reduce noisy defaults#58
Merged
Conversation
WalkthroughThis PR unifies logging configuration across Transponder by replacing application-specific ChangesUnified RUST_LOG Configuration and Application Logging
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
deploy/transponder.service.example (1)
29-29: ⚡ Quick winUse
infoas the production default for thetranspondertarget.Setting
transponder=debugin the systemd production example conflicts with the PR goal of quieter defaults and can create avoidable log volume in normal operation. Prefertransponder=infohere and document debug as a temporary troubleshooting override.Proposed change
-Environment=RUST_LOG=info,transponder=debug,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn +Environment=RUST_LOG=info,transponder=info,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn🤖 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 `@deploy/transponder.service.example` at line 29, The systemd example sets RUST_LOG with the transponder target at debug causing noisy production logs; update the RUST_LOG environment value so the transponder target uses info instead of debug (change "transponder=debug" to "transponder=info" in the Environment line that defines RUST_LOG) and add a brief note that debug logging is available as a temporary troubleshooting override (e.g., instruct operators to set transponder=debug when needed).
🤖 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 `@compose.prod.yml`:
- Line 27: The production compose default RUST_LOG currently enables verbose
transponder=debug; update the RUST_LOG default in the compose prod env entry
(the RUST_LOG line) to remove the transponder=debug setting or set it to a
non-debug level (e.g., transponder=info) so production containers do not start
with verbose transponder debug logs enabled.
In `@src/push/apns.rs`:
- Line 12: The APNs push-result logs were promoted to info! but must stay at
debug level; locate the logging calls in src/push/apns.rs that currently use
info! for success/invalid-token outcomes (and any similar uses in lines
~258-281) and change them back to debug! (ensure the tracing import still
includes debug). While updating, remove any user-identifying data from the log
messages so they remain non-sensitive and follow the push logging policy.
In `@src/push/fcm.rs`:
- Line 13: The push-result logs in src/push/fcm.rs were upgraded to info! but
must remain at debug level and must not contain any user-identifying
information; locate the push-result logging sites in this file (the
success/not-found logs around the push/result handling and the block around
lines ~355-381) and change any info! (or other non-debug) invocations to debug!
while ensuring the logged messages contain only generic push/result info (no
user IDs, emails, or tokens); keep the existing tracing import usage (debug) and
update any related tests or comments if they assert on log level.
---
Nitpick comments:
In `@deploy/transponder.service.example`:
- Line 29: The systemd example sets RUST_LOG with the transponder target at
debug causing noisy production logs; update the RUST_LOG environment value so
the transponder target uses info instead of debug (change "transponder=debug" to
"transponder=info" in the Environment line that defines RUST_LOG) and add a
brief note that debug logging is available as a temporary troubleshooting
override (e.g., instruct operators to set transponder=debug when needed).
🪄 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: d606ff5a-9561-4084-a312-b73017c21310
📒 Files selected for processing (13)
AGENTS.mdREADME.mdcompose.prod.ymldeploy/production.env.exampledeploy/transponder.service.exampledocker-compose.ymldocs/deployment.mdjustfilesrc/main.rssrc/nostr/client.rssrc/nostr/events.rssrc/push/apns.rssrc/push/fcm.rs
| TRANSPONDER_HEALTH_BIND_ADDRESS: "0.0.0.0:8080" | ||
| TRANSPONDER_LOGGING_LEVEL: ${TRANSPONDER_LOGGING_LEVEL:-info} | ||
| TRANSPONDER_LOGGING_FORMAT: json | ||
| RUST_LOG: ${RUST_LOG:-info,transponder=debug,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn} |
There was a problem hiding this comment.
Use a non-debug default for transponder in production.
The default includes transponder=debug, which keeps verbose app logs enabled by default in compose.prod.yml.
Suggested patch
- RUST_LOG: ${RUST_LOG:-info,transponder=debug,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn}
+ RUST_LOG: ${RUST_LOG:-info,transponder=info,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn}📝 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
| RUST_LOG: ${RUST_LOG:-info,transponder=debug,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn} | |
| RUST_LOG: ${RUST_LOG:-info,transponder=info,nostr_relay_pool=info,nostr_sdk=info,nostr=info,reqwest=warn,hyper=warn,hyper_util=warn,h2=warn,tower=warn,rustls=warn,tungstenite=warn,tokio_tungstenite=warn} |
🤖 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 `@compose.prod.yml` at line 27, The production compose default RUST_LOG
currently enables verbose transponder=debug; update the RUST_LOG default in the
compose prod env entry (the RUST_LOG line) to remove the transponder=debug
setting or set it to a non-debug level (e.g., transponder=info) so production
containers do not start with verbose transponder debug logs enabled.
| use serde::{Deserialize, Serialize}; | ||
| use tokio::sync::RwLock; | ||
| use tracing::{debug, error, trace, warn}; | ||
| use tracing::{debug, error, info, trace, warn}; |
There was a problem hiding this comment.
Keep APNs push-result logs at debug level.
The success/invalid-token result logs were promoted to info!, which increases noise and diverges from the push logging policy.
Suggested patch
-use tracing::{debug, error, info, trace, warn};
+use tracing::{debug, error, trace, warn};
@@
- info!("APNs notification accepted");
+ debug!("APNs notification accepted");
@@
- info!("APNs token no longer valid");
+ debug!("APNs token no longer valid");As per coding guidelines: src/push/**/*.rs: Use tracing at debug level for push results, never include user-identifying information.
Also applies to: 258-281
🤖 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 `@src/push/apns.rs` at line 12, The APNs push-result logs were promoted to
info! but must stay at debug level; locate the logging calls in src/push/apns.rs
that currently use info! for success/invalid-token outcomes (and any similar
uses in lines ~258-281) and change them back to debug! (ensure the tracing
import still includes debug). While updating, remove any user-identifying data
from the log messages so they remain non-sensitive and follow the push logging
policy.
| use serde::{Deserialize, Serialize}; | ||
| use tokio::sync::RwLock; | ||
| use tracing::{debug, error, trace, warn}; | ||
| use tracing::{debug, error, info, trace, warn}; |
There was a problem hiding this comment.
Keep FCM push-result logs at debug level.
The success/not-found push-result logs were promoted to info!, which is noisier than the repo push logging policy.
Suggested patch
-use tracing::{debug, error, info, trace, warn};
+use tracing::{debug, error, trace, warn};
@@
- info!("FCM notification accepted");
+ debug!("FCM notification accepted");
@@
- info!("FCM token not found");
+ debug!("FCM token not found");As per coding guidelines: src/push/**/*.rs: Use tracing at debug level for push results, never include user-identifying information.
Also applies to: 355-381
🤖 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 `@src/push/fcm.rs` at line 13, The push-result logs in src/push/fcm.rs were
upgraded to info! but must remain at debug level and must not contain any
user-identifying information; locate the push-result logging sites in this file
(the success/not-found logs around the push/result handling and the block around
lines ~355-381) and change any info! (or other non-debug) invocations to debug!
while ensuring the logged messages contain only generic push/result info (no
user IDs, emails, or tokens); keep the existing tracing import usage (debug) and
update any related tests or comments if they assert on log level.
❌ Code Coverage Report
View detailed coverageDownload the Or run locally: cargo llvm-cov --html
open target/llvm-cov/html/index.html |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
EnvFilterinitialization fromRUST_LOGorTRANSPONDER_LOGGING_LEVELevent_id, while leaving the rest of the application logging changes in placeRUST_LOGvalues explicitly for Transponder and its dependenciesTesting
cargo fmtcargo testcargo clippy -- -D warningsSummary by CodeRabbit
Documentation
Chores