Skip to content

Conversation

@buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Dec 2, 2025

📜 Description

Span first initiative

💡 Motivation and Context

Part of #3331

💚 How did you test it?

Unit tests

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

#skip-changelog

@buenaflor buenaflor changed the title feat(spans-first): Add active span and more Span API scaffolding feat(spans-first): Add active span parenting behaviour and more Span API scaffolding Dec 2, 2025
@buenaflor buenaflor changed the title feat(spans-first): Add active span parenting behaviour and more Span API scaffolding feat(spans-first): Add active span parenting behaviour and more api scaffolding Dec 2, 2025
@codecov
Copy link

codecov bot commented Dec 2, 2025

Codecov Report

❌ Patch coverage is 88.31169% with 9 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (feat/span-first@87d79a3). Learn more about missing BASE report.

Files with missing lines Patch % Lines
packages/dart/lib/src/protocol/noop_span.dart 25.00% 6 Missing ⚠️
packages/dart/lib/src/hub_adapter.dart 0.00% 2 Missing ⚠️
packages/dart/lib/src/noop_hub.dart 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                Coverage Diff                 @@
##             feat/span-first    #3377   +/-   ##
==================================================
  Coverage                   ?   88.64%           
==================================================
  Files                      ?      295           
  Lines                      ?    10077           
  Branches                   ?        0           
==================================================
  Hits                       ?     8933           
  Misses                     ?     1144           
  Partials                   ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@buenaflor
Copy link
Contributor Author

cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Scope clear() does not reset active spans list

The Scope.clear() method, intended to reset the scope, doesn't clear the _activeSpans list. This leaves stale spans, leading to incorrect parenting for new spans and inconsistency with other cleared collection fields.

packages/dart/lib/src/scope.dart#L297-L316

/// Resets the Scope to its default state
Future<void> clear() async {
clearAttachments();
level = null;
span = null;
_transaction = null;
_fingerprint = [];
_tags.clear();
_extra.clear();
_eventProcessors.clear();
_replayId = null;
propagationContext = PropagationContext();
_attributes.clear();
_clearBreadcrumbsSync();
_setUserSync(null);
await clearBreadcrumbs();
await setUser(null);
}

packages/dart/lib/src/scope.dart#L45-L78

final List<Span> _activeSpans = [];
@visibleForTesting
List<Span> get activeSpans => List.unmodifiable(_activeSpans);
/// Returns the currently active span, or `null` if no span is active.
///
/// The active span is the most recently set span via [setActiveSpan].
/// When starting a new span with `active: true` (the default), the new span
/// becomes a child of this active span.
@internal
Span? getActiveSpan() {
return _activeSpans.lastOrNull;
}
/// Sets the given [span] as the currently active span.
///
/// Active spans are used to automatically parent new spans.
/// When a new span is started with `active: true` (the default), it becomes
/// a child of the currently active span.
@internal
void setActiveSpan(Span span) {
_activeSpans.add(span);
}
/// Removes the given [span] from the active spans list.
///
/// This should be called when a span ends to remove it from the active
/// span hierarchy.
@internal
void removeActiveSpan(Span span) {
_activeSpans.remove(span);
}

Fix in Cursor Fix in Web


@buenaflor buenaflor changed the title feat(spans-first): Add active span parenting behaviour and more api scaffolding [Span First #3]: Add active span parenting behaviour and more api scaffolding Dec 2, 2025
buenaflor and others added 4 commits December 3, 2025 00:05
…ove attribute management. Added spanId to SimpleSpan and NoOpSpan, and updated UnsetSpan to throw errors for unimplemented methods. Enhanced tests to verify spanId creation.
- 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.
@getsentry getsentry deleted a comment from cursor bot Dec 10, 2025
@buenaflor
Copy link
Contributor Author

cursor review

…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.
@buenaflor
Copy link
Contributor Author

cursor review

@buenaflor buenaflor marked this pull request as ready for review December 10, 2025 14:11
@buenaflor buenaflor requested a review from denrase as a code owner December 10, 2025 14:11
- 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.
@buenaflor buenaflor requested a review from denrase December 10, 2025 15:27
- 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.
- 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.
@buenaflor buenaflor merged commit 5d19311 into feat/span-first Dec 11, 2025
136 of 141 checks passed
@buenaflor buenaflor deleted the feat/spans/active-spans-impl branch December 11, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants