-
Notifications
You must be signed in to change notification settings - Fork 7
chore(api): reduce log verbosity for routine operations #301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThree source files updated to downgrade certain debug log statements to trace level. Changes affect authentication middleware, scope validation middleware, and server logging for health checks, billing telemetry, and periodic refresh operations. No functional behavior, control flow, or API modifications introduced. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/basilica-api/src/api/middleware/auth.rs (1)
211-214: Consider downgrading JWT auth success to trace for consistency.The API key authentication success message was downgraded to trace (line 110), but JWT authentication success remains at debug. Since both are high-frequency routine operations, consider applying the same log level reduction here for consistency.
🔎 Suggested change for consistency
- debug!( + trace!( "JWT authentication successful for user: {}. Scopes: {:?}", claims.sub, claims.scope );
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
crates/basilica-api/src/api/middleware/auth.rs(2 hunks)crates/basilica-api/src/api/middleware/scope.rs(2 hunks)crates/basilica-api/src/server.rs(4 hunks)
⏰ 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). (2)
- GitHub Check: lint-complex
- GitHub Check: build-api (stable)
🔇 Additional comments (6)
crates/basilica-api/src/api/middleware/auth.rs (1)
110-113: LGTM: Appropriate log level reduction for routine operation.Moving API key authentication success from debug to trace is appropriate for reducing log noise in production, as this is a high-frequency routine operation.
crates/basilica-api/src/api/middleware/scope.rs (1)
66-72: LGTM: Appropriate log level reduction for high-frequency operation.Downgrading successful scope authorization messages to trace is appropriate. This middleware runs on every authenticated request, making it a high-frequency operation that can generate significant log noise at debug level.
crates/basilica-api/src/server.rs (4)
190-196: LGTM: Appropriate log level reduction for periodic billing task.Downgrading the billing telemetry success message to trace is appropriate. This task runs every 30 seconds (line 1046) and generates routine operational logs that don't need debug-level visibility in production.
882-882: LGTM: Appropriate log level reduction for periodic health checks.Downgrading the validator health check success message to trace is appropriate. This task runs at regular intervals and generates high-frequency routine logs.
969-978: LGTM: Appropriate and nuanced log level adjustments for periodic refresh task.The logging changes for GPU offerings refresh are well-designed:
- Trace for routine refresh attempts (line 969)
- Debug for successful fetches with count (line 973, downgraded from info)
- Trace for no-op refreshes during cooldown (lines 975-977)
This provides appropriate signal-to-noise ratio while maintaining visibility into meaningful state changes.
1022-1025: LGTM: Appropriate log level reduction for periodic health checks.Downgrading the secure cloud health check completion message to trace is appropriate. This is a routine periodic task that doesn't need debug-level visibility in production.
Downgrades log levels for high-frequency routine messages to reduce noise in production logs:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.