Skip to content

Commit 5d19311

Browse files
buenaflorbuenaflor
authored
[Span First #3]: Add active span parenting behaviour and more api scaffolding (#3377)
* Update * Updateg * Update * Update * Update * Remove unnecessary import * Remove unnecessary import * Update * Update * Update * Update * Update * Update * Update * Update * Update * Refactor Span implementations to standardize spanId handling and improve attribute management. Added spanId to SimpleSpan and NoOpSpan, and updated UnsetSpan to throw errors for unimplemented methods. Enhanced tests to verify spanId creation. * Add tests for finished spans as parents and handling of disabled tracing - Introduced a test to verify that a finished span can be used as a parent for a new span. - Added a test to ensure that when tracing is disabled, starting a span results in a NoOpSpan and does not affect active spans. * Implement idempotent behavior for ending spans and enhance tests for span finalization - Added a check in the `end` method of `SimpleSpan` to prevent multiple calls from altering the end timestamp after the span is finished. - Introduced a new test to verify that calling `end` on an already finished span does not change its state or timestamp. - Updated existing tests to use a consistent method for obtaining the hub instance. * Fix end method in SimpleSpan to ensure captureSpan is called after setting isFinished flag * Update scope_test to use getActiveSpan method for retrieving the last active span * Refactor Span documentation and clean up tests - Removed redundant comment about parent span being null in the Span interface. - Enhanced documentation for the attributes getter to specify the use of Map.unmodifiable. - Cleaned up the hub_span_test by removing an incomplete test case regarding finished spans as parents. - Updated scope_test to include assertions for active spans in the cloned state. * Update TODO comment for clarity * Update TODO comment for span processing * Rename test for setting span status to improve clarity * Fix end method in SimpleSpan to ensure endTimestamp is set as UTC - Updated the end method to convert the endTimestamp to UTC correctly. - Added a test to verify that endTimestamp is set as UTC when the end method is called. * Update span_test to ensure endTimestamp is set as UTC in tests - Modified the test to convert the end timestamp to UTC before passing it to the span's end method, ensuring consistency with the expected behavior.
1 parent 87d79a3 commit 5d19311

File tree

12 files changed

+815
-40
lines changed

12 files changed

+815
-40
lines changed

‎packages/dart/lib/src/hub.dart‎

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import 'protocol/unset_span.dart';
1111
import 'sentry_tracer.dart';
1212
import 'sentry_traces_sampler.dart';
1313
import 'protocol/noop_span.dart';
14+
import 'protocol/simple_span.dart';
1415
import 'transport/data_category.dart';
1516

1617
/// Configures the scope through the callback.
@@ -586,12 +587,48 @@ class Hub {
586587
SentryLevel.warning,
587588
"Instance is disabled and this 'startSpan' call is a no-op.",
588589
);
589-
} else if (_options.isTracingEnabled()) {
590-
// TODO: implementation of span api behaviour according to https://develop.sentry.dev/sdk/telemetry/spans/span-api/
591590
return NoOpSpan();
592591
}
593592

594-
return NoOpSpan();
593+
if (!_options.isTracingEnabled()) {
594+
return NoOpSpan();
595+
}
596+
597+
// Determine the parent span based on the parentSpan parameter:
598+
// - If parentSpan is UnsetSpan (default), use the currently active span
599+
// - If parentSpan is a specific Span, use that as the parent
600+
// - If parentSpan is null, create a root/segment span (no parent)
601+
final Span? resolvedParentSpan;
602+
if (parentSpan is UnsetSpan) {
603+
resolvedParentSpan = scope.getActiveSpan();
604+
} else {
605+
resolvedParentSpan = parentSpan;
606+
}
607+
608+
final span =
609+
SimpleSpan(name: name, parentSpan: resolvedParentSpan, hub: this);
610+
if (attributes != null) {
611+
span.setAttributes(attributes);
612+
}
613+
if (active) {
614+
scope.setActiveSpan(span);
615+
}
616+
617+
return span;
618+
}
619+
620+
void captureSpan(Span span) {
621+
if (!_isEnabled) {
622+
_options.log(
623+
SentryLevel.warning,
624+
"Instance is disabled and this 'captureSpan' call is a no-op.",
625+
);
626+
return;
627+
}
628+
629+
scope.removeActiveSpan(span);
630+
631+
// TODO(next-pr): run this span through span specific pipeline and then forward to span buffer
595632
}
596633

