[receiver/zipkin] add protocols option

receiver/zipkinreceiver/README.md

@@ -29,9 +29,44 @@ receivers:
 
 The following settings are configurable:
 
-- `endpoint` (default = localhost:9411): host:port on which the receiver is going to receive data.See our [security best practices doc](https://opentelemetry.io/docs/security/config-best-practices/#protect-against-denial-of-service-attacks) to understand how to set the endpoint in different environments.  You can review the [full list of `ServerConfig`](https://github.com/open-telemetry/opentelemetry-collector/tree/main/config/confighttp).
+### Legacy Configuration (Deprecated)
+
+- `endpoint` (default = localhost:9411): host:port on which the receiver is going to receive data. See our [security best practices doc](https://opentelemetry.io/docs/security/config-best-practices/#protect-against-denial-of-service-attacks) to understand how to set the endpoint in different environments. 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.
 
+### Protocol-Based Configuration
+
+Starting from v0.128.0, the zipkin receiver supports a protocol-based configuration format:
+
+```yaml
+receivers:
+  zipkin:
+    protocols:
+      http:
+        endpoint: localhost:9411
+    parse_string_tags: false
+```
+
+Currently, only the HTTP protocol is supported, but this structure allows for future expansion to other protocols (such as UDP). The protocol-based design makes the receiver more extensible and follows the pattern used by other receivers in the OpenTelemetry Collector.
+
+#### Future Protocol Support
+
+The architecture has been designed to easily add support for additional protocols in the future. For example, a UDP protocol could be added with configuration like:
+
+```yaml
+receivers:
+  zipkin:
+    protocols:
+      http:
+        endpoint: localhost:9411
+      # UDP support is not yet implemented, this is just an example
+      # udp:
+      #   endpoint: localhost:9412
+    parse_string_tags: false
+```
+
+The legacy configuration format is still supported for backward compatibility, but it is recommended to use the new protocol-based configuration format for new deployments.
+
 ## Advanced Configuration
 
 Several helper files are leveraged to provide additional capabilities automatically:

receiver/zipkinreceiver/config.go

@@ -4,14 +4,51 @@
 package zipkinreceiver // import "github.com/open-telemetry/opentelemetry-collector-contrib/receiver/zipkinreceiver"
 
 import (
+	"errors"
+	"fmt"
+
 	"go.opentelemetry.io/collector/component"
 	"go.opentelemetry.io/collector/config/confighttp"
+	"go.opentelemetry.io/collector/confmap"
+)
+
+const (
+	protoHTTP = "protocols::http"
+	// Future protocol constants can be added here
+	// protoUDP = "protocols::udp"
 )
 
+// HTTPConfig defines configuration for Zipkin HTTP receiver.
+type HTTPConfig struct {
+	ServerConfig confighttp.ServerConfig `mapstructure:",squash"`
+	// prevent unkeyed literal initialization
+	_ struct{}
+}
+
+// UDPConfig defines configuration for a potential future Zipkin UDP receiver.
+// type UDPConfig struct {
+// 	Endpoint string `mapstructure:"endpoint"`
+// 	// Add other UDP-specific configuration options here
+// }
+
+// Protocols holds the configuration for all supported protocols.
+type Protocols struct {
+	HTTP *HTTPConfig `mapstructure:"http"`
+	// Future protocols can be added here
+	// UDP *UDPConfig `mapstructure:"udp"`
+
+	// prevent unkeyed literal initialization
+	_ struct{}
+}
+
 // Config defines configuration for Zipkin receiver.
 type Config struct {
-	// Configures the receiver server protocol.
+	// Protocols is the configuration for the supported protocols, currently only HTTP.
+	Protocols `mapstructure:"protocols"`
+
+	// Deprecated: [v0.128.0] Use protocols.http 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"`
@@ -24,5 +61,48 @@ var _ component.Config = (*Config)(nil)
 
 // Validate checks the receiver configuration is valid
 func (cfg *Config) Validate() error {
+	// Check if legacy configuration is used
+	if cfg.Endpoint != "" {
+		// Legacy configuration is used, no need to validate protocols
+		return nil
+	}
+
+	// Check if protocols configuration is used
+	if cfg.HTTP == nil {
+		// Currently, only HTTP protocol is supported
+		// When more protocols are added, this validation can be updated
+		// to check if at least one protocol is configured
+		return errors.New("must specify at least one protocol when using protocols configuration")
+	}
+
+	return nil
+}
+
+// Unmarshal is a custom unmarshaler for Config
+func (cfg *Config) Unmarshal(conf *confmap.Conf) error {
+	// First, unmarshal the standard fields
+	err := conf.Unmarshal(cfg)
+	if err != nil {
+		return err
+	}
+
+	// Check if any protocol configuration is used
+	// Currently only checking for HTTP, but can be expanded for future protocols
+	if !conf.IsSet(protoHTTP) {
+		// No protocols section, using legacy configuration
+		cfg.HTTP = &HTTPConfig{
+			ServerConfig: cfg.ServerConfig,
+		}
+		return nil
+	}
+
+	// If protocols section exists, clear the legacy endpoint to avoid confusion
+	if cfg.Endpoint != "" && conf.IsSet(protoHTTP) {
+		// Both legacy and new configuration used, log a warning
+		fmt.Println("Warning: Both legacy endpoint and protocols.http configuration found. Using protocols.http configuration.")
+		// Clear the legacy endpoint
+		cfg.Endpoint = ""
+	}
+
 	return nil
 }

receiver/zipkinreceiver/config_test.go

@@ -37,6 +37,14 @@
 				ServerConfig: confighttp.ServerConfig{
 					Endpoint: "localhost:8765",
 				},
+				// Populate protocols with HTTP config
+				Protocols: Protocols{
+					HTTP: &HTTPConfig{
+						ServerConfig: confighttp.ServerConfig{
+							Endpoint: "localhost:8765",
+						},
+					},
+				},
 				ParseStringTags: false,
 			},
 		},
@@ -46,9 +54,53 @@
 				ServerConfig: confighttp.ServerConfig{
 					Endpoint: defaultHTTPEndpoint,
 				},
+				// Populate protocols with default HTTP config
+				Protocols: Protocols{
+					HTTP: &HTTPConfig{
+						ServerConfig: confighttp.ServerConfig{
+							Endpoint: defaultHTTPEndpoint,
+						},
+					},
+				},
 				ParseStringTags: true,
 			},
 		},
