From 995230c8ec079028ab408ec97bd88ea60fb04085 Mon Sep 17 00:00:00 2001 From: Nandor Magyar Date: Fri, 19 Apr 2024 17:17:19 +0200 Subject: [PATCH] fix: crane uses port/targetPort in serviceMonitors for metrics (#964) --- golang/api/v1/deploy.go | 4 ++- golang/internal/mapper/grpc.go | 2 +- golang/pkg/crane/k8s/service_monitor.go | 29 ++++++++++---------- golang/pkg/crane/k8s/service_monitor_test.go | 2 +- protobuf/go/agent/agent.pb.go | 8 +++--- protobuf/proto/agent.proto | 2 +- web/crux/proto/agent.proto | 2 +- web/crux/src/app/deploy/deploy.mapper.ts | 2 +- web/crux/src/grpc/protobuf/proto/agent.ts | 8 +++--- 9 files changed, 31 insertions(+), 28 deletions(-) diff --git a/golang/api/v1/deploy.go b/golang/api/v1/deploy.go index c3607bd844..2b444ff5b9 100644 --- a/golang/api/v1/deploy.go +++ b/golang/api/v1/deploy.go @@ -256,8 +256,10 @@ type ContainerConfig struct { } type Metrics struct { + // Path the path to be scraped, if not defined /metrics is used Path string `json:"path"` - Port string `json:"port"` + // Port exposed port of the service where metrics are available + Port int `json:"port"` } func (c *ContainerConfig) Strings(appConfig *config.CommonConfiguration) []string { diff --git a/golang/internal/mapper/grpc.go b/golang/internal/mapper/grpc.go index c7a77d427b..55898e5c63 100644 --- a/golang/internal/mapper/grpc.go +++ b/golang/internal/mapper/grpc.go @@ -224,7 +224,7 @@ func mapCraneConfig(crane *agent.CraneContainerConfig, containerConfig *v1.Conta if crane.Metrics != nil { containerConfig.Metrics = &v1.Metrics{ Path: crane.Metrics.Path, - Port: crane.Metrics.Path, + Port: int(crane.Metrics.Port), } } } diff --git a/golang/pkg/crane/k8s/service_monitor.go b/golang/pkg/crane/k8s/service_monitor.go index c8f119f558..f7fd2bb403 100644 --- a/golang/pkg/crane/k8s/service_monitor.go +++ b/golang/pkg/crane/k8s/service_monitor.go @@ -11,6 +11,7 @@ import ( smv1 "github.com/prometheus-operator/prometheus-operator/pkg/client/applyconfiguration/monitoring/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/util/intstr" ) type ServiceMonitor struct { @@ -58,28 +59,28 @@ func (sm *ServiceMonitor) Deploy(namespace, serviceName string, metricParams v1. return err } - portName := metricParams.Port - - if portName == "" { - portName = firstPort - } - metricsPath := metricParams.Path if metricsPath == "" { metricsPath = "/metrics" } + endpoint := smv1.Endpoint().WithPath(metricsPath) + if metricParams.Port != 0 { + endpoint = endpoint.WithTargetPort(intstr.FromInt(metricParams.Port)) + } else { + endpoint = endpoint.WithPort(firstPort) + } + smApplyConfig := smv1.ServiceMonitor(serviceName, namespace). WithSpec(smv1.ServiceMonitorSpec(). - WithEndpoints( - smv1.Endpoint().WithPort(portName).WithPath(metricsPath), - ).WithSelector( - metav1.LabelSelector{ - MatchLabels: map[string]string{ - "app": serviceName, + WithEndpoints(endpoint). + WithSelector( + metav1.LabelSelector{ + MatchLabels: map[string]string{ + "app": serviceName, + }, }, - }, - )) + )) _, err = sm.ClientSet.MonitoringV1().ServiceMonitors(namespace).Apply(sm.Ctx, smApplyConfig, metav1.ApplyOptions{ FieldManager: sm.appConfig.FieldManagerName, diff --git a/golang/pkg/crane/k8s/service_monitor_test.go b/golang/pkg/crane/k8s/service_monitor_test.go index a483f559b0..adb7cfb352 100644 --- a/golang/pkg/crane/k8s/service_monitor_test.go +++ b/golang/pkg/crane/k8s/service_monitor_test.go @@ -31,7 +31,7 @@ func TestMain(m *testing.M) { func TestServiceMonitorSpawning(t *testing.T) { m, err := k8s.NewServiceMonitor(context.Background(), k8s.NewClient(getTestConfig())) assert.Nil(t, err, "client is spawned without errors") - err = m.Deploy("crane-sm-test", "test-sm", v1.Metrics{Path: "/metrics", Port: "tcp-metrics"}, "") + err = m.Deploy("crane-sm-test", "test-sm", v1.Metrics{Path: "/metrics", Port: 8080}, "") assert.Nil(t, err, "no errors expected with default parameters") } diff --git a/protobuf/go/agent/agent.pb.go b/protobuf/go/agent/agent.pb.go index 3075cf5e37..2212e21d73 100644 --- a/protobuf/go/agent/agent.pb.go +++ b/protobuf/go/agent/agent.pb.go @@ -1433,7 +1433,7 @@ type Metrics struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Port string `protobuf:"bytes,1,opt,name=port,proto3" json:"port,omitempty"` + Port int32 `protobuf:"varint,1,opt,name=port,proto3" json:"port,omitempty"` Path string `protobuf:"bytes,2,opt,name=path,proto3" json:"path,omitempty"` } @@ -1469,11 +1469,11 @@ func (*Metrics) Descriptor() ([]byte, []int) { return file_protobuf_proto_agent_proto_rawDescGZIP(), []int{18} } -func (x *Metrics) GetPort() string { +func (x *Metrics) GetPort() int32 { if x != nil { return x.Port } - return "" + return 0 } func (x *Metrics) GetPath() string { @@ -2763,7 +2763,7 @@ var file_protobuf_proto_agent_proto_rawDesc = []byte{ 0x65, 0x79, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x6b, 0x65, 0x79, 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x3a, 0x02, 0x38, 0x01, 0x22, 0x31, 0x0a, 0x07, 0x4d, 0x65, 0x74, 0x72, 0x69, - 0x63, 0x73, 0x12, 0x12, 0x0a, 0x04, 0x70, 0x6f, 0x72, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, + 0x63, 0x73, 0x12, 0x12, 0x0a, 0x04, 0x70, 0x6f, 0x72, 0x74, 0x18, 0x01, 0x20, 0x01, 0x28, 0x05, 0x52, 0x04, 0x70, 0x6f, 0x72, 0x74, 0x12, 0x12, 0x0a, 0x04, 0x70, 0x61, 0x74, 0x68, 0x18, 0x02, 0x20, 0x01, 0x28, 0x09, 0x52, 0x04, 0x70, 0x61, 0x74, 0x68, 0x22, 0x96, 0x01, 0x0a, 0x0d, 0x45, 0x78, 0x70, 0x65, 0x63, 0x74, 0x65, 0x64, 0x53, 0x74, 0x61, 0x74, 0x65, 0x12, 0x2c, 0x0a, 0x05, diff --git a/protobuf/proto/agent.proto b/protobuf/proto/agent.proto index c80b78380e..90ae346d5b 100644 --- a/protobuf/proto/agent.proto +++ b/protobuf/proto/agent.proto @@ -187,7 +187,7 @@ message Marker { } message Metrics { - string port = 1; + int32 port = 1; string path = 2; } diff --git a/web/crux/proto/agent.proto b/web/crux/proto/agent.proto index c80b78380e..90ae346d5b 100644 --- a/web/crux/proto/agent.proto +++ b/web/crux/proto/agent.proto @@ -187,7 +187,7 @@ message Marker { } message Metrics { - string port = 1; + int32 port = 1; string path = 2; } diff --git a/web/crux/src/app/deploy/deploy.mapper.ts b/web/crux/src/app/deploy/deploy.mapper.ts index 3e9c4f6c9a..77dbf0f8fa 100644 --- a/web/crux/src/app/deploy/deploy.mapper.ts +++ b/web/crux/src/app/deploy/deploy.mapper.ts @@ -387,7 +387,7 @@ export default class DeployMapper { metrics: config.metrics?.enabled ? { path: config.metrics.path ?? null, - port: config.metrics.port?.toString() ?? null, + port: config.metrics.port ?? null, } : null, } diff --git a/web/crux/src/grpc/protobuf/proto/agent.ts b/web/crux/src/grpc/protobuf/proto/agent.ts index 248c0cf9f3..d41320e393 100644 --- a/web/crux/src/grpc/protobuf/proto/agent.ts +++ b/web/crux/src/grpc/protobuf/proto/agent.ts @@ -267,7 +267,7 @@ export interface Marker_IngressEntry { } export interface Metrics { - port: string + port: number path: string } @@ -1049,17 +1049,17 @@ export const Marker_IngressEntry = { } function createBaseMetrics(): Metrics { - return { port: '', path: '' } + return { port: 0, path: '' } } export const Metrics = { fromJSON(object: any): Metrics { - return { port: isSet(object.port) ? String(object.port) : '', path: isSet(object.path) ? String(object.path) : '' } + return { port: isSet(object.port) ? Number(object.port) : 0, path: isSet(object.path) ? String(object.path) : '' } }, toJSON(message: Metrics): unknown { const obj: any = {} - message.port !== undefined && (obj.port = message.port) + message.port !== undefined && (obj.port = Math.round(message.port)) message.path !== undefined && (obj.path = message.path) return obj },