-
-
Notifications
You must be signed in to change notification settings - Fork 277
enh(internal): debug log api #3425
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: main
Are you sure you want to change the base?
Changes from all commits
3b8e07f
cd107e8
90ffcc2
196e2a2
0ffd9fb
0b4f10d
50661e7
a7f32a8
92165ac
15e714b
b38b3c8
007a0b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,106 @@ | ||
| import 'dart:developer' as dev; | ||
|
|
||
| import 'package:meta/meta.dart'; | ||
|
|
||
| import '../../sentry.dart'; | ||
|
|
||
| /// Lightweight isolate compatible diagnostic logger for the Sentry SDK. | ||
| /// | ||
| /// Logger naming convention: | ||
| /// - `sentry` – core dart package | ||
| /// - `sentry.flutter` – flutter package | ||
| /// - `sentry.{integration}` – integration packages (dio, hive, etc.) | ||
| /// | ||
| /// Each package should have at least one top-level instance. | ||
| /// | ||
| /// Example: | ||
| /// ```dart | ||
| /// const debugLogger = SentryDebugLogger('sentry.flutter'); | ||
| /// | ||
| /// debugLogger.warning('My Message') | ||
| ///``` | ||
| @internal | ||
| class SentryDebugLogger { | ||
| final String _name; | ||
|
|
||
| const SentryDebugLogger(this._name); | ||
|
|
||
| static bool _isEnabled = false; | ||
| static SentryLevel _minLevel = SentryLevel.warning; | ||
|
|
||
| @visibleForTesting | ||
| static bool get isEnabled => _isEnabled; | ||
|
|
||
| @visibleForTesting | ||
| static SentryLevel get minLevel => _minLevel; | ||
|
|
||
| /// Configure logging for the current isolate. | ||
| /// | ||
| /// This needs to be called for each new spawned isolate before logging. | ||
| static void configure({ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this ever changing during runtime? I don't understand why we now have both an instance and static vars. If we go the instance, can't we just call `Logger(name, minLevel)'? If it needs to be enabled later, we can just set it on the instance. So either static only, or instance only, no need to mix both.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
the reason why it's an instance is so we have the benefit of creating multiple logger instance with the same configuration within the same isolate the configuration is static per isolate, I don't think we will ever have a use case where we want multiple loggers with different minLevels Example: Flutter const logger = SentryDebugLogger(sentry.flutter)
// SentryDebugLogger.configure(...) called in SentryOptions
logger.info('info in main isolate)' // by default uses the main isolate static config
// spawn a new isolate
// within that isolate
// SentryDebugLogger.configure(...) called directly after isolate has spawned
logger.info('info in other isolate') // you can still use that same logger instance
this doesn't work because you can only configure the logger through the static function
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what we could do is to separate the config and have only the config be static so we don't mix it into the log class /// Per-isolate logging configuration.
@internal
class SentryLogConfig {
static bool isEnabled = false;
static SentryLevel minLevel = SentryLevel.warning;
/// Configure logging for the current isolate.
static void configure({
required bool isEnabled,
SentryLevel minLevel = SentryLevel.warning,
}) {
SentryLogConfig.isEnabled = isEnabled;
SentryLogConfig.minLevel = minLevel;
}
}
// within the logger:
@pragma('vm:prefer-inline')
void _log(SentryLevel level, String message, {Object? error, StackTrace? stackTrace}) {
if (!SentryLogConfig.isEnabled) return;
if (level.ordinal < SentryLogConfig.minLevel.ordinal) return;
dev.log(
'[${level.name}] $message',
name: _name,
level: level.toDartLogLevel(),
error: error,
stackTrace: stackTrace,
time: DateTime.now(),
);
}alternative 1 - update the config on the instance: const logger = SentryDebugLogger(sentry.flutter);
// later at some point
logger.updateConfig(isEnabled: xyz, name: 'sentry.flutter');caveat: we would need to update the config for every new logger alternative 2 - update the config using a callback on a global config instance: const loggerConfig = LoggerConfig() // updated later at some point when Sentry initializes
final logger = SentryDebugLogger(sentry.flutter, () => loggerConfig); |
||
| required bool isEnabled, | ||
| SentryLevel minLevel = SentryLevel.warning, | ||
| }) { | ||
| _isEnabled = isEnabled; | ||
| _minLevel = minLevel; | ||
| } | ||
|
|
||
| void debug( | ||
| String message, { | ||
| Object? error, | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a case where we would log an error + stacktrace as debug? Same for
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm not sure, just added it for consistency |
||
| StackTrace? stackTrace, | ||
| }) => | ||
| _log(SentryLevel.debug, message, error: error, stackTrace: stackTrace); | ||
|
|
||
| void info( | ||
| String message, { | ||
| Object? error, | ||
| StackTrace? stackTrace, | ||
| }) => | ||
| _log(SentryLevel.info, message, error: error, stackTrace: stackTrace); | ||
|
|
||
| void warning( | ||
| String message, { | ||
| Object? error, | ||
| StackTrace? stackTrace, | ||
| }) => | ||
| _log(SentryLevel.warning, message, error: error, stackTrace: stackTrace); | ||
|
|
||
| void error( | ||
| String message, { | ||
| Object? error, | ||
| StackTrace? stackTrace, | ||
| }) => | ||
| _log(SentryLevel.error, message, error: error, stackTrace: stackTrace); | ||
|
|
||
| void fatal( | ||
| String message, { | ||
| Object? error, | ||
| StackTrace? stackTrace, | ||
| }) => | ||
| _log(SentryLevel.fatal, message, error: error, stackTrace: stackTrace); | ||
|
|
||
| @pragma('vm:prefer-inline') | ||
| void _log( | ||
| SentryLevel level, | ||
| String message, { | ||
| Object? error, | ||
| StackTrace? stackTrace, | ||
| }) { | ||
| if (!_isEnabled) return; | ||
| if (level.ordinal < _minLevel.ordinal) return; | ||
buenaflor marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| dev.log( | ||
| '[${level.name}] $message', | ||
| name: _name, | ||
| level: level.toDartLogLevel(), | ||
| error: error, | ||
| stackTrace: stackTrace, | ||
| time: DateTime.now(), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| /// Logger for the Sentry Dart SDK. | ||
| @internal | ||
| const debugLogger = SentryDebugLogger('sentry'); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,158 @@ | ||
| import 'package:sentry/sentry.dart'; | ||
| import 'package:sentry/src/utils/debug_logger.dart'; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| import '../test_utils.dart'; | ||
|
|
||
| void main() { | ||
| group(SentryDebugLogger, () { | ||
| tearDown(() { | ||
| // Reset to default state after each test | ||
| SentryDebugLogger.configure(isEnabled: false); | ||
| }); | ||
|
|
||
| group('configuration', () { | ||
| test('enables logging when isEnabled is true', () { | ||
| SentryDebugLogger.configure(isEnabled: true); | ||
|
|
||
| expect(SentryDebugLogger.isEnabled, isTrue); | ||
| }); | ||
|
|
||
| test('disables logging when isEnabled is false', () { | ||
| SentryDebugLogger.configure(isEnabled: false); | ||
|
|
||
| expect(SentryDebugLogger.isEnabled, isFalse); | ||
| }); | ||
|
|
||
| test('sets minimum level', () { | ||
| SentryDebugLogger.configure( | ||
| isEnabled: true, | ||
| minLevel: SentryLevel.error, | ||
| ); | ||
|
|
||
| expect(SentryDebugLogger.isEnabled, isTrue); | ||
| expect(SentryDebugLogger.minLevel, equals(SentryLevel.error)); | ||
| }); | ||
|
|
||
| test('defaults minLevel to warning', () { | ||
| SentryDebugLogger.configure(isEnabled: true); | ||
|
|
||
| expect(SentryDebugLogger.minLevel, equals(SentryLevel.warning)); | ||
| }); | ||
|
|
||
| test('SentryOptions.debug enables logger', () { | ||
| final options = defaultTestOptions(); | ||
|
|
||
| expect(options.debug, isFalse); | ||
| options.debug = true; | ||
|
|
||
| expect(SentryDebugLogger.isEnabled, isTrue); | ||
| }); | ||
|
|
||
| test('SentryOptions.diagnosticLevel sets minLevel when set before debug', | ||
| () { | ||
| final options = defaultTestOptions(); | ||
|
|
||
| options.diagnosticLevel = SentryLevel.error; | ||
| options.debug = true; | ||
|
|
||
| expect(SentryDebugLogger.isEnabled, isTrue); | ||
| expect(SentryDebugLogger.minLevel, equals(SentryLevel.error)); | ||
| }); | ||
|
|
||
| test( | ||
| 'SentryOptions.diagnosticLevel updates minLevel when set after debug', | ||
| () { | ||
| final options = defaultTestOptions(); | ||
|
|
||
| options.debug = true; | ||
| expect(SentryDebugLogger.minLevel, equals(SentryLevel.warning)); | ||
|
|
||
| options.diagnosticLevel = SentryLevel.error; | ||
|
|
||
| expect(SentryDebugLogger.isEnabled, isTrue); | ||
| expect(SentryDebugLogger.minLevel, equals(SentryLevel.error)); | ||
| }); | ||
| }); | ||
|
|
||
| group('logging when enabled', () { | ||
| setUp(() { | ||
| SentryDebugLogger.configure( | ||
| isEnabled: true, minLevel: SentryLevel.debug); | ||
| }); | ||
|
|
||
| test('debug logs without throwing', () { | ||
| expect( | ||
| () => debugLogger.debug('debug message'), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
|
|
||
| test('info logs without throwing', () { | ||
| expect( | ||
| () => debugLogger.info('info message'), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
|
|
||
| test('warning logs without throwing', () { | ||
| expect( | ||
| () => debugLogger.warning('warning message'), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
|
|
||
| test('error logs without throwing', () { | ||
| expect( | ||
| () => debugLogger.error('error message'), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
|
|
||
| test('fatal logs without throwing', () { | ||
| expect( | ||
| () => debugLogger.fatal('fatal message'), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
|
|
||
| test('accepts error object', () { | ||
| expect( | ||
| () => debugLogger.error( | ||
| 'error occurred', | ||
| error: Exception('test exception'), | ||
| ), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
|
|
||
| test('accepts stackTrace', () { | ||
| expect( | ||
| () => debugLogger.error( | ||
| 'error occurred', | ||
| error: Exception('test'), | ||
| stackTrace: StackTrace.current, | ||
| ), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| group('logger instances', () { | ||
| test('debugLogger constant is available', () { | ||
| expect(debugLogger, isA<SentryDebugLogger>()); | ||
| }); | ||
|
|
||
| test('can create logger with custom name', () { | ||
| const customLogger = SentryDebugLogger('sentry.flutter'); | ||
|
|
||
| SentryDebugLogger.configure(isEnabled: true); | ||
|
|
||
| expect( | ||
| () => customLogger.info('test from flutter logger'), | ||
| returnsNormally, | ||
| ); | ||
| }); | ||
| }); | ||
| }); | ||
| } |
This file was deleted.
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.
I think
SentryInternalLoggerwould be a more appropriate name.