Skip to content

Commit

Permalink
ddtrace/tracer: address race detector warning from TestDefaultHTTPCli…
Browse files Browse the repository at this point in the history
…ent (#1347)

Doing a deep comparision (through assert.Equal/assert.NotEqual) of
http.Clients in these tests can trigger a race detector warning:

	=== FAIL: ddtrace/tracer TestDefaultHTTPClient/socket (0.00s)
	==================
	WARNING: DATA RACE
	Read at 0x000001e0a080 by goroutine 123:
	  [ ... elided ... ]
	  reflect.DeepEqual()
	      /usr/local/Cellar/go/1.18.3/libexec/src/reflect/deepequal.go:237 +0x3f9
	  github.com/stretchr/testify/assert.ObjectsAreEqual()
	      /Users/nick.ripley/go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:65 +0x184
	  github.com/stretchr/testify/assert.NotEqual()
	      /Users/nick.ripley/go/pkg/mod/github.com/stretchr/testify@v1.7.0/assert/assertions.go:694 +0x204
	  gopkg.in/DataDog/dd-trace-go.v1/ddtrace/tracer.TestDefaultHTTPClient.func2()
	      /Users/nick.ripley/dd/dd-trace-go/ddtrace/tracer/option_test.go:433 +0x3a4
	  testing.tRunner()
	      /usr/local/Cellar/go/1.18.3/libexec/src/testing/testing.go:1439 +0x213
	  testing.(*T).Run.func1()
	      /usr/local/Cellar/go/1.18.3/libexec/src/testing/testing.go:1486 +0x47

	Previous write at 0x000001e0a080 by goroutine 19:
	  sync/atomic.AddInt32()
	      /usr/local/Cellar/go/1.18.3/libexec/src/runtime/race_amd64.s:279 +0xb
	  sync/atomic.AddInt32()
	      <autogenerated>:1 +0x1a
	  net/http.(*Transport).removeIdleConn.func1()
	      /usr/local/Cellar/go/1.18.3/libexec/src/net/http/transport.go:1096 +0x39
	  runtime.deferreturn()
	      /usr/local/Cellar/go/1.18.3/libexec/src/runtime/panic.go:436 +0x32
	  net/http.(*persistConn).readLoop.func1()
	      /usr/local/Cellar/go/1.18.3/libexec/src/net/http/transport.go:2062 +0x72
	  runtime.deferreturn()
	      /usr/local/Cellar/go/1.18.3/libexec/src/runtime/panic.go:436 +0x32
	  net/http.(*Transport).dialConn.func5()
	      /usr/local/Cellar/go/1.18.3/libexec/src/net/http/transport.go:1750 +0x39

This is a data race but not actually a bug in the tracer library, just a
bug in the tests. We can use assert.Same/assert.NotSame to compare the
*http.Clients as pointers, since the tests are just checking whether or
not the default client is returned.
  • Loading branch information
nsrip-dd committed Jun 18, 2022
1 parent f1b2983 commit 0870477
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions ddtrace/tracer/option_test.go
Expand Up @@ -416,7 +416,10 @@ func TestTracerOptionsDefaults(t *testing.T) {

func TestDefaultHTTPClient(t *testing.T) {
t.Run("no-socket", func(t *testing.T) {
assert.Equal(t, defaultHTTPClient(), defaultClient)
// We care that whether clients are different, but doing a deep
// comparison is overkill and can trigger the race detector, so
// just compare the pointers.
assert.Same(t, defaultHTTPClient(), defaultClient)
})

t.Run("socket", func(t *testing.T) {
Expand All @@ -430,7 +433,7 @@ func TestDefaultHTTPClient(t *testing.T) {
defer os.RemoveAll(f.Name())
defer func(old string) { defaultSocketAPM = old }(defaultSocketAPM)
defaultSocketAPM = f.Name()
assert.NotEqual(t, defaultHTTPClient(), defaultClient)
assert.NotSame(t, defaultHTTPClient(), defaultClient)
})
}

Expand Down

0 comments on commit 0870477

Please sign in to comment.