From d93ace2b6d241e44dac8785a79fae21a4c1b389e Mon Sep 17 00:00:00 2001 From: Stefano Date: Thu, 4 Apr 2024 19:11:35 +0200 Subject: [PATCH] Add timing metric and beforeMetric callback (part 3) (#1954) * added SentryOptions.beforeMetricCallback * added beforeMetricCallback logic in metrics_aggregator.dart * added timing metric api with span auto start * timing api uses span duration as value for the emitted metric if possible * Feat/metrics span summary p4 (#1958) * added local_metrics_aggregator.dart to spans * metrics_aggregator.dart now adds to current span's localMetricsAggregator * added metric_summary.dart * added metricSummary to spans and transaction JSONs --- .../src/metrics/local_metrics_aggregator.dart | 37 +++++ dart/lib/src/metrics/metric.dart | 8 +- dart/lib/src/metrics/metrics_aggregator.dart | 31 ++++ dart/lib/src/metrics/metrics_api.dart | 62 ++++++++ dart/lib/src/noop_sentry_span.dart | 4 + dart/lib/src/protocol.dart | 1 + dart/lib/src/protocol/metric_summary.dart | 43 ++++++ dart/lib/src/protocol/sentry_span.dart | 20 +++ dart/lib/src/protocol/sentry_transaction.dart | 18 +++ dart/lib/src/sentry_options.dart | 29 +++- dart/lib/src/sentry_span_interface.dart | 4 + dart/lib/src/sentry_tracer.dart | 5 + .../local_metrics_aggregator_test.dart | 40 +++++ dart/test/metrics/metric_test.dart | 67 +++++--- .../test/metrics/metrics_aggregator_test.dart | 143 +++++++++++++++++- dart/test/metrics/metrics_api_test.dart | 78 +++++++++- dart/test/sentry_options_test.dart | 28 +++- dart/test/sentry_span_test.dart | 39 +++++ dart/test/sentry_tracer_test.dart | 15 ++ dart/test/sentry_transaction_test.dart | 46 +++++- flutter/example/lib/main.dart | 32 +++- flutter/test/mocks.mocks.dart | 2 + 22 files changed, 713 insertions(+), 39 deletions(-) create mode 100644 dart/lib/src/metrics/local_metrics_aggregator.dart create mode 100644 dart/lib/src/protocol/metric_summary.dart create mode 100644 dart/test/metrics/local_metrics_aggregator_test.dart diff --git a/dart/lib/src/metrics/local_metrics_aggregator.dart b/dart/lib/src/metrics/local_metrics_aggregator.dart new file mode 100644 index 0000000000..92076ef807 --- /dev/null +++ b/dart/lib/src/metrics/local_metrics_aggregator.dart @@ -0,0 +1,37 @@ +import 'dart:core'; +import 'package:meta/meta.dart'; +import '../protocol/metric_summary.dart'; +import 'metric.dart'; + +@internal +class LocalMetricsAggregator { + // format: > + final Map> _buckets = {}; + + void add(final Metric metric, final num value) { + final bucket = + _buckets.putIfAbsent(metric.getSpanAggregationKey(), () => {}); + + bucket.update(metric.getCompositeKey(), (m) => m..add(value), + ifAbsent: () => Metric.fromType( + type: MetricType.gauge, + key: metric.key, + value: value, + unit: metric.unit, + tags: metric.tags) as GaugeMetric); + } + + Map> getSummaries() { + final Map> summaries = {}; + for (final entry in _buckets.entries) { + final String exportKey = entry.key; + + final metricSummaries = entry.value.values + .map((gauge) => MetricSummary.fromGauge(gauge)) + .toList(); + + summaries[exportKey] = metricSummaries; + } + return summaries; + } +} diff --git a/dart/lib/src/metrics/metric.dart b/dart/lib/src/metrics/metric.dart index 4b86b5cb87..52831e73ce 100644 --- a/dart/lib/src/metrics/metric.dart +++ b/dart/lib/src/metrics/metric.dart @@ -112,6 +112,10 @@ abstract class Metric { return ('${type.statsdType}_${key}_${unit.name}_$serializedTags'); } + /// Return a key created by [key], [type] and [unit]. + /// This key should be used to aggregate the metric locally in a span. + String getSpanAggregationKey() => '${type.statsdType}:$key@${unit.name}'; + /// Remove forbidden characters from the metric key and tag key. String _normalizeKey(String input) => input.replaceAll(forbiddenKeyCharsRegex, '_'); @@ -186,13 +190,9 @@ class GaugeMetric extends Metric { @visibleForTesting num get last => _last; - @visibleForTesting num get minimum => _minimum; - @visibleForTesting num get maximum => _maximum; - @visibleForTesting num get sum => _sum; - @visibleForTesting int get count => _count; } diff --git a/dart/lib/src/metrics/metrics_aggregator.dart b/dart/lib/src/metrics/metrics_aggregator.dart index 5551c396c0..2dd9cb2c91 100644 --- a/dart/lib/src/metrics/metrics_aggregator.dart +++ b/dart/lib/src/metrics/metrics_aggregator.dart @@ -59,6 +59,30 @@ class MetricsAggregator { return; } + // run before metric callback if set + if (_options.beforeMetricCallback != null) { + try { + final shouldEmit = _options.beforeMetricCallback!(key, tags: tags); + if (!shouldEmit) { + _options.logger( + SentryLevel.info, + 'Metric was dropped by beforeMetric', + ); + return; + } + } catch (exception, stackTrace) { + _options.logger( + SentryLevel.error, + 'The BeforeMetric callback threw an exception', + exception: exception, + stackTrace: stackTrace, + ); + if (_options.automatedTestMode) { + rethrow; + } + } + } + final bucketKey = _getBucketKey(_options.clock()); final bucket = _buckets.putIfAbsent(bucketKey, () => {}); final metric = Metric.fromType( @@ -76,6 +100,13 @@ class MetricsAggregator { ifAbsent: () => metric, ); + // For sets, we only record that a value has been added to the set but not which one. + // See develop docs: https://develop.sentry.dev/sdk/metrics/#sets + _hub + .getSpan() + ?.localMetricsAggregator + ?.add(metric, metricType == MetricType.set ? addedWeight : value); + // Schedule the metrics flushing. _scheduleFlush(); } diff --git a/dart/lib/src/metrics/metrics_api.dart b/dart/lib/src/metrics/metrics_api.dart index 380c21e92f..1ef8df77c3 100644 --- a/dart/lib/src/metrics/metrics_api.dart +++ b/dart/lib/src/metrics/metrics_api.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert'; import '../../sentry.dart'; import '../utils/crc32_utils.dart'; @@ -115,4 +116,65 @@ class MetricsApi { map.putIfAbsent(key, () => value); } } + + /// Emits a Distribution metric, identified by [key], with the time it takes + /// to run [function]. + /// You can set the [unit] and the optional [tags] to associate to the metric. + void timing(final String key, + {required FutureOr Function() function, + final DurationSentryMeasurementUnit unit = + DurationSentryMeasurementUnit.second, + final Map? tags}) async { + // Start a span for the metric + final span = _hub.getSpan()?.startChild('metric.timing', description: key); + // Set the user tags to the span as well + if (span != null && tags != null) { + for (final entry in tags.entries) { + span.setTag(entry.key, entry.value); + } + } + final before = _hub.options.clock(); + try { + if (function is Future Function()) { + await function(); + } else { + function(); + } + } finally { + final after = _hub.options.clock(); + Duration duration = after.difference(before); + // If we have a span, we use its duration as value for the emitted metric + if (span != null) { + await span.finish(); + duration = + span.endTimestamp?.difference(span.startTimestamp) ?? duration; + } + final value = _convertMicrosTo(unit, duration.inMicroseconds); + + _hub.metricsAggregator?.emit(MetricType.distribution, key, value, unit, + _enrichWithDefaultTags(tags)); + } + } + + double _convertMicrosTo( + final DurationSentryMeasurementUnit unit, final int micros) { + switch (unit) { + case DurationSentryMeasurementUnit.nanoSecond: + return micros * 1000; + case DurationSentryMeasurementUnit.microSecond: + return micros.toDouble(); + case DurationSentryMeasurementUnit.milliSecond: + return micros / 1000.0; + case DurationSentryMeasurementUnit.second: + return micros / 1000000.0; + case DurationSentryMeasurementUnit.minute: + return micros / 60000000.0; + case DurationSentryMeasurementUnit.hour: + return micros / 3600000000.0; + case DurationSentryMeasurementUnit.day: + return micros / 86400000000.0; + case DurationSentryMeasurementUnit.week: + return micros / 86400000000.0 / 7.0; + } + } } diff --git a/dart/lib/src/noop_sentry_span.dart b/dart/lib/src/noop_sentry_span.dart index 2156aeb678..45d72b94c9 100644 --- a/dart/lib/src/noop_sentry_span.dart +++ b/dart/lib/src/noop_sentry_span.dart @@ -1,3 +1,4 @@ +import 'metrics/local_metrics_aggregator.dart'; import 'protocol.dart'; import 'tracing.dart'; import 'utils.dart'; @@ -95,4 +96,7 @@ class NoOpSentrySpan extends ISentrySpan { @override void scheduleFinish() {} + + @override + LocalMetricsAggregator? get localMetricsAggregator => null; } diff --git a/dart/lib/src/protocol.dart b/dart/lib/src/protocol.dart index 3dac1a6f3c..6d89823721 100644 --- a/dart/lib/src/protocol.dart +++ b/dart/lib/src/protocol.dart @@ -8,6 +8,7 @@ export 'protocol/sentry_device.dart'; export 'protocol/dsn.dart'; export 'protocol/sentry_gpu.dart'; export 'protocol/mechanism.dart'; +export 'protocol/metric_summary.dart'; export 'protocol/sentry_message.dart'; export 'protocol/sentry_operating_system.dart'; export 'protocol/sentry_request.dart'; diff --git a/dart/lib/src/protocol/metric_summary.dart b/dart/lib/src/protocol/metric_summary.dart new file mode 100644 index 0000000000..b0c617fb30 --- /dev/null +++ b/dart/lib/src/protocol/metric_summary.dart @@ -0,0 +1,43 @@ +import '../metrics/metric.dart'; + +class MetricSummary { + final num min; + final num max; + final num sum; + final int count; + final Map? tags; + + MetricSummary.fromGauge(GaugeMetric gauge) + : min = gauge.minimum, + max = gauge.maximum, + sum = gauge.sum, + count = gauge.count, + tags = gauge.tags; + + const MetricSummary( + {required this.min, + required this.max, + required this.sum, + required this.count, + required this.tags}); + + /// Deserializes a [MetricSummary] from JSON [Map]. + factory MetricSummary.fromJson(Map data) => MetricSummary( + min: data['min'], + max: data['max'], + count: data['count'], + sum: data['sum'], + tags: data['tags']?.cast(), + ); + + /// Produces a [Map] that can be serialized to JSON. + Map toJson() { + return { + 'min': min, + 'max': max, + 'count': count, + 'sum': sum, + if (tags?.isNotEmpty ?? false) 'tags': tags, + }; + } +} diff --git a/dart/lib/src/protocol/sentry_span.dart b/dart/lib/src/protocol/sentry_span.dart index e03410d715..780578d182 100644 --- a/dart/lib/src/protocol/sentry_span.dart +++ b/dart/lib/src/protocol/sentry_span.dart @@ -1,6 +1,7 @@ import 'dart:async'; import '../hub.dart'; +import '../metrics/local_metrics_aggregator.dart'; import '../protocol.dart'; import '../sentry_tracer.dart'; @@ -12,6 +13,7 @@ typedef OnFinishedCallback = Future Function({DateTime? endTimestamp}); class SentrySpan extends ISentrySpan { final SentrySpanContext _context; DateTime? _endTimestamp; + Map>? _metricSummaries; late final DateTime _startTimestamp; final Hub _hub; @@ -22,6 +24,7 @@ class SentrySpan extends ISentrySpan { SpanStatus? _status; final Map _tags = {}; OnFinishedCallback? _finishedCallback; + late final LocalMetricsAggregator? _localMetricsAggregator; @override final SentryTracesSamplingDecision? samplingDecision; @@ -37,6 +40,9 @@ class SentrySpan extends ISentrySpan { _startTimestamp = startTimestamp?.toUtc() ?? _hub.options.clock(); _finishedCallback = finishedCallback; _origin = _context.origin; + _localMetricsAggregator = _hub.options.enableSpanLocalMetricAggregation + ? LocalMetricsAggregator() + : null; } @override @@ -65,6 +71,7 @@ class SentrySpan extends ISentrySpan { if (_throwable != null) { _hub.setSpanContext(_throwable, this, _tracer.name); } + _metricSummaries = _localMetricsAggregator?.getSummaries(); await _finishedCallback?.call(endTimestamp: _endTimestamp); return super.finish(status: status, endTimestamp: _endTimestamp); } @@ -154,6 +161,9 @@ class SentrySpan extends ISentrySpan { @override set origin(String? origin) => _origin = origin; + @override + LocalMetricsAggregator? get localMetricsAggregator => _localMetricsAggregator; + Map toJson() { final json = _context.toJson(); json['start_timestamp'] = @@ -174,6 +184,16 @@ class SentrySpan extends ISentrySpan { if (_origin != null) { json['origin'] = _origin; } + + final metricSummariesMap = _metricSummaries?.entries ?? Iterable.empty(); + if (metricSummariesMap.isNotEmpty) { + final map = {}; + for (final entry in metricSummariesMap) { + final summary = entry.value.map((e) => e.toJson()); + map[entry.key] = summary.toList(growable: false); + } + json['_metrics_summary'] = map; + } return json; } diff --git a/dart/lib/src/protocol/sentry_transaction.dart b/dart/lib/src/protocol/sentry_transaction.dart index e00fa23355..eea319aa41 100644 --- a/dart/lib/src/protocol/sentry_transaction.dart +++ b/dart/lib/src/protocol/sentry_transaction.dart @@ -13,6 +13,7 @@ class SentryTransaction extends SentryEvent { @internal final SentryTracer tracer; late final Map measurements; + late final Map>? metricSummaries; late final SentryTransactionInfo? transactionInfo; SentryTransaction( @@ -37,6 +38,7 @@ class SentryTransaction extends SentryEvent { super.request, String? type, Map? measurements, + Map>? metricSummaries, SentryTransactionInfo? transactionInfo, }) : super( timestamp: timestamp ?? tracer.endTimestamp, @@ -52,6 +54,8 @@ class SentryTransaction extends SentryEvent { final spanContext = tracer.context; spans = tracer.children; this.measurements = measurements ?? {}; + this.metricSummaries = + metricSummaries ?? tracer.localMetricsAggregator?.getSummaries(); contexts.trace = spanContext.toTraceContext( sampled: tracer.samplingDecision?.sampled, @@ -85,6 +89,16 @@ class SentryTransaction extends SentryEvent { json['transaction_info'] = transactionInfo.toJson(); } + final metricSummariesMap = metricSummaries?.entries ?? Iterable.empty(); + if (metricSummariesMap.isNotEmpty) { + final map = {}; + for (final entry in metricSummariesMap) { + final summary = entry.value.map((e) => e.toJson()); + map[entry.key] = summary.toList(growable: false); + } + json['_metrics_summary'] = map; + } + return json; } @@ -123,6 +137,7 @@ class SentryTransaction extends SentryEvent { List? threads, String? type, Map? measurements, + Map>? metricSummaries, SentryTransactionInfo? transactionInfo, }) => SentryTransaction( @@ -148,6 +163,9 @@ class SentryTransaction extends SentryEvent { type: type ?? this.type, measurements: (measurements != null ? Map.from(measurements) : null) ?? this.measurements, + metricSummaries: + (metricSummaries != null ? Map.from(metricSummaries) : null) ?? + this.metricSummaries, transactionInfo: transactionInfo ?? this.transactionInfo, ); } diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart index a08d67ca37..eea642ae48 100644 --- a/dart/lib/src/sentry_options.dart +++ b/dart/lib/src/sentry_options.dart @@ -165,6 +165,10 @@ class SentryOptions { /// to the scope. When nothing is returned from the function, the breadcrumb is dropped BeforeBreadcrumbCallback? beforeBreadcrumb; + /// This function is called right before a metric is about to be emitted. + /// Can return true to emit the metric, or false to drop it. + BeforeMetricCallback? beforeMetricCallback; + /// Sets the release. SDK will try to automatically configure a release out of the box /// See [docs for further information](https://docs.sentry.io/platforms/flutter/configuration/releases/) String? release; @@ -390,7 +394,7 @@ class SentryOptions { bool enableMetrics = false; @experimental - bool _enableDefaultTagsForMetrics = false; + bool _enableDefaultTagsForMetrics = true; /// Enables enriching metrics with default tags. Requires [enableMetrics]. /// More on https://develop.sentry.dev/delightful-developer-metrics/sending-metrics-sdk/#automatic-tags-extraction @@ -406,6 +410,22 @@ class SentryOptions { set enableDefaultTagsForMetrics(final bool enableDefaultTagsForMetrics) => _enableDefaultTagsForMetrics = enableDefaultTagsForMetrics; + @experimental + bool _enableSpanLocalMetricAggregation = true; + + /// Enables span metrics aggregation. Requires [enableMetrics]. + /// More on https://develop.sentry.dev/sdk/metrics/#span-aggregation + @experimental + bool get enableSpanLocalMetricAggregation => + enableMetrics && _enableSpanLocalMetricAggregation; + + /// Enables span metrics aggregation. Requires [enableMetrics]. + /// More on https://develop.sentry.dev/sdk/metrics/#span-aggregation + @experimental + set enableSpanLocalMetricAggregation( + final bool enableSpanLocalMetricAggregation) => + _enableSpanLocalMetricAggregation = enableSpanLocalMetricAggregation; + /// Only for internal use. Changed SDK behaviour when set to true: /// - Rethrow exceptions that occur in user provided closures @internal @@ -526,6 +546,13 @@ typedef BeforeBreadcrumbCallback = Breadcrumb? Function( Hint? hint, }); +/// This function is called right before a metric is about to be emitted. +/// Can return true to emit the metric, or false to drop it. +typedef BeforeMetricCallback = bool Function( + String key, { + Map? tags, +}); + /// Used to provide timestamp for logging. typedef ClockProvider = DateTime Function(); diff --git a/dart/lib/src/sentry_span_interface.dart b/dart/lib/src/sentry_span_interface.dart index cdc121f849..1d142c45b9 100644 --- a/dart/lib/src/sentry_span_interface.dart +++ b/dart/lib/src/sentry_span_interface.dart @@ -1,5 +1,6 @@ import 'package:meta/meta.dart'; +import 'metrics/local_metrics_aggregator.dart'; import 'protocol.dart'; import 'tracing.dart'; @@ -46,6 +47,9 @@ abstract class ISentrySpan { /// See https://develop.sentry.dev/sdk/performance/trace-origin set origin(String? origin); + @internal + LocalMetricsAggregator? get localMetricsAggregator; + /// Returns the end timestamp if finished DateTime? get endTimestamp; diff --git a/dart/lib/src/sentry_tracer.dart b/dart/lib/src/sentry_tracer.dart index c7f6993ad3..d9ea75d256 100644 --- a/dart/lib/src/sentry_tracer.dart +++ b/dart/lib/src/sentry_tracer.dart @@ -3,6 +3,7 @@ import 'dart:async'; import 'package:meta/meta.dart'; import '../sentry.dart'; +import 'metrics/local_metrics_aggregator.dart'; import 'profiling.dart'; import 'sentry_tracer_finish_status.dart'; import 'utils/sample_rate_format.dart'; @@ -413,4 +414,8 @@ class SentryTracer extends ISentrySpan { }); } } + + @override + LocalMetricsAggregator? get localMetricsAggregator => + _rootSpan.localMetricsAggregator; } diff --git a/dart/test/metrics/local_metrics_aggregator_test.dart b/dart/test/metrics/local_metrics_aggregator_test.dart new file mode 100644 index 0000000000..42648fdaf2 --- /dev/null +++ b/dart/test/metrics/local_metrics_aggregator_test.dart @@ -0,0 +1,40 @@ +import 'package:sentry/src/metrics/local_metrics_aggregator.dart'; +import 'package:test/expect.dart'; +import 'package:test/scaffolding.dart'; + +import '../mocks.dart'; + +void main() { + group('add', () { + late LocalMetricsAggregator aggregator; + + setUp(() { + aggregator = LocalMetricsAggregator(); + }); + + test('same metric multiple times aggregates them', () async { + aggregator.add(fakeMetric, 1); + aggregator.add(fakeMetric, 2); + final summaries = aggregator.getSummaries(); + expect(summaries.length, 1); + final summary = summaries.values.first; + expect(summary.length, 1); + }); + + test('same metric different tags aggregates summary bucket', () async { + aggregator.add(fakeMetric, 1); + aggregator.add(fakeMetric..tags.clear(), 2); + final summaries = aggregator.getSummaries(); + expect(summaries.length, 1); + final summary = summaries.values.first; + expect(summary.length, 2); + }); + + test('different metrics does not aggregate them', () async { + aggregator.add(fakeMetric, 1); + aggregator.add(fakeMetric2, 2); + final summaries = aggregator.getSummaries(); + expect(summaries.length, 2); + }); + }); +} diff --git a/dart/test/metrics/metric_test.dart b/dart/test/metrics/metric_test.dart index f5f3116230..d123916eea 100644 --- a/dart/test/metrics/metric_test.dart +++ b/dart/test/metrics/metric_test.dart @@ -57,10 +57,9 @@ void main() { test('encode CounterMetric', () async { final int bucketKey = 10; - final String expectedStatsd = + final expectedStatsd = 'key_metric_@hour:2.1|c|#tag1:tag value 1,key_2:@13/-d_s|T10'; - final String actualStatsd = - fixture.counterMetric.encodeToStatsd(bucketKey); + final actualStatsd = fixture.counterMetric.encodeToStatsd(bucketKey); expect(actualStatsd, expectedStatsd); }); }); @@ -72,12 +71,12 @@ void main() { fixture = Fixture(); }); - test('getCompositeKey escapes commas from tags', () async { + test('escapes commas from tags', () async { final Iterable tags = fixture.counterMetric.tags.values; - final String joinedTags = tags.join(); + final joinedTags = tags.join(); final Iterable expectedTags = tags.map((e) => e.replaceAll(',', '\\,')); - final String actualKey = fixture.counterMetric.getCompositeKey(); + final actualKey = fixture.counterMetric.getCompositeKey(); expect(joinedTags.contains(','), true); expect(joinedTags.contains('\\,'), false); @@ -87,31 +86,63 @@ void main() { } }); - test('getCompositeKey CounterMetric', () async { - final String expectedKey = + test('CounterMetric', () async { + final expectedKey = 'c_key metric!_hour_tag1=tag\\, value 1,key 2=&@"13/-d_s'; - final String actualKey = fixture.counterMetric.getCompositeKey(); + final actualKey = fixture.counterMetric.getCompositeKey(); expect(actualKey, expectedKey); }); - test('getCompositeKey GaugeMetric', () async { - final String expectedKey = + test('GaugeMetric', () async { + final expectedKey = 'g_key metric!_hour_tag1=tag\\, value 1,key 2=&@"13/-d_s'; - final String actualKey = fixture.gaugeMetric.getCompositeKey(); + final actualKey = fixture.gaugeMetric.getCompositeKey(); expect(actualKey, expectedKey); }); - test('getCompositeKey DistributionMetric', () async { - final String expectedKey = + test('DistributionMetric', () async { + final expectedKey = 'd_key metric!_hour_tag1=tag\\, value 1,key 2=&@"13/-d_s'; - final String actualKey = fixture.distributionMetric.getCompositeKey(); + final actualKey = fixture.distributionMetric.getCompositeKey(); expect(actualKey, expectedKey); }); - test('getCompositeKey SetMetric', () async { - final String expectedKey = + test('SetMetric', () async { + final expectedKey = 's_key metric!_hour_tag1=tag\\, value 1,key 2=&@"13/-d_s'; - final String actualKey = fixture.setMetric.getCompositeKey(); + final actualKey = fixture.setMetric.getCompositeKey(); + expect(actualKey, expectedKey); + }); + }); + + group('getSpanAggregationKey', () { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + test('CounterMetric', () async { + final expectedKey = 'c:key metric!@hour'; + final actualKey = fixture.counterMetric.getSpanAggregationKey(); + expect(actualKey, expectedKey); + }); + + test('GaugeMetric', () async { + final expectedKey = 'g:key metric!@hour'; + final actualKey = fixture.gaugeMetric.getSpanAggregationKey(); + expect(actualKey, expectedKey); + }); + + test('DistributionMetric', () async { + final expectedKey = 'd:key metric!@hour'; + final actualKey = fixture.distributionMetric.getSpanAggregationKey(); + expect(actualKey, expectedKey); + }); + + test('SetMetric', () async { + final expectedKey = 's:key metric!@hour'; + final actualKey = fixture.setMetric.getSpanAggregationKey(); expect(actualKey, expectedKey); }); }); diff --git a/dart/test/metrics/metrics_aggregator_test.dart b/dart/test/metrics/metrics_aggregator_test.dart index 1b411b8f0f..5636e7ebf6 100644 --- a/dart/test/metrics/metrics_aggregator_test.dart +++ b/dart/test/metrics/metrics_aggregator_test.dart @@ -60,6 +60,91 @@ void main() { }); }); + group('span local aggregation', () { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + test('emit calls add', () async { + final MetricsAggregator sut = fixture.getSut(hub: fixture.hub); + final t = fixture.hub.startTransaction('test', 'op', bindToScope: true); + + var spanSummary = t.localMetricsAggregator?.getSummaries().values; + expect(spanSummary, isEmpty); + + sut.testEmit(); + + spanSummary = t.localMetricsAggregator?.getSummaries().values; + expect(spanSummary, isNotEmpty); + }); + + test('emit counter', () async { + final MetricsAggregator sut = fixture.getSut(hub: fixture.hub); + final t = fixture.hub.startTransaction('test', 'op', bindToScope: true); + + sut.testEmit(type: MetricType.counter, value: 1); + sut.testEmit(type: MetricType.counter, value: 4); + + final spanSummary = t.localMetricsAggregator?.getSummaries().values.first; + expect(spanSummary!.length, 1); + expect(spanSummary.first.sum, 5); + expect(spanSummary.first.min, 1); + expect(spanSummary.first.max, 4); + expect(spanSummary.first.count, 2); + expect(spanSummary.first.tags, mockTags); + }); + + test('emit distribution', () async { + final MetricsAggregator sut = fixture.getSut(hub: fixture.hub); + final t = fixture.hub.startTransaction('test', 'op', bindToScope: true); + + sut.testEmit(type: MetricType.distribution, value: 1); + sut.testEmit(type: MetricType.distribution, value: 4); + + final spanSummary = t.localMetricsAggregator?.getSummaries().values.first; + expect(spanSummary!.length, 1); + expect(spanSummary.first.sum, 5); + expect(spanSummary.first.min, 1); + expect(spanSummary.first.max, 4); + expect(spanSummary.first.count, 2); + expect(spanSummary.first.tags, mockTags); + }); + + test('emit gauge', () async { + final MetricsAggregator sut = fixture.getSut(hub: fixture.hub); + final t = fixture.hub.startTransaction('test', 'op', bindToScope: true); + + sut.testEmit(type: MetricType.gauge, value: 1); + sut.testEmit(type: MetricType.gauge, value: 4); + + final spanSummary = t.localMetricsAggregator?.getSummaries().values.first; + expect(spanSummary!.length, 1); + expect(spanSummary.first.sum, 5); + expect(spanSummary.first.min, 1); + expect(spanSummary.first.max, 4); + expect(spanSummary.first.count, 2); + expect(spanSummary.first.tags, mockTags); + }); + + test('emit set', () async { + final MetricsAggregator sut = fixture.getSut(hub: fixture.hub); + final t = fixture.hub.startTransaction('test', 'op', bindToScope: true); + + sut.testEmit(type: MetricType.set, value: 1); + sut.testEmit(type: MetricType.set, value: 4); + + final spanSummary = t.localMetricsAggregator?.getSummaries().values.first; + expect(spanSummary!.length, 1); + expect(spanSummary.first.sum, 2); + expect(spanSummary.first.min, 1); + expect(spanSummary.first.max, 1); + expect(spanSummary.first.count, 2); + expect(spanSummary.first.tags, mockTags); + }); + }); + group('emit in same time bucket', () { late Fixture fixture; @@ -280,6 +365,54 @@ void main() { }); }); + group('beforeMetric', () { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + test('emits if not set', () async { + final MetricsAggregator sut = fixture.getSut(maxWeight: 4); + sut.testEmit(key: 'key1'); + final metricsCaptured = sut.buckets.values.first.values; + expect(metricsCaptured.length, 1); + expect(metricsCaptured.first.key, 'key1'); + }); + + test('drops if it return false', () async { + final MetricsAggregator sut = fixture.getSut(maxWeight: 4); + fixture.options.beforeMetricCallback = (key, {tags}) => key != 'key2'; + sut.testEmit(key: 'key1'); + sut.testEmit(key: 'key2'); + final metricsCaptured = sut.buckets.values.first.values; + expect(metricsCaptured.length, 1); + expect(metricsCaptured.first.key, 'key1'); + }); + + test('emits if it return true', () async { + final MetricsAggregator sut = fixture.getSut(maxWeight: 4); + fixture.options.beforeMetricCallback = (key, {tags}) => true; + sut.testEmit(key: 'key1'); + sut.testEmit(key: 'key2'); + final metricsCaptured = sut.buckets.values.first.values; + expect(metricsCaptured.length, 2); + expect(metricsCaptured.first.key, 'key1'); + expect(metricsCaptured.last.key, 'key2'); + }); + + test('emits if it throws', () async { + final MetricsAggregator sut = fixture.getSut(maxWeight: 4); + fixture.options.beforeMetricCallback = (key, {tags}) => throw Exception(); + sut.testEmit(key: 'key1'); + sut.testEmit(key: 'key2'); + final metricsCaptured = sut.buckets.values.first.values; + expect(metricsCaptured.length, 2); + expect(metricsCaptured.first.key, 'key1'); + expect(metricsCaptured.last.key, 'key2'); + }); + }); + group('overweight', () { late Fixture fixture; @@ -324,14 +457,22 @@ final DateTime mockTimestamp = DateTime.fromMillisecondsSinceEpoch(1); class Fixture { final options = SentryOptions(dsn: fakeDsn); final mockHub = MockHub(); + late final hub = Hub(options); + + Fixture() { + options.tracesSampleRate = 1; + options.enableMetrics = true; + options.enableSpanLocalMetricAggregation = true; + } MetricsAggregator getSut({ + Hub? hub, Duration flushInterval = const Duration(milliseconds: 1), int flushShiftMs = 0, int maxWeight = 100000, }) { return MetricsAggregator( - hub: mockHub, + hub: hub ?? mockHub, options: options, flushInterval: flushInterval, flushShiftMs: flushShiftMs, diff --git a/dart/test/metrics/metrics_api_test.dart b/dart/test/metrics/metrics_api_test.dart index 698cb5f172..9345720102 100644 --- a/dart/test/metrics/metrics_api_test.dart +++ b/dart/test/metrics/metrics_api_test.dart @@ -1,8 +1,12 @@ +import 'dart:async'; + +import 'package:sentry/sentry.dart'; import 'package:sentry/src/metrics/metric.dart'; import 'package:sentry/src/metrics/metrics_api.dart'; -import 'package:test/expect.dart'; -import 'package:test/scaffolding.dart'; +import 'package:sentry/src/sentry_tracer.dart'; +import 'package:test/test.dart'; +import '../mocks.dart'; import '../mocks/mock_hub.dart'; void main() { @@ -68,11 +72,79 @@ void main() { expect(sentMetrics.first.type, MetricType.set); expect((sentMetrics.first as SetMetric).values, {1, 2, 4, 494360628}); }); + + test('timing emits distribution', () async { + final delay = Duration(milliseconds: 100); + final completer = Completer(); + MetricsApi api = fixture.getSut(); + + // The timing API tries to start a child span + expect(fixture.mockHub.getSpanCalls, 0); + api.timing('key', + function: () => Future.delayed(delay, () => completer.complete())); + expect(fixture.mockHub.getSpanCalls, 1); + + await completer.future; + Iterable sentMetrics = + fixture.mockHub.metricsAggregator!.buckets.values.first.values; + + // The timing API emits a distribution metric + expect(sentMetrics.first.type, MetricType.distribution); + // The default unit is second + expect(sentMetrics.first.unit, DurationSentryMeasurementUnit.second); + // It awaits for the function completion, which means 100 milliseconds in + // this case. Since the unit is second, its value (duration) is >= 0.1 + expect( + (sentMetrics.first as DistributionMetric).values.first >= 0.1, true); + }); + + test('timing starts a span', () async { + final delay = Duration(milliseconds: 100); + final completer = Completer(); + fixture._options.tracesSampleRate = 1; + fixture._options.enableMetrics = true; + MetricsApi api = fixture.getSut(hub: fixture.hub); + + // Start a transaction so that timing api can start a child span + final transaction = fixture.hub.startTransaction( + 'name', + 'operation', + bindToScope: true, + ) as SentryTracer; + expect(transaction.children, isEmpty); + + // Timing starts a span + api.timing('my key', + unit: DurationSentryMeasurementUnit.milliSecond, + function: () => Future.delayed(delay, () => completer.complete())); + final span = transaction.children.first; + expect(span.finished, false); + expect(span.context.operation, 'metric.timing'); + expect(span.context.description, 'my key'); + + // Timing finishes the span when the function is finished, which takes 100 milliseconds + await completer.future; + expect(span.finished, true); + final spanDuration = span.endTimestamp!.difference(span.startTimestamp); + expect(spanDuration.inMilliseconds >= 100, true); + await Future.delayed(Duration()); + + Iterable sentMetrics = + fixture.hub.metricsAggregator!.buckets.values.first.values; + + // The emitted metric value should match the span duration + expect(sentMetrics.first.unit, DurationSentryMeasurementUnit.milliSecond); + // Duration.inMilliseconds returns an int, so we have to assert it + expect((sentMetrics.first as DistributionMetric).values.first.toInt(), + spanDuration.inMilliseconds); + }); }); } class Fixture { + final _options = SentryOptions(dsn: fakeDsn); final mockHub = MockHub(); + late final hub = Hub(_options); - MetricsApi getSut() => MetricsApi(hub: mockHub); + MetricsApi getSut({Hub? hub}) => MetricsApi(hub: hub ?? mockHub); } diff --git a/dart/test/sentry_options_test.dart b/dart/test/sentry_options_test.dart index 438913ed10..273366e442 100644 --- a/dart/test/sentry_options_test.dart +++ b/dart/test/sentry_options_test.dart @@ -140,17 +140,16 @@ void main() { expect(options.enableMetrics, false); }); - test('default tags for metrics are disabled by default', () { + test('default tags for metrics are enabled by default', () { final options = SentryOptions(dsn: fakeDsn); options.enableMetrics = true; - expect(options.enableDefaultTagsForMetrics, false); + expect(options.enableDefaultTagsForMetrics, true); }); test('default tags for metrics are disabled if metrics are disabled', () { final options = SentryOptions(dsn: fakeDsn); options.enableMetrics = false; - options.enableDefaultTagsForMetrics = true; expect(options.enableDefaultTagsForMetrics, false); }); @@ -162,4 +161,27 @@ void main() { expect(options.enableDefaultTagsForMetrics, true); }); + + test('span local metric aggregation is enabled by default', () { + final options = SentryOptions(dsn: fakeDsn); + options.enableMetrics = true; + + expect(options.enableSpanLocalMetricAggregation, true); + }); + + test('span local metric aggregation is disabled if metrics are disabled', () { + final options = SentryOptions(dsn: fakeDsn); + options.enableMetrics = false; + + expect(options.enableSpanLocalMetricAggregation, false); + }); + + test('span local metric aggregation is enabled if metrics are enabled, too', + () { + final options = SentryOptions(dsn: fakeDsn); + options.enableMetrics = true; + options.enableSpanLocalMetricAggregation = true; + + expect(options.enableSpanLocalMetricAggregation, true); + }); } diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index e878be5cf2..e161ceee2f 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -2,6 +2,7 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:test/test.dart'; +import 'mocks.dart'; import 'mocks/mock_hub.dart'; void main() { @@ -125,11 +126,14 @@ void main() { }); test('span serializes', () async { + fixture.hub.options.enableMetrics = true; + fixture.hub.options.enableSpanLocalMetricAggregation = true; final sut = fixture.getSut(); sut.setTag('test', 'test'); sut.setData('test', 'test'); sut.origin = 'manual'; + sut.localMetricsAggregator?.add(fakeMetric, 0); await sut.finish(status: SpanStatus.aborted()); @@ -141,6 +145,26 @@ void main() { expect(map['tags']['test'], 'test'); expect(map['status'], 'aborted'); expect(map['origin'], 'manual'); + expect(map['_metrics_summary'], isNotNull); + }); + + test('adding a metric after span finish does not serialize', () async { + fixture.hub.options.enableMetrics = true; + fixture.hub.options.enableSpanLocalMetricAggregation = true; + final sut = fixture.getSut(); + await sut.finish(status: SpanStatus.aborted()); + sut.localMetricsAggregator?.add(fakeMetric, 0); + + expect(sut.toJson()['_metrics_summary'], isNull); + }); + + test('adding a metric when option is disabled does not serialize', () async { + fixture.hub.options.enableMetrics = false; + final sut = fixture.getSut(); + sut.localMetricsAggregator?.add(fakeMetric, 0); + await sut.finish(status: SpanStatus.aborted()); + + expect(sut.toJson()['_metrics_summary'], isNull); }); test('finished returns false if not yet', () { @@ -271,6 +295,21 @@ void main() { final sut = fixture.getSut(); expect(sut.origin, 'manual'); }); + + test('localMetricsAggregator is set when option is enabled', () async { + fixture.hub.options.enableMetrics = true; + fixture.hub.options.enableSpanLocalMetricAggregation = true; + final sut = fixture.getSut(); + expect(fixture.hub.options.enableSpanLocalMetricAggregation, true); + expect(sut.localMetricsAggregator, isNotNull); + }); + + test('localMetricsAggregator is null when option is disabled', () async { + fixture.hub.options.enableSpanLocalMetricAggregation = false; + final sut = fixture.getSut(); + expect(fixture.hub.options.enableSpanLocalMetricAggregation, false); + expect(sut.localMetricsAggregator, null); + }); } class Fixture { diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index e57cb415ea..77914e66d3 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -468,6 +468,21 @@ void main() { expect(sut.measurements.isEmpty, true); }); + + test('localMetricsAggregator is set when option is enabled', () async { + fixture.hub.options.enableMetrics = true; + fixture.hub.options.enableSpanLocalMetricAggregation = true; + final sut = fixture.getSut(); + expect(fixture.hub.options.enableSpanLocalMetricAggregation, true); + expect(sut.localMetricsAggregator, isNotNull); + }); + + test('localMetricsAggregator is null when option is disabled', () async { + fixture.hub.options.enableMetrics = false; + final sut = fixture.getSut(); + expect(fixture.hub.options.enableSpanLocalMetricAggregation, false); + expect(sut.localMetricsAggregator, null); + }); }); group('$SentryBaggageHeader', () { diff --git a/dart/test/sentry_transaction_test.dart b/dart/test/sentry_transaction_test.dart index fb784abd6d..7de0549a13 100644 --- a/dart/test/sentry_transaction_test.dart +++ b/dart/test/sentry_transaction_test.dart @@ -2,6 +2,7 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:test/test.dart'; +import 'mocks.dart'; import 'mocks/mock_hub.dart'; void main() { @@ -9,6 +10,7 @@ void main() { SentryTracer _createTracer({ bool? sampled = true, + Hub? hub, }) { final context = SentryTransactionContext( 'name', @@ -16,11 +18,16 @@ void main() { samplingDecision: SentryTracesSamplingDecision(sampled!), transactionNameSource: SentryTransactionNameSource.component, ); - return SentryTracer(context, MockHub()); + return SentryTracer(context, hub ?? MockHub()); } test('toJson serializes', () async { - final tracer = _createTracer(); + fixture.options.enableSpanLocalMetricAggregation = true; + fixture.options.enableMetrics = true; + + final tracer = _createTracer(hub: fixture.hub); + tracer.localMetricsAggregator?.add(fakeMetric, 0); + final child = tracer.startChild('child'); await child.finish(); await tracer.finish(); @@ -32,6 +39,7 @@ void main() { expect(map['start_timestamp'], isNotNull); expect(map['spans'], isNotNull); expect(map['transaction_info']['source'], 'component'); + expect(map['_metrics_summary'], isNotNull); }); test('returns finished if it is', () async { @@ -66,9 +74,43 @@ void main() { expect(sut.sampled, false); }); + + test('add a metric to localAggregator adds it to metricSummary', () async { + fixture.options.enableSpanLocalMetricAggregation = true; + fixture.options.enableMetrics = true; + + final tracer = _createTracer(hub: fixture.hub) + ..localMetricsAggregator?.add(fakeMetric, 0); + await tracer.finish(); + + final sut = fixture.getSut(tracer); + expect(sut.metricSummaries, isNotEmpty); + }); + + test('add metric after creation does not add it to metricSummary', () async { + fixture.options.enableSpanLocalMetricAggregation = true; + fixture.options.enableMetrics = true; + + final tracer = _createTracer(hub: fixture.hub); + await tracer.finish(); + final sut = fixture.getSut(tracer); + tracer.localMetricsAggregator?.add(fakeMetric, 0); + + expect(sut.metricSummaries, isEmpty); + }); + + test('metricSummary is null by default', () async { + final tracer = _createTracer(); + await tracer.finish(); + final sut = fixture.getSut(tracer); + expect(sut.metricSummaries, null); + }); } class Fixture { + final SentryOptions options = SentryOptions(dsn: fakeDsn); + late final Hub hub = Hub(options); + SentryTransaction getSut(SentryTracer tracer) { return SentryTransaction(tracer); } diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 73fc7da8c2..5d52ab6080 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -2,6 +2,7 @@ import 'dart:async'; import 'dart:convert'; +import 'dart:math'; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; @@ -530,15 +531,32 @@ class MainScaffold extends StatelessWidget { ), TooltipButton( onPressed: () async { - Sentry.metrics().increment('key1'); - Sentry.metrics().increment('key3', - unit: DurationSentryMeasurementUnit.minute); - Sentry.metrics().increment('key1', - value: 2, tags: {'myTag': 'myValue', 'myTag2': 'myValue2'}); + final span = Sentry.getSpan() ?? + Sentry.startTransaction( + 'testMetrics', 'span summary example', + bindToScope: true); + Sentry.metrics().increment('increment key', + unit: DurationSentryMeasurementUnit.day); + Sentry.metrics().distribution('distribution key', + value: Random().nextDouble() * 10); + Sentry.metrics().set('set int key', + value: Random().nextInt(100), + tags: {'myTag': 'myValue', 'myTag2': 'myValue2'}); + Sentry.metrics().set('set string key', + stringValue: 'Random n ${Random().nextInt(100)}'); + Sentry.metrics() + .gauge('gauge key', value: Random().nextDouble() * 10); + Sentry.metrics().timing( + 'timing key', + function: () async => await Future.delayed( + Duration(milliseconds: Random().nextInt(100)), + () => span.finish()), + unit: DurationSentryMeasurementUnit.milliSecond, + ); }, text: - 'Demonstrates the metrics. It creates 3 counter metrics and send them to Sentry.', - buttonTitle: 'Counter Metric', + 'Demonstrates the metrics. It creates several metrics and send them to Sentry.', + buttonTitle: 'Metrics', ), if (UniversalPlatform.isIOS || UniversalPlatform.isMacOS) const CocoaExample(), diff --git a/flutter/test/mocks.mocks.dart b/flutter/test/mocks.mocks.dart index d747a3889f..ee81d44430 100644 --- a/flutter/test/mocks.mocks.dart +++ b/flutter/test/mocks.mocks.dart @@ -640,6 +640,7 @@ class MockSentryTransaction extends _i1.Mock implements _i3.SentryTransaction { List<_i3.SentryThread>? threads, String? type, Map? measurements, + Map>? metricSummaries, _i3.SentryTransactionInfo? transactionInfo, }) => (super.noSuchMethod( @@ -674,6 +675,7 @@ class MockSentryTransaction extends _i1.Mock implements _i3.SentryTransaction { #threads: threads, #type: type, #measurements: measurements, + #metricSummaries: metricSummaries, #transactionInfo: transactionInfo, }, ),