-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Update SpanSnapshot to use parent SpanContext #1748
Conversation
Having only the parent span ID and a separate field to communicate if the parent was remote does not provide a comprehensive view of the parent span nor is it an efficient way to transmit this information. Update the SpanSnapshot to have a `Parent` field that contains the parent span context. This field replaces the ParentSpanID and HasRemoteParent fields.
Codecov Report
@@ Coverage Diff @@
## main #1748 +/- ##
=====================================
Coverage 78.0% 78.0%
=====================================
Files 132 132
Lines 6941 6944 +3
=====================================
+ Hits 5417 5420 +3
Misses 1274 1274
Partials 250 250
|
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.
Not totally related to the PR itself but I see that there are other parts of the code that need some refactoring after the isRemote()
change on the SpanContext
just like this, are there any issue tracking them?
@paivagustavo there is this issue which is being addressed in this PR, but if you see other places where it needs to be refactored please do indeed open an issue or PR. |
Having only the parent span ID and a separate field to communicate if the parent was remote does not provide a comprehensive view of the parent span nor is it an efficient way to transmit this information. Update the SpanSnapshot to have a
Parent
field that contains the parent span context. This field replaces the ParentSpanID and HasRemoteParent fields.Additionally to this being an improvement to the functionality of the SDK, it also helps move towards having the
SpanSnapshot
implement theReadonlySpan
interface.Part of #1380