-
Notifications
You must be signed in to change notification settings - Fork 289
Conversation
Signed-off-by: Yuri Shkuro <ys@uber.com>
Signed-off-by: Yuri Shkuro <ys@uber.com>
Codecov Report
@@ Coverage Diff @@
## master #450 +/- ##
=======================================
Coverage 84.69% 84.69%
=======================================
Files 33 33
Lines 1503 1503
=======================================
Hits 1273 1273
Misses 168 168
Partials 62 62 Continue to review full report at Codecov.
|
Signed-off-by: Yuri Shkuro <ys@uber.com>
@@ -79,37 +62,26 @@ func NewRemotelyControlledSampler( | |||
return sampler | |||
} | |||
|
|||
func applySamplerOptions(opts ...SamplerOption) samplerOptions { |
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.
moved to sampler_remote_options
Signed-off-by: Yuri Shkuro <ys@uber.com>
@@ -48,29 +45,15 @@ type RemotelyControlledSampler struct { | |||
doneChan chan *sync.WaitGroup | |||
} | |||
|
|||
type httpSamplingManager struct { |
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.
moved to bottom of the file
samplerV1 := s.getSamplerForOperation(operationName) | ||
sampled, tags := samplerV1.IsSampled(span.context.TraceID(), operationName) | ||
return SamplingDecision{sample: sampled, retryable: false, tags: tags} |
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.
This is new behavior - do we require this right now?
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.
You mean new to Go client?
I don't think we can avoid it. The main change is not actually here, but in OnCreateSpan, which must return retryable=true in order for tag sampling to work (otherwise the state will be finalized). So if we already have that in OnCreateSpan, adding OnSetOperationName() doesn't change things that much, imo.
Part of #449.