Add appProtocol for otlp and jaeger receiver parsers

Signed-off-by: Sergei Semenchuk <pdp.eleven11@gmail.com>
open-telemetry · Feb 10, 2022 · 17db6e6 · 17db6e6
1 parent e8d6f8b
commit 17db6e6
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 15 deletions.
diff --git a/pkg/collector/adapters/config_to_ports_test.go b/pkg/collector/adapters/config_to_ports_test.go
@@ -53,6 +53,14 @@ func TestExtractPortsFromConfig(t *testing.T) {
     protocols:
       thrift_http:
         endpoint: 0.0.0.0:15268
+  otlp:
+    protocols:
+      grpc:
+      http:
+  otlp/2:
+    protocols:
+      grpc:
+        endpoint: 0.0.0.0:55555
 `
 
 	// prepare
@@ -63,7 +71,7 @@ func TestExtractPortsFromConfig(t *testing.T) {
 	// test
 	ports, err := adapters.ConfigToReceiverPorts(logger, config)
 	assert.NoError(t, err)
-	assert.Len(t, ports, 6)
+	assert.Len(t, ports, 10)
 
 	// verify
 	expectedPorts := map[int32]bool{}
@@ -73,6 +81,9 @@ func TestExtractPortsFromConfig(t *testing.T) {
 	expectedPorts[int32(6831)] = false
 	expectedPorts[int32(6833)] = false
 	expectedPorts[int32(15268)] = false
+	expectedPorts[int32(4318)] = false
+	expectedPorts[int32(55681)] = false
+	expectedPorts[int32(55555)] = false
 
 	expectedNames := map[string]bool{}
 	expectedNames["examplereceiver"] = false
@@ -81,20 +92,40 @@ func TestExtractPortsFromConfig(t *testing.T) {
 	expectedNames["jaeger-thrift-compact"] = false
 	expectedNames["jaeger-thrift-binary"] = false
 	expectedNames["jaeger-custom-thrift-http"] = false
+	expectedNames["otlp-grpc"] = false
+	expectedNames["otlp-http"] = false
+	expectedNames["otlp-http-legacy"] = false
+	expectedNames["otlp-2-grpc"] = false
+
+	expectedAppProtocols := map[string]string{}
+	expectedAppProtocols["otlp-grpc"] = "grpc"
+	expectedAppProtocols["otlp-http"] = "http"
+	expectedAppProtocols["otlp-http-legacy"] = "http"
+	expectedAppProtocols["jaeger-custom-thrift-http"] = "http"
+	expectedAppProtocols["jaeger-grpc"] = "grpc"
+	expectedAppProtocols["otlp-2-grpc"] = "grpc"
 
 	// make sure we only have the ports in the set
 	for _, port := range ports {
 		assert.NotNil(t, expectedPorts[port.Port])
 		assert.NotNil(t, expectedNames[port.Name])
 		expectedPorts[port.Port] = true
 		expectedNames[port.Name] = true
+
+		if appProtocol, ok := expectedAppProtocols[port.Name]; ok {
+			assert.Equal(t, appProtocol, *port.AppProtocol)
+		}
 	}
 
 	// and make sure all the ports from the set are there
 	for _, val := range expectedPorts {
 		assert.True(t, val)
 	}
 
+	// make sure we only have the ports names in the set
+	for _, val := range expectedNames {
+		assert.True(t, val)
+	}
 }
 
 func TestNoPortsParsed(t *testing.T) {

diff --git a/pkg/collector/parser/receiver_jaeger.go b/pkg/collector/parser/receiver_jaeger.go
@@ -63,16 +63,19 @@ func (j *JaegerReceiverParser) Ports() ([]corev1.ServicePort, error) {
 		name              string
 		defaultPort       int32
 		transportProtocol corev1.Protocol
+		appProtocol       string
 	}{
 		{
 			name:              "grpc",
 			defaultPort:       defaultGRPCPort,
 			transportProtocol: corev1.ProtocolTCP,
+			appProtocol:       "grpc",
 		},
 		{
 			name:              "thrift_http",
 			defaultPort:       defaultThriftHTTPPort,
 			transportProtocol: corev1.ProtocolTCP,
+			appProtocol:       "http",
 		},
 		{
 			name:              "thrift_compact",
@@ -109,6 +112,11 @@ func (j *JaegerReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			// set the appropriate transport protocol (i.e. TCP/UDP) for this kind of receiver protocol
 			protocolPort.Protocol = protocol.transportProtocol
 
+			if protocol.appProtocol != "" {
+				c := protocol.appProtocol
+				protocolPort.AppProtocol = &c
+			}
+
 			// at this point, we *have* a port specified, add it to the list of ports
 			ports = append(ports, *protocolPort)
 		}

diff --git a/pkg/collector/parser/receiver_otlp.go b/pkg/collector/parser/receiver_otlp.go
@@ -32,6 +32,11 @@ const (
 	defaultOTLPHTTPPort       int32 = 4318
 )
 
+var (
+	grpc = "grpc"
+	http = "http"
+)
+
 // OTLPReceiverParser parses the configuration for OTLP receivers.
 type OTLPReceiverParser struct {
 	logger logr.Logger
@@ -64,27 +69,30 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
 		defaultPorts []corev1.ServicePort
 	}{
 		{
-			name: "grpc",
+			name: grpc,
 			defaultPorts: []corev1.ServicePort{
 				{
-					Name:       portName(fmt.Sprintf("%s-grpc", o.name), defaultOTLPGRPCPort),
-					Port:       defaultOTLPGRPCPort,
-					TargetPort: intstr.FromInt(int(defaultOTLPGRPCPort)),
+					Name:        portName(fmt.Sprintf("%s-grpc", o.name), defaultOTLPGRPCPort),
+					Port:        defaultOTLPGRPCPort,
+					TargetPort:  intstr.FromInt(int(defaultOTLPGRPCPort)),
+					AppProtocol: &grpc,
 				},
 			},
 		},
 		{
-			name: "http",
+			name: http,
 			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", o.name), defaultOTLPHTTPPort),
+					Port:        defaultOTLPHTTPPort,
+					TargetPort:  intstr.FromInt(int(defaultOTLPHTTPPort)),
+					AppProtocol: &http,
 				},
 				{
-					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
+					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
+					AppProtocol: &http,
 				},
 			},
 		},
@@ -106,6 +114,14 @@ func (o *OTLPReceiverParser) Ports() ([]corev1.ServicePort, error) {
 			if protocolPort == nil {
 				ports = append(ports, protocol.defaultPorts...)
 			} else {
+				// infer protocol and appProtocol from protocol.name
+				if protocol.name == grpc {
+					protocolPort.Protocol = corev1.ProtocolTCP
+					protocolPort.AppProtocol = &grpc
+				} else if protocol.name == http {
+					protocolPort.Protocol = corev1.ProtocolTCP
+					protocolPort.AppProtocol = &http
+				}
 				ports = append(ports, *protocolPort)
 			}
 		}

diff --git a/pkg/collector/reconcile/service_test.go b/pkg/collector/reconcile/service_test.go
@@ -115,10 +115,12 @@ func TestDesiredService(t *testing.T) {
 	})
 	t.Run("should return service with port mentioned in Instance.Spec.Ports and inferred ports", func(t *testing.T) {
 
+		grpc := "grpc"
 		jaegerPorts := v1.ServicePort{
-			Name:     "jaeger-grpc",
-			Protocol: "TCP",
-			Port:     14250,
+			Name:        "jaeger-grpc",
+			Protocol:    "TCP",
+			Port:        14250,
+			AppProtocol: &grpc,
 		}
 		ports := append(params().Instance.Spec.Ports, jaegerPorts)
 		expected := service("test-collector", ports)