Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Allow to set endTimestamp manually when finishing span
  • Loading branch information
fatih ergin committed Mar 8, 2022
commit 2b2fd7d223e361f8826dc9647cd1f60cbc4addfb
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

## Unreleased

* Feat: Allow to set startTimestamp & endTimestamp manually to SentrySpan (#676)
* Bump: Sentry-Cocoa to 7.10.0 (#777)
* Feat: Additional Dart/Flutter context information (#778)
* Bump: Kotlin plugin to 1.5.31 (#763)
Expand Down
2 changes: 1 addition & 1 deletion dart/lib/src/noop_sentry_span.dart
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class NoOpSentrySpan extends ISentrySpan {
}

@override
Future<void> finish({SpanStatus? status}) async {}
Future<void> finish({SpanStatus? status, DateTime? endTimestamp}) async {}

@override
void removeData(String key) {}
Expand Down
32 changes: 22 additions & 10 deletions dart/lib/src/protocol/sentry_span.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import '../utils.dart';

class SentrySpan extends ISentrySpan {
final SentrySpanContext _context;
DateTime? _timestamp;
DateTime? _endTimestamp;
late final DateTime _startTimestamp;
final Hub _hub;

Expand All @@ -19,7 +19,7 @@ class SentrySpan extends ISentrySpan {

SpanStatus? _status;
final Map<String, String> _tags = {};
void Function()? _finishedCallback;
void Function({DateTime? endTimestamp})? _finishedCallback;

@override
bool? sampled;
Expand All @@ -30,7 +30,7 @@ class SentrySpan extends ISentrySpan {
this._hub, {
DateTime? startTimestamp,
bool? sampled,
Function()? finishedCallback,
Function({DateTime? endTimestamp})? finishedCallback,
}) {
_startTimestamp = startTimestamp?.toUtc() ?? getUtcDateTime();
this.sampled = sampled;
Expand All @@ -43,17 +43,28 @@ class SentrySpan extends ISentrySpan {
return;
}

final utcDateTime = getUtcDateTime();

if (status != null) {
_status = status;
}
_timestamp = endTimestamp ?? getUtcDateTime();

if (endTimestamp?.isBefore(_startTimestamp) ?? false) {
_hub.options.logger(
SentryLevel.warning,
'End timestamp ($endTimestamp) cannot be before start timestamp ($_startTimestamp)',
);
_endTimestamp = utcDateTime;
} else {
_endTimestamp = endTimestamp?.toUtc() ?? utcDateTime;
}

// associate error
if (_throwable != null) {
_hub.setSpanContext(_throwable, this, _tracer.name);
}
_finishedCallback?.call();
await super.finish(status: status);
_finishedCallback?.call(endTimestamp: _endTimestamp);
await super.finish(status: status, endTimestamp: _endTimestamp);
}

@override
Expand Down Expand Up @@ -128,7 +139,7 @@ class SentrySpan extends ISentrySpan {
DateTime get startTimestamp => _startTimestamp;

@override
DateTime? get endTimestamp => _timestamp;
DateTime? get endTimestamp => _endTimestamp;

@override
SentrySpanContext get context => _context;
Expand All @@ -137,8 +148,9 @@ class SentrySpan extends ISentrySpan {
final json = _context.toJson();
json['start_timestamp'] =
formatDateAsIso8601WithMillisPrecision(_startTimestamp);
if (_timestamp != null) {
json['timestamp'] = formatDateAsIso8601WithMillisPrecision(_timestamp!);
if (_endTimestamp != null) {
json['timestamp'] =
formatDateAsIso8601WithMillisPrecision(_endTimestamp!);
}
if (_data.isNotEmpty) {
json['data'] = _data;
Expand All @@ -153,7 +165,7 @@ class SentrySpan extends ISentrySpan {
}

@override
bool get finished => _timestamp != null;
bool get finished => _endTimestamp != null;

@override
dynamic get throwable => _throwable;
Expand Down
2 changes: 1 addition & 1 deletion dart/lib/src/sentry_span_interface.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ abstract class ISentrySpan {
void removeData(String key);

/// Sets span timestamp marking this span as finished.
Future<void> finish({SpanStatus? status}) async {}
Future<void> finish({SpanStatus? status, DateTime? endTimestamp}) async {}

/// Gets span status.
SpanStatus? get status;
Expand Down
33 changes: 22 additions & 11 deletions dart/lib/src/sentry_tracer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,27 @@ class SentryTracer extends ISentrySpan {
}

@override
Future<void> finish({SpanStatus? status}) async {
Future<void> finish({SpanStatus? status, DateTime? endTimestamp}) async {
final commonEndTimestamp = endTimestamp ?? getUtcDateTime();
_autoFinishAfterTimer?.cancel();
_finishStatus = SentryTracerFinishStatus.finishing(status);
if (!_rootSpan.finished &&
(!_waitForChildren || _haveAllChildrenFinished())) {
_rootSpan.status ??= status;

// finish unfinished spans otherwise transaction gets dropped
for (final span in _children) {
if (!span.finished) {
await span.finish(status: SpanStatus.deadlineExceeded());
}
}
// remove span where its endTimestamp is before startTimestamp
_children.removeWhere(
(span) => !_hasSpanSuitableTimestamps(span, commonEndTimestamp));

var _rootEndTimestamp = getUtcDateTime();
// finish unfinished spans otherwise transaction gets dropped
final spansToBeFinished = _children.where((span) => !span.finished);
await Future.forEach(
spansToBeFinished,
(SentrySpan span) async => await span.finish(
status: SpanStatus.deadlineExceeded(),
endTimestamp: commonEndTimestamp));

var _rootEndTimestamp = commonEndTimestamp;
if (_trimEnd && children.isNotEmpty) {
final childEndTimestamps = children
.where((child) => child.endTimestamp != null)
Expand Down Expand Up @@ -184,11 +190,11 @@ class SentryTracer extends ISentrySpan {
description: description);

final child = SentrySpan(this, context, _hub,
sampled: _rootSpan.sampled,
startTimestamp: startTimestamp, finishedCallback: () {
sampled: _rootSpan.sampled, startTimestamp: startTimestamp,
finishedCallback: ({DateTime? endTimestamp}) {
final finishStatus = _finishStatus;
if (finishStatus.finishing) {
finish(status: finishStatus.status);
finish(status: finishStatus.status, endTimestamp: endTimestamp);
}
});

Expand Down Expand Up @@ -241,4 +247,9 @@ class SentryTracer extends ISentrySpan {
}
return true;
}

bool _hasSpanSuitableTimestamps(
SentrySpan span, DateTime endTimestampCandidate) =>
!span.startTimestamp
.isAfter((span.endTimestamp ?? endTimestampCandidate));
}
38 changes: 35 additions & 3 deletions dart/test/sentry_span_test.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:sentry/sentry.dart';
import 'package:sentry/src/sentry_tracer.dart';
import 'package:sentry/src/utils.dart';
import 'package:test/test.dart';

import 'mocks/mock_hub.dart';
Expand All @@ -16,6 +17,17 @@ void main() {
expect(sut.startTimestamp.isUtc, true);
});

test('convert given endTimestamp to utc date time', () async {
final nonUtcEndTimestamp = DateTime.now().toLocal();

final sut = fixture.getSut(startTimestamp: nonUtcEndTimestamp);

await sut.finish(endTimestamp: nonUtcEndTimestamp);

expect(nonUtcEndTimestamp.isUtc, false);
expect(sut.endTimestamp?.isUtc, true);
});

test('finish sets status', () async {
final sut = fixture.getSut();

Expand All @@ -32,6 +44,15 @@ void main() {
expect(sut.endTimestamp, isNotNull);
});

test('finish uses given end timestamp', () async {
final sut = fixture.getSut();
final endTimestamp = getUtcDateTime();

expect(sut.endTimestamp, isNull);
await sut.finish(endTimestamp: endTimestamp);
expect(sut.endTimestamp, endTimestamp);
});

test('finish sets throwable', () {
final sut = fixture.getSut();
sut.throwable = StateError('message');
Expand All @@ -41,6 +62,17 @@ void main() {
expect(fixture.hub.spanContextCals, 1);
});

test(
'finish does not set endTimestamp if given end timestamp is before start timestamp',
() async {
final sut = fixture.getSut();

final invalidEndTimestamp = sut.startTimestamp.add(-Duration(hours: 1));
await sut.finish(endTimestamp: invalidEndTimestamp);

expect(sut.endTimestamp, isNot(equals(invalidEndTimestamp)));
});

test('span adds data', () {
final sut = fixture.getSut();

Expand Down Expand Up @@ -194,7 +226,7 @@ void main() {

test('callback called on finish', () async {
var numberOfCallbackCalls = 0;
final sut = fixture.getSut(finishedCallback: () {
final sut = fixture.getSut(finishedCallback: ({DateTime? endTimestamp}) {
numberOfCallbackCalls += 1;
});

Expand All @@ -206,7 +238,7 @@ void main() {
test('optional endTimestamp set instead of current time', () async {
final sut = fixture.getSut();

final endTimestamp = DateTime.now().add(Duration(days: 1));
final endTimestamp = getUtcDateTime().add(Duration(days: 1));

await sut.finish(endTimestamp: endTimestamp);

Expand All @@ -225,7 +257,7 @@ class Fixture {
SentrySpan getSut(
{DateTime? startTimestamp,
bool? sampled = true,
Function()? finishedCallback}) {
Function({DateTime? endTimestamp})? finishedCallback}) {
tracer = SentryTracer(context, hub);

return SentrySpan(
Expand Down
68 changes: 43 additions & 25 deletions dart/test/sentry_tracer_test.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:sentry/sentry.dart';
import 'package:sentry/src/sentry_tracer.dart';
import 'package:sentry/src/utils.dart';
import 'package:test/test.dart';

import 'mocks/mock_hub.dart';
Expand Down Expand Up @@ -34,6 +35,32 @@ void main() {
expect(trace?.status.toString(), 'aborted');
});

test('tracer finishes with end timestamp', () async {
final sut = fixture.getSut();
final endTimestamp = getUtcDateTime();

await sut.finish(endTimestamp: endTimestamp);

expect(sut.endTimestamp, endTimestamp);
});

test(
'tracer finish sets given end timestamp to all children while finishing them',
() async {
final sut = fixture.getSut();

final childA = sut.startChild('operation-a', description: 'description');
final childB = sut.startChild('operation-b', description: 'description');
final endTimestamp = getUtcDateTime();

await sut.finish(endTimestamp: endTimestamp);
await childA.finish();
await childB.finish();

expect(childA.endTimestamp, endTimestamp);
expect(childB.endTimestamp, endTimestamp);
});

test('tracer finishes unfinished spans', () async {
final sut = fixture.getSut();
sut.startChild('child');
Expand Down Expand Up @@ -238,50 +265,41 @@ void main() {
});

test('end trimmed to last child', () async {
final sut = fixture.getSut(
trimEnd: true, autoFinishAfter: Duration(milliseconds: 200));
final sut = fixture.getSut(trimEnd: true);
final endTimestamp = getUtcDateTime().add(Duration(minutes: 1));
final olderEndTimeStamp = endTimestamp.add(Duration(seconds: 1));
final oldestEndTimeStamp = olderEndTimeStamp.add(Duration(seconds: 1));

final childA = sut.startChild('operation-a', description: 'description');
final childB = sut.startChild('operation-b', description: 'description');

await childA.finish();
await Future.delayed(Duration(milliseconds: 10));
await childB.finish();
await Future.delayed(Duration(milliseconds: 210));
await childA.finish(endTimestamp: endTimestamp);
await childB.finish(endTimestamp: olderEndTimeStamp);
await sut.finish(endTimestamp: oldestEndTimeStamp);

expect(sut.endTimestamp, childB.endTimestamp);
});

test('end trimmed to child', () async {
final sut = fixture.getSut(
trimEnd: true, autoFinishAfter: Duration(milliseconds: 200));
final sut = fixture.getSut(trimEnd: true);
final endTimestamp = getUtcDateTime().add(Duration(minutes: 1));
final olderEndTimeStamp = endTimestamp.add(Duration(seconds: 1));

final childA = sut.startChild('operation-a', description: 'description');

await childA.finish();
await Future.delayed(Duration(milliseconds: 210));
await childA.finish(endTimestamp: endTimestamp);
await sut.finish(endTimestamp: olderEndTimeStamp);

expect(sut.endTimestamp, childA.endTimestamp);
});

test('end not trimmed when no child', () async {
final sut = fixture.getSut(
trimEnd: true, autoFinishAfter: Duration(milliseconds: 200));
final sut = fixture.getSut(trimEnd: true);
final endTimestamp = getUtcDateTime();

await Future.delayed(Duration(milliseconds: 210));

expect(sut.endTimestamp, isNotNull);
});

test('end not trimmed when no finished child', () async {
final sut = fixture.getSut(
trimEnd: true, autoFinishAfter: Duration(milliseconds: 200));

sut.startChild('operation-a', description: 'description');

await Future.delayed(Duration(milliseconds: 210));
await sut.finish(endTimestamp: endTimestamp);

expect(sut.endTimestamp, isNotNull);
expect(sut.endTimestamp, endTimestamp);
});

test('does not add more spans than configured in options', () async {
Expand Down
6 changes: 4 additions & 2 deletions flutter/test/mocks.mocks.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,10 @@ class MockNoOpSentrySpan extends _i1.Mock implements _i2.NoOpSentrySpan {
super.noSuchMethod(Invocation.setter(#status, status),
returnValueForMissingStub: null);
@override
_i7.Future<void> finish({_i3.SpanStatus? status}) =>
(super.noSuchMethod(Invocation.method(#finish, [], {#status: status}),
_i7.Future<void> finish({_i3.SpanStatus? status, DateTime? endTimestamp}) =>
(super.noSuchMethod(
Invocation.method(
#finish, [], {#status: status, #endTimestamp: endTimestamp}),
returnValue: Future<void>.value(),
returnValueForMissingStub: Future<void>.value()) as _i7.Future<void>);
@override
Expand Down