pr review

Signed-off-by: odubajDT <[email protected]>

receiver/zipkinreceiver/config.go

@@ -18,6 +18,8 @@ var disallowHTTPDefaultProtocol = featuregate.GlobalRegistry().MustRegister(
 	featuregate.WithRegisterFromVersion("v0.114.0"),
 )
 
+const deprecationConfigMsg = "the inline setting of http server parameters has been deprecated, please use .protocols.http parameter instead."
+
 // Config defines configuration for Zipkin receiver.
 type Config struct {
 	// Configures the receiver server protocol.
@@ -41,9 +43,12 @@ var _ component.Config = (*Config)(nil)
 
 // Validate checks the receiver configuration is valid
 func (cfg *Config) Validate() error {
-	if cfg.isServerConfigDefined() {
+	if isServerConfigDefined(cfg.ServerConfig) {
 		if disallowHTTPDefaultProtocol.IsEnabled() {
-			return fmt.Errorf("the inline setting of http server parameters has been deprecated, please use protocols.http parameter instead")
+			return fmt.Errorf(deprecationConfigMsg)
+		}
+		if isServerConfigDefined(cfg.Protocols.HTTP) {
+			return fmt.Errorf("cannot use .protocols.http together with default server config setup")
 		}
 		cfg.Protocols.HTTP = cfg.ServerConfig
 		cfg.ServerConfig = confighttp.ServerConfig{}
@@ -53,7 +58,7 @@ func (cfg *Config) Validate() error {
 }
 
 // IsServerConfigDefined checks if the ServerConfig is defined by the user
-func (cfg *Config) isServerConfigDefined() bool {
+func isServerConfigDefined(cfg confighttp.ServerConfig) bool {
 	return cfg.Endpoint != "" ||
 		cfg.TLSSetting != nil ||
 		cfg.CORS != nil ||

receiver/zipkinreceiver/config_test.go

@@ -4,6 +4,7 @@
 package zipkinreceiver
 
 import (
+	"fmt"
 	"path/filepath"
 	"testing"
 
@@ -17,7 +18,7 @@ import (
 	"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver/internal/metadata"
 )
 
-func TestLoadConfig(t *testing.T) {
+func TestValidateConfig(t *testing.T) {
 	t.Parallel()
 
 	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml"))
@@ -27,12 +28,8 @@ func TestLoadConfig(t *testing.T) {
 		id             component.ID
 		disallowInline bool
 		expected       component.Config
-		wantErr        bool
+		wantErr        error
 	}{
-		{
-			id:       component.NewID(metadata.Type),
-			expected: createDefaultConfig(),
-		},
 		{
 			id: component.NewIDWithName(metadata.Type, "customname"),
 			expected: &Config{
@@ -47,7 +44,7 @@ func TestLoadConfig(t *testing.T) {
 		{
 			id:             component.NewIDWithName(metadata.Type, "customname"),
 			disallowInline: true,
-			wantErr:        true,
+			wantErr:        fmt.Errorf(deprecationConfigMsg),
 		},
 		{
 			id: component.NewIDWithName(metadata.Type, "protocols"),
@@ -75,26 +72,26 @@ func TestLoadConfig(t *testing.T) {
 		{
 			id: component.NewIDWithName(metadata.Type, "parse_strings"),
 			expected: &Config{
-				Protocols: ProtocolTypes{
-					HTTP: confighttp.ServerConfig{
-						Endpoint: defaultBindEndpoint,
-					},
-				},
 				ParseStringTags: true,
 			},
 		},
 		{
 			id: component.NewIDWithName(metadata.Type, "parse_strings"),
 			expected: &Config{
-				Protocols: ProtocolTypes{
-					HTTP: confighttp.ServerConfig{
-						Endpoint: defaultBindEndpoint,
-					},
-				},
 				ParseStringTags: true,
 			},
 			disallowInline: true,
 		},
+		{
+			id:             component.NewIDWithName(metadata.Type, "deprecated"),
+			disallowInline: true,
+			wantErr:        fmt.Errorf(deprecationConfigMsg),
+		},
+		{
+			id:             component.NewIDWithName(metadata.Type, "deprecated"),
+			disallowInline: false,
+			wantErr:        fmt.Errorf("cannot use .protocols.http together with default server config setup"),
+		},
 	}
 
 	for _, tt := range tests {
@@ -105,6 +102,52 @@ func TestLoadConfig(t *testing.T) {
 					require.NoError(t, featuregate.GlobalRegistry().Set(disallowHTTPDefaultProtocol.ID(), false))
 				})
 			}
+			cfg := &Config{}
+
+			sub, err := cm.Sub(tt.id.String())
+			require.NoError(t, err)
+			require.NoError(t, sub.Unmarshal(cfg))
+
+			if tt.wantErr != nil {
+				assert.Equal(t, tt.wantErr, component.ValidateConfig(cfg))
+			} else {
+				assert.NoError(t, component.ValidateConfig(cfg))
+				assert.Equal(t, tt.expected, cfg)
+			}
+		})
+	}
+}
+
+func TestLoadConfig(t *testing.T) {
+	t.Parallel()
+
+	cm, err := confmaptest.LoadConf(filepath.Join("testdata", "config.yaml"))
+	require.NoError(t, err)
+
+	tests := []struct {
+		id       component.ID
+		expected component.Config
+		wantErr  bool
+	}{
+		{
+			id:       component.NewID(metadata.Type),
+			expected: createDefaultConfig(),
+		},
+		{
+			id: component.NewIDWithName(metadata.Type, "parse_strings"),
+			expected: &Config{
+				Protocols: ProtocolTypes{
+					HTTP: confighttp.ServerConfig{
+						Endpoint: defaultBindEndpoint,
+					},
+				},
+				ParseStringTags: true,
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.id.String(), func(t *testing.T) {
 			factory := NewFactory()
 			cfg := factory.CreateDefaultConfig()
 

receiver/zipkinreceiver/factory.go

@@ -51,5 +51,8 @@ func createTracesReceiver(
 	nextConsumer consumer.Traces,
 ) (receiver.Traces, error) {
 	rCfg := cfg.(*Config)
+	if isServerConfigDefined(rCfg.ServerConfig) {
+		set.Logger.Warn(deprecationConfigMsg)
+	}
 	return newReceiver(rCfg, nextConsumer, set)
 }

receiver/zipkinreceiver/factory_test.go

@@ -4,13 +4,18 @@
 package zipkinreceiver
 
 import (
+	"bytes"
 	"context"
 	"testing"
 
+	"github.com/open-telemetry/opentelemetry-collector-contrib/internal/common/testutil"
 	"github.com/stretchr/testify/assert"
 	"go.opentelemetry.io/collector/component/componenttest"
+	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/consumer/consumertest"
 	"go.opentelemetry.io/collector/receiver/receivertest"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 )
 
 func TestCreateDefaultConfig(t *testing.T) {
@@ -38,3 +43,34 @@ func TestCreateReceiver(t *testing.T) {
 	assert.NoError(t, err, "receiver creation failed")
 	assert.NotNil(t, tReceiver, "receiver creation failed")
 }
+
+func TestCreateReceiverDeprecatedWarning(t *testing.T) {
+	cfg := &Config{
+		ServerConfig: confighttp.ServerConfig{
+			Endpoint: testutil.EndpointForPort(defaultHTTPPort),
+		},
+		ParseStringTags: false,
+	}
+
+	var buffer bytes.Buffer
+	writer := zapcore.AddSync(&buffer)
+
+	encoder := zapcore.NewConsoleEncoder(zap.NewDevelopmentEncoderConfig())
+	core := zapcore.NewCore(encoder, writer, zapcore.DebugLevel)
+	logger := zap.New(core)
+
+	set := receivertest.NewNopSettings()
+	set.Logger = logger
+
+	_, _ = createTracesReceiver(
+		context.Background(),
+		set,
+		cfg,
+		consumertest.NewNop(),
+	)
+
+	logOutput := buffer.String()
+	if !bytes.Contains([]byte(logOutput), []byte(deprecationConfigMsg)) {
+		t.Errorf("Expected log message not found. Got: %s", logOutput)
+	}
+}

receiver/zipkinreceiver/testdata/config.yaml

@@ -7,3 +7,8 @@ zipkin/protocols:
       endpoint: "localhost:8765"
 zipkin/parse_strings:
   parse_string_tags: true
+zipkin/deprecated:
+  endpoint: "localhost:8766"
+  protocols:
+    http:
+      endpoint: "localhost:8765"