Shutdown everytime http server starts to fail to clean up GRPC server

open-telemetry · Jan 8, 2024 · 31feb8e · 31feb8e
1 parent 60d44f5
commit 31feb8e
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 4 deletions.
diff --git a/receiver/otlpreceiver/factory_test.go b/receiver/otlpreceiver/factory_test.go
@@ -151,7 +151,6 @@ func TestCreateTracesReceiver(t *testing.T) {
 			assert.NoError(t, err)
 			if tt.wantStartErr {
 				assert.Error(t, tr.Start(context.Background(), componenttest.NewNopHost()))
-				assert.NoError(t, tr.Shutdown(context.Background()))
 			} else {
 				assert.NoError(t, tr.Start(context.Background(), componenttest.NewNopHost()))
 				assert.NoError(t, tr.Shutdown(context.Background()))
@@ -261,7 +260,6 @@ func TestCreateMetricReceiver(t *testing.T) {
 			assert.NoError(t, err)
 			if tt.wantStartErr {
 				assert.Error(t, mr.Start(context.Background(), componenttest.NewNopHost()))
-				assert.NoError(t, mr.Shutdown(context.Background()))
 			} else {
 				require.NoError(t, mr.Start(context.Background(), componenttest.NewNopHost()))
 				assert.NoError(t, mr.Shutdown(context.Background()))
@@ -371,7 +369,6 @@ func TestCreateLogReceiver(t *testing.T) {
 			assert.NoError(t, err)
 			if tt.wantStartErr {
 				assert.Error(t, mr.Start(context.Background(), componenttest.NewNopHost()))
-				assert.NoError(t, mr.Shutdown(context.Background()))
 			} else {
 				require.NoError(t, mr.Start(context.Background(), componenttest.NewNopHost()))
 				assert.NoError(t, mr.Shutdown(context.Background()))

diff --git a/receiver/otlpreceiver/otlp.go b/receiver/otlpreceiver/otlp.go
@@ -10,6 +10,7 @@ import (
 	"net/http"
 	"sync"
 
+	"go.uber.org/multierr"
 	"go.uber.org/zap"
 	"google.golang.org/grpc"
 
@@ -168,11 +169,15 @@ func (r *otlpReceiver) startHTTPServer(host component.Host) error {
 
 // Start runs the trace receiver on the gRPC server. Currently
 // it also enables the metrics receiver too.
-func (r *otlpReceiver) Start(_ context.Context, host component.Host) error {
+func (r *otlpReceiver) Start(ctx context.Context, host component.Host) error {
 	if err := r.startGRPCServer(host); err != nil {
 		return err
 	}
 	if err := r.startHTTPServer(host); err != nil {
+		// It's possible that a valid GRPC server configuration was specified,
+		// but an invalid HTTP configuration. If that's the case, the successfully
+		// started GRPC server must be shutdown to ensure no goroutines are leaked.
+		err = multierr.Append(err, r.Shutdown(ctx))
 		return err
 	}