Skip to content
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

Record dropped spans in client reports #2154

Merged
merged 16 commits into from
Jul 11, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

### Features

- Record dropped spans in client reports ([#2154](https://github.com/getsentry/sentry-dart/pull/2154))
- Add memory usage to contexts ([#2133](https://github.com/getsentry/sentry-dart/pull/2133))
- Only for Linux/Windows applications, as iOS/Android/macOS use native SDKs

Expand Down
6 changes: 3 additions & 3 deletions dart/lib/src/client_reports/client_report_recorder.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ class ClientReportRecorder {
final ClockProvider _clock;
final Map<_QuantityKey, int> _quantities = {};

void recordLostEvent(
final DiscardReason reason, final DataCategory category) {
void recordLostEvent(final DiscardReason reason, final DataCategory category,
{int count = 1}) {
final key = _QuantityKey(reason, category);
var current = _quantities[key] ?? 0;
_quantities[key] = current + 1;
_quantities[key] = current + count;
}

ClientReport? flush() {
Expand Down
2 changes: 2 additions & 0 deletions dart/lib/src/client_reports/discarded_event.dart
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
return 'session';
case DataCategory.transaction:
return 'transaction';
case DataCategory.span:

Check warning on line 57 in dart/lib/src/client_reports/discarded_event.dart

View check run for this annotation

Codecov / codecov/patch

dart/lib/src/client_reports/discarded_event.dart#L57

Added line #L57 was not covered by tests
return 'span';
case DataCategory.attachment:
return 'attachment';
case DataCategory.security:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,6 @@ class NoOpClientReportRecorder implements ClientReportRecorder {
}

@override
void recordLostEvent(DiscardReason reason, DataCategory category) {}
void recordLostEvent(DiscardReason reason, DataCategory category,
{int count = 1}) {}
}
5 changes: 5 additions & 0 deletions dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,11 @@ class Hub {
DiscardReason.sampleRate,
DataCategory.transaction,
);
_options.recorder.recordLostEvent(
DiscardReason.sampleRate,
DataCategory.span,
count: transaction.spans.length + 1,
);
_options.logger(
SentryLevel.warning,
'Transaction ${transaction.eventId} was dropped due to sampling decision.',
Expand Down
58 changes: 38 additions & 20 deletions dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ class SentryClient {
Hint? hint,
}) async {
if (_sampleRate()) {
_recordLostEvent(event, DiscardReason.sampleRate);
_options.recorder
.recordLostEvent(DiscardReason.sampleRate, _getCategory(event));
_options.logger(
SentryLevel.debug,
'Event ${event.eventId.toString()} was dropped due to sampling decision.',
Expand Down Expand Up @@ -403,7 +404,9 @@ class SentryClient {
SentryEvent event,
Hint hint,
) async {
SentryEvent? eventOrTransaction = event;
SentryEvent? processedEvent = event;
final spanCountBeforeCallback =
event is SentryTransaction ? event.spans.length : 0;

final beforeSend = _options.beforeSend;
final beforeSendTransaction = _options.beforeSendTransaction;
Expand All @@ -412,18 +415,18 @@ class SentryClient {
try {
if (event is SentryTransaction && beforeSendTransaction != null) {
beforeSendName = 'beforeSendTransaction';
final e = beforeSendTransaction(event);
if (e is Future<SentryTransaction?>) {
eventOrTransaction = await e;
final callbackResult = beforeSendTransaction(event);
if (callbackResult is Future<SentryTransaction?>) {
processedEvent = await callbackResult;
} else {
eventOrTransaction = e;
processedEvent = callbackResult;
}
} else if (beforeSend != null) {
final e = beforeSend(event, hint);
if (e is Future<SentryEvent?>) {
eventOrTransaction = await e;
final callbackResult = beforeSend(event, hint);
if (callbackResult is Future<SentryEvent?>) {
processedEvent = await callbackResult;
} else {
eventOrTransaction = e;
processedEvent = callbackResult;
}
}
} catch (exception, stackTrace) {
Expand All @@ -438,15 +441,32 @@ class SentryClient {
}
}

if (eventOrTransaction == null) {
_recordLostEvent(event, DiscardReason.beforeSend);
if (processedEvent == null) {
_options.recorder
.recordLostEvent(DiscardReason.beforeSend, _getCategory(event));
if (event is SentryTransaction) {
// We dropped the whole transaction, the dropped count includes all child spans + 1 root span
_options.recorder.recordLostEvent(
DiscardReason.beforeSend, DataCategory.span,
count: spanCountBeforeCallback + 1);
}
_options.logger(
SentryLevel.debug,
'${event.runtimeType} was dropped by $beforeSendName callback',
);
} else if (event is SentryTransaction &&
processedEvent is SentryTransaction) {
// If beforeSend removed only some spans we still report them as dropped
final spanCountAfterCallback = processedEvent.spans.length;
final droppedSpanCount = spanCountBeforeCallback - spanCountAfterCallback;
if (droppedSpanCount > 0) {
_options.recorder.recordLostEvent(
DiscardReason.beforeSend, DataCategory.span,
count: droppedSpanCount);
}
}

return eventOrTransaction;
return processedEvent;
}

Future<SentryEvent?> _runEventProcessors(
Expand Down Expand Up @@ -475,7 +495,8 @@ class SentryClient {
}
}
if (processedEvent == null) {
_recordLostEvent(event, DiscardReason.eventProcessor);
_options.recorder
.recordLostEvent(DiscardReason.eventProcessor, _getCategory(event));
_options.logger(SentryLevel.debug, 'Event was dropped by a processor');
break;
}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the same logic of the beforeSend callback here?
report discarded spans from a processor even if the transaction is not dropped
(note that computation should continue in that case, so no need to break)

Expand All @@ -490,14 +511,11 @@ class SentryClient {
return false;
}

void _recordLostEvent(SentryEvent event, DiscardReason reason) {
DataCategory category;
DataCategory _getCategory(SentryEvent event) {
if (event is SentryTransaction) {
category = DataCategory.transaction;
} else {
category = DataCategory.error;
return DataCategory.transaction;
}
_options.recorder.recordLostEvent(reason, category);
return DataCategory.error;
}

Future<SentryId?> _attachClientReportsAndSend(SentryEnvelope envelope) {
Expand Down
32 changes: 20 additions & 12 deletions dart/lib/src/sentry_envelope_item.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import 'sentry_user_feedback.dart';

/// Item holding header information and JSON encoded data.
class SentryEnvelopeItem {
SentryEnvelopeItem(this.header, this.dataFactory);
/// The original, non-encoded object, used when direct access to the source data is needed.
Object? originalObject;
Comment on lines +15 to +16
Copy link
Contributor Author

@buenaflor buenaflor Jul 10, 2024

Choose a reason for hiding this comment

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

used so we don't have to deserialize the envelope when we want to access the number of spans in a transaction for example


SentryEnvelopeItem(this.header, this.dataFactory, {this.originalObject});

/// Creates a [SentryEnvelopeItem] which sends [SentryTransaction].
factory SentryEnvelopeItem.fromTransaction(SentryTransaction transaction) {
Expand All @@ -24,7 +27,8 @@ class SentryEnvelopeItem {
cachedItem.getDataLength,
contentType: 'application/json',
);
return SentryEnvelopeItem(header, cachedItem.getData);
return SentryEnvelopeItem(header, cachedItem.getData,
originalObject: transaction);
}

factory SentryEnvelopeItem.fromAttachment(SentryAttachment attachment) {
Expand All @@ -37,7 +41,8 @@ class SentryEnvelopeItem {
fileName: attachment.filename,
attachmentType: attachment.attachmentType,
);
return SentryEnvelopeItem(header, cachedItem.getData);
return SentryEnvelopeItem(header, cachedItem.getData,
originalObject: attachment);
}

/// Create a [SentryEnvelopeItem] which sends [SentryUserFeedback].
Expand All @@ -50,7 +55,8 @@ class SentryEnvelopeItem {
cachedItem.getDataLength,
contentType: 'application/json',
);
return SentryEnvelopeItem(header, cachedItem.getData);
return SentryEnvelopeItem(header, cachedItem.getData,
originalObject: feedback);
}

/// Create a [SentryEnvelopeItem] which holds the [SentryEvent] data.
Expand All @@ -59,13 +65,13 @@ class SentryEnvelopeItem {
_CachedItem(() async => utf8JsonEncoder.convert(event.toJson()));

return SentryEnvelopeItem(
SentryEnvelopeItemHeader(
SentryItemType.event,
cachedItem.getDataLength,
contentType: 'application/json',
),
cachedItem.getData,
);
SentryEnvelopeItemHeader(
SentryItemType.event,
cachedItem.getDataLength,
contentType: 'application/json',
),
cachedItem.getData,
originalObject: event);
}

/// Create a [SentryEnvelopeItem] which holds the [ClientReport] data.
Expand All @@ -80,6 +86,7 @@ class SentryEnvelopeItem {
contentType: 'application/json',
),
cachedItem.getData,
originalObject: clientReport,
);
}

Expand All @@ -102,7 +109,8 @@ class SentryEnvelopeItem {
cachedItem.getDataLength,
contentType: 'application/octet-stream',
);
return SentryEnvelopeItem(header, cachedItem.getData);
return SentryEnvelopeItem(header, cachedItem.getData,
originalObject: buckets);
}

/// Header with info about type and length of data in bytes.
Expand Down
22 changes: 21 additions & 1 deletion dart/lib/src/transport/data_category.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,28 @@ enum DataCategory {
error,
session,
transaction,
span,
attachment,
security,
metricBucket,
unknown
unknown;

static DataCategory fromItemType(String itemType) {
switch (itemType) {
case 'event':
return DataCategory.error;
case 'session':
return DataCategory.session;
case 'attachment':
return DataCategory.attachment;
case 'transaction':
return DataCategory.transaction;
// The envelope item type used for metrics is statsd,
// whereas the client report category is metric_bucket
case 'statsd':
return DataCategory.metricBucket;
default:
return DataCategory.unknown;
}
}
}
36 changes: 12 additions & 24 deletions dart/lib/src/transport/rate_limiter.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import '../../sentry.dart';
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/discard_reason.dart';
Expand All @@ -25,8 +23,17 @@ class RateLimiter {

_options.recorder.recordLostEvent(
DiscardReason.rateLimitBackoff,
_categoryFromItemType(item.header.type),
DataCategory.fromItemType(item.header.type),
);

final originalObject = item.originalObject;
if (originalObject is SentryTransaction) {
_options.recorder.recordLostEvent(
DiscardReason.rateLimitBackoff,
DataCategory.span,
count: originalObject.spans.length + 1,
);
}
}
}

Expand Down Expand Up @@ -80,7 +87,7 @@ class RateLimiter {
// Private

bool _isRetryAfter(String itemType) {
final dataCategory = _categoryFromItemType(itemType);
final dataCategory = DataCategory.fromItemType(itemType);
final currentDate = DateTime.fromMillisecondsSinceEpoch(
_options.clock().millisecondsSinceEpoch);

Expand All @@ -106,25 +113,6 @@ class RateLimiter {
return false;
}

DataCategory _categoryFromItemType(String itemType) {
switch (itemType) {
case 'event':
return DataCategory.error;
case 'session':
return DataCategory.session;
case 'attachment':
return DataCategory.attachment;
case 'transaction':
return DataCategory.transaction;
// The envelope item type used for metrics is statsd,
// whereas the client report category is metric_bucket
case 'statsd':
return DataCategory.metricBucket;
default:
return DataCategory.unknown;
}
}

void _applyRetryAfterOnlyIfLonger(DataCategory dataCategory, DateTime date) {
final oldDate = _rateLimitedUntil[dataCategory];

Expand Down
17 changes: 15 additions & 2 deletions dart/lib/src/utils/transport_utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,21 @@ class TransportUtils {
}

if (response.statusCode >= 400 && response.statusCode != 429) {
options.recorder
.recordLostEvent(DiscardReason.networkError, DataCategory.error);
for (final item in envelope.items) {
options.recorder.recordLostEvent(
DiscardReason.networkError,
DataCategory.fromItemType(item.header.type),
);

final originalObject = item.originalObject;
if (originalObject is SentryTransaction) {
options.recorder.recordLostEvent(
DiscardReason.networkError,
DataCategory.span,
count: originalObject.spans.length + 1,
);
}
}
}
} else {
options.logger(
Expand Down
15 changes: 13 additions & 2 deletions dart/test/hub_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -694,11 +694,22 @@ void main() {
test('record sample rate dropping transaction', () async {
final hub = fixture.getSut(sampled: false);
var transaction = SentryTransaction(fixture.tracer);
fixture.tracer.startChild('child1');
fixture.tracer.startChild('child2');
fixture.tracer.startChild('child3');

await hub.captureTransaction(transaction);

expect(fixture.recorder.reason, DiscardReason.sampleRate);
expect(fixture.recorder.category, DataCategory.transaction);
expect(fixture.recorder.discardedEvents.length, 2);

// we dropped the whole tracer and it has 3 span children so the span count should be 4
// 3 children + 1 root span
final spanCount = fixture.recorder.discardedEvents
.firstWhere((element) =>
element.category == DataCategory.span &&
element.reason == DiscardReason.sampleRate)
.quantity;
expect(spanCount, 4);
});
});

Expand Down
Loading
Loading