fix(sdk/trace): return spec-compliant TraceIdRatioBased description (#8027)

Optimize performance when sampling is disabled - no need to calculate
fraction based on trace id (in `traceIDRatioSampler`) if fraction is
zero.

---------

Co-authored-by: David Ashpole <dashpole@google.com>
Co-authored-by: Damien Mathieu <42@dmathieu.com>

CHANGELOG.md

@@ -22,6 +22,11 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - Deprecate `INVALID` in `go.opentelemetry.io/otel/attribute`. Use `EMPTY` instead. (#8038)
 
+### Fixed
+
+- Return spec-compliant `TraceIdRatioBased` description. This is a breaking behavioral change, but it is necessary to
+  make the implementation [spec-compliant](https://opentelemetry.io/docs/specs/otel/trace/sdk/#traceidratiobased). (#8027)
+
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->
 

sdk/trace/sampling.go

@@ -69,17 +69,17 @@ type traceIDRatioSampler struct {
 }
 
 func (ts traceIDRatioSampler) ShouldSample(p SamplingParameters) SamplingResult {
-	psc := trace.SpanContextFromContext(p.ParentContext)
+	state := trace.SpanContextFromContext(p.ParentContext).TraceState()
 	x := binary.BigEndian.Uint64(p.TraceID[8:16]) >> 1
 	if x < ts.traceIDUpperBound {
 		return SamplingResult{
 			Decision:   RecordAndSample,
-			Tracestate: psc.TraceState(),
+			Tracestate: state,
 		}
 	}
 	return SamplingResult{
 		Decision:   Drop,
-		Tracestate: psc.TraceState(),
+		Tracestate: state,
 	}
 }
 
@@ -94,12 +94,20 @@ func (ts traceIDRatioSampler) Description() string {
 //
 //nolint:revive // revive complains about stutter of `trace.TraceIDRatioBased`
 func TraceIDRatioBased(fraction float64) Sampler {
+	// Cannot use AlwaysSample() and NeverSample(), must return spec-compliant descriptions.
+	// See https://opentelemetry.io/docs/specs/otel/trace/sdk/#traceidratiobased.
 	if fraction >= 1 {
-		return AlwaysSample()
+		return predeterminedSampler{
+			description: "TraceIDRatioBased{1}",
+			decision:    RecordAndSample,
+		}
 	}
 
 	if fraction <= 0 {
-		fraction = 0
+		return predeterminedSampler{
+			description: "TraceIDRatioBased{0}",
+			decision:    Drop,
+		}
 	}
 
 	return &traceIDRatioSampler{
@@ -118,6 +126,7 @@ func (alwaysOnSampler) ShouldSample(p SamplingParameters) SamplingResult {
 }
 
 func (alwaysOnSampler) Description() string {
+	// https://opentelemetry.io/docs/specs/otel/trace/sdk/#alwayson
 	return "AlwaysOnSampler"
 }
 
@@ -139,6 +148,7 @@ func (alwaysOffSampler) ShouldSample(p SamplingParameters) SamplingResult {
 }
 
 func (alwaysOffSampler) Description() string {
+	// https://opentelemetry.io/docs/specs/otel/trace/sdk/#alwaysoff
 	return "AlwaysOffSampler"
 }
 
@@ -147,6 +157,22 @@ func NeverSample() Sampler {
 	return alwaysOffSampler{}
 }
 
+type predeterminedSampler struct {
+	description string
+	decision    SamplingDecision
+}
+
+func (s predeterminedSampler) ShouldSample(p SamplingParameters) SamplingResult {
+	return SamplingResult{
+		Decision:   s.decision,
+		Tracestate: trace.SpanContextFromContext(p.ParentContext).TraceState(),
+	}
+}
+
+func (s predeterminedSampler) Description() string {
+	return s.description
+}
+
 // ParentBased returns a sampler decorator which behaves differently,
 // based on the parent of the span. If the span has no parent,
 // the decorated sampler is used to make sampling decision. If the span has

sdk/trace/sampling_test.go

@@ -306,3 +306,13 @@ func TestAlwaysRecordDefaultDescription(t *testing.T) {
 		)
 	}
 }
+
+func TestDescriptions(t *testing.T) {
+	assert.Equal(t, "AlwaysOnSampler", AlwaysSample().Description())
+	assert.Equal(t, "AlwaysOffSampler", NeverSample().Description())
+	assert.Equal(t, "TraceIDRatioBased{0.5}", TraceIDRatioBased(0.5).Description())
+	assert.Equal(t, "TraceIDRatioBased{1}", TraceIDRatioBased(1).Description())
+	assert.Equal(t, "TraceIDRatioBased{1}", TraceIDRatioBased(1.5).Description())
+	assert.Equal(t, "TraceIDRatioBased{0}", TraceIDRatioBased(0).Description())
+	assert.Equal(t, "TraceIDRatioBased{0}", TraceIDRatioBased(-0.5).Description())
+}