feat: fix graceful shutdown, add config validation

config/config.go

@@ -135,3 +135,35 @@ type Config struct {
 	// This allows using existing OpenTracing instrumentation with OpenTelemetry
 	OTLPUseOpenTracingBridge bool `envconfig:"OTLP_USE_OPENTRACING_BRIDGE" default:"true"`
 }
+
+// Validate checks the configuration for common misconfigurations and returns
+// a list of warning messages. It does not return an error to avoid breaking
+// existing services — warnings are meant to be logged at startup.
+func (c Config) Validate() []string {
+	var warnings []string
+
+	if c.GRPCPort < 0 || c.GRPCPort > 65535 {
+		warnings = append(warnings, "GRPCPort is out of valid range (0-65535)")
+	}
+	if c.HTTPPort < 0 || c.HTTPPort > 65535 {
+		warnings = append(warnings, "HTTPPort is out of valid range (0-65535)")
+	}
+	if c.GRPCPort == c.HTTPPort && c.GRPCPort != 0 {
+		warnings = append(warnings, "GRPCPort and HTTPPort are the same, this will cause a port conflict")
+	}
+	if c.NewRelicOpentelemetrySample < 0 || c.NewRelicOpentelemetrySample > 1.0 {
+		warnings = append(warnings, "NewRelicOpentelemetrySample should be between 0.0 and 1.0")
+	}
+	if c.OTLPSamplingRatio < 0 || c.OTLPSamplingRatio > 1.0 {
+		warnings = append(warnings, "OTLPSamplingRatio should be between 0.0 and 1.0")
+	}
+	if (c.GRPCTLSCertFile == "") != (c.GRPCTLSKeyFile == "") {
+		warnings = append(warnings, "GRPCTLSCertFile and GRPCTLSKeyFile must both be set or both be empty")
+	}
+	if c.ShutdownDurationInSeconds > 0 && c.HealthcheckWaitDurationInSeconds > 0 &&
+		c.HealthcheckWaitDurationInSeconds >= c.ShutdownDurationInSeconds {
+		warnings = append(warnings, "HealthcheckWaitDurationInSeconds should be less than ShutdownDurationInSeconds")
+	}
+
+	return warnings
+}

config/config_test.go

@@ -0,0 +1,108 @@
+package config
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestValidateDefaults(t *testing.T) {
+	c := Config{
+		GRPCPort:                        9090,
+		HTTPPort:                        9091,
+		ShutdownDurationInSeconds:       15,
+		HealthcheckWaitDurationInSeconds: 7,
+	}
+	warnings := c.Validate()
+	if len(warnings) > 0 {
+		t.Errorf("default config should have no warnings, got: %v", warnings)
+	}
+}
+
+func TestValidatePortZero(t *testing.T) {
+	// Port 0 means ephemeral — should be valid
+	c := Config{GRPCPort: 0, HTTPPort: 0}
+	warnings := c.Validate()
+	for _, w := range warnings {
+		if strings.Contains(w, "Port") && strings.Contains(w, "range") {
+			t.Errorf("port 0 should be valid, got warning: %s", w)
+		}
+		if strings.Contains(w, "port conflict") {
+			t.Errorf("port 0 should not warn about conflict, got warning: %s", w)
+		}
+	}
+}
+
+func TestValidatePortConflict(t *testing.T) {
+	c := Config{GRPCPort: 8080, HTTPPort: 8080}
+	warnings := c.Validate()
+	found := false
+	for _, w := range warnings {
+		if strings.Contains(w, "port conflict") {
+			found = true
+		}
+	}
+	if !found {
+		t.Error("same non-zero ports should warn about conflict")
+	}
+}
+
+func TestValidateSamplingRatio(t *testing.T) {
+	c := Config{
+		GRPCPort:                    9090,
+		HTTPPort:                    9091,
+		NewRelicOpentelemetrySample: 1.5,
+		OTLPSamplingRatio:          -0.1,
+	}
+	warnings := c.Validate()
+	foundNR := false
+	foundOTLP := false
+	for _, w := range warnings {
+		if strings.Contains(w, "NewRelicOpentelemetrySample") {
+			foundNR = true
+		}
+		if strings.Contains(w, "OTLPSamplingRatio") {
+			foundOTLP = true
+		}
+	}
+	if !foundNR || !foundOTLP {
+		t.Errorf("expected warnings for both sampling fields, got: %v", warnings)
+	}
+}
+
+func TestValidateTLSMismatch(t *testing.T) {
+	c := Config{
+		GRPCPort:        9090,
+		HTTPPort:        9091,
+		GRPCTLSCertFile: "/path/to/cert",
+		// GRPCTLSKeyFile intentionally empty
+	}
+	warnings := c.Validate()
+	found := false
+	for _, w := range warnings {
+		if strings.Contains(w, "TLS") {
+			found = true
+		}
+	}
+	if !found {
+		t.Error("mismatched TLS cert/key should produce a warning")
+	}
+}
+
+func TestValidateShutdownTiming(t *testing.T) {
+	c := Config{
+		GRPCPort:                        9090,
+		HTTPPort:                        9091,
+		ShutdownDurationInSeconds:       5,
+		HealthcheckWaitDurationInSeconds: 10,
+	}
+	warnings := c.Validate()
+	found := false
+	for _, w := range warnings {
+		if strings.Contains(w, "HealthcheckWaitDurationInSeconds") {
+			found = true
+		}
+	}
+	if !found {
+		t.Error("healthcheck duration >= shutdown duration should produce a warning")
+	}
+}

