From 99a9a9f6e5005f0b603f5ae6a8779e59ab87ba1c Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 09:56:09 +0200 Subject: [PATCH 1/8] feat: add span level measurements --- dart/lib/src/noop_sentry_span.dart | 4 ++++ dart/lib/src/protocol/sentry_span.dart | 15 +++++++++++++- dart/lib/src/sentry_span_interface.dart | 5 ++++- dart/lib/src/sentry_tracer.dart | 20 ++++++++++--------- dart/test/sentry_span_test.dart | 26 +++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 11 deletions(-) diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index 45d72b94c9..ff1d17bcca 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -99,4 +99,8 @@ class NoOpSentrySpan extends ISentrySpan { @override LocalMetricsAggregator? get localMetricsAggregator => null; + + @override + // TODO: implement measurements + Map get measurements => throw UnimplementedError(); } diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index 6358b4802a..db9b35b5c4 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -26,6 +26,7 @@ class SentrySpan extends ISentrySpan { final SentryTracer _tracer; final Map _data = {}; + final Map _measurements = {}; dynamic _throwable; SpanStatus? _status; @@ -228,6 +229,8 @@ class SentrySpan extends ISentrySpan { Map get tags => _tags; Map get data => _data; + @override + Map get measurements => _measurements; @override SentryTraceHeader toSentryTrace() => SentryTraceHeader( @@ -242,7 +245,17 @@ class SentrySpan extends ISentrySpan { num value, { SentryMeasurementUnit? unit, }) { - _tracer.setMeasurement(name, value, unit: unit); + if (finished) { + _hub.options.logger(SentryLevel.debug, + "The span is already finished. Measurement $name cannot be set"); + return; + } + _measurements[name] = SentryMeasurement(name, value, unit: unit); + // We set the measurement in the transaction, too, but we have to check if this is the root span + // of the transaction, to avoid an infinite recursion + if (!_isRootSpan) { + _tracer.setMeasurementFromChild(name, value, unit: unit); + } } @override diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index 1d142c45b9..00d3709577 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -71,13 +71,16 @@ abstract class ISentrySpan { /// Returns the trace information that could be sent as a sentry-trace header. SentryTraceHeader toSentryTrace(); - /// Set observed measurement for this transaction. + /// Set observed measurement for this span or transaction. void setMeasurement( String name, num value, { SentryMeasurementUnit? unit, }); + /// Returns the measurements for a span or transaction. + Map get measurements; + /// Returns the baggage that can be sent as "baggage" header. SentryBaggageHeader? toBaggageHeader(); diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index 66c179e386..c1b4b4184d 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -17,7 +17,8 @@ class SentryTracer extends ISentrySpan { late final SentrySpan _rootSpan; final List _children = []; final Map _extra = {}; - final Map _measurements = {}; + @override + Map get measurements => _rootSpan.measurements; Timer? _autoFinishAfterTimer; Duration? _autoFinishAfter; @@ -149,7 +150,7 @@ class SentryTracer extends ISentrySpan { } final transaction = SentryTransaction(this); - transaction.measurements.addAll(_measurements); + transaction.measurements.addAll(_rootSpan.measurements); profileInfo = (status == null || status == SpanStatus.ok()) ? await profiler?.finishFor(transaction) @@ -320,9 +321,6 @@ class SentryTracer extends ISentrySpan { @override SentryTraceHeader toSentryTrace() => _rootSpan.toSentryTrace(); - @visibleForTesting - Map get measurements => - Map.unmodifiable(_measurements); bool _haveAllChildrenFinished() { for (final child in children) { @@ -340,11 +338,15 @@ class SentryTracer extends ISentrySpan { @override void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { - if (finished) { - return; + _rootSpan.setMeasurement(name, value, unit: unit); + } + + void setMeasurementFromChild(String name, num value, + {SentryMeasurementUnit? unit}) { + // We don't want to overwrite the root span measurement, if it comes from a child. + if (!_rootSpan.measurements.containsKey(name)) { + setMeasurement(name, value, unit: unit); } - final measurement = SentryMeasurement(name, value, unit: unit); - _measurements[name] = measurement; } @override diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index e161ceee2f..cc67bd9a7b 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -310,6 +310,32 @@ void main() { expect(fixture.hub.options.enableSpanLocalMetricAggregation, false); expect(sut.localMetricsAggregator, null); }); + + test('setMeasurement sets a measurement', () async { + final sut = fixture.getSut(); + sut.setMeasurement("test", 1); + expect(sut.measurements.containsKey("test"), true); + expect(sut.measurements["test"]!.value, 1); + }); + + test('setMeasurement does not set a measurement if a span is finished', + () async { + final sut = fixture.getSut(); + await sut.finish(); + sut.setMeasurement("test", 1); + expect(sut.measurements.isEmpty, true); + }); + + test('setMeasurement also set a measurement to the transaction root span', + () async { + final sut = fixture.getSut(); + final child = sut.tracer.startChild("child"); + child.setMeasurement("child", 2); + expect(child.measurements.containsKey("child"), true); + expect(child.measurements["child"]!.value, 2); + expect(sut.measurements.containsKey("child"), true); + expect(sut.measurements["child"]!.value, 2); + }); } class Fixture { From f67d2d7136931dedf5a98abb785932e85ab231db Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 10:01:57 +0200 Subject: [PATCH 2/8] add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3517cd150b..eba994b935 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Improvements +- Add support for span level measurements. - Add error type identifier to improve obfuscated Flutter issue titles ([#2170](https://github.com/getsentry/sentry-dart/pull/2170)) - Example: transforms issue titles from `GA` to `FlutterError` or `minified:nE` to `FlutterError` - This is enabled automatically and will change grouping if you already have issues with obfuscated titles From c53f9644dd46803d6099a6f5a591a56bc7c71992 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 10:05:49 +0200 Subject: [PATCH 3/8] add issue link to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eba994b935..1826e9ead9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ ### Improvements -- Add support for span level measurements. +- Add support for span level measurements. ([#1855](https://github.com/getsentry/sentry-dart/issues/1855)) - Add error type identifier to improve obfuscated Flutter issue titles ([#2170](https://github.com/getsentry/sentry-dart/pull/2170)) - Example: transforms issue titles from `GA` to `FlutterError` or `minified:nE` to `FlutterError` - This is enabled automatically and will change grouping if you already have issues with obfuscated titles From 2943b5076f7214370296f508f221880743dd2754 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 14:53:27 +0200 Subject: [PATCH 4/8] fix: correct changelog --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1826e9ead9..b63d500d63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,10 +1,13 @@ # Changelog +## Unreleased + +- Add support for span level measurements. ([#1855](https://github.com/getsentry/sentry-dart/issues/1855)) + ## 8.6.0 ### Improvements -- Add support for span level measurements. ([#1855](https://github.com/getsentry/sentry-dart/issues/1855)) - Add error type identifier to improve obfuscated Flutter issue titles ([#2170](https://github.com/getsentry/sentry-dart/pull/2170)) - Example: transforms issue titles from `GA` to `FlutterError` or `minified:nE` to `FlutterError` - This is enabled automatically and will change grouping if you already have issues with obfuscated titles From 45ead380b222d344f5ce15feea9e0fce52f63da8 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 15:12:47 +0200 Subject: [PATCH 5/8] moved the stored measurements from root span to tracer --- dart/lib/src/noop_sentry_span.dart | 4 ---- dart/lib/src/protocol/sentry_span.dart | 10 +--------- dart/lib/src/sentry_span_interface.dart | 2 -- dart/lib/src/sentry_tracer.dart | 19 ++++++++++++------- dart/test/sentry_span_test.dart | 16 +++------------- dart/test/sentry_tracer_test.dart | 16 ++++++++++++++++ 6 files changed, 32 insertions(+), 35 deletions(-) diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index ff1d17bcca..45d72b94c9 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -99,8 +99,4 @@ class NoOpSentrySpan extends ISentrySpan { @override LocalMetricsAggregator? get localMetricsAggregator => null; - - @override - // TODO: implement measurements - Map get measurements => throw UnimplementedError(); } diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index db9b35b5c4..00f6ec8f5a 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -26,7 +26,6 @@ class SentrySpan extends ISentrySpan { final SentryTracer _tracer; final Map _data = {}; - final Map _measurements = {}; dynamic _throwable; SpanStatus? _status; @@ -229,8 +228,6 @@ class SentrySpan extends ISentrySpan { Map get tags => _tags; Map get data => _data; - @override - Map get measurements => _measurements; @override SentryTraceHeader toSentryTrace() => SentryTraceHeader( @@ -250,12 +247,7 @@ class SentrySpan extends ISentrySpan { "The span is already finished. Measurement $name cannot be set"); return; } - _measurements[name] = SentryMeasurement(name, value, unit: unit); - // We set the measurement in the transaction, too, but we have to check if this is the root span - // of the transaction, to avoid an infinite recursion - if (!_isRootSpan) { - _tracer.setMeasurementFromChild(name, value, unit: unit); - } + _tracer.setMeasurementFromChild(name, value, unit: unit); } @override diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index 00d3709577..d5747129fb 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -78,8 +78,6 @@ abstract class ISentrySpan { SentryMeasurementUnit? unit, }); - /// Returns the measurements for a span or transaction. - Map get measurements; /// Returns the baggage that can be sent as "baggage" header. SentryBaggageHeader? toBaggageHeader(); diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index c1b4b4184d..1507143d69 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -17,8 +17,9 @@ class SentryTracer extends ISentrySpan { late final SentrySpan _rootSpan; final List _children = []; final Map _extra = {}; - @override - Map get measurements => _rootSpan.measurements; + + final Map _measurements = {}; + Map get measurements => _measurements; Timer? _autoFinishAfterTimer; Duration? _autoFinishAfter; @@ -150,7 +151,7 @@ class SentryTracer extends ISentrySpan { } final transaction = SentryTransaction(this); - transaction.measurements.addAll(_rootSpan.measurements); + transaction.measurements.addAll(_measurements); profileInfo = (status == null || status == SpanStatus.ok()) ? await profiler?.finishFor(transaction) @@ -321,7 +322,6 @@ class SentryTracer extends ISentrySpan { @override SentryTraceHeader toSentryTrace() => _rootSpan.toSentryTrace(); - bool _haveAllChildrenFinished() { for (final child in children) { if (!child.finished) { @@ -338,13 +338,18 @@ class SentryTracer extends ISentrySpan { @override void setMeasurement(String name, num value, {SentryMeasurementUnit? unit}) { - _rootSpan.setMeasurement(name, value, unit: unit); + if (finished) { + _hub.options.logger(SentryLevel.debug, + "The tracer is already finished. Measurement $name cannot be set"); + return; + } + _measurements[name] = SentryMeasurement(name, value, unit: unit); } void setMeasurementFromChild(String name, num value, {SentryMeasurementUnit? unit}) { - // We don't want to overwrite the root span measurement, if it comes from a child. - if (!_rootSpan.measurements.containsKey(name)) { + // We don't want to overwrite span measurement, if it comes from a child. + if (!_measurements.containsKey(name)) { setMeasurement(name, value, unit: unit); } } diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index cc67bd9a7b..26dde8fbdf 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -314,8 +314,8 @@ void main() { test('setMeasurement sets a measurement', () async { final sut = fixture.getSut(); sut.setMeasurement("test", 1); - expect(sut.measurements.containsKey("test"), true); - expect(sut.measurements["test"]!.value, 1); + expect(sut.tracer.measurements.containsKey("test"), true); + expect(sut.tracer.measurements["test"]!.value, 1); }); test('setMeasurement does not set a measurement if a span is finished', @@ -323,19 +323,9 @@ void main() { final sut = fixture.getSut(); await sut.finish(); sut.setMeasurement("test", 1); - expect(sut.measurements.isEmpty, true); + expect(sut.tracer.measurements.isEmpty, true); }); - test('setMeasurement also set a measurement to the transaction root span', - () async { - final sut = fixture.getSut(); - final child = sut.tracer.startChild("child"); - child.setMeasurement("child", 2); - expect(child.measurements.containsKey("child"), true); - expect(child.measurements["child"]!.value, 2); - expect(sut.measurements.containsKey("child"), true); - expect(sut.measurements["child"]!.value, 2); - }); } class Fixture { diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index a1251c224b..ba57aeb405 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -467,6 +467,22 @@ void main() { expect(fixture.hub.options.enableSpanLocalMetricAggregation, false); expect(sut.localMetricsAggregator, null); }); + + test('setMeasurement sets a measurement', () async { + final sut = fixture.getSut(); + sut.setMeasurement("test", 1); + expect(sut.measurements.containsKey("test"), true); + expect(sut.measurements["test"]!.value, 1); + }); + + test('setMeasurementFromChild does not override existing measurements', + () async { + final sut = fixture.getSut(); + sut.setMeasurement("test", 1); + sut.setMeasurementFromChild("test", 5); + expect(sut.measurements.containsKey("test"), true); + expect(sut.measurements["test"]!.value, 1); + }); }); group('$SentryBaggageHeader', () { From c5c0bf4677b321e1340194602f6127be999af7a4 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 15:20:25 +0200 Subject: [PATCH 6/8] changed issue number to pr number in changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b63d500d63..a35dc712a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## Unreleased -- Add support for span level measurements. ([#1855](https://github.com/getsentry/sentry-dart/issues/1855)) +- Add support for span level measurements. ([#2214](https://github.com/getsentry/sentry-dart/pull/2214)) ## 8.6.0 From e85d6f42307afb61c462555531a7bed21c25f1de Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 5 Aug 2024 15:39:19 +0200 Subject: [PATCH 7/8] fixed formatting --- dart/lib/src/sentry_span_interface.dart | 1 - dart/test/sentry_span_test.dart | 1 - 2 files changed, 2 deletions(-) diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index d5747129fb..979822c2ac 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -78,7 +78,6 @@ abstract class ISentrySpan { SentryMeasurementUnit? unit, }); - /// Returns the baggage that can be sent as "baggage" header. SentryBaggageHeader? toBaggageHeader(); diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index 26dde8fbdf..d063cf787e 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -325,7 +325,6 @@ void main() { sut.setMeasurement("test", 1); expect(sut.tracer.measurements.isEmpty, true); }); - } class Fixture { From 30c9e3d9b24920d124414c245fec57b393fedb55 Mon Sep 17 00:00:00 2001 From: Martin Haintz Date: Mon, 5 Aug 2024 17:44:41 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md Co-authored-by: Giancarlo Buenaflor --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a35dc712a1..1f7ce06059 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +### Features + - Add support for span level measurements. ([#2214](https://github.com/getsentry/sentry-dart/pull/2214)) ## 8.6.0