+		{
+			id: component.NewIDWithName(metadata.Type, "protocols_config"),
+			expected: &Config{
+				// For new config format, protocols is populated with HTTP config
+				Protocols: Protocols{
+					HTTP: &HTTPConfig{
+						ServerConfig: confighttp.ServerConfig{
+							Endpoint: "localhost:9411",
+						},
+					},
+				},
+				// When using protocols config, the legacy endpoint should be empty
+				ServerConfig: confighttp.ServerConfig{
+					Endpoint: "",
+				},
+				ParseStringTags: false,
+			},
+		},
+		{
+			id: component.NewIDWithName(metadata.Type, "protocols_with_legacy"),
+			expected: &Config{
+				// We expect protocols to override the legacy endpoint
+				Protocols: Protocols{
+					HTTP: &HTTPConfig{
+						ServerConfig: confighttp.ServerConfig{
+							Endpoint: "localhost:9412",
+						},
+					},
+				},
+				// Legacy endpoint should be cleared when protocols.http is set
+				ServerConfig: confighttp.ServerConfig{
+					Endpoint: "",
+				},
+				ParseStringTags: false,
+			},
+		},
 	}
 
 	for _, tt := range tests {
@@ -61,7 +113,37 @@
 			require.NoError(t, sub.Unmarshal(cfg))
 
 			assert.NoError(t, xconfmap.Validate(cfg))
-			assert.Equal(t, tt.expected, cfg)
+
+			// Instead of comparing the entire config, just check the specific fields we care about
+			actualCfg := cfg.(*Config)
+			expectedCfg := tt.expected.(*Config)
+
+			// Check endpoint
+			assert.Equal(t, expectedCfg.Endpoint, actualCfg.Endpoint)
+
+			// Check parse_string_tags
+			assert.Equal(t, expectedCfg.ParseStringTags, actualCfg.ParseStringTags)
+
+			if tt.id.Name() == "customname" {
+				// We want to prioritise protocols.http over the root ServerConfig
+				// set this field if using legacy config
+				require.NotNil(t, actualCfg.HTTP)
+				assert.Equal(t, "localhost:8765", actualCfg.HTTP.ServerConfig.Endpoint)
+			}
+
+			// For the protocols_config test, check that protocols.http is configured correctly
+			if tt.id.Name() == "protocols_config" {
+				require.NotNil(t, actualCfg.HTTP)
+				assert.Equal(t, "localhost:9411", actualCfg.HTTP.ServerConfig.Endpoint)
+			}
+			// For the protocols_config test, check that protocols.http is configured correctly
+			if tt.id.Name() == "protocols_with_legacy" {
+				// Check that endpoint is set in protocols.http
+				require.NotNil(t, actualCfg.HTTP)
+				assert.Equal(t, "localhost:9412", actualCfg.HTTP.ServerConfig.Endpoint)
+				// Check that legacy endpoint is cleared
+				assert.Empty(t, actualCfg.Endpoint, "Legacy endpoint should be cleared when protocols.http is set")
+			}
 		})
 	}
 }

