ProbabilitySampler always sample when incoming trace-id is 64-bit (B3/Jaeger)

[shanipribadi]

Probability Sampler right now takes the 8 Most Significant Bytes (leftmost / upper), parse it as a BigEndian uint64, divides by 2, and then compares it against an upper bound value based on fraction * 2^63.

opencensus-go/trace/sampling.go

Lines 50 to 56 in 0521206

	traceIDUpperBound := uint64(fraction * (1 << 63))
	return Sampler(func(p SamplingParameters) SamplingDecision {
	if p.ParentContext.IsSampled() {
	return SamplingDecision{Sample: true}
	}
	x := binary.BigEndian.Uint64(p.TraceID[0:8]) >> 1
	return SamplingDecision{Sample: x < traceIDUpperBound}

But if we are using B3 (Zipkin) or Jaeger propagation format for incoming trace-id and the incoming trace-id is 64-bit,
the 8 MSBytes taken by the sampler will always be 0 valued, and is always evaluated to be lower than the trace upper bound.
This makes all requests coming from systems that are pushing 64-bit trace-id to always be sampled.

References: b3 propagation and jaeger propagation are stored in the rightmost/lower bytes (LSB) in case it's 64-bit or less.

opencensus-go/plugin/ochttp/propagation/b3/b3.go

Lines 75 to 78 in 0521206

	if len(b) <= 8 {
	// The lower 64-bits.
	start := 8 + (8 - len(b))
	copy(traceID[start:], b)

https://github.com/census-ecosystem/opencensus-go-exporter-jaeger/blob/30c8b0fe8ad9d0eac5785893f3941b2e72c5aaaa/propagation/http.go#L60

The same behaviour also appears in opencensus-java https://github.com/census-instrumentation/opencensus-java/blob/a6ec0416fd0c33a5cb04083b58bacab20b2dea98/api/src/main/java/io/opencensus/trace/TraceId.java#L226 (getLowerLong returns the hi bytes rather than the lower bytes)
as well as in opentelemetry-go https://github.com/open-telemetry/opentelemetry-go/blob/c05c3e237d94cc57e5a87c8fa4b39aaa8b862f65/sdk/trace/sampling.go#L84 (I assume since it copied the same logic from opencensus)

Given that the Sampler interface is available, people can just create their own fixed ProbabilitySampler implementation,
But having this behaviour in ProbabilitySampler is surprising, and potentially can be costly when it happens on production traffic without it being documented.