-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow auto termination of spans #1748
Conversation
7d42475
to
a1b0918
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1748 +/- ##
==========================================
+ Coverage 85.29% 85.42% +0.12%
==========================================
Files 464 464
Lines 10791 10818 +27
Branches 1595 1601 +6
==========================================
+ Hits 9204 9241 +37
+ Misses 870 860 -10
Partials 717 717
|
da0c39e
to
8c566c8
Compare
@@ -101,4 +102,50 @@ class SpanRepository { | |||
} | |||
|
|||
private fun notTracked(spanId: String): Boolean = activeSpans[spanId] == null && completedSpans[spanId] == null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will you be adding unit tests that verify this works with different edge-case combinations of descendants which might change the traversal logic? Like if a child is no longer active, but the grand child is active?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Question about adding more test cases for different combinations of span states. We need to make sure that after backgrounding, all active spans belonging to traces whose root as auto terminate configured will be terminated, irrespective of the state of its ancestors.
d2571be
to
e7a6db8
Compare
8c566c8
to
7ee25e6
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files |
@bidetofevil I added some extra test cases to the integration test & made some slight tweaks to the implementation, so putting this out for review again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
7ee25e6
to
e880753
Compare
Goal
Implements auto-termination of spans. This is achieved by checking whether a span has
AutoTerminationMode.ON_BACKGROUND
just before the session payload is constructed, and terminating it and its descendants if this is true.Testing
Added integration tests.