pr feedback

sentry_logging/lib/src/logging_integration.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 
 import 'package:logging/logging.dart';
 import 'package:sentry/sentry.dart';
+import 'version.dart';
 import 'extension.dart';
 
 /// An [Integration] which listens to all messages of the
@@ -13,16 +14,21 @@ class LoggingIntegration extends Integration<SentryOptions> {
   /// messages with errors as an [SentryEvent] instead of an [Breadcrumb].
   /// Setting [logExceptionAsEvent] to false captures everything as
   /// [Breadcrumb]s.
-  LoggingIntegration({bool logExceptionAsEvent = true})
-      : _logExceptionsAsEvents = logExceptionAsEvent;
+  LoggingIntegration({
+    Level minBreadcrumbLevel = Level.INFO,
+    Level minEventLevel = Level.SEVERE,
+  })  : _minBreadcrumbLevel = minBreadcrumbLevel,
+        _minEventLevel = minEventLevel;
 
-  final bool _logExceptionsAsEvents;
+  final Level _minBreadcrumbLevel;
+  final Level _minEventLevel;
   late StreamSubscription<LogRecord> _subscription;
   late Hub _hub;
 
   @override
   FutureOr<void> call(Hub hub, SentryOptions options) {
     _hub = hub;
+    _setSdkVersion(options);
     _subscription = Logger.root.onRecord.listen(
       _onLog,
       onError: (Object error, StackTrace stackTrace) {
@@ -38,21 +44,33 @@ class LoggingIntegration extends Integration<SentryOptions> {
     await _subscription.cancel();
   }
 
-  void _onLog(LogRecord record) {
-    // Everything is just logged as a breadcrumb
-    if (!_logExceptionsAsEvents) {
-      _hub.addBreadcrumb(record.toBreadcrumb());
-      return;
-    }
+  void _setSdkVersion(SentryOptions options) {
+    final sdk = SdkVersion(
+      name: sdkName,
+      version: sdkVersion,
+      integrations: options.sdk.integrations,
+      packages: options.sdk.packages,
+    );
+    sdk.addPackage('pub:sentry_logging', sdkVersion);
+    options.sdk = sdk;
+  }
 
-    // If a LogRecord contains an exception, it gets reported as an SentryEvent
-    if (record.error == null) {
-      _hub.addBreadcrumb(record.toBreadcrumb());
-    } else {
+  bool _isLoggable(Level logLevel, Level minLevel) {
+    return logLevel > minLevel;
+  }
+
+  void _onLog(LogRecord record) {
+    // The event must be logged first, otherwise the log would also be added
+    // to the breadcrumbs for itself.
+    if (_isLoggable(record.level, _minEventLevel)) {
       _hub.captureEvent(
         record.toEvent(),
         stackTrace: record.stackTrace,
       );
     }
+
+    if (_isLoggable(record.level, _minBreadcrumbLevel)) {
+      _hub.addBreadcrumb(record.toBreadcrumb());
+    }
   }
 }

sentry_logging/lib/src/version.dart

@@ -0,0 +1,5 @@
+/// The SDK version reported to Sentry.io in the submitted events.
+const String sdkVersion = '6.2.0';
+
+/// The default SDK name reported to Sentry.io in the submitted events.
+const String sdkName = 'sentry.dart.sentry_logging';

sentry_logging/pubspec.yaml

@@ -14,3 +14,4 @@ dependencies:
 dev_dependencies:
   lints: ^1.0.0
   test: ^1.16.0
+  yaml: ^3.1.0 # needed for version match (code and pubspec)

sentry_logging/test/logging_integration_test.dart

@@ -1,6 +1,7 @@
 import 'package:logging/logging.dart';
 import 'package:sentry/sentry.dart';
 import 'package:sentry_logging/sentry_logging.dart';
+import 'package:sentry_logging/src/version.dart';
 import 'package:test/expect.dart';
 import 'package:test/scaffolding.dart';
 
@@ -22,9 +23,21 @@ void main() {
     );
   });
 
-  test('logger gets recorded', () async {
+  test('options.sdk.integrations contains version', () async {
     final sut = fixture.createSut();
     await sut.call(fixture.hub, fixture.options);
+    await sut.close();
+
+    expect(fixture.options.sdk.name, sdkName);
+    final package = fixture.options.sdk.packages
+        .firstWhere((it) => it.name == 'pub:sentry_logging');
+    expect(package.name, 'pub:sentry_logging');
+    expect(package.version, sdkVersion);
+  });
+
+  test('logger gets recorded if level over minlevel', () async {
+    final sut = fixture.createSut(minBreadcrumbLevel: Level.CONFIG);
+    await sut.call(fixture.hub, fixture.options);
 
     final log = Logger('FooBarLogger');
     log.warning(
@@ -45,26 +58,22 @@ void main() {
     expect(crumb.type, 'debug');
   });
 
-  test('exceptions is recorded as breadcrumb if logExceptionsAsEvents = false',
-      () async {
-    final sut = fixture.createSut(logExceptionsAsEvents: false);
+  test('logger gets not recorded if level under minlevel', () async {
+    final sut = fixture.createSut(minBreadcrumbLevel: Level.SEVERE);
     await sut.call(fixture.hub, fixture.options);
 
     final log = Logger('FooBarLogger');
     log.warning(
       'A log message',
-      Exception('foo bar'),
-      StackTrace.current,
     );
+
     expect(fixture.hub.events.length, 0);
-    expect(fixture.hub.breadcrumbs.length, 1);
-    final crumb = fixture.hub.breadcrumbs.first;
-    expect(crumb.data?.length, 4);
+    expect(fixture.hub.breadcrumbs.length, 0);
   });
 
-  test('exceptions is recorded as event if logExceptionsAsEvents = true',
+  test('exception is recorded as event if minEventLevel over minlevel',
       () async {
-    final sut = fixture.createSut(logExceptionsAsEvents: true);
+    final sut = fixture.createSut(minEventLevel: Level.INFO);
     await sut.call(fixture.hub, fixture.options);
 
     final exception = Exception('foo bar');
@@ -76,22 +85,45 @@ void main() {
       exception,
       stackTrace,
     );
-    expect(fixture.hub.breadcrumbs.length, 0);
     expect(fixture.hub.events.length, 1);
+    expect(fixture.hub.events.first.event.breadcrumbs, null);
     final event = fixture.hub.events.first.event;
     expect(event.level, SentryLevel.warning);
     expect(event.logger, 'FooBarLogger');
     expect(event.throwable, exception);
     expect(event.extra?['LogRecord.sequenceNumber'], isNotNull);
     expect(fixture.hub.events.first.stackTrace, stackTrace);
   });
+
+  test('exception is not recorded as event if minEventLevel under minlevel',
+      () async {
+    final sut = fixture.createSut(minEventLevel: Level.SEVERE);
+    await sut.call(fixture.hub, fixture.options);
+
+    final exception = Exception('foo bar');
+    final stackTrace = StackTrace.current;
+
+    final log = Logger('FooBarLogger');
+    log.warning(
+      'A log message',
+      exception,
+      stackTrace,
+    );
+    expect(fixture.hub.events.length, 0);
+  });
 }
 
 class Fixture {
   SentryOptions options = SentryOptions(dsn: fakeDsn);
   MockHub hub = MockHub();
 
-  LoggingIntegration createSut({bool logExceptionsAsEvents = true}) {
-    return LoggingIntegration(logExceptionAsEvent: logExceptionsAsEvents);
+  LoggingIntegration createSut({
+    Level minBreadcrumbLevel = Level.INFO,
+    Level minEventLevel = Level.SEVERE,
+  }) {
+    return LoggingIntegration(
+      minBreadcrumbLevel: minBreadcrumbLevel,
+      minEventLevel: minEventLevel,
+    );
   }
 }

sentry_logging/test/version_test.dart

@@ -0,0 +1,19 @@
+@TestOn('vm')
+
+import 'dart:io';
+
+import 'package:sentry_logging/src/version.dart';
+import 'package:test/test.dart';
+import 'package:yaml/yaml.dart' as yaml;
+
+void main() {
+  test(
+    'sdkVersion matches that of pubspec.yaml',
+    () {
+      final dynamic pubspec =
+          yaml.loadYaml(File('pubspec.yaml').readAsStringSync());
+      expect(sdkVersion, pubspec['version']);
+    },
+    skip: 'works locally but not on CI',
+  );
+}