diff --git a/.chloggen/jaeger-query-grpc.yaml b/.chloggen/jaeger-query-grpc.yaml new file mode 100644 index 0000000000..41250bafd7 --- /dev/null +++ b/.chloggen/jaeger-query-grpc.yaml @@ -0,0 +1,6 @@ +change_type: enhancement +component: collector +note: "Support gRPC endpoint for jaeger_query extension and expose it when configured." +issues: [4358] +subtext: | + - HTTP remains defaulted to 16686; gRPC has no default and is exposed only when `jaeger_query.grpc.endpoint` is set. diff --git a/internal/components/extensions/jaeger_query_extension.go b/internal/components/extensions/jaeger_query_extension.go index 60735a9c2b..e32392f648 100644 --- a/internal/components/extensions/jaeger_query_extension.go +++ b/internal/components/extensions/jaeger_query_extension.go @@ -16,8 +16,9 @@ import ( ) const ( - name = "jaeger_query" - port = 16686 + name = "jaeger_query" + grpcPortName = "jq-grpc" + httpPort = 16686 ) var ( @@ -26,14 +27,19 @@ var ( type JaegerQueryExtensionConfig struct { HTTP jaegerHTTPAddress `mapstructure:"http,omitempty" yaml:"http,omitempty"` + GRPC jaegerGRPCAddress `mapstructure:"grpc,omitempty" yaml:"grpc,omitempty"` } type jaegerHTTPAddress struct { Endpoint string `mapstructure:"endpoint,omitempty" yaml:"endpoint,omitempty"` } -func (g *JaegerQueryExtensionConfig) GetPortNumOrDefault(logger logr.Logger, p int32) int32 { - num, err := g.GetPortNum() +type jaegerGRPCAddress struct { + Endpoint string `mapstructure:"endpoint,omitempty" yaml:"endpoint,omitempty"` +} + +func (g *JaegerQueryExtensionConfig) GetHTTPPortNumOrDefault(logger logr.Logger, p int32) int32 { + num, err := g.GetHTTPPortNum() if err != nil { logger.V(3).Info("no port set, using default: %d", p) return p @@ -41,31 +47,59 @@ func (g *JaegerQueryExtensionConfig) GetPortNumOrDefault(logger logr.Logger, p i return num } -// GetPortNum attempts to get the port for the given config. If it cannot, the UnsetPort and the given missingPortError +// GetHTTPPortNum attempts to get the port for the given config. If it cannot, the UnsetPort and the given missingPortError // are returned. -func (g *JaegerQueryExtensionConfig) GetPortNum() (int32, error) { +func (g *JaegerQueryExtensionConfig) GetHTTPPortNum() (int32, error) { if len(g.HTTP.Endpoint) > 0 { return components.PortFromEndpoint(g.HTTP.Endpoint) } return components.UnsetPort, components.PortNotFoundErr } +func (g *JaegerQueryExtensionConfig) GetGRPCPortNum() (int32, error) { + if len(g.GRPC.Endpoint) > 0 { + return components.PortFromEndpoint(g.GRPC.Endpoint) + } + return components.UnsetPort, components.PortNotFoundErr +} + func ParseJaegerQueryExtensionConfig(logger logr.Logger, name string, defaultPort *corev1.ServicePort, cfg *JaegerQueryExtensionConfig) ([]corev1.ServicePort, error) { if cfg == nil { return nil, nil } - if _, err := cfg.GetPortNum(); err != nil && defaultPort.Port == components.UnsetPort { - logger.WithValues("receiver", defaultPort.Name).Error(err, "couldn't parse the endpoint's port and no default port set") - return []corev1.ServicePort{}, err + + httpPortNum := cfg.GetHTTPPortNumOrDefault(logger, defaultPort.Port) + grpcPortNum := components.UnsetPort + if p, err := cfg.GetGRPCPortNum(); err == nil { + grpcPortNum = p + } + + if httpPortNum == components.UnsetPort && grpcPortNum == components.UnsetPort { + logger.WithValues("receiver", defaultPort.Name).Error(components.PortNotFoundErr, "couldn't parse the endpoint's port and no default port set") + return []corev1.ServicePort{}, components.PortNotFoundErr + } + + var ports []corev1.ServicePort + + // - Preserve HTTP port name as "jaeger-query" for backward compatibility. + // - Use a short, stable name for gRPC ("jq-grpc") to avoid the 15-char Kubernetes limit. + if httpPortNum != components.UnsetPort { + httpSvcPort := *defaultPort + httpSvcPort.Name = naming.PortName(name, httpPortNum) + ports = append(ports, components.ConstructServicePort(&httpSvcPort, httpPortNum)) + } + + if grpcPortNum != components.UnsetPort { + grpcSvcPort := *defaultPort + grpcSvcPort.Name = naming.PortName(grpcPortName, grpcPortNum) + ports = append(ports, components.ConstructServicePort(&grpcSvcPort, grpcPortNum)) } - port := cfg.GetPortNumOrDefault(logger, defaultPort.Port) - svcPort := defaultPort - svcPort.Name = naming.PortName(name, port) - return []corev1.ServicePort{components.ConstructServicePort(svcPort, port)}, nil + + return ports, nil } func NewJaegerQueryExtensionParserBuilder() components.Builder[*JaegerQueryExtensionConfig] { - return components.NewBuilder[*JaegerQueryExtensionConfig]().WithPort(port).WithName(name).WithPortParser(ParseJaegerQueryExtensionConfig).WithDefaultsApplier(endpointDefaulter).WithDefaultRecAddress(components.DefaultRecAddress).WithTargetPort(port) + return components.NewBuilder[*JaegerQueryExtensionConfig]().WithPort(httpPort).WithName(name).WithPortParser(ParseJaegerQueryExtensionConfig).WithDefaultsApplier(endpointDefaulter).WithDefaultRecAddress(components.DefaultRecAddress).WithTargetPort(httpPort) } func endpointDefaulter(_ logr.Logger, defaultRecAddr string, port int32, config *JaegerQueryExtensionConfig) (map[string]interface{}, error) { @@ -82,7 +116,21 @@ func endpointDefaulter(_ logr.Logger, defaultRecAddr string, port int32, config } } + // Apply address defaulting for gRPC only if an endpoint is provided + if config.GRPC.Endpoint != "" { + v := strings.Split(config.GRPC.Endpoint, ":") + if len(v) < 2 || v[0] == "" { + config.GRPC.Endpoint = fmt.Sprintf("%s:%s", defaultRecAddr, v[len(v)-1]) + } + } + res := make(map[string]interface{}) err := mapstructure.Decode(config, &res) + // Avoid emitting empty grpc map when not configured + if config.GRPC.Endpoint == "" { + if m, ok := res["grpc"].(map[string]interface{}); ok && len(m) == 0 { + delete(res, "grpc") + } + } return res, err } diff --git a/internal/components/extensions/jaeger_query_extension_test.go b/internal/components/extensions/jaeger_query_extension_test.go index e0c9e7e174..9b0e37aebb 100644 --- a/internal/components/extensions/jaeger_query_extension_test.go +++ b/internal/components/extensions/jaeger_query_extension_test.go @@ -24,16 +24,51 @@ func TestJaegerQueryExtensionParser(t *testing.T) { defaultCfg, err := genericBuilder.GetDefaultConfig(logr.Discard(), nil) require.NoError(t, err) - ports, err := genericBuilder.Ports(logr.Discard(), "jaeger_query", defaultCfg) - require.NoError(t, err) - assert.Equal(t, []corev1.ServicePort{{ - Name: "jaeger-query", - Port: 16686, - TargetPort: intstr.FromInt32(16686), - }}, ports) + tests := []struct { + name string + config interface{} + expected []corev1.ServicePort + }{ + { + name: "default http only", + config: defaultCfg, + expected: []corev1.ServicePort{{ + Name: "jaeger-query", Port: 16686, TargetPort: intstr.FromInt32(16686), + }}, + }, + { + name: "grpc only configured", + config: map[string]interface{}{"grpc": map[string]interface{}{"endpoint": "0.0.0.0:16685"}}, + expected: []corev1.ServicePort{ + {Name: "jaeger-query", Port: 16686, TargetPort: intstr.FromInt32(16686)}, + {Name: "jq-grpc", Port: 16685, TargetPort: intstr.FromInt32(16685)}, + }, + }, + { + name: "http and grpc configured (non-default ports)", + config: map[string]interface{}{ + "http": map[string]interface{}{"endpoint": "0.0.0.0:17686"}, + "grpc": map[string]interface{}{"endpoint": "0.0.0.0:17685"}, + }, + expected: []corev1.ServicePort{ + {Name: "jaeger-query", Port: 17686, TargetPort: intstr.FromInt32(17686)}, + {Name: "jq-grpc", Port: 17685, TargetPort: intstr.FromInt32(17685)}, + }, + }, + } + + for _, tc := range tests { + tc := tc + t.Run(tc.name, func(t *testing.T) { + var cfg = tc.config + ports, err := genericBuilder.Ports(logr.Discard(), "jaeger_query", cfg) + require.NoError(t, err) + assert.ElementsMatch(t, tc.expected, ports) + }) + } } -func TestJaegerQueryExtensionParser_config(t *testing.T) { +func TestJaegerQueryExtensionParser_GetDefaultConfig(t *testing.T) { jaegerBuilder := NewJaegerQueryExtensionParserBuilder() genericBuilder, err := jaegerBuilder.Build() require.NoError(t, err) @@ -44,14 +79,53 @@ func TestJaegerQueryExtensionParser_config(t *testing.T) { want interface{} }{ { - name: "valid config", - config: map[string]interface{}{"http": map[string]interface{}{"endpoint": "127.0.0.0:16686"}}, - want: map[string]interface{}{"http": map[string]interface{}{"endpoint": "127.0.0.0:16686"}}, + name: "http: preserves provided endpoint", + config: map[string]interface{}{"http": map[string]interface{}{"endpoint": "127.0.0.0:17686"}}, + want: map[string]interface{}{"http": map[string]interface{}{"endpoint": "127.0.0.0:17686"}}, + }, + { + name: "http: defaults host when missing", + config: map[string]interface{}{"http": map[string]interface{}{"endpoint": ":17686"}}, + want: map[string]interface{}{ + "http": map[string]interface{}{"endpoint": "0.0.0.0:17686"}, + }, }, { - name: "missing config", + name: "http: defaults when missing", want: map[string]interface{}{"http": map[string]interface{}{"endpoint": "0.0.0.0:16686"}}, }, + { + name: "grpc: preserves provided endpoint; http defaults", + config: map[string]interface{}{"grpc": map[string]interface{}{"endpoint": "127.0.0.0:17685"}}, + want: map[string]interface{}{ + "http": map[string]interface{}{"endpoint": "0.0.0.0:16686"}, + "grpc": map[string]interface{}{"endpoint": "127.0.0.0:17685"}, + }, + }, + { + name: "grpc: defaults host when missing", + config: map[string]interface{}{"grpc": map[string]interface{}{"endpoint": ":17685"}}, + want: map[string]interface{}{ + "http": map[string]interface{}{"endpoint": "0.0.0.0:16686"}, + "grpc": map[string]interface{}{"endpoint": "0.0.0.0:17685"}, + }, + }, + { + name: "grpc: empty section removed", + config: map[string]interface{}{"grpc": map[string]interface{}{}}, + want: map[string]interface{}{"http": map[string]interface{}{"endpoint": "0.0.0.0:16686"}}, + }, + { + name: "http+grpc: preserves provided endpoints", + config: map[string]interface{}{ + "http": map[string]interface{}{"endpoint": "127.0.0.1:17686"}, + "grpc": map[string]interface{}{"endpoint": "127.0.0.1:17685"}, + }, + want: map[string]interface{}{ + "http": map[string]interface{}{"endpoint": "127.0.0.1:17686"}, + "grpc": map[string]interface{}{"endpoint": "127.0.0.1:17685"}, + }, + }, } for _, test := range tests { diff --git a/internal/manifests/collector/service_test.go b/internal/manifests/collector/service_test.go index 2660d7beb4..92d3dbd141 100644 --- a/internal/manifests/collector/service_test.go +++ b/internal/manifests/collector/service_test.go @@ -371,8 +371,8 @@ func TestExtensionService(t *testing.T) { Extensions: &v1beta1.AnyConfig{ Object: map[string]interface{}{ "jaeger_query": map[string]interface{}{ - "http": map[string]interface{}{ - "endpoint": "0.0.0.0:16686", + "grpc": map[string]interface{}{ + "endpoint": "0.0.0.0:17685", }, }, }, @@ -382,13 +382,8 @@ func TestExtensionService(t *testing.T) { }, }, expectedPorts: []v1.ServicePort{ - { - Name: "jaeger-query", - Port: 16686, - TargetPort: intstr.IntOrString{ - IntVal: 16686, - }, - }, + {Name: "jaeger-query", Port: 16686, TargetPort: intstr.IntOrString{IntVal: 16686}}, + {Name: "jq-grpc", Port: 17685, TargetPort: intstr.IntOrString{IntVal: 17685}}, }, }, { @@ -409,10 +404,10 @@ func TestExtensionService(t *testing.T) { Object: map[string]interface{}{ "jaeger_query": map[string]interface{}{ "http": map[string]interface{}{ - "endpoint": "0.0.0.0:16686", + "endpoint": "0.0.0.0:17686", }, "grpc": map[string]interface{}{ - "endpoint": "0.0.0.0:16686", + "endpoint": "0.0.0.0:17685", }, }, }, @@ -422,13 +417,8 @@ func TestExtensionService(t *testing.T) { }, }, expectedPorts: []v1.ServicePort{ - { - Name: "jaeger-query", - Port: 16686, - TargetPort: intstr.IntOrString{ - IntVal: 16686, - }, - }, + {Name: "jaeger-query", Port: 17686, TargetPort: intstr.IntOrString{IntVal: 17686}}, + {Name: "jq-grpc", Port: 17685, TargetPort: intstr.IntOrString{IntVal: 17685}}, }, }, {