From caa9897d05938877c3eecf3e234dff099e1ca190 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:37:51 +0100 Subject: [PATCH 1/8] Fix child timestamp trimming --- dart/lib/src/sentry_tracer.dart | 33 +++++++++++++++++++------------ dart/test/sentry_tracer_test.dart | 22 +++++++++++++++++++++ 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index c57912fe46..56be03a690 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -49,9 +49,9 @@ class SentryTracer extends ISentrySpan { /// highest timestamp of child spans, trimming the duration of the /// transaction. This is useful to discard extra time in the transaction that /// is not accounted for in child spans, like what happens in the - /// [SentryNavigatorObserver](https://pub.dev/documentation/sentry_flutter/latest/sentry_flutter/SentryNavigatorObserver-class.html) - /// idle transactions, where we finish the transaction after a given - /// "idle time" and we don't want this "idle time" to be part of the transaction. + /// [SentryNavigatorObserver] idle transactions, where we finish the + /// transaction after a given "idle time" and we don't want this "idle time" + /// to be part of the transaction. SentryTracer( SentryTransactionContext transactionContext, this._hub, { @@ -109,18 +109,24 @@ class SentryTracer extends ISentrySpan { } var _rootEndTimestamp = commonEndTimestamp; + + // Trim the end timestamp of the transaction to the very last timestamp of child spans if (_trimEnd && children.isNotEmpty) { - final childEndTimestamps = children - .where((child) => child.endTimestamp != null) - .map((child) => child.endTimestamp!); - - if (childEndTimestamps.isNotEmpty) { - final oldestChildEndTimestamp = - childEndTimestamps.reduce((a, b) => a.isAfter(b) ? a : b); - if (_rootEndTimestamp.isAfter(oldestChildEndTimestamp)) { - _rootEndTimestamp = oldestChildEndTimestamp; + DateTime? latestEndTime; + + for (var child in children) { + final childEndTimestamp = child.endTimestamp; + if (childEndTimestamp != null) { + if (latestEndTime == null || + childEndTimestamp.isAfter(latestEndTime)) { + latestEndTime = child.endTimestamp; + } } } + + if (latestEndTime != null) { + _rootEndTimestamp = latestEndTime; + } } // the callback should run before because if the span is finished, @@ -362,7 +368,8 @@ class SentryTracer extends ISentrySpan { Dsn.parse(_hub.options.dsn!).publicKey, release: _hub.options.release, environment: _hub.options.environment, - userId: null, // because of PII not sending it for now + userId: null, + // because of PII not sending it for now userSegment: user?.segment, transaction: _isHighQualityTransactionName(transactionNameSource) ? name : null, diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 48ff011e82..d2fa187995 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -386,6 +386,28 @@ void main() { expect(sut.endTimestamp, endTimestamp); }); + test('end trimmed to latest child end timestamp', () async { + final sut = fixture.getSut(trimEnd: true); + final rootEndInitial = getUtcDateTime(); + final childEnd1 = rootEndInitial; + final childEnd2 = rootEndInitial.add(Duration(seconds: 1)); + final childEnd3 = rootEndInitial; + + final childA = sut.startChild('operation-a', description: 'description'); + final childB = sut.startChild('operation-b', description: 'description'); + final childC = sut.startChild('operation-c', description: 'description'); + + await childA.finish(endTimestamp: childEnd1); + await childB.finish(endTimestamp: childEnd2); + await childC.finish(endTimestamp: childEnd3); + + await sut.finish(endTimestamp: rootEndInitial); + + expect(sut.endTimestamp, equals(childB.endTimestamp), + reason: + 'The root end timestamp should be updated to match the latest child end timestamp.'); + }); + test('does not add more spans than configured in options', () async { fixture.hub.options.maxSpans = 2; final sut = fixture.getSut(); From 438c8a2be73c2c82c82f3032bc9542ac16f80d6e Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:49:46 +0100 Subject: [PATCH 2/8] Update CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7f259f8b0..6442a03abe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Fixes + +- Fix transaction end timestamp trimming ([#1916](https://github.com/getsentry/sentry-dart/pull/1916)) + - Transaction end timestamps are now correctly trimmed to the latest child span end timestamp + ### Features - Use `recordHttpBreadcrumbs` to set iOS `enableNetworkBreadcrumbs` ([#1884](https://github.com/getsentry/sentry-dart/pull/1884)) From c4def3a5a550e644677aa55605c1f9c3428284e3 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:53:30 +0100 Subject: [PATCH 3/8] Run formatting --- dart/test/sentry_tracer_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index d2fa187995..42c7e6f18f 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -405,7 +405,7 @@ void main() { expect(sut.endTimestamp, equals(childB.endTimestamp), reason: - 'The root end timestamp should be updated to match the latest child end timestamp.'); + 'The root end timestamp should be updated to match the latest child end timestamp.'); }); test('does not add more spans than configured in options', () async { From bbb60eccdc14f495ea40be8f5df7550be0510186 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:56:38 +0100 Subject: [PATCH 4/8] Update docs --- dart/lib/src/sentry_tracer.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 56be03a690..e1be32267d 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -49,9 +49,9 @@ class SentryTracer extends ISentrySpan { /// highest timestamp of child spans, trimming the duration of the /// transaction. This is useful to discard extra time in the transaction that /// is not accounted for in child spans, like what happens in the - /// [SentryNavigatorObserver] idle transactions, where we finish the - /// transaction after a given "idle time" and we don't want this "idle time" - /// to be part of the transaction. + /// [SentryNavigatorObserver](https://pub.dev/documentation/sentry_flutter/latest/sentry_flutter/SentryNavigatorObserver-class.html) + /// idle transactions, where we finish the transaction after a given + /// "idle time" and we don't want this "idle time" to be part of the transaction. SentryTracer( SentryTransactionContext transactionContext, this._hub, { From f9baa715bed84c57cbf822b2544663bef6fb99c9 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Tue, 5 Mar 2024 19:57:35 +0100 Subject: [PATCH 5/8] Revert --- dart/lib/src/sentry_tracer.dart | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index e1be32267d..ed3fb35799 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -368,8 +368,7 @@ class SentryTracer extends ISentrySpan { Dsn.parse(_hub.options.dsn!).publicKey, release: _hub.options.release, environment: _hub.options.environment, - userId: null, - // because of PII not sending it for now + userId: null, // because of PII not sending it for now userSegment: user?.segment, transaction: _isHighQualityTransactionName(transactionNameSource) ? name : null, From 22a50551468150a391a657ce4119e9360d8b9327 Mon Sep 17 00:00:00 2001 From: Giancarlo Buenaflor Date: Wed, 6 Mar 2024 11:27:05 +0100 Subject: [PATCH 6/8] Update dart/test/sentry_tracer_test.dart Co-authored-by: Philipp Hofmann --- dart/test/sentry_tracer_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 42c7e6f18f..6a23d21999 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -389,9 +389,9 @@ void main() { test('end trimmed to latest child end timestamp', () async { final sut = fixture.getSut(trimEnd: true); final rootEndInitial = getUtcDateTime(); - final childEnd1 = rootEndInitial; - final childEnd2 = rootEndInitial.add(Duration(seconds: 1)); - final childEnd3 = rootEndInitial; + final childAEnd = rootEndInitial; + final childBEnd = rootEndInitial.add(Duration(seconds: 1)); + final childCEnd = rootEndInitial; final childA = sut.startChild('operation-a', description: 'description'); final childB = sut.startChild('operation-b', description: 'description'); From 8a25ff3fec74c2d1f94e18ba14e78a2f97fe6e1f Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 11:39:07 +0100 Subject: [PATCH 7/8] change var to final --- dart/lib/src/sentry_tracer.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index ed3fb35799..c7f6993ad3 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -114,7 +114,7 @@ class SentryTracer extends ISentrySpan { if (_trimEnd && children.isNotEmpty) { DateTime? latestEndTime; - for (var child in children) { + for (final child in children) { final childEndTimestamp = child.endTimestamp; if (childEndTimestamp != null) { if (latestEndTime == null || From 85daa5cb1d4236647cf48726c4459ad313af0790 Mon Sep 17 00:00:00 2001 From: GIancarlo Buenaflor Date: Wed, 6 Mar 2024 11:40:56 +0100 Subject: [PATCH 8/8] fix test --- dart/test/sentry_tracer_test.dart | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 6a23d21999..1a328702d1 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -397,9 +397,9 @@ void main() { final childB = sut.startChild('operation-b', description: 'description'); final childC = sut.startChild('operation-c', description: 'description'); - await childA.finish(endTimestamp: childEnd1); - await childB.finish(endTimestamp: childEnd2); - await childC.finish(endTimestamp: childEnd3); + await childA.finish(endTimestamp: childAEnd); + await childB.finish(endTimestamp: childBEnd); + await childC.finish(endTimestamp: childCEnd); await sut.finish(endTimestamp: rootEndInitial);