Disable Zipkin server if port/address is not configured (#2554)

In the past Zipkin server would not be started if the port was not configured, but not it's always started.

* run OTEL tests as part of `test-ci` (without coverage)
* change CollectorZipkinHTTPHostPort default to ""
* add logging when Zipkin server is not started
* changed `ports.GetAddressFromCLIOptions()` to return empty string when `port=0 && hostPort=""` (previously it was returning `":"`, which didn't make sense, it's better to return default value for string when no parts of the address are specified)

Makefile

@@ -391,7 +391,9 @@ install-tools:
 install-ci: install-tools
 
 .PHONY: test-ci
-test-ci: build-examples lint cover
+# TODO (ys) added test-otel to at least ensure tests run in CI,
+#      but this needs to be changed in the lint and cover targets instead
+test-ci: build-examples lint cover test-otel
 
 .PHONY: thrift
 thrift: idl/thrift/jaeger.thrift thrift-image

cmd/collector/app/builder_flags.go

@@ -101,7 +101,7 @@ func AddOTELJaegerFlags(flags *flag.FlagSet) {
 
 // AddOTELZipkinFlags adds flag that are exposed by OTEL Zipkin receiver
 func AddOTELZipkinFlags(flags *flag.FlagSet) {
-	flags.String(CollectorZipkinHTTPHostPort, ports.PortToHostPort(0), "The host:port (e.g. 127.0.0.1:9411 or :9411) of the collector's Zipkin server")
+	flags.String(CollectorZipkinHTTPHostPort, "", "The host:port (e.g. 127.0.0.1:9411 or :9411) of the collector's Zipkin server (disabled by default)")
 }
 
 // InitFromViper initializes CollectorOptions with properties from viper

cmd/collector/app/server/zipkin.go

@@ -42,6 +42,7 @@ type ZipkinServerParams struct {
 // StartZipkinServer based on the given parameters
 func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) {
 	if params.HostPort == "" {
+		params.Logger.Info("Not listening for Zipkin HTTP traffic, port not configured")
 		return nil, nil
 	}
 

cmd/opentelemetry/app/defaultconfig/default_config.go

@@ -39,7 +39,6 @@ import (
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry/app/exporter/memoryexporter"
 	jaegerresource "github.com/jaegertracing/jaeger/cmd/opentelemetry/app/processor/resourceprocessor"
 	"github.com/jaegertracing/jaeger/cmd/opentelemetry/app/receiver/kafkareceiver"
-	"github.com/jaegertracing/jaeger/ports"
 )
 
 const (
@@ -164,7 +163,7 @@ func createReceivers(component ComponentType, factories component.Factories) con
 		"otlp":   factories.Receivers["otlp"].CreateDefaultConfig(),
 	}
 	zipkin := factories.Receivers["zipkin"].CreateDefaultConfig().(*zipkinreceiver.Config)
-	if zipkin.Endpoint != "" && zipkin.Endpoint != ports.PortToHostPort(0) {
+	if zipkin.Endpoint != "" {
 		recvs["zipkin"] = zipkin
 	}
 	return recvs

cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go

@@ -29,7 +29,6 @@ import (
 	"go.opentelemetry.io/collector/receiver/zipkinreceiver"
 
 	jConfig "github.com/jaegertracing/jaeger/pkg/config"
-	"github.com/jaegertracing/jaeger/ports"
 )
 
 func TestDefaultValues(t *testing.T) {
@@ -39,7 +38,7 @@ func TestDefaultValues(t *testing.T) {
 
 	factory := &Factory{Viper: v, Wrapped: zipkinreceiver.NewFactory()}
 	cfg := factory.CreateDefaultConfig().(*zipkinreceiver.Config)
-	assert.Equal(t, ports.PortToHostPort(0), cfg.Endpoint)
+	assert.Equal(t, "", cfg.Endpoint)
 }
 
 func TestLoadConfigAndFlags(t *testing.T) {

ports/ports.go

@@ -60,6 +60,10 @@ func GetAddressFromCLIOptions(port int, hostPort string) string {
 		return PortToHostPort(port)
 	}
 
+	if hostPort == "" {
+		return ""
+	}
+
 	if strings.Contains(hostPort, ":") {
 		return hostPort
 	}

ports/ports_test.go

@@ -15,6 +15,7 @@
 package ports
 
 import (
+	"fmt"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -25,17 +26,21 @@ func TestPortToHostPort(t *testing.T) {
 }
 
 func TestGetAddressFromCLIOptionsLegacy(t *testing.T) {
-	assert.Equal(t, ":123", GetAddressFromCLIOptions(123, ""))
-}
-
-func TestGetAddressFromCLIOptionHostPort(t *testing.T) {
-	assert.Equal(t, "127.0.0.1:123", GetAddressFromCLIOptions(0, "127.0.0.1:123"))
-}
-
-func TestGetAddressFromCLIOptionOnlyPort(t *testing.T) {
-	assert.Equal(t, ":123", GetAddressFromCLIOptions(0, ":123"))
-}
-
-func TestGetAddressFromCLIOptionOnlyPortWithoutColon(t *testing.T) {
-	assert.Equal(t, ":123", GetAddressFromCLIOptions(0, "123"))
+	tests := []struct {
+		port     int
+		hostPort string
+		out      string
+	}{
+		{port: 123, hostPort: "", out: ":123"},
+		{port: 0, hostPort: "127.0.0.1:123", out: "127.0.0.1:123"},
+		{port: 0, hostPort: ":123", out: ":123"},
+		{port: 123, hostPort: "567", out: ":123"},
+		{port: 0, hostPort: "123", out: ":123"},
+		{port: 0, hostPort: "", out: ""},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%+v", test), func(t *testing.T) {
+			assert.Equal(t, test.out, GetAddressFromCLIOptions(test.port, test.hostPort))
+		})
+	}
 }