Breaking config change: Update sapm and signalfx to use HttpServerConfig

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+## 🛑 Breaking changes 🛑
+
+- TLS config changed for `sapmreceiver` (#488) and `signalfxreceiver` receivers (#488)
+
 ## v0.6.0
 
 # 🎉 OpenTelemetry Collector Contrib v0.6.0 (Beta) 🎉

receiver/sapmreceiver/README.md

@@ -14,7 +14,7 @@ The following settings are required:
 
 The following setting are optional:
 
-* `tls` (no default): This is an optional object used to specify if TLS should
+* `tls_settings` (no default): This is an optional object used to specify if TLS should
   be used for incoming connections.
     * `cert_file`: Specifies the certificate file to use for TLS connection.
       Note: Both `key_file` and `cert_file` are required for TLS connection. 

receiver/sapmreceiver/config.go

@@ -15,23 +15,16 @@
 package sapmreceiver
 
 import (
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configmodels"
-	"go.opentelemetry.io/collector/config/configtls"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/splunk"
 )
 
 // Config defines configuration for SAPM receiver.
 type Config struct {
 	configmodels.ReceiverSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
-
-	// TODO: Use one of the configs from core.
-	// The target endpoint.
-	Endpoint string `mapstructure:"endpoint"`
-
-	// Configures the receiver to use TLS.
-	// The default value is nil, which will cause the receiver to not use TLS.
-	TLSCredentials *configtls.TLSSetting `mapstructure:"tls, omitempty"`
+	confighttp.HTTPServerSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
 
 	splunk.AccessTokenPassthroughConfig `mapstructure:",squash"`
 }

receiver/sapmreceiver/config_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configmodels"
 	"go.opentelemetry.io/collector/config/configtest"
 	"go.opentelemetry.io/collector/config/configtls"
@@ -53,7 +54,9 @@ func TestLoadConfig(t *testing.T) {
 				TypeVal: typeStr,
 				NameVal: "sapm/customname",
 			},
-			Endpoint: "0.0.0.0:7276",
+			HTTPServerSettings: confighttp.HTTPServerSettings{
+				Endpoint: "0.0.0.0:7276",
+			},
 		})
 
 	r2 := cfg.Receivers["sapm/tls"].(*Config)
@@ -63,10 +66,14 @@ func TestLoadConfig(t *testing.T) {
 				TypeVal: typeStr,
 				NameVal: "sapm/tls",
 			},
-			Endpoint: ":7276",
-			TLSCredentials: &configtls.TLSSetting{
-				CertFile: "/test.crt",
-				KeyFile:  "/test.key",
+			HTTPServerSettings: confighttp.HTTPServerSettings{
+				Endpoint: ":7276",
+				TLSSetting: &configtls.TLSServerSetting{
+					TLSSetting: configtls.TLSSetting{
+						CertFile: "/test.crt",
+						KeyFile:  "/test.key",
+					},
+				},
 			},
 		})
 
@@ -77,7 +84,9 @@ func TestLoadConfig(t *testing.T) {
 				TypeVal: typeStr,
 				NameVal: "sapm/passthrough",
 			},
-			Endpoint: ":7276",
+			HTTPServerSettings: confighttp.HTTPServerSettings{
+				Endpoint: ":7276",
+			},
 			AccessTokenPassthroughConfig: splunk.AccessTokenPassthroughConfig{
 				AccessTokenPassthrough: true,
 			},

receiver/sapmreceiver/factory.go

@@ -23,6 +23,7 @@ import (
 	"strconv"
 
 	"go.opentelemetry.io/collector/component"
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configmodels"
 	"go.opentelemetry.io/collector/consumer"
 	"go.opentelemetry.io/collector/receiver/receiverhelper"
@@ -50,7 +51,9 @@ func createDefaultConfig() configmodels.Receiver {
 			TypeVal: typeStr,
 			NameVal: typeStr,
 		},
-		Endpoint: defaultEndpoint,
+		HTTPServerSettings: confighttp.HTTPServerSettings{
+			Endpoint: defaultEndpoint,
+		},
 	}
 }
 

receiver/sapmreceiver/testdata/config.yaml

@@ -11,7 +11,7 @@ receivers:
 
   # The following demonstrates how to specify TLS for the receiver.
   sapm/tls:
-    tls:
+    tls_settings:
       cert_file: /test.crt
       key_file: /test.key
 

receiver/sapmreceiver/trace_receiver.go

@@ -71,7 +71,7 @@ func (sr *sapmReceiver) handleRequest(ctx context.Context, req *http.Request) er
 	}
 
 	transport := "http"
