-
-
Notifications
You must be signed in to change notification settings - Fork 832
fix(website): Resolve duplicate logging in production #805
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
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughRefactors logger initialization to select transports based on environment: production uses Cloud Logging via LoggingWinston; development uses a Console transport with timestamp and JSON formatting. The logger level logic and public API remain unchanged. Changes
Sequence Diagram(s)sequenceDiagram
actor App
participant Logger
participant Console as Console Transport
participant Cloud as Cloud Logging (LoggingWinston)
Note over Logger: Initialization
App->>Logger: create logger
alt NODE_ENV=production
Logger->>Cloud: attach transport
else development/other
Logger->>Console: attach transport
end
Note over Logger: Usage
App->>Logger: log(message, meta)
alt production
Logger->>Cloud: write entry
else development
Logger->>Console: write line (timestamp + JSON)
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
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.
Summary of Changes
Hello @yamadashy, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses an issue where logs were duplicated in production environments. The fix involves refining the logging configuration to ensure that only the appropriate logging transport is active based on the deployment environment, thereby streamlining log collection and preventing redundant entries.
Highlights
- Logging Duplication Fix: Resolves the problem of duplicate log entries appearing in production monitoring systems by ensuring only one logging transport is active.
- Environment-Specific Logging: Implements conditional logic within the
createLogger
function to activate different logging transports based on theNODE_ENV
environment variable. - Production Logging: Configures production environments to exclusively use Cloud Logging, preventing redundant console output.
- Development Logging: Ensures development environments continue to use console logging for local debugging purposes, without interfering with production logs.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Claude encountered an error —— View job I'll analyze this and get back to you. |
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.
Pull Request Overview
This PR fixes a duplicate logging issue in production where logs were appearing twice in monitoring systems. The root cause was having both Cloud Logging and Console transports active simultaneously in production.
- Refactored logger configuration to use environment-specific transports
- Production environments now use only Cloud Logging transport to eliminate duplicates
- Development environments continue using console logging for local debugging
website/server/src/utils/logger.ts
Outdated
// Configure transports based on environment | ||
function createLogger() { | ||
const transports: winston.transport[] = [ | ||
new winston.transports.Console({ | ||
format: winston.format.combine(winston.format.timestamp(), winston.format.json()), | ||
}), | ||
]; | ||
const transports: winston.transport[] = []; | ||
|
||
// Add Cloud Logging transport only in production | ||
if (process.env.NODE_ENV === 'production') { |
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.
[nitpick] Consider using a more robust environment check or constants to avoid magic strings. For example, define a constant like const IS_PRODUCTION = process.env.NODE_ENV === 'production'
or use an environment configuration utility.
Copilot uses AI. Check for mistakes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #805 +/- ##
=======================================
Coverage 87.41% 87.41%
=======================================
Files 113 113
Lines 6493 6493
Branches 1331 1331
=======================================
Hits 5676 5676
Misses 817 817 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying repomix with
|
Latest commit: |
4e20fc5
|
Status: | ✅ Deploy successful! |
Preview URL: | https://b4122c69.repomix.pages.dev |
Branch Preview URL: | https://feat-website-log.repomix.pages.dev |
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.
Code Review
This pull request correctly resolves the duplicate logging issue in production by conditionally configuring Winston transports based on NODE_ENV
. The logic is sound and effectively separates production from development logging. I have provided one suggestion to refactor the implementation for improved conciseness and maintainability.
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 (2)
website/server/src/utils/logger.ts (2)
7-7
: Nit: Consider typing transports as TransportStream[] for clearer intent.Some setups’ TS typings are happier with the explicit TransportStream type, and it reads cleaner when mixing community transports like LoggingWinston.
Apply within the selected line range:
- const transports: winston.transport[] = []; + const transports: TransportStream[] = [];Add the supporting import at the top of the file:
import type TransportStream from 'winston-transport';
13-20
: Optional: Silence logs during tests to keep CI output clean.If your test runner sets NODE_ENV=test, adding a test-specific branch helps avoid noisy logs and speeds up log-heavy suites.
- } else { - // In development, use console logging - transports.push( - new winston.transports.Console({ - format: winston.format.combine(winston.format.timestamp(), winston.format.json()), - }), - ); - } + } else if (process.env.NODE_ENV === 'test') { + // In tests, silence logging + transports.push( + new winston.transports.Console({ + silent: true, + }), + ); + } else { + // In development, use console logging + transports.push( + new winston.transports.Console({ + format: winston.format.combine(winston.format.timestamp(), winston.format.json()), + }), + ); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
website/server/src/utils/logger.ts
(1 hunks)
🔇 Additional comments (2)
website/server/src/utils/logger.ts (2)
10-20
: Production-only Cloud Logging and dev-only Console fixes the double logs — LGTM.This environment-based transport split directly addresses the duplication in production while preserving local console output. No functional concerns here.
10-20
: All logging is centralized in utils/logger.ts—no stray instances found
A repository-wide search confirms onlywebsite/server/src/utils/logger.ts
containswinston.createLogger
,LoggingWinston
,new winston.transports.Console
, or directwinston.<level>
calls. No other logger initializations or transports exist elsewhere, so duplicates are avoided.
- Separate transport configuration for production and development - Production: Use only Cloud Logging transport to prevent duplicates - Development: Use only Console transport for local debugging - Fixes issue where Cloud Logging and Console transports caused duplicate log entries
- Extract isProduction constant to avoid magic strings - Use ternary operator for more concise transport initialization - Reuse isProduction constant for log level configuration - Improve code readability and maintainability Addresses feedback from @Copilot and @gemini-code-assist[bot]
722e1de
to
4e20fc5
Compare
Fixes duplicate logging issue in production environment where logs appeared twice in monitoring systems.
Summary
Changes
website/server/src/utils/logger.ts
to conditionally configure winston transports based onNODE_ENV
Checklist
npm run test
npm run lint