From 4731426c620c704ccb3fa0d7861597fcfa87bdad Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 12 Oct 2020 12:31:18 -0400 Subject: [PATCH 1/5] Do not start zipkin server without explicitly provided port Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/zipkin.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/collector/app/server/zipkin.go b/cmd/collector/app/server/zipkin.go index b4fca5afe5a..aeeb283f2ab 100644 --- a/cmd/collector/app/server/zipkin.go +++ b/cmd/collector/app/server/zipkin.go @@ -41,7 +41,7 @@ type ZipkinServerParams struct { // StartZipkinServer based on the given parameters func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) { - if params.HostPort == "" { + if params.HostPort == "" || params.HostPort == ":" { return nil, nil } From a8c89179413457653f98d7e751e487e8ac62bbcd Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 12 Oct 2020 12:35:44 -0400 Subject: [PATCH 2/5] finish it Signed-off-by: Yuri Shkuro --- cmd/collector/app/builder_flags.go | 2 +- cmd/collector/app/server/zipkin.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/collector/app/builder_flags.go b/cmd/collector/app/builder_flags.go index 1ac9d0cef39..839329812cf 100644 --- a/cmd/collector/app/builder_flags.go +++ b/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 diff --git a/cmd/collector/app/server/zipkin.go b/cmd/collector/app/server/zipkin.go index aeeb283f2ab..7c2bbf9c4e2 100644 --- a/cmd/collector/app/server/zipkin.go +++ b/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.HostPort == ":" { + params.Logger.Info("Not listening for Zipkin HTTP traffic, port not configured") return nil, nil } From feb9416b36aec1bbb7b16aeaa1da4947d8cf8a6f Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 12 Oct 2020 14:37:35 -0400 Subject: [PATCH 3/5] Test otel in CI Signed-off-by: Yuri Shkuro --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 2b79ad65707..e641b6d0e85 100644 --- a/Makefile +++ b/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 From 9e774003d49d0a5ea09f6a16d45906245666d2ff Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 12 Oct 2020 15:32:52 -0400 Subject: [PATCH 4/5] Clean-up Signed-off-by: Yuri Shkuro --- cmd/collector/app/server/zipkin.go | 2 +- .../zipkinreceiver/zipkin_receiver_test.go | 3 +- ports/ports.go | 4 +++ ports/ports_test.go | 31 +++++++++++-------- 4 files changed, 24 insertions(+), 16 deletions(-) diff --git a/cmd/collector/app/server/zipkin.go b/cmd/collector/app/server/zipkin.go index 7c2bbf9c4e2..d9006d0b6cf 100644 --- a/cmd/collector/app/server/zipkin.go +++ b/cmd/collector/app/server/zipkin.go @@ -41,7 +41,7 @@ type ZipkinServerParams struct { // StartZipkinServer based on the given parameters func StartZipkinServer(params *ZipkinServerParams) (*http.Server, error) { - if params.HostPort == "" || params.HostPort == ":" { + if params.HostPort == "" { params.Logger.Info("Not listening for Zipkin HTTP traffic, port not configured") return nil, nil } diff --git a/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go b/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go index 52a11b086bb..145333f7a3c 100644 --- a/cmd/opentelemetry/app/receiver/zipkinreceiver/zipkin_receiver_test.go +++ b/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) { diff --git a/ports/ports.go b/ports/ports.go index 95cbee755c9..449ed9a918d 100644 --- a/ports/ports.go +++ b/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 } diff --git a/ports/ports_test.go b/ports/ports_test.go index 51385afec03..2072b2bc19c 100644 --- a/ports/ports_test.go +++ b/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)) + }) + } } From aa4ffc4f781594f8a2659b3d8388aae7090c5239 Mon Sep 17 00:00:00 2001 From: Yuri Shkuro Date: Mon, 12 Oct 2020 22:01:17 -0400 Subject: [PATCH 5/5] remove extra condition Signed-off-by: Yuri Shkuro --- cmd/opentelemetry/app/defaultconfig/default_config.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/opentelemetry/app/defaultconfig/default_config.go b/cmd/opentelemetry/app/defaultconfig/default_config.go index b849066fa33..9f719b355ca 100644 --- a/cmd/opentelemetry/app/defaultconfig/default_config.go +++ b/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