core.go

@@ -439,6 +439,16 @@ func (c *cb) Run() error {
 		errChan <- c.runHTTP(ctx, c.httpServer)
 	}()
 	err = <-errChan
+	// Stop the peer server only on unexpected failures to avoid racing
+	// with an in-progress graceful shutdown.
+	if err != nil && !errors.Is(err, http.ErrServerClosed) && !errors.Is(err, grpc.ErrServerStopped) {
+		if c.grpcServer != nil {
+			c.grpcServer.Stop()
+		}
+		if c.httpServer != nil {
+			c.httpServer.Close()
+		}
+	}
 	c.gracefulWait.Wait() // if graceful shutdown is in progress wait for it to finish
 	c.close()
 	return err
@@ -482,12 +492,9 @@ func (c *cb) Stop(dur time.Duration) error {
 	}
 	log.Info(context.Background(), "msg", "Server shut down started, bye bye")
 	if c.httpServer != nil {
-		go func(ctx context.Context, c *cb) {
-			err := c.httpServer.Shutdown(ctx)
-			if err != nil {
-				log.Error(context.Background(), "msg", "http server shutdown error", "err", err)
-			}
-		}(ctx, c) // shutdown http server gracefully
+		if err := c.httpServer.Shutdown(ctx); err != nil {
+			log.Error(context.Background(), "msg", "http server shutdown error", "err", err)
+		}
 	}
 	if c.grpcServer != nil {
 		timedCall(ctx, c.grpcServer.GracefulStop)
@@ -529,5 +536,11 @@ func New(c config.Config) CB {
 		svc:    make([]CBService, 0),
 	}
 	impl.processConfig()
+	// Log validation warnings after processConfig so the logger is configured
+	if warnings := impl.config.Validate(); len(warnings) > 0 {
+		for _, w := range warnings {
+			log.Warn(context.Background(), "msg", "config validation warning", "warning", w)
+		}
+	}
 	return impl
 }

go.mod

@@ -5,11 +5,11 @@ go 1.25.0
 require (
 	github.com/NYTimes/gziphandler v1.1.1
 	github.com/afex/hystrix-go v0.0.0-20180502004556-fa1af6a1f4f5
-	github.com/go-coldbrew/errors v0.2.1
+	github.com/go-coldbrew/errors v0.2.2
 	github.com/go-coldbrew/hystrixprometheus v0.1.2
-	github.com/go-coldbrew/interceptors v0.1.10
-	github.com/go-coldbrew/log v0.2.5
-	github.com/go-coldbrew/options v0.2.4
+	github.com/go-coldbrew/interceptors v0.1.11
+	github.com/go-coldbrew/log v0.2.6
+	github.com/go-coldbrew/options v0.2.5
 	github.com/go-coldbrew/tracing v0.0.7
 	github.com/golang/protobuf v1.5.4
 	github.com/grpc-ecosystem/go-grpc-middleware v1.4.0
@@ -31,18 +31,25 @@ require (
 )
 
 require (
+	github.com/HdrHistogram/hdrhistogram-go v1.2.0 // indirect
+	github.com/adhocore/gronx v1.19.1 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
 	github.com/cenkalti/backoff/v5 v5.0.3 // indirect
 	github.com/certifi/gocertifi v0.0.0-20210507211836-431795d63e8d // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
+	github.com/dlclark/regexp2 v1.9.0 // indirect
+	github.com/fsnotify/fsnotify v1.9.0 // indirect
 	github.com/getsentry/raven-go v0.2.0 // indirect
 	github.com/go-kit/log v0.2.1 // indirect
 	github.com/go-logfmt/logfmt v0.6.1 // indirect
 	github.com/go-logr/logr v1.4.3 // indirect
 	github.com/go-logr/stdr v1.2.2 // indirect
 	github.com/google/uuid v1.6.0 // indirect
+	github.com/gopherjs/gopherjs v1.20.1 // indirect
+	github.com/gorilla/websocket v1.5.0 // indirect
+	github.com/jtolds/gls v4.20.0+incompatible // indirect
+	github.com/k2io/hookingo v1.0.6 // indirect
 	github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect
-	github.com/newrelic/csec-go-agent v1.6.0 // indirect
 	github.com/newrelic/go-agent/v3/integrations/nrgrpc v1.4.7 // indirect
 	github.com/pkg/errors v0.9.1 // indirect
 	github.com/prometheus/client_model v0.6.2 // indirect
@@ -63,4 +70,5 @@ require (
 	google.golang.org/genproto/googleapis/api v0.0.0-20260319201613-d00831a3d3e7 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20260319201613-d00831a3d3e7 // indirect
 	gopkg.in/airbrake/gobrake.v2 v2.0.9 // indirect
+	gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7 // indirect
 )