From 0080f2c0fbe0773400c9a6a3f576d0c677bf3f22 Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Wed, 3 Jan 2024 15:55:22 -0800 Subject: [PATCH 1/3] [chore][config/configgrpc] Enable goleak Enables goleak to run on the configgrpc package. Requires ignoring the opencensus-go leak. --- config/configgrpc/configgrpc_test.go | 7 ++++++- config/configgrpc/package_test.go | 17 +++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 config/configgrpc/package_test.go diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index d881f71c89d..da92c5a6ddb 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -641,6 +641,7 @@ func TestHttpReception(t *testing.T) { assert.NoError(t, errResp) assert.NotNil(t, resp) } + assert.NoError(t, grpcClientConn.Close()) cancelFunc() s.Stop() }) @@ -685,6 +686,7 @@ func TestReceiveOnUnixDomainSocket(t *testing.T) { resp, errResp := c.Export(ctx, ptraceotlp.NewExportRequest(), grpc.WaitForReady(true)) assert.NoError(t, errResp) assert.NotNil(t, resp) + assert.NoError(t, grpcClientConn.Close()) cancelFunc() srv.Stop() } @@ -848,6 +850,7 @@ func TestClientInfoInterceptors(t *testing.T) { t.Run(tC.desc, func(t *testing.T) { mock := &grpcTraceServer{} var l net.Listener + var grpcClientConn *grpc.ClientConn // prepare the server { @@ -886,7 +889,8 @@ func TestClientInfoInterceptors(t *testing.T) { require.NoError(t, tt.Shutdown(context.Background())) }() - grpcClientConn, errClient := gcs.ToClientConn(context.Background(), componenttest.NewNopHost(), tt.TelemetrySettings) + var errClient error + grpcClientConn, errClient = gcs.ToClientConn(context.Background(), componenttest.NewNopHost(), tt.TelemetrySettings) require.NoError(t, errClient) cl := ptraceotlp.NewGRPCClient(grpcClientConn) @@ -902,6 +906,7 @@ func TestClientInfoInterceptors(t *testing.T) { // the client address is something like 127.0.0.1:41086 assert.Contains(t, cl.Addr.String(), "127.0.0.1") + assert.NoError(t, grpcClientConn.Close()) }) } } diff --git a/config/configgrpc/package_test.go b/config/configgrpc/package_test.go new file mode 100644 index 00000000000..519657748da --- /dev/null +++ b/config/configgrpc/package_test.go @@ -0,0 +1,17 @@ +// Copyright The OpenTelemetry Authors +// SPDX-License-Identifier: Apache-2.0 + +package configgrpc + +import ( + "testing" + + "go.uber.org/goleak" +) + +// The IgnoreTopFunction call prevents catching the leak generated by opencensus +// defaultWorker.Start which at this time is part of the package's init call. +// See https://github.com/open-telemetry/opentelemetry-collector/issues/9165#issuecomment-1874836336 for more context. +func TestMain(m *testing.M) { + goleak.VerifyTestMain(m, goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start")) +} From 8026868465c6e6f60ae5c292424447945b3c7967 Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Wed, 3 Jan 2024 16:10:45 -0800 Subject: [PATCH 2/3] make gotidy --- config/configgrpc/go.mod | 1 + config/configgrpc/go.sum | 1 + 2 files changed, 2 insertions(+) diff --git a/config/configgrpc/go.mod b/config/configgrpc/go.mod index 261a07f8bbf..64d973258e6 100644 --- a/config/configgrpc/go.mod +++ b/config/configgrpc/go.mod @@ -17,6 +17,7 @@ require ( go.opentelemetry.io/collector/pdata v1.0.0 go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.46.1 go.opentelemetry.io/otel v1.21.0 + go.uber.org/goleak v1.3.0 go.uber.org/zap v1.26.0 google.golang.org/grpc v1.60.1 ) diff --git a/config/configgrpc/go.sum b/config/configgrpc/go.sum index 89c95888bf4..a1f1604c5ba 100644 --- a/config/configgrpc/go.sum +++ b/config/configgrpc/go.sum @@ -292,6 +292,7 @@ go.opentelemetry.io/otel/sdk/metric v1.21.0/go.mod h1:FJ8RAsoPGv/wYMgBdUJXOm+6pz go.opentelemetry.io/otel/trace v1.21.0 h1:WD9i5gzvoUPuXIXH24ZNBudiarZDKuekPqi/E8fpfLc= go.opentelemetry.io/otel/trace v1.21.0/go.mod h1:LGbsEB0f9LGjN+OZaQQ26sohbOmiMR+BaslueVtS/qQ= go.uber.org/goleak v1.3.0 h1:2K3zAYmnTNqV73imy9J1T3WC+gmCePx2hEGkimedGto= +go.uber.org/goleak v1.3.0/go.mod h1:CoHD4mav9JJNrW/WLlf7HGZPjdw8EucARQHekz1X6bE= go.uber.org/multierr v1.11.0 h1:blXXJkSxSSfBVBlC76pxqeO+LN3aDfLQo+309xJstO0= go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN80Y= go.uber.org/zap v1.26.0 h1:sI7k6L95XOKS281NhVKOFCUNIvv9e0w4BF8N3u+tCRo= From 4783b9c650ec88c6926fd0f11c3dcf698393fd74 Mon Sep 17 00:00:00 2001 From: Curtis Robert Date: Thu, 4 Jan 2024 09:10:23 -0800 Subject: [PATCH 3/3] Defer close calls to have them next to start for clarity --- config/configgrpc/configgrpc_test.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/config/configgrpc/configgrpc_test.go b/config/configgrpc/configgrpc_test.go index da92c5a6ddb..7539b2c2eb3 100644 --- a/config/configgrpc/configgrpc_test.go +++ b/config/configgrpc/configgrpc_test.go @@ -632,6 +632,7 @@ func TestHttpReception(t *testing.T) { } grpcClientConn, errClient := gcs.ToClientConn(context.Background(), componenttest.NewNopHost(), tt.TelemetrySettings) assert.NoError(t, errClient) + defer func() { assert.NoError(t, grpcClientConn.Close()) }() c := ptraceotlp.NewGRPCClient(grpcClientConn) ctx, cancelFunc := context.WithTimeout(context.Background(), 2*time.Second) resp, errResp := c.Export(ctx, ptraceotlp.NewExportRequest(), grpc.WaitForReady(true)) @@ -641,7 +642,6 @@ func TestHttpReception(t *testing.T) { assert.NoError(t, errResp) assert.NotNil(t, resp) } - assert.NoError(t, grpcClientConn.Close()) cancelFunc() s.Stop() }) @@ -681,12 +681,12 @@ func TestReceiveOnUnixDomainSocket(t *testing.T) { } grpcClientConn, errClient := gcs.ToClientConn(context.Background(), componenttest.NewNopHost(), tt.TelemetrySettings) assert.NoError(t, errClient) + defer func() { assert.NoError(t, grpcClientConn.Close()) }() c := ptraceotlp.NewGRPCClient(grpcClientConn) ctx, cancelFunc := context.WithTimeout(context.Background(), 2*time.Second) resp, errResp := c.Export(ctx, ptraceotlp.NewExportRequest(), grpc.WaitForReady(true)) assert.NoError(t, errResp) assert.NotNil(t, resp) - assert.NoError(t, grpcClientConn.Close()) cancelFunc() srv.Stop() } @@ -850,7 +850,6 @@ func TestClientInfoInterceptors(t *testing.T) { t.Run(tC.desc, func(t *testing.T) { mock := &grpcTraceServer{} var l net.Listener - var grpcClientConn *grpc.ClientConn // prepare the server { @@ -889,9 +888,9 @@ func TestClientInfoInterceptors(t *testing.T) { require.NoError(t, tt.Shutdown(context.Background())) }() - var errClient error - grpcClientConn, errClient = gcs.ToClientConn(context.Background(), componenttest.NewNopHost(), tt.TelemetrySettings) + grpcClientConn, errClient := gcs.ToClientConn(context.Background(), componenttest.NewNopHost(), tt.TelemetrySettings) require.NoError(t, errClient) + defer func() { assert.NoError(t, grpcClientConn.Close()) }() cl := ptraceotlp.NewGRPCClient(grpcClientConn) ctx, cancelFunc := context.WithTimeout(context.Background(), 2*time.Second) @@ -906,7 +905,6 @@ func TestClientInfoInterceptors(t *testing.T) { // the client address is something like 127.0.0.1:41086 assert.Contains(t, cl.Addr.String(), "127.0.0.1") - assert.NoError(t, grpcClientConn.Close()) }) } }