[receiver/zipkin] move global server configuration parameters to a separate parameter

.chloggen/refactor-zipkinreceiver.yaml

@@ -0,0 +1,27 @@
+# Use this changelog template to create an entry for release notes.
+
+# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
+change_type: breaking
+
+# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
+component: zipkinreceiver
+
+# A brief description of the change.  Surround your text with quotes ("") if it needs to start with a backtick (`).
+note: "Move global server configuration parameters to a separate parameter."
+
+# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
+issues: [35730]
+
+# (Optional) One or more lines of additional information to render under the primary note.
+# These lines will be padded with 2 spaces and then inserted directly into the document.
+# Use pipe (|) for multiline entries.
+subtext:
+
+# If your change doesn't affect end users or the exported elements of any package,
+# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
+# Optional: The change log or logs in which this entry should be included.
+# e.g. '[user]' or '[user, api]'
+# Include 'user' if the change is relevant to end users.
+# Include 'api' if there is a change to a library API.
+# Default: '[user]'
+change_logs: []

exporter/logzioexporter/example/config.yaml

@@ -2,7 +2,9 @@ receivers:
   opencensus:
     endpoint: :55678
   zipkin:
-    endpoint: :9411
+    protocols:
+      http:
+        endpoint: :9411
   jaeger:
     protocols:
       thrift_http:

exporter/zipkinexporter/zipkin_test.go

