Fix span IsRecording when not sampling (#1750)

* Adjust TestRecording fucntion to validate with and without sampling. correct span.isRecording logic to return false when not being sampled

* Changed to TestExectutionTracerTaskEnd to only expect 1 span to increment instead of all 3

* added changelog entry

* Updated CHANGELOG.md

* Remove newline from isRecording()

* Rewrite TestSpanIsRecording to include table based
tests and check for when span is ended immediately

* Update sdk/trace/trace_test.go

Improve readability of test name

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

* Update sdk/trace/trace_test.go

Improve readability of test name

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>

* Update CHANGELOG.md

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update sdk/trace_test.go test comments

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

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)

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

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)
 	}
 }