diff --git a/CHANGELOG.md b/CHANGELOG.md index bd18ccc99..3b2463819 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Fixed + +- The `Span.IsRecording` implementation from `go.opentelemetry.io/otel/sdk/trace` always returns false when not being sampled. (#1750) + ### Changed - Jaeger exporter was updated to use thrift v0.14.1. (#1712) diff --git a/sdk/trace/span.go b/sdk/trace/span.go index 312294ae6..4cc86489d 100644 --- a/sdk/trace/span.go +++ b/sdk/trace/span.go @@ -160,7 +160,8 @@ func (s *span) IsRecording() bool { } s.mu.Lock() defer s.mu.Unlock() - return s.endTime.IsZero() + + return !s.startTime.IsZero() && s.endTime.IsZero() } // SetStatus sets the status of this span in the form of a code and a diff --git a/sdk/trace/trace_test.go b/sdk/trace/trace_test.go index c1b43a976..41f64c18e 100644 --- a/sdk/trace/trace_test.go +++ b/sdk/trace/trace_test.go @@ -223,13 +223,35 @@ func TestSetName(t *testing.T) { } } -func TestRecordingIsOn(t *testing.T) { - tp := NewTracerProvider() - _, span := tp.Tracer("Recording on").Start(context.Background(), "StartSpan") - defer span.End() - if span.IsRecording() == false { - t.Error("new span is not recording events") - } +func TestSpanIsRecording(t *testing.T) { + t.Run("while Span active", func(t *testing.T) { + for name, tc := range map[string]struct { + sampler Sampler + want bool + }{ + "Always sample, recording on": {sampler: AlwaysSample(), want: true}, + "Never sample recording off": {sampler: NeverSample(), want: false}, + } { + tp := NewTracerProvider(WithSampler(tc.sampler)) + _, span := tp.Tracer(name).Start(context.Background(), "StartSpan") + defer span.End() + got := span.IsRecording() + assert.Equal(t, got, tc.want, name) + } + }) + + t.Run("after Span end", func(t *testing.T) { + for name, tc := range map[string]Sampler{ + "Always Sample": AlwaysSample(), + "Never Sample": NeverSample(), + } { + tp := NewTracerProvider(WithSampler(tc)) + _, span := tp.Tracer(name).Start(context.Background(), "StartSpan") + span.End() + got := span.IsRecording() + assert.False(t, got, name) + } + }) } func TestSampling(t *testing.T) { @@ -1014,6 +1036,7 @@ func TestExecutionTracerTaskEnd(t *testing.T) { s.executionTracerTaskEnd = executionTracerTaskEnd spans = append(spans, s) // parent not sampled + tp.sampler = AlwaysSample() _, apiSpan = tr.Start(context.Background(), "foo") s = apiSpan.(*span) s.executionTracerTaskEnd = executionTracerTaskEnd @@ -1022,7 +1045,9 @@ func TestExecutionTracerTaskEnd(t *testing.T) { for _, span := range spans { span.End() } - if got, want := n, uint64(len(spans)); got != want { + // Only one span should be sampled meaning only one execution of + // executionTracerTaskEnd. + if got, want := n, uint64(1); got != want { t.Fatalf("Execution tracer task ended for %v spans; want %v", got, want) } }