597634
@internal

‎packages/dart/lib/src/hub_adapter.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,4 +221,7 @@ class HubAdapter implements Hub {
221221

222222
@override
223223
void removeAttribute(String key) => Sentry.currentHub.removeAttribute(key);
224+
225+
@override
226+
void captureSpan(Span span) => Sentry.currentHub.captureSpan(span);
224227
}

‎packages/dart/lib/src/noop_hub.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,4 +161,7 @@ class NoOpHub implements Hub {
161161
Map<String, SentryAttribute>? attributes,
162162
}) =>
163163
NoOpSpan();
164+
165+
@override
166+
void captureSpan(Span span) {}
164167
}

‎packages/dart/lib/src/protocol/noop_span.dart‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,40 @@ class NoOpSpan implements Span {
44
const NoOpSpan();
55

66
@override
7-
void end({DateTime? endTimestamp}) {}
7+
SpanId get spanId => SpanId.empty();
88

99
@override
10-
void setAttribute(String key, SentryAttribute value) {}
10+
final String name = 'NoOpSpan';
1111

1212
@override
13-
void setAttributes(Map<String, SentryAttribute> attributes) {}
13+
set name(String name) {}
14+
15+
@override
16+
final SpanV2Status status = SpanV2Status.ok;
17+
18+
@override
19+
set status(SpanV2Status status) {}
20+
21+
@override
22+
Span? get parentSpan => null;
23+
24+
@override
25+
DateTime? get endTimestamp => null;
26+
27+
@override
28+
Map<String, SentryAttribute> get attributes => {};
29+
30+
@override
31+
bool get isFinished => false;
1432

1533
@override
16-
void setName(String name) {}
34+
void setAttribute(String key, SentryAttribute value) {}
1735

1836
@override
19-
void setStatus(SpanV2Status status) {}
37+
void setAttributes(Map<String, SentryAttribute> attributes) {}
38+
39+
@override
40+
void end({DateTime? endTimestamp}) {}
2041

2142
@override
2243
Map<String, dynamic> toJson() => {};

‎packages/dart/lib/src/protocol/simple_span.dart‎

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,67 @@
11
import '../../sentry.dart';
22

33
class SimpleSpan implements Span {
4+
final SpanId _spanId;
5+
final Hub _hub;
46
@override
5-
void end({DateTime? endTimestamp}) {
6-
// TODO: implement end
7-
}
7+
final Span? parentSpan;
8+
final Map<String, SentryAttribute> _attributes = {};
9+
10+
String _name;
11+
SpanV2Status _status = SpanV2Status.ok;
12+
DateTime? _endTimestamp;
13+
bool _isFinished = false;
14+
15+
SimpleSpan({
16+
required String name,
17+
this.parentSpan,
18+
Hub? hub,
19+
}) : _spanId = SpanId.newId(),
20+
_hub = hub ?? HubAdapter(),
21+
_name = name;
822

923
@override
10-
void setAttribute(String key, SentryAttribute value) {
11-
// TODO: implement setAttribute
12-
}
24+
SpanId get spanId => _spanId;
1325

1426
@override
15-
void setAttributes(Map<String, SentryAttribute> attributes) {
16-
// TODO: implement setAttributes
27+
String get name => _name;
28+
29+
@override
30+
set name(String value) => _name = value;
31+
32+
@override
33+
SpanV2Status get status => _status;
34+
35+
@override
36+
set status(SpanV2Status value) => _status = value;
37+
38+
@override
39+
DateTime? get endTimestamp => _endTimestamp;
40+
41+
@override
42+
Map<String, SentryAttribute> get attributes => Map.unmodifiable(_attributes);
43+
44+
@override
45+
bool get isFinished => _isFinished;
46+
47+
@override
48+
void setAttribute(String key, SentryAttribute value) {
49+
_attributes[key] = value;
1750
}
1851

1952
@override
20-
void setName(String name) {
21-
// TODO: implement setName
53+
void setAttributes(Map<String, SentryAttribute> attributes) {
54+
_attributes.addAll(attributes);
2255
}
2356

2457
@override
25-
void setStatus(SpanV2Status status) {
26-
// TODO: implement setStatus
58+
void end({DateTime? endTimestamp}) {
59+
if (_isFinished) {
60+
return;
61+
}
62+
_endTimestamp = (endTimestamp ?? DateTime.now()).toUtc();
63+
_isFinished = true;
64+
_hub.captureSpan(this);
2765
}
2866

2967
@override

‎packages/dart/lib/src/protocol/span.dart‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,43 @@ import 'package:meta/meta.dart';
22

33
import '../../sentry.dart';
44

5-
/// Represents the Span model based on https://develop.sentry.dev/sdk/telemetry/spans/span-api/
5+
// Span specs: https://develop.sentry.dev/sdk/telemetry/spans/span-api/
6+
7+
/// Represents a basic telemetry span.
68
abstract class Span {
79
@internal
810
const Span();
911

12+
/// Gets the id of the span.
13+
SpanId get spanId;
14+
15+
/// Gets the name of the span.
16+
String get name;
17+
18+
/// Sets the name of the span.
19+
set name(String name);
20+
21+
/// Gets the parent span.
22+
Span? get parentSpan;
23+
24+
/// Gets the status of the span.
25+
SpanV2Status get status;
26+
27+
/// Sets the status of the span.
28+
set status(SpanV2Status status);
29+
30+
/// Gets the end timestamp of the span.
31+
DateTime? get endTimestamp;
32+
33+
/// Gets a read-only view of the attributes of the span using [Map.unmodifiable](https://api.flutter.dev/flutter/dart-core/Map/Map.unmodifiable.html).
34+
///
35+
/// The returned map must not be mutated by callers.
36+
Map<String, SentryAttribute> get attributes;
37+
1038
/// Ends the span.
1139
///
1240
/// [endTimestamp] can be used to override the end time.
13-
/// If omitted, the span ends using the current time.
41+
/// If omitted, the span ends using the current time when end is executed.
1442
void end({DateTime? endTimestamp});
1543

1644
/// Sets a single attribute.
@@ -23,11 +51,8 @@ abstract class Span {
2351
/// Overrides if the attributes already exist.
2452
void setAttributes(Map<String, SentryAttribute> attributes);
2553

26-
/// Sets the status of the span.
27-
void setStatus(SpanV2Status status);
28-
29-
/// Sets the name of the span.
30-
void setName(String name);
54+
@internal
55+
bool get isFinished;
3156

3257
@internal
3358
Map<String, dynamic> toJson();

‎packages/dart/lib/src/protocol/unset_span.dart‎

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,32 +9,58 @@ class UnsetSpan extends Span {
99
const UnsetSpan();
1010

1111
@override
12-
void end({DateTime? endTimestamp}) {
13-
throw UnimplementedError('$UnsetSpan methods should not be used');
14-
}
12+
SpanId get spanId =>
13+
throw UnimplementedError('$UnsetSpan apis should not be used');
1514

1615
@override
17-
void setAttribute(String key, SentryAttribute value) {
18-
throw UnimplementedError('$UnsetSpan methods should not be used');
19-
}
16+
String get name =>
17+
throw UnimplementedError('$UnsetSpan apis should not be used');
2018

2119
@override
22-
void setAttributes(Map<String, SentryAttribute> attributes) {
23-
throw UnimplementedError('$UnsetSpan methods should not be used');
20+
set name(String name) =>
21+
throw UnimplementedError('$UnsetSpan apis should not be used');
22+
23+
@override
24+
SpanV2Status get status =>
25+
throw UnimplementedError('$UnsetSpan apis should not be used');
26+
27+
@override
28+
set status(SpanV2Status status) =>
29+
throw UnimplementedError('$UnsetSpan apis should not be used');
30+
31+
@override
32+
Span? get parentSpan =>
33+
throw UnimplementedError('$UnsetSpan apis should not be used');
34+
35+
@override
36+
DateTime? get endTimestamp =>
37+
throw UnimplementedError('$UnsetSpan apis should not be used');
38+
39+
@override
40+
Map<String, SentryAttribute> get attributes =>
41+
throw UnimplementedError('$UnsetSpan apis should not be used');
42+
43+
@override
44+
bool get isFinished =>
45+
throw UnimplementedError('$UnsetSpan apis should not be used');
46+
47+
@override
48+
void setAttribute(String key, SentryAttribute value) {
49+
throw UnimplementedError('$UnsetSpan apis should not be used');
2450
}
2551

2652
@override
27-
void setName(String name) {
28-
throw UnimplementedError('$UnsetSpan methods should not be used');
53+
void setAttributes(Map<String, SentryAttribute> attributes) {
54+
throw UnimplementedError('$UnsetSpan apis should not be used');
2955
}
3056

3157
@override
32-
void setStatus(SpanV2Status status) {
33-
throw UnimplementedError('$UnsetSpan methods should not be used');
58+
void end({DateTime? endTimestamp}) {
59+
throw UnimplementedError('$UnsetSpan apis should not be used');
3460
}
3561

3662
@override
3763
Map<String, dynamic> toJson() {
38-
throw UnimplementedError('$UnsetSpan methods should not be used');
64+
throw UnimplementedError('$UnsetSpan apis should not be used');
3965
}
4066
}

‎packages/dart/lib/src/scope.dart‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,42 @@ class Scope {
4343
/// Returns active transaction or null if there is no active transaction.
4444
ISentrySpan? span;
4545

46+
/// List of active spans.
47+
/// The last span in the list is the current active span.
48+
final List<Span> _activeSpans = [];
49+
50+
@visibleForTesting
51+
List<Span> get activeSpans => List.unmodifiable(_activeSpans);
52+
53+
/// Returns the currently active span, or `null` if no span is active.
54+
///
55+
/// The active span is the most recently set span via [setActiveSpan].
56+
/// When starting a new span with `active: true` (the default), the new span
57+
/// becomes a child of this active span.
58+
@internal
59+
Span? getActiveSpan() {
60+
return _activeSpans.lastOrNull;
61+
}
62+
63+
/// Sets the given [span] as the currently active span.
64+
///
65+
/// Active spans are used to automatically parent new spans.
66+
/// When a new span is started with `active: true` (the default), it becomes
67+
/// a child of the currently active span.
68+
@internal
69+
void setActiveSpan(Span span) {
70+
_activeSpans.add(span);
71+
}
72+
73+
/// Removes the given [span] from the active spans list.
74+
///
75+
/// This should be called when a span ends to remove it from the active
76+
/// span list.
77+
@internal
78+
void removeActiveSpan(Span span) {
79+
_activeSpans.remove(span);
80+
}
81+
4682
/// The propagation context for connecting errors and spans to traces.
4783
/// There should always be a propagation context available at all times.
4884
///
@@ -273,6 +309,7 @@ class Scope {
273309
_replayId = null;
274310
propagationContext = PropagationContext();
275311
_attributes.clear();
312+
_activeSpans.clear();
276313

277314
_clearBreadcrumbsSync();
278315
_setUserSync(null);
@@ -480,6 +517,10 @@ class Scope {
480517
clone.setAttributes(Map.from(_attributes));
481518
}
482519

520+
if (_activeSpans.isNotEmpty) {
521+
clone._activeSpans.addAll(_activeSpans);
522+
}
523+
483524
return clone;
484525
}
485526

0 commit comments

Comments
 (0)