receiver/zipkinreceiver/factory.go

@@ -31,9 +31,15 @@ func NewFactory() receiver.Factory {
 
 // createDefaultConfig creates the default configuration for Zipkin receiver.
 func createDefaultConfig() component.Config {
+	httpConfig := confighttp.NewDefaultServerConfig()
+	httpConfig.Endpoint = defaultHTTPEndpoint
+	// For backward compatibility, use the legacy configuration format
 	return &Config{
-		ServerConfig: confighttp.ServerConfig{
-			Endpoint: defaultHTTPEndpoint,
+		ServerConfig: httpConfig,
+		Protocols: Protocols{
+			HTTP: &HTTPConfig{
+				ServerConfig: httpConfig,
+			},
 		},
 		ParseStringTags: false,
 	}

receiver/zipkinreceiver/testdata/config.yaml

@@ -3,3 +3,12 @@ zipkin/customname:
   endpoint: "localhost:8765"
 zipkin/parse_strings:
   parse_string_tags: true
+zipkin/protocols_config:
+  protocols:
+    http:
+      endpoint: "localhost:9411"
+zipkin/protocols_with_legacy:
+  endpoint: "localhost:9411"
+  protocols:
+    http:
+      endpoint: "localhost:9412"

receiver/zipkinreceiver/trace_receiver.go

@@ -21,6 +21,7 @@ import (
 	"go.opentelemetry.io/collector/pdata/ptrace"
 	"go.opentelemetry.io/collector/receiver"
 	"go.opentelemetry.io/collector/receiver/receiverhelper"
+	"go.uber.org/zap"
 
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/zipkin/zipkinv1"
 	"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/translator/zipkin/zipkinv2"
@@ -94,17 +95,48 @@ func (zr *zipkinReceiver) Start(ctx context.Context, host component.Host) error
 		return errors.New("nil host")
 	}
 
+	// Always prioritize Protocols.HTTP over root ServerConfig
+	// If Protocols.HTTP is not set, copy the root ServerConfig to it
+	if zr.config.HTTP == nil && zr.config.Endpoint != "" {
+		zr.config.HTTP = &HTTPConfig{
+			ServerConfig: zr.config.ServerConfig,
+		}
+	}
+
+	// Start HTTP server if HTTP protocol is enabled
+	if zr.config.HTTP != nil {
+		if err := zr.startHTTPServer(ctx, host); err != nil {
+			return err
+		}
+	}
+
+	// Future protocols can be added here, for example:
+	// if zr.config.UDP != nil {
+	//     if err := zr.startUDPServer(ctx, host); err != nil {
+	//         return err
+	//     }
+	// }
+
+	return nil
+}
+
+// startHTTPServer initializes and starts the HTTP server for receiving Zipkin spans
+func (zr *zipkinReceiver) startHTTPServer(ctx context.Context, host component.Host) error {
 	var err error
-	zr.server, err = zr.config.ToServer(ctx, host, zr.settings.TelemetrySettings, zr)
+	httpConfig := zr.config.HTTP.ServerConfig
+
+	zr.server, err = httpConfig.ToServer(ctx, host, zr.settings.TelemetrySettings, zr)
 	if err != nil {
 		return err
 	}
 
 	var listener net.Listener
-	listener, err = zr.config.ToListener(ctx)
+	zr.settings.Logger.Info("Starting HTTP server", zap.String("endpoint", httpConfig.Endpoint))
+	listener, err = httpConfig.ToListener(ctx)
 	if err != nil {
 		return err
 	}
+
 	zr.shutdownWG.Add(1)
 	go func() {
 		defer zr.shutdownWG.Done()
@@ -149,12 +181,18 @@ func (zr *zipkinReceiver) v2ToTraceSpans(blob []byte, hdr http.Header) (reqs ptr
 
 // Shutdown tells the receiver that should stop reception,
 // giving it a chance to perform any necessary clean-up and shutting down
-// its HTTP server.
+// its servers.
 func (zr *zipkinReceiver) Shutdown(context.Context) error {
 	var err error
+
+	// Shutdown HTTP server if it was started
 	if zr.server != nil {
 		err = zr.server.Close()
 	}
+
+	// Future protocol shutdown can be added here
+	// For example: if zr.udpServer != nil { zr.udpServer.Close() }
+
 	zr.shutdownWG.Wait()
 	return err
 }