Missing sampler.type & sampler.param attributes/ tags in Jaeger samplers #3653
Description
Background
Hi, thanks for anyone who contributed to jaeger remote sampler!
I am trying to use jaeger remote sampler, but I notice in the currently implementation of each jaeger sampler, sampler.type and sampler.param attributes/ tags are not set, e.g in probabilisticSampler:
func (s *probabilisticSampler) ShouldSample(p trace.SamplingParameters) trace.SamplingResult {
...
return trace.SamplingResult{
Decision: trace.Drop,
Tracestate: psc.TraceState(),
}
}
While in the deprecated jaeger-client-go, when a sampler is making the sampling decision, sampler.type and sampler.param tags are set, e.g
func (s *ProbabilisticSampler) init(samplingRate float64) *ProbabilisticSampler {
...
s.tags = []Tag{
{key: SamplerTypeTagKey, value: SamplerTypeProbabilistic},
{key: SamplerParamTagKey, value: s.samplingRate},
}
return s
}
func (s *ProbabilisticSampler) IsSampled(id TraceID, operation string) (bool, []Tag) {
return s.samplingBoundary >= id.Low&maxRandomNumber, s.tags
}
These two sampler tags are essential to Jaeger adaptive sampling (more detailed explanation here).
In Jaeger backend (more specifically jaeger-collector), spans without sampler.type tag will not contribute to the span throughput which is needed for further adaptive probability calculation.
// handleRootSpan returns a function that records throughput for root spans
func handleRootSpan(aggregator strategystore.Aggregator, logger *zap.Logger) ProcessSpan {
return func(span *model.Span, tenant string) {
....
samplerType, samplerParam := span.GetSamplerParams(logger)
if samplerType == "" {
return
}
aggregator.RecordThroughput(service, span.OperationName, samplerType, samplerParam)
}
}
So without the sampler tags being set, the Jaeger adaptive sampling will not work as expected.
Proposed Solution
Add sampler.type and sampler.param attributes as part of SamplingResult in ShouldSample for each sampler.