Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(internal/testutil): fix race when accessing trace spans #10186

Merged
merged 11 commits into from
May 23, 2024
2 changes: 1 addition & 1 deletion bigquery/oc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestOCTracing(t *testing.T) {
q := client.Query("select *")
q.Run(ctx) // Doesn't matter if we get an error; span should be created either way

if len(te.Spans) == 0 {
if len(te.Spans()) == 0 {
t.Fatalf("Expected some spans to be created, but got %d", 0)
}
}
2 changes: 1 addition & 1 deletion datastore/oc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestOCTracing(t *testing.T) {
t.Fatalf("client.Put: %v", err)
}

if len(te.Spans) == 0 {
if len(te.Spans()) == 0 {
t.Fatalf("Expected some span to be created, but got %d", 0)
}
}
13 changes: 11 additions & 2 deletions internal/testutil/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import (
// TestExporter is a test utility exporter. It should be created with NewtestExporter.
type TestExporter struct {
mu sync.Mutex
Spans []*trace.SpanData
spans []*trace.SpanData

Stats chan *view.Data
Views []*view.View
Expand Down Expand Up @@ -56,7 +56,16 @@ func NewTestExporter(views ...*view.View) *TestExporter {
func (te *TestExporter) ExportSpan(s *trace.SpanData) {
te.mu.Lock()
defer te.mu.Unlock()
te.Spans = append(te.Spans, s)
te.spans = append(te.spans, s)
}

// Spans returns the exported spans.
func (te *TestExporter) Spans() []*trace.SpanData {
te.mu.Lock()
defer te.mu.Unlock()
spans := make([]*trace.SpanData, len(te.spans))
copy(spans, te.spans)
return spans
}

// ExportView exports a view.
Expand Down
2 changes: 1 addition & 1 deletion internal/trace/trace_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func TestStartSpan_OpenCensus(t *testing.T) {
if IsOpenTelemetryTracingEnabled() {
t.Errorf("got true, want false")
}
spans := te.Spans
spans := te.Spans()
if len(spans) != 1 {
t.Fatalf("got %d, want 1", len(spans))
}
Expand Down
21 changes: 9 additions & 12 deletions spanner/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4548,12 +4548,11 @@ func TestClient_DoForEachRow_ShouldNotEndSpanWithIteratorDoneError(t *testing.T)
case <-time.After(1 * time.Second):
t.Fatal("No stats were exported before timeout")
}
// Preferably we would want to lock the TestExporter here, but the mutex TestExporter.mu is not exported, so we
// cannot do that.
if len(te.Spans) == 0 {
spans := te.Spans()
if len(spans) == 0 {
t.Fatal("No spans were exported")
}
s := te.Spans[len(te.Spans)-1].Status
s := spans[len(spans)-1].Status
if s.Code != int32(codes.OK) {
t.Errorf("Span status mismatch\nGot: %v\nWant: %v", s.Code, codes.OK)
}
Expand Down Expand Up @@ -4598,12 +4597,11 @@ func TestClient_DoForEachRow_ShouldEndSpanWithQueryError(t *testing.T) {
case <-time.After(1 * time.Second):
t.Fatal("No stats were exported before timeout")
}
// Preferably we would want to lock the TestExporter here, but the mutex TestExporter.mu is not exported, so we
// cannot do that.
if len(te.Spans) == 0 {
spans := te.Spans()
if len(spans) == 0 {
t.Fatal("No spans were exported")
}
s := te.Spans[len(te.Spans)-1].Status
s := spans[len(spans)-1].Status
if s.Code != int32(codes.InvalidArgument) {
t.Errorf("Span status mismatch\nGot: %v\nWant: %v", s.Code, codes.InvalidArgument)
}
Expand Down Expand Up @@ -5366,12 +5364,11 @@ func checkBatchWriteSpan(t *testing.T, errors []error, code codes.Code) {
case <-time.After(1 * time.Second):
t.Fatal("No stats were exported before timeout")
}
// Preferably we would want to lock the TestExporter here, but the mutex TestExporter.mu is not exported, so we
// cannot do that.
if len(te.Spans) == 0 {
spans := te.Spans()
if len(spans) == 0 {
t.Fatal("No spans were exported")
}
s := te.Spans[len(te.Spans)-1].Status
s := spans[len(spans)-1].Status
if s.Code != int32(code) {
t.Errorf("Span status mismatch\nGot: %v\nWant: %v", s.Code, code)
}
Expand Down
2 changes: 1 addition & 1 deletion storage/integration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5675,7 +5675,7 @@ func TestIntegration_OCTracing(t *testing.T) {
bkt := client.Bucket(bucket)
bkt.Attrs(ctx)

if len(te.Spans) == 0 {
if len(te.Spans()) == 0 {
t.Fatalf("Expected some spans to be created, but got %d", 0)
}
})
Expand Down
Loading