From 708afecc4e7ab938f0ad14cbd16270500833f966 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 28 Sep 2022 15:29:15 +0200 Subject: [PATCH 1/9] Does not allow setting measurement if finished --- dart/lib/src/sentry_tracer.dart | 3 +++ dart/test/sentry_tracer_test.dart | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index f81d14c1fb..08a9cee3de 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -284,6 +284,9 @@ class SentryTracer extends ISentrySpan { @override void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { + if (finished) { + return; + } final measurement = SentryMeasurement(name, value, unit: unit); _measurements[name] = measurement; } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 8dfc8ed138..676e00572f 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -373,6 +373,16 @@ void main() { expect(sut.startChild('child3'), isA()); }); + + test('tracer does not allow setting measurement if finished', () async { + final sut = fixture.getSut(); + + await sut.finish(); + + sut.setMeasurement('key', 1.0); + + expect(sut.measurements.isEmpty, true); + }); }); group('$SentryBaggageHeader', () { From 56ab789752cfc25750f626f6bdfa1e7aa076acaf Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 28 Sep 2022 15:30:38 +0200 Subject: [PATCH 2/9] pr id --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee4367c976..7038fc23f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Align span spec for serialize ops ([#1024](https://github.com/getsentry/sentry-dart/pull/1024)) - Pin sentry version ([#1020](https://github.com/getsentry/sentry-dart/pull/1020)) +- Tracer does not allow setting measurement if finished ([#1026](https://github.com/getsentry/sentry-dart/pull/1026)) ### Features From f19e5a49ff3f3ab049b9698f2f3ba4cd6282b0f3 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 10:05:00 +0200 Subject: [PATCH 3/9] fix --- dart/lib/src/sentry_tracer.dart | 4 ++- .../navigation/sentry_navigator_observer.dart | 26 ++++++++----------- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 08a9cee3de..317be3fccd 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -106,8 +106,10 @@ class SentryTracer extends ISentrySpan { } } - await _rootSpan.finish(endTimestamp: _rootEndTimestamp); + // the callback should run before because if the span is finished, + // we cannot attach data, its immutable after being finished. await _onFinish?.call(this); + await _rootSpan.finish(endTimestamp: _rootEndTimestamp); // remove from scope await _hub.configureScope((scope) { diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index b32d18e1ae..76d6ed39f9 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -1,5 +1,4 @@ // ignore: implementation_imports -import 'package:sentry/src/sentry_tracer.dart'; import 'package:flutter/widgets.dart'; import '../../sentry_flutter.dart'; @@ -181,20 +180,17 @@ class SentryNavigatorObserver extends RouteObserver> { autoFinishAfter: _autoFinishAfter, trimEnd: true, onFinish: (transaction) async { - // ignore: invalid_use_of_internal_member - if (transaction is SentryTracer) { - final nativeFrames = await _native - .endNativeFramesCollection(transaction.context.traceId); - if (nativeFrames != null) { - final measurements = nativeFrames.toMeasurements(); - for (final item in measurements.entries) { - final measurement = item.value; - transaction.setMeasurement( - item.key, - measurement.value, - unit: measurement.unit, - ); - } + final nativeFrames = await _native + .endNativeFramesCollection(transaction.context.traceId); + if (nativeFrames != null) { + final measurements = nativeFrames.toMeasurements(); + for (final item in measurements.entries) { + final measurement = item.value; + transaction.setMeasurement( + item.key, + measurement.value, + unit: measurement.unit, + ); } } }, From 6530b5600c4203c1f6354bbde7f7b239c97b87b9 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 10:15:11 +0200 Subject: [PATCH 4/9] sets none to default --- dart/lib/src/protocol/sentry_span.dart | 2 +- dart/lib/src/sentry_tracer.dart | 6 +++++- dart/test/sentry_tracer_test.dart | 22 ++++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index bad87759b9..b7690be242 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -187,7 +187,7 @@ class SentrySpan extends ISentrySpan { void setMeasurement( String name, num value, { - SentryMeasurementUnit? unit, + SentryMeasurementUnit? unit = SentryMeasurementUnit.none, }) { _tracer.setMeasurement(name, value, unit: unit); } diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 317be3fccd..0973c6470b 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -285,7 +285,11 @@ class SentryTracer extends ISentrySpan { .isAfter((span.endTimestamp ?? endTimestampCandidate)); @override - void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { + void setMeasurement( + String name, + num value, { + SentryMeasurementUnit? unit = SentryMeasurementUnit.none, + }) { if (finished) { return; } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index 676e00572f..fa145fbbec 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -374,6 +374,28 @@ void main() { expect(sut.startChild('child3'), isA()); }); + test('tracer sets measurement', () async { + final sut = fixture.getSut(); + + sut.setMeasurement('key', 1.0); + + expect(sut.measurements['key']!.value, 1.0); + expect(sut.measurements['key']?.unit, SentryMeasurementUnit.none); + + await sut.finish(); + }); + + test('tracer sets custom measurement unit', () async { + final sut = fixture.getSut(); + + sut.setMeasurement('key', 1.0, unit: SentryMeasurementUnit.hour); + + expect(sut.measurements['key']!.value, 1.0); + expect(sut.measurements['key']?.unit, SentryMeasurementUnit.hour); + + await sut.finish(); + }); + test('tracer does not allow setting measurement if finished', () async { final sut = fixture.getSut(); From e2a36d890bdd6840a376b6f760ec0353de77a7e2 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 10:53:53 +0200 Subject: [PATCH 5/9] fix --- dart/lib/src/protocol/sentry_span.dart | 6 +----- dart/lib/src/sentry_tracer.dart | 6 +----- dart/test/sentry_tracer_test.dart | 1 - 3 files changed, 2 insertions(+), 11 deletions(-) diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index b7690be242..1dc56ade25 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -184,11 +184,7 @@ class SentrySpan extends ISentrySpan { ); @override - void setMeasurement( - String name, - num value, { - SentryMeasurementUnit? unit = SentryMeasurementUnit.none, - }) { + void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { _tracer.setMeasurement(name, value, unit: unit); } diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 0973c6470b..317be3fccd 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -285,11 +285,7 @@ class SentryTracer extends ISentrySpan { .isAfter((span.endTimestamp ?? endTimestampCandidate)); @override - void setMeasurement( - String name, - num value, { - SentryMeasurementUnit? unit = SentryMeasurementUnit.none, - }) { + void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { if (finished) { return; } diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index fa145fbbec..06185e9e4c 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -380,7 +380,6 @@ void main() { sut.setMeasurement('key', 1.0); expect(sut.measurements['key']!.value, 1.0); - expect(sut.measurements['key']?.unit, SentryMeasurementUnit.none); await sut.finish(); }); From 9094eec621848820782267676d2f9a308d7adb6e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 10:55:27 +0200 Subject: [PATCH 6/9] remove import --- flutter/lib/src/navigation/sentry_navigator_observer.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/flutter/lib/src/navigation/sentry_navigator_observer.dart b/flutter/lib/src/navigation/sentry_navigator_observer.dart index 76d6ed39f9..e504acc824 100644 --- a/flutter/lib/src/navigation/sentry_navigator_observer.dart +++ b/flutter/lib/src/navigation/sentry_navigator_observer.dart @@ -1,4 +1,3 @@ -// ignore: implementation_imports import 'package:flutter/widgets.dart'; import '../../sentry_flutter.dart'; From 66c5ceb7617e17ce4e1ff9aead3e05d651aca1bc Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 10:56:28 +0200 Subject: [PATCH 7/9] changelog --- CHANGELOG.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b429227b70..50edae673e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,17 @@ # Changelog +## Unreleased + +### Fixes + +- Tracer does not allow setting measurement if finished ([#1026](https://github.com/getsentry/sentry-dart/pull/1026)) + ## 6.11.1 ### Fixes - Align span spec for serialize ops ([#1024](https://github.com/getsentry/sentry-dart/pull/1024)) - Pin sentry version ([#1020](https://github.com/getsentry/sentry-dart/pull/1020)) -- Tracer does not allow setting measurement if finished ([#1026](https://github.com/getsentry/sentry-dart/pull/1026)) ### Features From 6b191abcc10b6f33307a4bd221c8da25f158df8e Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 10:57:34 +0200 Subject: [PATCH 8/9] fix --- dart/lib/src/protocol/sentry_span.dart | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index 1dc56ade25..bad87759b9 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -184,7 +184,11 @@ class SentrySpan extends ISentrySpan { ); @override - void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { + void setMeasurement( + String name, + num value, { + SentryMeasurementUnit? unit, + }) { _tracer.setMeasurement(name, value, unit: unit); } From 9f74defae11a01ac298fb402efb2133651a83ed3 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Thu, 29 Sep 2022 13:44:06 +0200 Subject: [PATCH 9/9] fix tests --- dart/lib/src/protocol/sentry_span.dart | 4 ++-- dart/lib/src/sentry_tracer.dart | 18 ++++++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index bad87759b9..0abee1c990 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -19,7 +19,7 @@ class SentrySpan extends ISentrySpan { SpanStatus? _status; final Map _tags = {}; - void Function({DateTime? endTimestamp})? _finishedCallback; + Function({DateTime? endTimestamp})? _finishedCallback; @override final SentryTracesSamplingDecision? samplingDecision; @@ -62,7 +62,7 @@ class SentrySpan extends ISentrySpan { if (_throwable != null) { _hub.setSpanContext(_throwable, this, _tracer.name); } - _finishedCallback?.call(endTimestamp: _endTimestamp); + await _finishedCallback?.call(endTimestamp: _endTimestamp); return super.finish(status: status, endTimestamp: _endTimestamp); } diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 317be3fccd..d3944d4d60 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -218,14 +218,7 @@ class SentryTracer extends ISentrySpan { _hub, samplingDecision: _rootSpan.samplingDecision, startTimestamp: startTimestamp, - finishedCallback: ({ - DateTime? endTimestamp, - }) { - final finishStatus = _finishStatus; - if (finishStatus.finishing) { - finish(status: finishStatus.status, endTimestamp: endTimestamp); - } - }, + finishedCallback: _finishedCallback, ); _children.add(child); @@ -233,6 +226,15 @@ class SentryTracer extends ISentrySpan { return child; } + Future _finishedCallback({ + DateTime? endTimestamp, + }) async { + final finishStatus = _finishStatus; + if (finishStatus.finishing) { + await finish(status: finishStatus.status, endTimestamp: endTimestamp); + } + } + @override SpanStatus? get status => _rootSpan.status;