Skip to content

Support grpc endpoint for jaeger_query_extension #4388

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Support grpc endpoint for jaeger_query_extension #4388

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .chloggen/jaeger-query-grpc.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
76 changes: 62 additions & 14 deletions internal/components/extensions/jaeger_query_extension.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ import (
)

const (
name = "jaeger_query"
port = 16686
name = "jaeger_query"
grpcPortName = "jq-grpc"
httpPort = 16686
)

var (
Expand All @@ -26,46 +27,79 @@ 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
}
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) {
Expand All @@ -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
}
98 changes: 86 additions & 12 deletions internal/components/extensions/jaeger_query_extension_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down
26 changes: 8 additions & 18 deletions internal/manifests/collector/service_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
},
},
Expand All @@ -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}},
},
},
{
Expand All @@ -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",
},
},
},
Expand All @@ -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}},
},
},
{
Expand Down
Loading