feat: file logging and configurable minimum log level#6
Merged
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 82b99ca. Configure here.
82b99ca to
edd9975
Compare
Two additions to mini_chromium's logging: 1. LOG_TO_FILE destination. Implements the previously stubbed LOG_TO_FILE destination and adds a simplified base::OpenFile() helper (after upstream Chromium's helper of the same name). Callers pass the path via LoggingSettings::log_file_path. The file is opened lazily for appending on the first emitted message and held by a std::unique_ptr<FILE> with a silent closer so an fclose failure during static destruction cannot re-enter Flush() and deadlock on the file lock. File state (lock, path, handle, and an `enabled` flag) is grouped in a file-scope LogFile struct; an empty path or a failed open is handled by clearing `enabled`, so a transient open failure cannot clobber a concurrent InitLogging(). Concurrent LOG calls are serialized with a base::Lock. 2. Configurable minimum log level. Replaces the hardcoded GetMinLogLevel() with a runtime-configurable value. Adds SetMinLogLevel() and a min_log_level field on LoggingSettings (applied by InitLogging). Default remains LOG_INFO, so existing callers are unaffected. g_logging_destination and g_min_log_level are std::atomic so reads on the LOG hot path don't need the file lock and writes from InitLogging / SetMinLogLevel don't race concurrent readers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
86f7d9c to
fd5d7ae
Compare
This was referenced Apr 18, 2026
mujacica
approved these changes
Apr 20, 2026
JoshuaMoelans
left a comment
Member
There was a problem hiding this comment.
a few open questions left, but otherwise LGTM!
The source file was omitted when file_util.cc was introduced alongside file_util.h. CMake already lists both. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JoshuaMoelans
approved these changes
Apr 20, 2026
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
3 participants
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.
File logging and configurable minimum log level for sentry-native/crashpad:
Two additions to mini_chromium's logging:
LOG_TO_FILEdestination. Implements the previously stubbedLOG_TO_FILEdestination and adds a simplifiedbase::OpenFile()helper (after upstream Chromium's helper of the same name). Callers pass the path viaLoggingSettings::log_file_path. The file is opened lazily for appending on the first emitted message and held by astd::unique_ptr<FILE>with a silent closer, so anfclosefailure during static destruction cannot re-enterFlush()and deadlock on the file lock. File state (lock, path, handle, and anenabledflag) is grouped in a file-scopeLogFilestruct; an empty path or a failed open is handled by clearingenabled, so a transient open failure cannot clobber a concurrentInitLogging(). Concurrent LOG calls are serialized with abase::Lock.Configurable minimum log level. Replaces the hardcoded
GetMinLogLevel()with a runtime-configurable value. AddsSetMinLogLevel()and amin_log_levelfield onLoggingSettings(applied byInitLogging). Default remainsLOG_INFO, so existing callers are unaffected.g_logging_destinationandg_min_log_levelarestd::atomicso reads on the LOG hot path don't need the file lock and writes fromInitLogging/SetMinLogLeveldon't race concurrent readers.See also: