From 909fbdfd24d9924538d4d5ec7f169f60341ab88c Mon Sep 17 00:00:00 2001
From: Sri Harsha CH <57220027+harshachinta@users.noreply.github.com>
Date: Thu, 8 Feb 2024 23:39:45 +0530
Subject: [PATCH] fix: add mutex to internal trace otel test variable (#9390)

* fix: add mutex to otel test variable

* feat: code refactoring

* feat: comments refactoring
---
 internal/trace/trace.go                       | 23 +++++++++++++------
 internal/trace/trace_test.go                  | 12 +++++-----
 .../test/opentelemetry/test/ot_traces_test.go |  6 ++---
 3 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/internal/trace/trace.go b/internal/trace/trace.go
index eabed000f309..97738b2cbee2 100644
--- a/internal/trace/trace.go
+++ b/internal/trace/trace.go
@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"os"
 	"strings"
+	"sync"
 
 	"go.opencensus.io/trace"
 	"go.opentelemetry.io/otel"
@@ -50,17 +51,23 @@ const (
 )
 
 var (
-	// OpenTelemetryTracingEnabled is true if the environment variable
+	// openTelemetryTracingEnabledMu guards access to openTelemetryTracingEnabled field
+	openTelemetryTracingEnabledMu = sync.RWMutex{}
+	// openTelemetryTracingEnabled is true if the environment variable
 	// GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING is set to the
 	// case-insensitive value "opentelemetry".
-	//
-	// Do not access directly. Use instead IsOpenTelemetryTracingEnabled or
-	// IsOpenCensusTracingEnabled. Intended for use only in unit tests. Restore
-	// original value after each test.
-	OpenTelemetryTracingEnabled bool = strings.EqualFold(strings.TrimSpace(
+	openTelemetryTracingEnabled bool = strings.EqualFold(strings.TrimSpace(
 		os.Getenv(TelemetryPlatformTracingVar)), TelemetryPlatformTracingOpenTelemetry)
 )
 
+// SetOpenTelemetryTracingEnabledField programmatically sets the value provided by GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING for the purpose of unit testing.
+// Do not invoke it directly. Intended for use only in unit tests. Restore original value after each test.
+func SetOpenTelemetryTracingEnabledField(enabled bool) {
+	openTelemetryTracingEnabledMu.Lock()
+	defer openTelemetryTracingEnabledMu.Unlock()
+	openTelemetryTracingEnabled = enabled
+}
+
 // IsOpenCensusTracingEnabled returns true if the environment variable
 // GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING is NOT set to the
 // case-insensitive value "opentelemetry".
@@ -72,7 +79,9 @@ func IsOpenCensusTracingEnabled() bool {
 // GOOGLE_API_GO_EXPERIMENTAL_TELEMETRY_PLATFORM_TRACING is set to the
 // case-insensitive value "opentelemetry".
 func IsOpenTelemetryTracingEnabled() bool {
-	return OpenTelemetryTracingEnabled
+	openTelemetryTracingEnabledMu.RLock()
+	defer openTelemetryTracingEnabledMu.RUnlock()
+	return openTelemetryTracingEnabled
 }
 
 // StartSpan adds a span to the trace with the given name. If IsOpenCensusTracingEnabled
diff --git a/internal/trace/trace_test.go b/internal/trace/trace_test.go
index bfc02e284ee3..2e03d2a89df0 100644
--- a/internal/trace/trace_test.go
+++ b/internal/trace/trace_test.go
@@ -41,11 +41,11 @@ var (
 )
 
 func TestStartSpan_OpenCensus(t *testing.T) {
-	old := OpenTelemetryTracingEnabled
-	OpenTelemetryTracingEnabled = false
+	old := IsOpenTelemetryTracingEnabled()
+	SetOpenTelemetryTracingEnabledField(false)
 	te := testutil.NewTestExporter()
 	t.Cleanup(func() {
-		OpenTelemetryTracingEnabled = old
+		SetOpenTelemetryTracingEnabledField(old)
 		te.Unregister()
 	})
 
@@ -95,12 +95,12 @@ func TestStartSpan_OpenCensus(t *testing.T) {
 }
 
 func TestStartSpan_OpenTelemetry(t *testing.T) {
-	old := OpenTelemetryTracingEnabled
-	OpenTelemetryTracingEnabled = true
+	old := IsOpenTelemetryTracingEnabled()
+	SetOpenTelemetryTracingEnabledField(true)
 	ctx := context.Background()
 	te := testutil.NewOpenTelemetryTestExporter()
 	t.Cleanup(func() {
-		OpenTelemetryTracingEnabled = old
+		SetOpenTelemetryTracingEnabledField(old)
 		te.Unregister(ctx)
 	})
 
diff --git a/spanner/test/opentelemetry/test/ot_traces_test.go b/spanner/test/opentelemetry/test/ot_traces_test.go
index 23d267a0bca4..c2a4ed942f98 100644
--- a/spanner/test/opentelemetry/test/ot_traces_test.go
+++ b/spanner/test/opentelemetry/test/ot_traces_test.go
@@ -33,11 +33,11 @@ import (
 func TestSpannerTracesWithOpenTelemetry(t *testing.T) {
 	ctx := context.Background()
 	te := newOpenTelemetryTestExporter(false, true)
-	old := trace.OpenTelemetryTracingEnabled
-	trace.OpenTelemetryTracingEnabled = true
+	old := trace.IsOpenTelemetryTracingEnabled()
+	trace.SetOpenTelemetryTracingEnabledField(true)
 
 	t.Cleanup(func() {
-		trace.OpenTelemetryTracingEnabled = old
+		trace.SetOpenTelemetryTracingEnabledField(old)
 		te.Unregister(ctx)
 	})