Skip to content

Feat: Record & Send Client Reports #813

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

Merged
merged 20 commits into from
Apr 13, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions dart/lib/sentry.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export 'src/protocol.dart';
export 'src/scope.dart';
export 'src/sentry.dart';
export 'src/sentry_envelope.dart';
export 'src/sentry_envelope_item.dart';
export 'src/sentry_client.dart';
export 'src/sentry_options.dart';
// useful for integrations
Expand All @@ -28,3 +29,8 @@ export 'src/sentry_user_feedback.dart';
// tracing
export 'src/tracing.dart';
export 'src/sentry_measurement.dart';
// client reports
export 'src/client_reports/client_report.dart';
export 'src/client_reports/client_report_recorder.dart';
export 'src/client_reports/discard_reason.dart';
export 'src/transport/data_category.dart';
3 changes: 0 additions & 3 deletions dart/lib/src/client_reports/client_report.dart
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import 'package:meta/meta.dart';

import 'discarded_event.dart';
import '../utils.dart';

@internal
class ClientReport {
ClientReport(this.timestamp, this.discardedEvents);

Expand Down
3 changes: 0 additions & 3 deletions dart/lib/src/client_reports/client_report_recorder.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import 'package:meta/meta.dart';

import '../sentry_options.dart';
import 'client_report.dart';
import 'discarded_event.dart';
import 'discard_reason.dart';
import '../transport/data_category.dart';

@internal
class ClientReportRecorder {
ClientReportRecorder(this._clock);

Expand Down
3 changes: 0 additions & 3 deletions dart/lib/src/client_reports/discard_reason.dart
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
import 'package:meta/meta.dart';

/// A reason that defines why events were lost, see
/// https://develop.sentry.dev/sdk/client-reports/#envelope-item-payload.
@internal
enum DiscardReason {
beforeSend,
eventProcessor,
Expand Down
3 changes: 0 additions & 3 deletions dart/lib/src/client_reports/discarded_event.dart
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
import 'package:meta/meta.dart';

import 'discard_reason.dart';
import '../transport/data_category.dart';

@internal
class DiscardedEvent {
DiscardedEvent(this.reason, this.category, this.quantity);

Expand Down
13 changes: 5 additions & 8 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,9 @@ import 'dart:collection';

import 'package:meta/meta.dart';

import 'protocol.dart';
import 'scope.dart';
import 'sentry_client.dart';
import 'sentry_options.dart';
import '../sentry.dart';
import 'sentry_tracer.dart';
import 'sentry_traces_sampler.dart';
import 'sentry_user_feedback.dart';
import 'tracing.dart';

/// Configures the scope through the callback.
typedef ScopeCallback = void Function(Scope);
Expand Down Expand Up @@ -462,14 +457,16 @@ class Hub {
'Capturing unfinished transaction: ${transaction.eventId}',
);
} else {
final item = _peek();

if (!transaction.sampled) {
item.client.recordLostEvent(
DiscardReason.sampleRate, DataCategory.transaction);
_options.logger(
SentryLevel.warning,
'Transaction ${transaction.eventId} was dropped due to sampling decision.',
);
} else {
final item = _peek();

try {
sentryId = await item.client.captureTransaction(
transaction,
Expand Down
5 changes: 5 additions & 0 deletions dart/lib/src/noop_sentry_client.dart
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import 'dart:async';

import 'client_reports/discard_reason.dart';
import 'protocol.dart';
import 'scope.dart';
import 'sentry_client.dart';
import 'sentry_envelope.dart';
import 'sentry_user_feedback.dart';
import 'transport/data_category.dart';

class NoOpSentryClient implements SentryClient {
NoOpSentryClient._();
Expand Down Expand Up @@ -60,4 +62,7 @@ class NoOpSentryClient implements SentryClient {
Scope? scope,
}) async =>
SentryId.empty();

@override
void recordLostEvent(DiscardReason reason, DataCategory category) {}
}
26 changes: 24 additions & 2 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ import 'transport/http_transport.dart';
import 'transport/noop_transport.dart';
import 'version.dart';
import 'sentry_envelope.dart';
import 'client_reports/client_report_recorder.dart';
import 'client_reports/discard_reason.dart';
import 'transport/data_category.dart';

/// Default value for [User.ipAddress]. It gets set when an event does not have
/// a user and IP address. Only applies if [SentryOptions.sendDefaultPii] is set
Expand All @@ -35,9 +38,10 @@ class SentryClient {
/// Instantiates a client using [SentryOptions]
factory SentryClient(SentryOptions options) {
if (options.transport is NoOpTransport) {
options.transport = HttpTransport(options, RateLimiter(options.clock));
final recorder = ClientReportRecorder(options.clock);
final rateLimiter = RateLimiter(options.clock, recorder);
options.transport = HttpTransport(options, rateLimiter, recorder);
}

return SentryClient._(options);
}

Expand All @@ -53,6 +57,7 @@ class SentryClient {
dynamic hint,
}) async {
if (_sampleRate()) {
_recordLostEvent(event, DiscardReason.sampleRate);
_options.logger(
SentryLevel.debug,
'Event ${event.eventId.toString()} was dropped due to sampling decision.',
Expand Down Expand Up @@ -87,6 +92,7 @@ class SentryClient {

final beforeSend = _options.beforeSend;
if (beforeSend != null) {
final beforeSendEvent = preparedEvent;
try {
preparedEvent = await beforeSend(preparedEvent, hint: hint);
} catch (exception, stackTrace) {
Expand All @@ -98,6 +104,7 @@ class SentryClient {
);
}
if (preparedEvent == null) {
_recordLostEvent(beforeSendEvent, DiscardReason.beforeSend);
_options.logger(
SentryLevel.debug,
'Event was dropped by BeforeSend callback',
Expand Down Expand Up @@ -278,6 +285,10 @@ class SentryClient {

void close() => _options.httpClient.close();

void recordLostEvent(DiscardReason reason, DataCategory category) {
_options.transport.recordLostEvent(reason, category);
}

Future<SentryEvent?> _processEvent(
SentryEvent event, {
dynamic hint,
Expand All @@ -296,6 +307,7 @@ class SentryClient {
);
}
if (processedEvent == null) {
_recordLostEvent(event, DiscardReason.eventProcessor);
_options.logger(SentryLevel.debug, 'Event was dropped by a processor');
break;
}
Expand All @@ -309,4 +321,14 @@ class SentryClient {
}
return false;
}

void _recordLostEvent(SentryEvent event, DiscardReason reason) {
DataCategory category;
if (event is SentryTransaction) {
category = DataCategory.transaction;
} else {
category = DataCategory.error;
}
recordLostEvent(reason, category);
}
}
13 changes: 11 additions & 2 deletions dart/lib/src/sentry_envelope.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'dart:convert';
import 'client_reports/client_report.dart';
import 'protocol.dart';
import 'sentry_item_type.dart';
import 'sentry_options.dart';
Expand All @@ -12,7 +13,7 @@ import 'sentry_user_feedback.dart';
class SentryEnvelope {
SentryEnvelope(this.header, this.items);

/// Header descriping envelope content.
/// Header describing envelope content.
final SentryEnvelopeHeader header;

/// All items contained in the envelope.
Expand Down Expand Up @@ -74,7 +75,7 @@ class SentryEnvelope {
if (length < 0) {
continue;
}
// Olny attachments should be filtered according to
// Only attachments should be filtered according to
// SentryOptions.maxAttachmentSize
if (item.header.type == SentryItemType.attachment) {
if (await item.header.length() > options.maxAttachmentSize) {
Expand All @@ -88,4 +89,12 @@ class SentryEnvelope {
}
}
}

/// Add an envelope item containing client report data.
void addClientReport(ClientReport? clientReport) {
if (clientReport != null) {
final envelopeItem = SentryEnvelopeItem.fromClientReport(clientReport);
items.add(envelopeItem);
}
}
}
20 changes: 17 additions & 3 deletions dart/lib/src/transport/http_transport.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ import 'dart:convert';

import 'package:http/http.dart';

import '../client_reports/client_report_recorder.dart';
import '../client_reports/discard_reason.dart';
import 'data_category.dart';
import 'noop_encode.dart' if (dart.library.io) 'encode.dart';
import '../noop_client.dart';
import '../protocol.dart';
Expand All @@ -19,19 +22,22 @@ class HttpTransport implements Transport {

final RateLimiter _rateLimiter;

final ClientReportRecorder _clientReportRecorder;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe ClientReportRecorder should be a property of SentryOptions? cc @philipphofmann

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't put dependencies / objects to the options in ObjC. I guess you should follow your SDKs conventions and you can answer best how to follow these I think.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really have a central object/place to init dependencies AFAIK, but we rather worked with singletons for now, or other objects construct their dependencies internally, like in this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the refactoring to not expose the new classes I moved the instance of ClientReportRecorder to options. It is mainly used to access the instance in Hub, SentryClient and tests.


late _CredentialBuilder _credentialBuilder;

final Map<String, String> _headers;

factory HttpTransport(SentryOptions options, RateLimiter rateLimiter) {
factory HttpTransport(SentryOptions options, RateLimiter rateLimiter,
ClientReportRecorder clientReportRecorder) {
if (options.httpClient is NoOpClient) {
options.httpClient = Client();
}

return HttpTransport._(options, rateLimiter);
return HttpTransport._(options, rateLimiter, clientReportRecorder);
}

HttpTransport._(this._options, this._rateLimiter)
HttpTransport._(this._options, this._rateLimiter, this._clientReportRecorder)
: _dsn = Dsn.parse(_options.dsn!),
_headers = _buildHeaders(
_options.platformChecker.isWeb,
Expand All @@ -51,6 +57,9 @@ class HttpTransport implements Transport {
return SentryId.empty();
}

final clientReport = _clientReportRecorder.flush();
filteredEnvelope.addClientReport(clientReport);

final streamedRequest = await _createStreamedRequest(filteredEnvelope);
final response = await _options.httpClient
.send(streamedRequest)
Expand Down Expand Up @@ -118,6 +127,11 @@ class HttpTransport implements Transport {
_rateLimiter.updateRetryAfterLimits(
sentryRateLimitHeader, retryAfterHeader, response.statusCode);
}

@override
void recordLostEvent(DiscardReason reason, DataCategory category) {
_clientReportRecorder.recordLostEvent(reason, category);
}
}

class _CredentialBuilder {
Expand Down
5 changes: 5 additions & 0 deletions dart/lib/src/transport/noop_transport.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@ import '../sentry_envelope.dart';

import '../protocol.dart';
import 'transport.dart';
import '../client_reports/discard_reason.dart';
import 'data_category.dart';

class NoOpTransport implements Transport {
@override
Future<SentryId?> send(SentryEnvelope envelope) async => null;

@override
void recordLostEvent(DiscardReason reason, DataCategory category) {}
}
19 changes: 17 additions & 2 deletions dart/lib/src/transport/rate_limiter.dart
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import '../transport/rate_limit_parser.dart';

import '../sentry_options.dart';
import '../sentry_envelope.dart';
import '../sentry_envelope_item.dart';
import 'rate_limit.dart';
import 'data_category.dart';
import '../client_reports/client_report_recorder.dart';
import '../client_reports/discard_reason.dart';

/// Controls retry limits on different category types sent to Sentry.
class RateLimiter {
RateLimiter(this._clockProvider);
RateLimiter(this._clockProvider, this._clientReportRecorder);

final ClockProvider _clockProvider;
final ClientReportRecorder _clientReportRecorder;
final _rateLimitedUntil = <DataCategory, DateTime>{};

/// Filter out envelopes that are rate limited.
Expand All @@ -22,6 +24,19 @@ class RateLimiter {
if (_isRetryAfter(item.header.type)) {
dropItems ??= [];
dropItems.add(item);

final category = _categoryFromItemType(item.header.type);
if (category == DataCategory.transaction) {
_clientReportRecorder.recordLostEvent(
DiscardReason.rateLimitBackoff,
DataCategory.transaction,
);
} else {
_clientReportRecorder.recordLostEvent(
DiscardReason.rateLimitBackoff,
DataCategory.error,
);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions dart/lib/src/transport/transport.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@ import 'dart:async';

import '../sentry_envelope.dart';
import '../protocol.dart';
import '../client_reports/discard_reason.dart';
import 'data_category.dart';

/// A transport is in charge of sending the event/envelope either via http
/// or caching in the disk.
abstract class Transport {
Future<SentryId?> send(SentryEnvelope envelope);

void recordLostEvent(final DiscardReason reason, final DataCategory category);
}
6 changes: 3 additions & 3 deletions dart/test/client_reports/client_report_test.dart
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import 'package:collection/collection.dart';
import 'package:sentry/src/client_reports/client_report.dart';
import 'package:sentry/src/client_reports/discarded_event.dart';
import 'package:sentry/src/client_reports/outcome.dart';
import 'package:sentry/src/transport/rate_limit_category.dart';
import 'package:sentry/src/client_reports/discard_reason.dart';
import 'package:sentry/src/transport/data_category.dart';
import 'package:test/test.dart';
import 'package:sentry/src/utils.dart';

Expand Down Expand Up @@ -47,7 +47,7 @@ class Fixture {
return ClientReport(
timestamp,
[
DiscardedEvent(Outcome.ratelimitBackoff, RateLimitCategory.error, 2),
DiscardedEvent(DiscardReason.rateLimitBackoff, DataCategory.error, 2),
],
);
}
Expand Down
Loading