-
-
Notifications
You must be signed in to change notification settings - Fork 277
Refacto : add a Transport class #123
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
Changes from 1 commit
a10d740
623c904
0dcb613
c97c98b
477fdac
b8fb322
fd92e3f
0944c68
46829fc
01d4e19
fbd001e
4d39531
34fb528
2437fa4
57798f5
789e154
1aa05d6
9895142
cb5e309
e563d2c
71b53c8
be3f0a2
43b798c
19dc91d
cec1f21
909d4f9
8b600dc
4ff58a6
5493e19
621d6c4
e11f653
88526e2
796b633
fb8b9e0
1dbcba4
8b56bce
daeb29e
098d58c
2833a94
35671d4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import 'dart:async'; | ||
| import 'dart:math'; | ||
|
|
||
| import 'package:meta/meta.dart'; | ||
| import 'package:sentry/sentry.dart'; | ||
|
|
@@ -17,13 +18,16 @@ abstract class SentryClient { | |
| /// `dart:html` is available, otherwise it will throw an unsupported error. | ||
| factory SentryClient(SentryOptions options) => createSentryClient(options); | ||
|
|
||
| SentryClient.base(this.options, {@required this.transport}); | ||
| SentryClient.base(this.options, {@required this.transport}) { | ||
| _random = options.sampleRate == null ? null : Random(); | ||
| } | ||
|
|
||
| @protected | ||
| SentryOptions options; | ||
|
|
||
| // TODO: this should be removed and used options.transport | ||
| @visibleForTesting | ||
| final Transport transport; | ||
| Transport transport; | ||
|
|
||
| /// Information about the current user. | ||
| /// | ||
|
|
@@ -37,6 +41,8 @@ abstract class SentryClient { | |
| /// * https://docs.sentry.io/learn/context/#capturing-the-user | ||
| User userContext; | ||
|
||
|
|
||
| Random _random; | ||
|
|
||
| /// Reports an [event] to Sentry.io. | ||
| Future<SentryId> captureEvent( | ||
| SentryEvent event, { | ||
|
|
@@ -45,6 +51,11 @@ abstract class SentryClient { | |
| }) async { | ||
| event = _processEvent(event, eventProcessors: options.eventProcessors); | ||
|
|
||
| // dropped by sampling or event processors | ||
| if (event == null) { | ||
| return Future.value(SentryId.empty()); | ||
| } | ||
|
|
||
| event = _applyScope(event: event, scope: scope); | ||
|
|
||
| event = event.copyWith( | ||
|
Contributor
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. projectId is not really necessaary, its part of the DSN URL
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. ✅ fixed |
||
|
|
@@ -97,6 +108,14 @@ abstract class SentryClient { | |
| dynamic hint, | ||
| List<EventProcessor> eventProcessors, | ||
| }) { | ||
| if (_sampleRate()) { | ||
| options.logger( | ||
| SentryLevel.debug, | ||
| 'Event ${event.eventId.toString()} was dropped due to sampling decision.', | ||
| ); | ||
| return null; | ||
| } | ||
|
|
||
| for (final processor in eventProcessors) { | ||
| try { | ||
| event = processor(event, hint); | ||
|
|
@@ -157,4 +176,11 @@ abstract class SentryClient { | |
| } | ||
| return event; | ||
| } | ||
|
|
||
| bool _sampleRate() { | ||
| if (options.sampleRate != null && _random != null) { | ||
| return (options.sampleRate < _random.nextDouble()); | ||
| } | ||
| return true; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,8 @@ import 'package:sentry/src/protocol.dart'; | |
|
|
||
| class MockSentryClient extends Mock implements SentryClient {} | ||
|
|
||
| class MockTransport extends Mock implements Transport {} | ||
|
|
||
| final fakeDsn = 'https://[email protected]/1234567'; | ||
|
|
||
| final fakeException = Exception('Error'); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import 'package:mockito/mockito.dart'; | ||
| import 'package:sentry/sentry.dart'; | ||
| import 'package:test/test.dart'; | ||
|
|
||
| import 'mocks.dart'; | ||
|
|
||
| void main() { | ||
| group('SentryClient sampling', () { | ||
| SentryOptions options; | ||
| Transport transport; | ||
|
|
||
| setUp(() { | ||
| options = SentryOptions(dsn: fakeDsn); | ||
| transport = MockTransport(); | ||
| }); | ||
|
|
||
| test('captures event, sample rate is 100% enabled', () { | ||
| options.sampleRate = 1.0; | ||
| final client = SentryClient(options); | ||
| client.transport = transport; | ||
| client.captureEvent(fakeEvent); | ||
|
|
||
| verify(client.transport.send(any)).called(1); | ||
| }); | ||
|
|
||
| test('do not capture event, sample rate is 0% disabled', () { | ||
| options.sampleRate = 0.0; | ||
| final client = SentryClient(options); | ||
| client.transport = transport; | ||
| client.captureEvent(fakeEvent); | ||
|
|
||
| verifyNever(client.transport.send(any)); | ||
| }); | ||
|
|
||
| test('do not capture event, sample rate is null', () { | ||
| options.sampleRate = null; | ||
| final client = SentryClient(options); | ||
| client.transport = transport; | ||
| client.captureEvent(fakeEvent); | ||
|
|
||
| verifyNever(client.transport.send(any)); | ||
| }); | ||
| }); | ||
| } |
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.
it could be private I believe
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.
✅ fixed