@@ -66,8 +66,10 @@ func TestZipkinExporter_roundtripJSON(t *testing.T) {
 	// Run the Zipkin receiver to "receive spans upload from a client application"
 	addr := testutil.GetAvailableLocalAddress(t)
 	recvCfg := &zipkinreceiver.Config{
-		ServerConfig: confighttp.ServerConfig{
-			Endpoint: addr,
+		Protocols: zipkinreceiver.ProtocolTypes{
+			HTTP: confighttp.ServerConfig{
+				Endpoint: addr,
+			},
 		},
 	}
 	zi, err := zipkinreceiver.NewFactory().CreateTraces(context.Background(), receivertest.NewNopSettings(), recvCfg, zexp)
@@ -314,8 +316,10 @@ func TestZipkinExporter_roundtripProto(t *testing.T) {
 	// Run the Zipkin receiver to "receive spans upload from a client application"
 	addr := testutil.GetAvailableLocalAddress(t)
 	recvCfg := &zipkinreceiver.Config{
-		ServerConfig: confighttp.ServerConfig{
-			Endpoint: addr,
+		Protocols: zipkinreceiver.ProtocolTypes{
+			HTTP: confighttp.ServerConfig{
+				Endpoint: addr,
+			},
 		},
 	}
 	zi, err := zipkinreceiver.NewFactory().CreateTraces(context.Background(), receivertest.NewNopSettings(), recvCfg, zexp)

receiver/zipkinreceiver/README.md

@@ -24,14 +24,23 @@ receiver definitions.
 ```yaml
 receivers:
   zipkin:
+    protocols:
+      http:
+    parse_string_tags: false
 ```
 
-The following settings are configurable:
+Supported parameters:
 
-- `endpoint` (default = localhost:9411): host:port on which the receiver is going to receive data.You can temporarily disable the `component.UseLocalHostAsDefaultHost` feature gate to change this to `0.0.0.0:9411`. This feature gate will be removed in a future release.  For full list of `ServerConfig` refer [here](https://github.com/open-telemetry/opentelemetry-collector/tree/main/config/confighttp).
 - `parse_string_tags` (default = false): if enabled, the receiver will attempt to parse string tags/binary annotations into int/bool/float.
+- `protocols` defines the protocol that will be used to receive data.
 
-## Advanced Configuration
+## HTTP protocol
+
+The following settings are configurable for the `http` protocol:
+
+- `endpoint` (default = localhost:9411): host:port on which the receiver is going to receive data. For full list of `ServerConfig` refer [here](https://github.com/open-telemetry/opentelemetry-collector/tree/main/config/confighttp).
+
+### Advanced Configuration
 
 Several helper files are leveraged to provide additional capabilities automatically:
 

receiver/zipkinreceiver/config.go

@@ -4,22 +4,71 @@
 package zipkinreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver"
 
 import (
+	"fmt"
+
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/confighttp"
+	"go.opentelemetry.io/collector/featuregate"
+)
+
+var disallowHTTPDefaultProtocol = featuregate.GlobalRegistry().MustRegister(
+	"zipkinreceiver.httpDefaultProtocol.disallow",
+	featuregate.StageAlpha,
+	featuregate.WithRegisterDescription("When enabled, usage of the default http configuration is disallowed"),
+	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.
+	//
+	// Deprecated: Parameter exists for historical compatibility
+	// and should not be used. To set the server configurations,
+	// use the Protocols parameter instead.
 	confighttp.ServerConfig `mapstructure:",squash"` // squash ensures fields are correctly decoded in embedded struct
 	// If enabled the zipkin receiver will attempt to parse string tags/binary annotations into int/bool/float.
 	// Disabled by default
 	ParseStringTags bool `mapstructure:"parse_string_tags"`
+	// Protocols define the supported protocols for the receiver
+	Protocols ProtocolTypes `mapstructure:"protocols"`
+}
+
+type ProtocolTypes struct {
+	HTTP confighttp.ServerConfig `mapstructure:"http"`
 }
 
 var _ component.Config = (*Config)(nil)
 
 // Validate checks the receiver configuration is valid
 func (cfg *Config) Validate() error {
+	if isServerConfigDefined(cfg.ServerConfig) {
+		if disallowHTTPDefaultProtocol.IsEnabled() {
+			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{}
+	}
+
 	return nil
 }
+
+// IsServerConfigDefined checks if the ServerConfig is defined by the user
+func isServerConfigDefined(cfg confighttp.ServerConfig) bool {
+	return cfg.Endpoint != "" ||
+		cfg.TLSSetting != nil ||
+		cfg.CORS != nil ||
+		cfg.Auth != nil ||
+		cfg.MaxRequestBodySize != 0 ||
+		cfg.IncludeMetadata ||
+		len(cfg.ResponseHeaders) != 0 ||
+		len(cfg.CompressionAlgorithms) != 0 ||
+		cfg.ReadHeaderTimeout != 0 ||
+		cfg.ReadTimeout != 0 ||
+		cfg.WriteTimeout != 0 ||
+		cfg.IdleTimeout != 0
+}

receiver/zipkinreceiver/config_test.go

@@ -4,6 +4,7 @@
 package zipkinreceiver
 
 import (
+	"fmt"
 	"path/filepath"
 	"testing"
 
@@ -12,38 +13,133 @@ import (
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/confighttp"
 	"go.opentelemetry.io/collector/confmap/confmaptest"
+	"go.opentelemetry.io/collector/featuregate"
 
 	"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"))
 	require.NoError(t, err)
 
 	tests := []struct {
-		id       component.ID
-		expected component.Config
+		id             component.ID
+		disallowInline bool
+		expected       component.Config
+		wantErr        error
 	}{
 		{
-			id:       component.NewID(metadata.Type),
-			expected: createDefaultConfig(),
+			id: component.NewIDWithName(metadata.Type, "customname"),
+			expected: &Config{
+				Protocols: ProtocolTypes{
+					HTTP: confighttp.ServerConfig{
+						Endpoint: "localhost:8765",
+					},
+				},
+				ParseStringTags: false,
+			},
 		},
 		{
-			id: component.NewIDWithName(metadata.Type, "customname"),
+			id:             component.NewIDWithName(metadata.Type, "customname"),
+			disallowInline: true,
+			wantErr:        fmt.Errorf(deprecationConfigMsg),
+		},
+		{
+			id: component.NewIDWithName(metadata.Type, "protocols"),
 			expected: &Config{
-				ServerConfig: confighttp.ServerConfig{
-					Endpoint: "localhost:8765",
+				Protocols: ProtocolTypes{
+					HTTP: confighttp.ServerConfig{
+						Endpoint: "localhost:8765",
+					},
 				},
 				ParseStringTags: false,
 			},
 		},
+		{
+			id: component.NewIDWithName(metadata.Type, "protocols"),
+			expected: &Config{
+				Protocols: ProtocolTypes{
+					HTTP: confighttp.ServerConfig{
+						Endpoint: "localhost:8765",
+					},
+				},
+				ParseStringTags: false,
+			},
+			disallowInline: true,
+		},
+		{
+			id: component.NewIDWithName(metadata.Type, "parse_strings"),
+			expected: &Config{
+				ParseStringTags: true,
+			},
+		},
+		{
+			id: component.NewIDWithName(metadata.Type, "parse_strings"),
+			expected: &Config{
+				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 {
+		t.Run(tt.id.String(), func(t *testing.T) {
+			if tt.disallowInline {
+				require.NoError(t, featuregate.GlobalRegistry().Set(disallowHTTPDefaultProtocol.ID(), true))
+				t.Cleanup(func() {
+					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{
-				ServerConfig: confighttp.ServerConfig{
-					Endpoint: defaultBindEndpoint,
+				Protocols: ProtocolTypes{
+					HTTP: confighttp.ServerConfig{
+						Endpoint: defaultBindEndpoint,
+					},
 				},
 				ParseStringTags: true,
 			},
@@ -59,8 +155,12 @@ func TestLoadConfig(t *testing.T) {
 			require.NoError(t, err)
 			require.NoError(t, sub.Unmarshal(cfg))
 
-			assert.NoError(t, component.ValidateConfig(cfg))
-			assert.Equal(t, tt.expected, cfg)
+			if tt.wantErr {
+				assert.Error(t, component.ValidateConfig(cfg))
+			} else {
+				assert.NoError(t, component.ValidateConfig(cfg))
+				assert.Equal(t, tt.expected, cfg)
+			}
 		})
 	}
 }

receiver/zipkinreceiver/factory.go

@@ -34,8 +34,10 @@ func NewFactory() receiver.Factory {
 // createDefaultConfig creates the default configuration for Zipkin receiver.
 func createDefaultConfig() component.Config {
 	return &Config{
-		ServerConfig: confighttp.ServerConfig{
-			Endpoint: testutil.EndpointForPort(defaultHTTPPort),
+		Protocols: ProtocolTypes{
+			HTTP: confighttp.ServerConfig{
+				Endpoint: testutil.EndpointForPort(defaultHTTPPort),
+			},
 		},
 		ParseStringTags: false,
 	}
@@ -49,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)
 }