-
Notifications
You must be signed in to change notification settings - Fork 657
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
Change should_sample
parameters to be spec compliant
#1764
Conversation
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.
Shouldn't we also update the should_sample to take trace_state from parent_context?
@lonewolf3739 |
Does it make sense to move that to should_sample and add a note saying that is the recommended way and discourage people from using |
@lonewolf3739
This makes sense. I'll make these changes. |
@lonewolf3739 |
should_sample
parameters to be spec compliant
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.
Approving, there is mostly a minor fix in documentation suggested.
Fixes #1763
SpanKind
toshould_sample
should_sample
usestrace_state
from the passed inContext
'sSpanContext
'sTraceState
instead of the explicitly passed inTraceState
.