-	if sr.config.TLSCredentials != nil {
+	if sr.config.TLSSetting != nil {
 		transport = "https"
 	}
 	ctx = obsreport.ReceiverContext(ctx, sr.config.Name(), transport, "")
@@ -173,9 +173,9 @@ func (sr *sapmReceiver) Start(_ context.Context, host component.Host) error {
 		var ln net.Listener
 
 		// set up the listener
-		ln, err = net.Listen("tcp", sr.config.Endpoint)
+		ln, err = sr.config.HTTPServerSettings.ToListener()
 		if err != nil {
-			err = fmt.Errorf("failed to bind to address %s: %v", sr.config.Endpoint, err)
+			err = fmt.Errorf("failed to bind to address %s: %w", sr.config.Endpoint, err)
 			return
 		}
 
@@ -184,14 +184,12 @@ func (sr *sapmReceiver) Start(_ context.Context, host component.Host) error {
 		nr.HandleFunc(sapmprotocol.TraceEndpointV2, sr.HTTPHandlerFunc)
 
 		// create a server with the handler
-		sr.server = &http.Server{Handler: nr}
+		sr.server = sr.config.HTTPServerSettings.ToServer(nr)
 
 		// run the server on a routine
 		go func() {
-			if sr.config.TLSCredentials != nil {
-				host.ReportFatalError(sr.server.ServeTLS(ln, sr.config.TLSCredentials.CertFile, sr.config.TLSCredentials.KeyFile))
-			} else {
-				host.ReportFatalError(sr.server.Serve(ln))
+			if errHTTP := sr.server.Serve(ln); errHTTP != nil {
+				host.ReportFatalError(errHTTP)
 			}
 		}()
 	})

receiver/sapmreceiver/trace_receiver_test.go

@@ -36,6 +36,7 @@ import (
 	"go.opencensus.io/trace"
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configtls"
 	"go.opentelemetry.io/collector/consumer/pdata"
 	"go.opentelemetry.io/collector/exporter/exportertest"
@@ -252,7 +253,9 @@ func TestReception(t *testing.T) {
 			args: args{
 				// 1. Create the SAPM receiver aka "server"
 				config: &Config{
-					Endpoint: defaultEndpoint,
+					HTTPServerSettings: confighttp.HTTPServerSettings{
+						Endpoint: defaultEndpoint,
+					},
 				},
 				sapm:   &splunksapm.PostSpansRequest{Batches: []*model.Batch{grpcFixture(now, time.Minute*10, time.Second*2)}},
 				zipped: false,
@@ -264,7 +267,9 @@ func TestReception(t *testing.T) {
 			name: "receive compressed sapm",
 			args: args{
 				config: &Config{
-					Endpoint: defaultEndpoint,
+					HTTPServerSettings: confighttp.HTTPServerSettings{
+						Endpoint: defaultEndpoint,
+					},
 				},
 				sapm:   &splunksapm.PostSpansRequest{Batches: []*model.Batch{grpcFixture(now, time.Minute*10, time.Second*2)}},
 				zipped: true,
@@ -276,10 +281,14 @@ func TestReception(t *testing.T) {
 			name: "connect via TLS compressed sapm",
 			args: args{
 				config: &Config{
-					Endpoint: tlsAddress,
-					TLSCredentials: &configtls.TLSSetting{
-						CertFile: ("./testdata/testcert.crt"),
-						KeyFile:  ("./testdata/testkey.key"),
+					HTTPServerSettings: confighttp.HTTPServerSettings{
+						Endpoint: tlsAddress,
+						TLSSetting: &configtls.TLSServerSetting{
+							TLSSetting: configtls.TLSSetting{
+								CertFile: "./testdata/testcert.crt",
+								KeyFile:  "./testdata/testkey.key",
+							},
+						},
 					},
 				},
 				sapm:   &splunksapm.PostSpansRequest{Batches: []*model.Batch{grpcFixture(now, time.Minute*10, time.Second*2)}},
@@ -344,7 +353,9 @@ func TestAccessTokenPassthrough(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			config := &Config{
-				Endpoint: defaultEndpoint,
+				HTTPServerSettings: confighttp.HTTPServerSettings{
+					Endpoint: defaultEndpoint,
+				},
 				AccessTokenPassthroughConfig: splunk.AccessTokenPassthroughConfig{
 					AccessTokenPassthrough: tt.accessTokenPassthrough,
 				},

receiver/signalfxreceiver/README.md

@@ -20,7 +20,7 @@ The following settings are optional:
   tandem with identical configuration option for [SignalFx
   exporter](../../exporter/signalfxexporter/README.md) to preserve datapoint
   origin.
-* `tls` (no default): This is an optional object used to specify if TLS should be used for
+* `tls_settings` (no default): This is an optional object used to specify if TLS should be used for
   incoming connections.
     * `cert_file`: Specifies the certificate file to use for TLS connection.
       Note: Both `key_file` and `cert_file` are required for TLS connection.

receiver/signalfxreceiver/config.go

@@ -15,23 +15,16 @@
 package signalfxreceiver
 
 import (
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configmodels"
-	"go.opentelemetry.io/collector/config/configtls"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/splunk"
 )
 
 // Config defines configuration for the SignalFx receiver.
 type Config struct {
 	configmodels.ReceiverSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
-
-	// TODO: Use one of the configs from core.
-	// The target endpoint.
-	Endpoint string `mapstructure:"endpoint"`
-
-	// Configures the receiver to use TLS.
-	// The default value is nil, which will cause the receiver to not use TLS.
-	TLSCredentials *configtls.TLSSetting `mapstructure:"tls, omitempty"`
+	confighttp.HTTPServerSettings `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
 
 	splunk.AccessTokenPassthroughConfig `mapstructure:",squash"`
 }

receiver/signalfxreceiver/config_test.go

@@ -21,6 +21,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configmodels"
 	"go.opentelemetry.io/collector/config/configtest"
 	"go.opentelemetry.io/collector/config/configtls"
@@ -53,7 +54,9 @@ func TestLoadConfig(t *testing.T) {
 				TypeVal: typeStr,
 				NameVal: "signalfx/allsettings",
 			},
-			Endpoint: "localhost:9943",
+			HTTPServerSettings: confighttp.HTTPServerSettings{
+				Endpoint: "localhost:9943",
+			},
 			AccessTokenPassthroughConfig: splunk.AccessTokenPassthroughConfig{
 				AccessTokenPassthrough: true,
 			},
@@ -66,10 +69,14 @@ func TestLoadConfig(t *testing.T) {
 				TypeVal: typeStr,
 				NameVal: "signalfx/tls",
 			},
-			Endpoint: ":9943",
-			TLSCredentials: &configtls.TLSSetting{
-				CertFile: "/test.crt",
-				KeyFile:  "/test.key",
+			HTTPServerSettings: confighttp.HTTPServerSettings{
+				Endpoint: ":9943",
+				TLSSetting: &configtls.TLSServerSetting{
+					TLSSetting: configtls.TLSSetting{
+						CertFile: "/test.crt",
+						KeyFile:  "/test.key",
+					},
+				},
 			},
 			AccessTokenPassthroughConfig: splunk.AccessTokenPassthroughConfig{
 				AccessTokenPassthrough: false,

receiver/signalfxreceiver/factory.go

@@ -22,6 +22,7 @@ import (
 
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/configerror"
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/config/configmodels"
 	"go.opentelemetry.io/collector/consumer"
 	"go.uber.org/zap"
@@ -60,7 +61,9 @@ func (f *Factory) CreateDefaultConfig() configmodels.Receiver {
 			TypeVal: typeStr,
 			NameVal: typeStr,
 		},
-		Endpoint: defaultEndpoint,
+		HTTPServerSettings: confighttp.HTTPServerSettings{
+			Endpoint: defaultEndpoint,
+		},
 	}
 }
 

receiver/signalfxreceiver/receiver.go

@@ -19,7 +19,9 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"sync"
 	"time"
@@ -105,19 +107,8 @@ func New(
 		logger:       logger,
 		config:       &config,
 		nextConsumer: nextConsumer,
-		server: &http.Server{
-			Addr: config.Endpoint,
-			// TODO: Evaluate what properties should be configurable, for now
-			//		set some hard-coded values.
-			ReadHeaderTimeout: defaultServerTimeout,
-			WriteTimeout:      defaultServerTimeout,
-		},
 	}
 
-	mx := mux.NewRouter()
-	mx.HandleFunc("/v2/datapoint", r.handleReq)
-	r.server.Handler = mx
-
 	return r, nil
 }
 
@@ -132,13 +123,27 @@ func (r *sfxReceiver) Start(_ context.Context, host component.Host) error {
 	r.startOnce.Do(func() {
 		err = nil
 
+		var ln net.Listener
+		// set up the listener
+		ln, err = r.config.HTTPServerSettings.ToListener()
+		if err != nil {
+			err = fmt.Errorf("failed to bind to address %s: %w", r.config.Endpoint, err)
+			return
+		}
+
+		mx := mux.NewRouter()
+		mx.HandleFunc("/v2/datapoint", r.handleReq)
+
+		r.server = r.config.HTTPServerSettings.ToServer(mx)
+
+		// TODO: Evaluate what properties should be configurable, for now
+		//		set some hard-coded values.
+		r.server.ReadHeaderTimeout = defaultServerTimeout
+		r.server.WriteTimeout = defaultServerTimeout
+
 		go func() {
-			if r.config.TLSCredentials != nil {
-				host.ReportFatalError(r.server.ListenAndServeTLS(r.config.TLSCredentials.CertFile, r.config.TLSCredentials.KeyFile))
-			} else {
-				if err = r.server.ListenAndServe(); err != nil && err != http.ErrServerClosed {
-					host.ReportFatalError(err)
-				}
+			if errHTTP := r.server.Serve(ln); errHTTP != nil {
+				host.ReportFatalError(errHTTP)
 			}
 		}()
 	})
@@ -161,7 +166,7 @@ func (r *sfxReceiver) Shutdown(context.Context) error {
 
 func (r *sfxReceiver) handleReq(resp http.ResponseWriter, req *http.Request) {
 	transport := "http"
-	if r.config.TLSCredentials != nil {
+	if r.config.TLSSetting != nil {
 		transport = "https"
 	}
 	ctx := obsreport.ReceiverContext(req.Context(), r.config.Name(), transport, r.config.Name())