- 
                Notifications
    You must be signed in to change notification settings 
- Fork 7.5k
feat(logging): start collecting logs at appMount #16338
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
base: master
Are you sure you want to change the base?
feat(logging): start collecting logs at appMount #16338
Conversation
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 moves the initialization of the logging system to the app mount phase to capture logs that occur before the configuration is fully merged. Previously, logs generated before the SET_CONFIG action were not captured by the logger.
- Adds early logging initialization during app mount with minimal configuration
- Refactors the logging initialization into smaller, focused functions
- Updates log collector configuration to happen after config is available
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description | 
|---|---|
| react/features/base/logging/reducer.ts | Adds optional maxEntryLength property to logCollector interface | 
| react/features/base/logging/middleware.ts | Implements early logging initialization and refactors logging setup into smaller functions | 
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| function _setupDebugLogging(isTestingEnabled: boolean) { | ||
| // The JitsiMeetInMemoryLogStorage cannot be accessed on mobile through | ||
| // the 'executeScript' method like it's done in torture tests for WEB. | ||
| if (!isTestingEnabled || typeof APP !== 'object') { | 
    
      
    
      Copilot
AI
    
    
    
      Aug 14, 2025 
    
  
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.
The condition logic has been inverted. The original code checked if (isTestingEnabled && typeof APP === 'object') but now it returns early when either condition is false, which means the debug logging setup will never execute.
| // This means if something happens between that interval and the logs don't get flushed, | ||
| // they will be lost (e.g., meeting tab is closed, browser crashes, uncaught exception). | ||
| // If undefined is passed, the default values will be used. | ||
| logCollector.maxEntryLength = rtcstatsLogFlushSizeBytes; | 
    
      
    
      Copilot
AI
    
    
    
      Aug 14, 2025 
    
  
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.
Directly assigning to logCollector.maxEntryLength may not properly update the log collector's configuration. The original code passed this value during construction. Consider using a proper configuration method if available.
| logCollector.maxEntryLength = rtcstatsLogFlushSizeBytes; | |
| if (typeof logCollector.setMaxEntryLength === 'function') { | |
| logCollector.setMaxEntryLength(rtcstatsLogFlushSizeBytes); | |
| } else { | |
| // Fallback to direct assignment if setter is not available. | |
| logCollector.maxEntryLength = rtcstatsLogFlushSizeBytes; | |
| } | 
Logs that would happen before SET_CONFIG logging middleware wouldn't be captured by the logger (which sends the data to rtcstats), logs such as
2025-08-11T08:28:49.586Z [features/base/config] Extending config with: {"videoQuality":{"preferredCodec":"VP9"}}This moves the init of the logger to app mount, allowing it to capture more logs, even before the config is completely merged.