From 31b949a4425aa35e6cec879b053d120fe80a528d Mon Sep 17 00:00:00 2001
From: Joao Grassi <joao@joaograssi.com>
Date: Fri, 6 Aug 2021 09:52:44 +0200
Subject: [PATCH] Change the default port for OLTP HTTP (#373)

* Change the default port for OLTP http
And marking the existing as legacy.

* Fix tests
---
 pkg/collector/parser/receiver_otlp.go      | 22 ++++++++++-----
 pkg/collector/parser/receiver_otlp_test.go | 31 +++++++++++++++++++---
 2 files changed, 42 insertions(+), 11 deletions(-)

diff --git a/pkg/collector/parser/receiver_otlp.go b/pkg/collector/parser/receiver_otlp.go
index 5e46ac345e..45eab11ed7 100644
--- a/pkg/collector/parser/receiver_otlp.go
+++ b/pkg/collector/parser/receiver_otlp.go
@@ -27,8 +27,9 @@ var _ ReceiverParser = &OTLPReceiverParser{}
 const (
 	parserNameOTLP = "__otlp"
 
-	defaultOTLPGRPCPort int32 = 4317
-	defaultOTLPHTTPPort int32 = 55681
+	defaultOTLPGRPCPort       int32 = 4317
+	defaultOTLPHTTPLegacyPort int32 = 55681
+	defaultOTLPHTTPPort       int32 = 4318
 )
 
 // OTLPReceiverParser parses the configuration for OTLP receivers.
@@ -74,11 +75,18 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
 		},
 		{
 			name: "http",
-			defaultPorts: []corev1.ServicePort{{
-				Name:       portName(fmt.Sprintf("%s-http", o.name), defaultOTLPHTTPPort),
-				Port:       defaultOTLPHTTPPort,
-				TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)),
-			}},
+			defaultPorts: []corev1.ServicePort{
+				{
+					Name:       portName(fmt.Sprintf("%s-http", o.name), defaultOTLPHTTPPort),
+					Port:       defaultOTLPHTTPPort,
+					TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)),
+				},
+				{
+					Name:       portName(fmt.Sprintf("%s-http-legacy", o.name), defaultOTLPHTTPLegacyPort),
+					Port:       defaultOTLPHTTPLegacyPort,
+					TargetPort: intstr.FromInt(int(defaultOTLPHTTPPort)), // we target the official port, not the legacy
+				},
+			},
 		},
 	} {
 		// do we have the protocol specified at all?
diff --git a/pkg/collector/parser/receiver_otlp_test.go b/pkg/collector/parser/receiver_otlp_test.go
index 51ca014d28..6abca3b5d1 100644
--- a/pkg/collector/parser/receiver_otlp_test.go
+++ b/pkg/collector/parser/receiver_otlp_test.go
@@ -40,16 +40,36 @@ func TestOTLPPortsOverridden(t *testing.T) {
 			"grpc": map[interface{}]interface{}{
 				"endpoint": "0.0.0.0:1234",
 			},
+			"http": map[interface{}]interface{}{
+				"endpoint": "0.0.0.0:1235",
+			},
 		},
 	})
 
+	expectedResults := map[string]struct {
+		portNumber int32
+		seen       bool
+	}{
+		"otlp-grpc": {portNumber: 1234},
+		"otlp-http": {portNumber: 1235},
+	}
+
 	// test
 	ports, err := builder.Ports()
 
 	// verify
 	assert.NoError(t, err)
-	assert.Len(t, ports, 1)
-	assert.EqualValues(t, 1234, ports[0].Port)
+	assert.Len(t, ports, len(expectedResults))
+
+	for _, port := range ports {
+		r := expectedResults[port.Name]
+		r.seen = true
+		expectedResults[port.Name] = r
+		assert.EqualValues(t, r.portNumber, port.Port)
+	}
+	for k, v := range expectedResults {
+		assert.True(t, v.seen, "the port %s wasn't included in the service ports", k)
+	}
 }
 
 func TestOTLPExposeDefaultPorts(t *testing.T) {
@@ -57,6 +77,7 @@ func TestOTLPExposeDefaultPorts(t *testing.T) {
 	builder := NewOTLPReceiverParser(logger, "otlp", map[interface{}]interface{}{
 		"protocols": map[interface{}]interface{}{
 			"grpc": map[interface{}]interface{}{},
+			"http": map[interface{}]interface{}{},
 		},
 	})
 
@@ -64,7 +85,9 @@ func TestOTLPExposeDefaultPorts(t *testing.T) {
 		portNumber int32
 		seen       bool
 	}{
-		"otlp-grpc": {portNumber: 4317},
+		"otlp-grpc":        {portNumber: 4317},
+		"otlp-http":        {portNumber: 4318},
+		"otlp-http-legacy": {portNumber: 55681},
 	}
 
 	// test
@@ -72,7 +95,7 @@ func TestOTLPExposeDefaultPorts(t *testing.T) {
 
 	// verify
 	assert.NoError(t, err)
-	assert.Len(t, ports, 1)
+	assert.Len(t, ports, len(expectedResults))
 
 	for _, port := range ports {
 		r := expectedResults[port.Name]