From 2e05242f84a10d6a15cfac970d360734083be061 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Wed, 26 Aug 2026 19:01:32 -0230 Subject: [PATCH 01/14] feat: telemetry MetricsStream reverse-scrape hub (JEP-0013 Phase 3) Add the MetricsStream protocol and Go hub so Prometheus can scrape merged exporter OpenMetrics from telemetry without an exporter client yet. Generated Python stubs are included for proto consistency. Co-authored-by: Cursor --- controller/cmd/telemetry/main.go | 49 ++- .../protocol/jumpstarter/v1/telemetry.pb.go | 402 ++++++++++++++++-- .../jumpstarter/v1/telemetry_grpc.pb.go | 47 +- controller/internal/service/metrics_merge.go | 239 +++++++++++ .../internal/service/metrics_merge_test.go | 172 ++++++++ controller/internal/service/metrics_stream.go | 241 +++++++++++ .../internal/service/metrics_stream_test.go | 402 ++++++++++++++++++ controller/internal/service/telemetry_http.go | 135 ++++++ .../internal/service/telemetry_identity.go | 62 +++ .../internal/service/telemetry_service.go | 83 ++-- protocol/proto/jumpstarter/v1/telemetry.proto | 33 +- .../jumpstarter/v1/telemetry_pb2.py | 34 +- .../jumpstarter/v1/telemetry_pb2.pyi | 102 ++++- .../jumpstarter/v1/telemetry_pb2_grpc.py | 51 ++- .../jumpstarter/v1/telemetry_pb2_grpc.pyi | 49 ++- 15 files changed, 1999 insertions(+), 102 deletions(-) create mode 100644 controller/internal/service/metrics_merge.go create mode 100644 controller/internal/service/metrics_merge_test.go create mode 100644 controller/internal/service/metrics_stream.go create mode 100644 controller/internal/service/metrics_stream_test.go create mode 100644 controller/internal/service/telemetry_http.go create mode 100644 controller/internal/service/telemetry_identity.go diff --git a/controller/cmd/telemetry/main.go b/controller/cmd/telemetry/main.go index 1c757d42e..cf2266048 100644 --- a/controller/cmd/telemetry/main.go +++ b/controller/cmd/telemetry/main.go @@ -14,9 +14,10 @@ See the License for the specific language governing permissions and limitations under the License. */ -// jumpstarter-telemetry receives structured log entries from exporters and clients -// via the PushLogs gRPC RPC and writes them to structured stdout for downstream -// log shippers (Promtail, Grafana Alloy, Vector) to forward to Loki. +// jumpstarter-telemetry reverse-scrapes exporter metrics via MetricsStream and +// receives structured log entries via PushLogs. Logs are written to structured +// stdout for downstream log shippers (Promtail, Grafana Alloy, Vector). +// Loki push is a later Phase 3 PR. // // TLS: always enabled. Set EXTERNAL_CERT_PEM and EXTERNAL_KEY_PEM to file paths of // operator-mounted cert/key (e.g. from a cert-manager Secret); when absent a @@ -30,8 +31,7 @@ limitations under the License. // certificate; the controller uses it to advertise the address to exporters via // GetServiceEndpoints. A mismatch causes TLS hostname verification failures. // -// Future phases will add direct Loki push and MetricsStream for reverse-scrape -// of exporter prometheus_client registries. +// HTTP: GET /metrics, /healthz, and /readyz bind separately (default :8080). package main import ( @@ -39,7 +39,9 @@ import ( "flag" "os" "os/signal" + "strings" "syscall" + "time" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log/zap" @@ -55,9 +57,36 @@ var ( buildDate = "unknown" ) +func splitCSV(s string) []string { + if s == "" { + return nil + } + parts := strings.Split(s, ",") + out := make([]string, 0, len(parts)) + for _, p := range parts { + p = strings.TrimSpace(p) + if p != "" { + out = append(out, p) + } + } + return out +} + func main() { var bindAddr string + var metricsAddr string + var scrapeTimeout time.Duration + var driverTypeEnum string + var exemplarKeys string flag.StringVar(&bindAddr, "grpc-bind", ":9093", "TCP address to bind the gRPC server to") + flag.StringVar(&metricsAddr, "metrics-bind-address", ":8080", + "TCP address for HTTP GET /metrics, /healthz, and /readyz. Use 0 to disable.") + flag.DurationVar(&scrapeTimeout, "scrape-timeout", 7*time.Second, + "Max wait for parallel MetricsStream scrape responses") + flag.StringVar(&driverTypeEnum, "driver-type-enum", strings.Join(service.DefaultDriverTypeEnum, ","), + "Comma-separated allowlist of driver_type values; others are remapped to other") + flag.StringVar(&exemplarKeys, "exemplar-keys", strings.Join(service.DefaultExemplarKeys, ","), + "Comma-separated allowlist of Prometheus exemplar keys") opts := zap.Options{} opts.BindFlags(flag.CommandLine) @@ -71,6 +100,8 @@ func main() { "gitCommit", gitCommit, "buildDate", buildDate, "bindAddr", bindAddr, + "metricsBindAddr", metricsAddr, + "scrapeTimeout", scrapeTimeout, ) ctx, cancel := context.WithCancel(context.Background()) @@ -87,8 +118,12 @@ func main() { } svc := &service.TelemetryService{ - BindAddr: bindAddr, - Signer: signer, + BindAddr: bindAddr, + MetricsBindAddr: metricsAddr, + ScrapeTimeout: scrapeTimeout, + DriverTypeEnum: splitCSV(driverTypeEnum), + ExemplarKeys: splitCSV(exemplarKeys), + Signer: signer, } // Register signal handler before starting the service so no signal diff --git a/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go b/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go index 00e583612..ee0ac7352 100644 --- a/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go +++ b/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go @@ -1,4 +1,4 @@ -// Copyright 2024 The Jumpstarter Authors +// Copyright 2026 The Jumpstarter Authors // Code generated by protoc-gen-go. DO NOT EDIT. // versions: @@ -24,6 +24,289 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) +// Exporter → Telemetry +type MetricsStreamRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Types that are valid to be assigned to Msg: + // + // *MetricsStreamRequest_Register + // *MetricsStreamRequest_ScrapeResponse + Msg isMetricsStreamRequest_Msg `protobuf_oneof:"msg"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsStreamRequest) Reset() { + *x = MetricsStreamRequest{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[0] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsStreamRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsStreamRequest) ProtoMessage() {} + +func (x *MetricsStreamRequest) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[0] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsStreamRequest.ProtoReflect.Descriptor instead. +func (*MetricsStreamRequest) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{0} +} + +func (x *MetricsStreamRequest) GetMsg() isMetricsStreamRequest_Msg { + if x != nil { + return x.Msg + } + return nil +} + +func (x *MetricsStreamRequest) GetRegister() *MetricsRegister { + if x != nil { + if x, ok := x.Msg.(*MetricsStreamRequest_Register); ok { + return x.Register + } + } + return nil +} + +func (x *MetricsStreamRequest) GetScrapeResponse() *MetricsScrapeResponse { + if x != nil { + if x, ok := x.Msg.(*MetricsStreamRequest_ScrapeResponse); ok { + return x.ScrapeResponse + } + } + return nil +} + +type isMetricsStreamRequest_Msg interface { + isMetricsStreamRequest_Msg() +} + +type MetricsStreamRequest_Register struct { + Register *MetricsRegister `protobuf:"bytes,1,opt,name=register,proto3,oneof"` // First message: identify this exporter. +} + +type MetricsStreamRequest_ScrapeResponse struct { + ScrapeResponse *MetricsScrapeResponse `protobuf:"bytes,2,opt,name=scrape_response,json=scrapeResponse,proto3,oneof"` // Subsequent: reply to a scrape. +} + +func (*MetricsStreamRequest_Register) isMetricsStreamRequest_Msg() {} + +func (*MetricsStreamRequest_ScrapeResponse) isMetricsStreamRequest_Msg() {} + +type MetricsRegister struct { + state protoimpl.MessageState `protogen:"open.v1"` + Identity string `protobuf:"bytes,1,opt,name=identity,proto3" json:"identity,omitempty"` // Exporter CRD name (verified against the auth token by the server). + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsRegister) Reset() { + *x = MetricsRegister{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[1] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsRegister) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsRegister) ProtoMessage() {} + +func (x *MetricsRegister) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[1] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsRegister.ProtoReflect.Descriptor instead. +func (*MetricsRegister) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{1} +} + +func (x *MetricsRegister) GetIdentity() string { + if x != nil { + return x.Identity + } + return "" +} + +type MetricsScrapeResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + MetricsText []byte `protobuf:"bytes,1,opt,name=metrics_text,json=metricsText,proto3" json:"metrics_text,omitempty"` // generate_latest() OpenMetrics output. + Timestamp *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=timestamp,proto3" json:"timestamp,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsScrapeResponse) Reset() { + *x = MetricsScrapeResponse{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsScrapeResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsScrapeResponse) ProtoMessage() {} + +func (x *MetricsScrapeResponse) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[2] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsScrapeResponse.ProtoReflect.Descriptor instead. +func (*MetricsScrapeResponse) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{2} +} + +func (x *MetricsScrapeResponse) GetMetricsText() []byte { + if x != nil { + return x.MetricsText + } + return nil +} + +func (x *MetricsScrapeResponse) GetTimestamp() *timestamppb.Timestamp { + if x != nil { + return x.Timestamp + } + return nil +} + +// Telemetry → Exporter +type MetricsStreamResponse struct { + state protoimpl.MessageState `protogen:"open.v1"` + // Types that are valid to be assigned to Msg: + // + // *MetricsStreamResponse_ScrapeRequest + Msg isMetricsStreamResponse_Msg `protobuf_oneof:"msg"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsStreamResponse) Reset() { + *x = MetricsStreamResponse{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsStreamResponse) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsStreamResponse) ProtoMessage() {} + +func (x *MetricsStreamResponse) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[3] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsStreamResponse.ProtoReflect.Descriptor instead. +func (*MetricsStreamResponse) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{3} +} + +func (x *MetricsStreamResponse) GetMsg() isMetricsStreamResponse_Msg { + if x != nil { + return x.Msg + } + return nil +} + +func (x *MetricsStreamResponse) GetScrapeRequest() *MetricsScrapeRequest { + if x != nil { + if x, ok := x.Msg.(*MetricsStreamResponse_ScrapeRequest); ok { + return x.ScrapeRequest + } + } + return nil +} + +type isMetricsStreamResponse_Msg interface { + isMetricsStreamResponse_Msg() +} + +type MetricsStreamResponse_ScrapeRequest struct { + ScrapeRequest *MetricsScrapeRequest `protobuf:"bytes,1,opt,name=scrape_request,json=scrapeRequest,proto3,oneof"` +} + +func (*MetricsStreamResponse_ScrapeRequest) isMetricsStreamResponse_Msg() {} + +// Empty request: "send your /metrics now". +type MetricsScrapeRequest struct { + state protoimpl.MessageState `protogen:"open.v1"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsScrapeRequest) Reset() { + *x = MetricsScrapeRequest{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[4] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsScrapeRequest) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsScrapeRequest) ProtoMessage() {} + +func (x *MetricsScrapeRequest) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[4] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsScrapeRequest.ProtoReflect.Descriptor instead. +func (*MetricsScrapeRequest) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{4} +} + // Request to push log entries to the telemetry service. type PushLogsRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -34,7 +317,7 @@ type PushLogsRequest struct { func (x *PushLogsRequest) Reset() { *x = PushLogsRequest{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[0] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[5] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -46,7 +329,7 @@ func (x *PushLogsRequest) String() string { func (*PushLogsRequest) ProtoMessage() {} func (x *PushLogsRequest) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[0] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[5] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -59,7 +342,7 @@ func (x *PushLogsRequest) ProtoReflect() protoreflect.Message { // Deprecated: Use PushLogsRequest.ProtoReflect.Descriptor instead. func (*PushLogsRequest) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{0} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{5} } func (x *PushLogsRequest) GetEntries() []*LogEntry { @@ -80,7 +363,7 @@ type PushLogsResponse struct { func (x *PushLogsResponse) Reset() { *x = PushLogsResponse{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[1] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[6] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -92,7 +375,7 @@ func (x *PushLogsResponse) String() string { func (*PushLogsResponse) ProtoMessage() {} func (x *PushLogsResponse) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[1] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[6] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -105,7 +388,7 @@ func (x *PushLogsResponse) ProtoReflect() protoreflect.Message { // Deprecated: Use PushLogsResponse.ProtoReflect.Descriptor instead. func (*PushLogsResponse) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{1} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{6} } func (x *PushLogsResponse) GetAccepted() uint32 { @@ -125,26 +408,27 @@ func (x *PushLogsResponse) GetDropped() uint32 { // A structured log entry from an exporter or client. // Maps directly to a Loki log entry with stream labels and body fields. type LogEntry struct { - state protoimpl.MessageState `protogen:"open.v1"` - Timestamp *timestamppb.Timestamp `protobuf:"bytes,1,opt,name=timestamp,proto3" json:"timestamp,omitempty"` // When the log was emitted. - Severity string `protobuf:"bytes,2,opt,name=severity,proto3" json:"severity,omitempty"` // Log severity: debug, info, warning, error, critical. - Message string `protobuf:"bytes,3,opt,name=message,proto3" json:"message,omitempty"` // Human-readable log message. - Component string `protobuf:"bytes,4,opt,name=component,proto3" json:"component,omitempty"` // Loki stream label: cli, exporter, controller, router, telemetry. - Exporter string `protobuf:"bytes,5,opt,name=exporter,proto3" json:"exporter,omitempty"` // Loki stream label: exporter CRD name (bounded by cluster size). - Lease string `protobuf:"bytes,6,opt,name=lease,proto3" json:"lease,omitempty"` // Log body only (high cardinality): active lease name. - Client string `protobuf:"bytes,7,opt,name=client,proto3" json:"client,omitempty"` // Log body only (high cardinality): client CRD name. - Operation string `protobuf:"bytes,8,opt,name=operation,proto3" json:"operation,omitempty"` // Log body: operation name (flash, power, etc.). - Result string `protobuf:"bytes,9,opt,name=result,proto3" json:"result,omitempty"` // Log body: operation outcome (success, failure, etc.). - DriverType string `protobuf:"bytes,10,opt,name=driver_type,json=driverType,proto3" json:"driver_type,omitempty"` // Log body: driver category (storage, power, network, etc.). - ExtraFields map[string]string `protobuf:"bytes,11,rep,name=extra_fields,json=extraFields,proto3" json:"extra_fields,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` // Additional structured key-value fields. - Namespace string `protobuf:"bytes,12,opt,name=namespace,proto3" json:"namespace,omitempty"` // Loki stream label: Kubernetes namespace (bounded by cluster size). + state protoimpl.MessageState `protogen:"open.v1"` + Timestamp *timestamppb.Timestamp `protobuf:"bytes,1,opt,name=timestamp,proto3" json:"timestamp,omitempty"` // When the log was emitted. + Severity string `protobuf:"bytes,2,opt,name=severity,proto3" json:"severity,omitempty"` // Log severity: debug, info, warning, error, critical. + Message string `protobuf:"bytes,3,opt,name=message,proto3" json:"message,omitempty"` // Human-readable log message. + Component string `protobuf:"bytes,4,opt,name=component,proto3" json:"component,omitempty"` // Loki stream label: cli, exporter, controller, router, telemetry. + Exporter string `protobuf:"bytes,5,opt,name=exporter,proto3" json:"exporter,omitempty"` // Loki stream label: exporter CRD name (bounded by cluster size). + Lease string `protobuf:"bytes,6,opt,name=lease,proto3" json:"lease,omitempty"` // Log body only (high cardinality): active lease name. + Client string `protobuf:"bytes,7,opt,name=client,proto3" json:"client,omitempty"` // Log body only (high cardinality): client CRD name. + Operation string `protobuf:"bytes,8,opt,name=operation,proto3" json:"operation,omitempty"` // Log body: operation name (flash, power, etc.). + Result string `protobuf:"bytes,9,opt,name=result,proto3" json:"result,omitempty"` // Log body: operation outcome (success, failure, etc.). + DriverType string `protobuf:"bytes,10,opt,name=driver_type,json=driverType,proto3" json:"driver_type,omitempty"` // Log body: driver category (storage, power, network, etc.). + ExtraFields map[string]string `protobuf:"bytes,11,rep,name=extra_fields,json=extraFields,proto3" json:"extra_fields,omitempty" protobuf_key:"bytes,1,opt,name=key" protobuf_val:"bytes,2,opt,name=value"` // Additional structured key-value fields. + // Capped at 16 entries, 64-char keys, 256-char values. + Namespace string `protobuf:"bytes,12,opt,name=namespace,proto3" json:"namespace,omitempty"` // Loki stream label: Kubernetes namespace (bounded by cluster size). unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } func (x *LogEntry) Reset() { *x = LogEntry{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[2] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[7] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -156,7 +440,7 @@ func (x *LogEntry) String() string { func (*LogEntry) ProtoMessage() {} func (x *LogEntry) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[2] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[7] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -169,7 +453,7 @@ func (x *LogEntry) ProtoReflect() protoreflect.Message { // Deprecated: Use LogEntry.ProtoReflect.Descriptor instead. func (*LogEntry) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{2} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{7} } func (x *LogEntry) GetTimestamp() *timestamppb.Timestamp { @@ -260,7 +544,20 @@ var File_jumpstarter_v1_telemetry_proto protoreflect.FileDescriptor const file_jumpstarter_v1_telemetry_proto_rawDesc = "" + "\n" + - "\x1ejumpstarter/v1/telemetry.proto\x12\x0ejumpstarter.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"E\n" + + "\x1ejumpstarter/v1/telemetry.proto\x12\x0ejumpstarter.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xae\x01\n" + + "\x14MetricsStreamRequest\x12=\n" + + "\bregister\x18\x01 \x01(\v2\x1f.jumpstarter.v1.MetricsRegisterH\x00R\bregister\x12P\n" + + "\x0fscrape_response\x18\x02 \x01(\v2%.jumpstarter.v1.MetricsScrapeResponseH\x00R\x0escrapeResponseB\x05\n" + + "\x03msg\"-\n" + + "\x0fMetricsRegister\x12\x1a\n" + + "\bidentity\x18\x01 \x01(\tR\bidentity\"t\n" + + "\x15MetricsScrapeResponse\x12!\n" + + "\fmetrics_text\x18\x01 \x01(\fR\vmetricsText\x128\n" + + "\ttimestamp\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\ttimestamp\"m\n" + + "\x15MetricsStreamResponse\x12M\n" + + "\x0escrape_request\x18\x01 \x01(\v2$.jumpstarter.v1.MetricsScrapeRequestH\x00R\rscrapeRequestB\x05\n" + + "\x03msg\"\x16\n" + + "\x14MetricsScrapeRequest\"E\n" + "\x0fPushLogsRequest\x122\n" + "\aentries\x18\x01 \x03(\v2\x18.jumpstarter.v1.LogEntryR\aentries\"H\n" + "\x10PushLogsResponse\x12\x1a\n" + @@ -283,8 +580,9 @@ const file_jumpstarter_v1_telemetry_proto_rawDesc = "" + "\tnamespace\x18\f \x01(\tR\tnamespace\x1a>\n" + "\x10ExtraFieldsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x012a\n" + - "\x10TelemetryService\x12M\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x012\xc3\x01\n" + + "\x10TelemetryService\x12`\n" + + "\rMetricsStream\x12$.jumpstarter.v1.MetricsStreamRequest\x1a%.jumpstarter.v1.MetricsStreamResponse(\x010\x01\x12M\n" + "\bPushLogs\x12\x1f.jumpstarter.v1.PushLogsRequest\x1a .jumpstarter.v1.PushLogsResponseB\xdf\x01\n" + "\x12com.jumpstarter.v1B\x0eTelemetryProtoP\x01Z`github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1;jumpstarterv1\xa2\x02\x03JXX\xaa\x02\x0eJumpstarter.V1\xca\x02\x0eJumpstarter\\V1\xe2\x02\x1aJumpstarter\\V1\\GPBMetadata\xea\x02\x0fJumpstarter::V1b\x06proto3" @@ -300,25 +598,36 @@ func file_jumpstarter_v1_telemetry_proto_rawDescGZIP() []byte { return file_jumpstarter_v1_telemetry_proto_rawDescData } -var file_jumpstarter_v1_telemetry_proto_msgTypes = make([]protoimpl.MessageInfo, 4) +var file_jumpstarter_v1_telemetry_proto_msgTypes = make([]protoimpl.MessageInfo, 9) var file_jumpstarter_v1_telemetry_proto_goTypes = []any{ - (*PushLogsRequest)(nil), // 0: jumpstarter.v1.PushLogsRequest - (*PushLogsResponse)(nil), // 1: jumpstarter.v1.PushLogsResponse - (*LogEntry)(nil), // 2: jumpstarter.v1.LogEntry - nil, // 3: jumpstarter.v1.LogEntry.ExtraFieldsEntry - (*timestamppb.Timestamp)(nil), // 4: google.protobuf.Timestamp + (*MetricsStreamRequest)(nil), // 0: jumpstarter.v1.MetricsStreamRequest + (*MetricsRegister)(nil), // 1: jumpstarter.v1.MetricsRegister + (*MetricsScrapeResponse)(nil), // 2: jumpstarter.v1.MetricsScrapeResponse + (*MetricsStreamResponse)(nil), // 3: jumpstarter.v1.MetricsStreamResponse + (*MetricsScrapeRequest)(nil), // 4: jumpstarter.v1.MetricsScrapeRequest + (*PushLogsRequest)(nil), // 5: jumpstarter.v1.PushLogsRequest + (*PushLogsResponse)(nil), // 6: jumpstarter.v1.PushLogsResponse + (*LogEntry)(nil), // 7: jumpstarter.v1.LogEntry + nil, // 8: jumpstarter.v1.LogEntry.ExtraFieldsEntry + (*timestamppb.Timestamp)(nil), // 9: google.protobuf.Timestamp } var file_jumpstarter_v1_telemetry_proto_depIdxs = []int32{ - 2, // 0: jumpstarter.v1.PushLogsRequest.entries:type_name -> jumpstarter.v1.LogEntry - 4, // 1: jumpstarter.v1.LogEntry.timestamp:type_name -> google.protobuf.Timestamp - 3, // 2: jumpstarter.v1.LogEntry.extra_fields:type_name -> jumpstarter.v1.LogEntry.ExtraFieldsEntry - 0, // 3: jumpstarter.v1.TelemetryService.PushLogs:input_type -> jumpstarter.v1.PushLogsRequest - 1, // 4: jumpstarter.v1.TelemetryService.PushLogs:output_type -> jumpstarter.v1.PushLogsResponse - 4, // [4:5] is the sub-list for method output_type - 3, // [3:4] is the sub-list for method input_type - 3, // [3:3] is the sub-list for extension type_name - 3, // [3:3] is the sub-list for extension extendee - 0, // [0:3] is the sub-list for field type_name + 1, // 0: jumpstarter.v1.MetricsStreamRequest.register:type_name -> jumpstarter.v1.MetricsRegister + 2, // 1: jumpstarter.v1.MetricsStreamRequest.scrape_response:type_name -> jumpstarter.v1.MetricsScrapeResponse + 9, // 2: jumpstarter.v1.MetricsScrapeResponse.timestamp:type_name -> google.protobuf.Timestamp + 4, // 3: jumpstarter.v1.MetricsStreamResponse.scrape_request:type_name -> jumpstarter.v1.MetricsScrapeRequest + 7, // 4: jumpstarter.v1.PushLogsRequest.entries:type_name -> jumpstarter.v1.LogEntry + 9, // 5: jumpstarter.v1.LogEntry.timestamp:type_name -> google.protobuf.Timestamp + 8, // 6: jumpstarter.v1.LogEntry.extra_fields:type_name -> jumpstarter.v1.LogEntry.ExtraFieldsEntry + 0, // 7: jumpstarter.v1.TelemetryService.MetricsStream:input_type -> jumpstarter.v1.MetricsStreamRequest + 5, // 8: jumpstarter.v1.TelemetryService.PushLogs:input_type -> jumpstarter.v1.PushLogsRequest + 3, // 9: jumpstarter.v1.TelemetryService.MetricsStream:output_type -> jumpstarter.v1.MetricsStreamResponse + 6, // 10: jumpstarter.v1.TelemetryService.PushLogs:output_type -> jumpstarter.v1.PushLogsResponse + 9, // [9:11] is the sub-list for method output_type + 7, // [7:9] is the sub-list for method input_type + 7, // [7:7] is the sub-list for extension type_name + 7, // [7:7] is the sub-list for extension extendee + 0, // [0:7] is the sub-list for field type_name } func init() { file_jumpstarter_v1_telemetry_proto_init() } @@ -326,13 +635,20 @@ func file_jumpstarter_v1_telemetry_proto_init() { if File_jumpstarter_v1_telemetry_proto != nil { return } + file_jumpstarter_v1_telemetry_proto_msgTypes[0].OneofWrappers = []any{ + (*MetricsStreamRequest_Register)(nil), + (*MetricsStreamRequest_ScrapeResponse)(nil), + } + file_jumpstarter_v1_telemetry_proto_msgTypes[3].OneofWrappers = []any{ + (*MetricsStreamResponse_ScrapeRequest)(nil), + } type x struct{} out := protoimpl.TypeBuilder{ File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_jumpstarter_v1_telemetry_proto_rawDesc), len(file_jumpstarter_v1_telemetry_proto_rawDesc)), NumEnums: 0, - NumMessages: 4, + NumMessages: 9, NumExtensions: 0, NumServices: 1, }, diff --git a/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go b/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go index 356b656a2..d5db4a9a7 100644 --- a/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go +++ b/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go @@ -1,4 +1,4 @@ -// Copyright 2024 The Jumpstarter Authors +// Copyright 2026 The Jumpstarter Authors // Code generated by protoc-gen-go-grpc. DO NOT EDIT. // versions: @@ -21,16 +21,20 @@ import ( const _ = grpc.SupportPackageIsVersion9 const ( - TelemetryService_PushLogs_FullMethodName = "/jumpstarter.v1.TelemetryService/PushLogs" + TelemetryService_MetricsStream_FullMethodName = "/jumpstarter.v1.TelemetryService/MetricsStream" + TelemetryService_PushLogs_FullMethodName = "/jumpstarter.v1.TelemetryService/PushLogs" ) // TelemetryServiceClient is the client API for TelemetryService service. // // For semantics around ctx use and closing/ending streaming RPCs, please refer to https://pkg.go.dev/google.golang.org/grpc/?tab=doc#ClientConn.NewStream. // -// A service that receives structured logs from exporters and clients. +// A service that reverse-scrapes exporter metrics and receives structured logs. // Implemented by jumpstarter-telemetry; not part of the controller. type TelemetryServiceClient interface { + // Persistent bidirectional stream: telemetry sends scrape requests, + // exporter responds with full metric snapshots (OpenMetrics text). + MetricsStream(ctx context.Context, opts ...grpc.CallOption) (grpc.BidiStreamingClient[MetricsStreamRequest, MetricsStreamResponse], error) // Push structured log entries to the telemetry service for Loki ingest. PushLogs(ctx context.Context, in *PushLogsRequest, opts ...grpc.CallOption) (*PushLogsResponse, error) } @@ -43,6 +47,19 @@ func NewTelemetryServiceClient(cc grpc.ClientConnInterface) TelemetryServiceClie return &telemetryServiceClient{cc} } +func (c *telemetryServiceClient) MetricsStream(ctx context.Context, opts ...grpc.CallOption) (grpc.BidiStreamingClient[MetricsStreamRequest, MetricsStreamResponse], error) { + cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) + stream, err := c.cc.NewStream(ctx, &TelemetryService_ServiceDesc.Streams[0], TelemetryService_MetricsStream_FullMethodName, cOpts...) + if err != nil { + return nil, err + } + x := &grpc.GenericClientStream[MetricsStreamRequest, MetricsStreamResponse]{ClientStream: stream} + return x, nil +} + +// This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. +type TelemetryService_MetricsStreamClient = grpc.BidiStreamingClient[MetricsStreamRequest, MetricsStreamResponse] + func (c *telemetryServiceClient) PushLogs(ctx context.Context, in *PushLogsRequest, opts ...grpc.CallOption) (*PushLogsResponse, error) { cOpts := append([]grpc.CallOption{grpc.StaticMethod()}, opts...) out := new(PushLogsResponse) @@ -57,9 +74,12 @@ func (c *telemetryServiceClient) PushLogs(ctx context.Context, in *PushLogsReque // All implementations must embed UnimplementedTelemetryServiceServer // for forward compatibility. // -// A service that receives structured logs from exporters and clients. +// A service that reverse-scrapes exporter metrics and receives structured logs. // Implemented by jumpstarter-telemetry; not part of the controller. type TelemetryServiceServer interface { + // Persistent bidirectional stream: telemetry sends scrape requests, + // exporter responds with full metric snapshots (OpenMetrics text). + MetricsStream(grpc.BidiStreamingServer[MetricsStreamRequest, MetricsStreamResponse]) error // Push structured log entries to the telemetry service for Loki ingest. PushLogs(context.Context, *PushLogsRequest) (*PushLogsResponse, error) mustEmbedUnimplementedTelemetryServiceServer() @@ -72,6 +92,9 @@ type TelemetryServiceServer interface { // pointer dereference when methods are called. type UnimplementedTelemetryServiceServer struct{} +func (UnimplementedTelemetryServiceServer) MetricsStream(grpc.BidiStreamingServer[MetricsStreamRequest, MetricsStreamResponse]) error { + return status.Error(codes.Unimplemented, "method MetricsStream not implemented") +} func (UnimplementedTelemetryServiceServer) PushLogs(context.Context, *PushLogsRequest) (*PushLogsResponse, error) { return nil, status.Error(codes.Unimplemented, "method PushLogs not implemented") } @@ -96,6 +119,13 @@ func RegisterTelemetryServiceServer(s grpc.ServiceRegistrar, srv TelemetryServic s.RegisterService(&TelemetryService_ServiceDesc, srv) } +func _TelemetryService_MetricsStream_Handler(srv interface{}, stream grpc.ServerStream) error { + return srv.(TelemetryServiceServer).MetricsStream(&grpc.GenericServerStream[MetricsStreamRequest, MetricsStreamResponse]{ServerStream: stream}) +} + +// This type alias is provided for backwards compatibility with existing code that references the prior non-generic stream type by name. +type TelemetryService_MetricsStreamServer = grpc.BidiStreamingServer[MetricsStreamRequest, MetricsStreamResponse] + func _TelemetryService_PushLogs_Handler(srv interface{}, ctx context.Context, dec func(interface{}) error, interceptor grpc.UnaryServerInterceptor) (interface{}, error) { in := new(PushLogsRequest) if err := dec(in); err != nil { @@ -126,6 +156,13 @@ var TelemetryService_ServiceDesc = grpc.ServiceDesc{ Handler: _TelemetryService_PushLogs_Handler, }, }, - Streams: []grpc.StreamDesc{}, + Streams: []grpc.StreamDesc{ + { + StreamName: "MetricsStream", + Handler: _TelemetryService_MetricsStream_Handler, + ServerStreams: true, + ClientStreams: true, + }, + }, Metadata: "jumpstarter/v1/telemetry.proto", } diff --git a/controller/internal/service/metrics_merge.go b/controller/internal/service/metrics_merge.go new file mode 100644 index 000000000..378e54af3 --- /dev/null +++ b/controller/internal/service/metrics_merge.go @@ -0,0 +1,239 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "bytes" + "errors" + "io" + "sort" + "strings" + + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" +) + +const ( + labelExporter = "exporter" + labelDriverType = "driver_type" + driverTypeOther = "other" + + // scrapeTimeoutsMetric is incremented once per exporter that does not + // answer a MetricsStream scrape within scrapeTimeout (JEP-0013). + scrapeTimeoutsMetric = "jumpstarter_scrape_timeouts_total" +) + +// DefaultDriverTypeEnum is the JEP-0013 default allowlist for driver_type. +var DefaultDriverTypeEnum = []string{ + "power", "storage", "network", "serial", "console", "video", "composite", +} + +// DefaultExemplarKeys is the JEP-0013 default exemplar allowlist. +var DefaultExemplarKeys = []string{"client", "lease_id"} + +type mergeConfig struct { + exporterName string + driverTypes map[string]struct{} + exemplarKeys map[string]struct{} +} + +type exporterSnapshot struct { + name string + text []byte +} + +func setToMap(values []string) map[string]struct{} { + out := make(map[string]struct{}, len(values)) + for _, v := range values { + if v == "" { + continue + } + out[v] = struct{}{} + } + return out +} + +func (s *TelemetryService) mergeConfigFor(name string) mergeConfig { + types := s.DriverTypeEnum + if len(types) == 0 { + types = DefaultDriverTypeEnum + } + keys := s.ExemplarKeys + if len(keys) == 0 { + keys = DefaultExemplarKeys + } + return mergeConfig{ + exporterName: name, + driverTypes: setToMap(types), + exemplarKeys: setToMap(keys), + } +} + +func parseMetricFamilies(text []byte) ([]*dto.MetricFamily, error) { + dec := expfmt.NewDecoder(bytes.NewReader(text), expfmt.NewFormat(expfmt.TypeOpenMetrics)) + var out []*dto.MetricFamily + for { + mf := new(dto.MetricFamily) + err := dec.Decode(mf) + if errors.Is(err, io.EOF) { + return out, nil + } + if err != nil { + return nil, err + } + out = append(out, mf) + } +} + +func encodeMetricFamilies(w io.Writer, families []*dto.MetricFamily) error { + sorted := append([]*dto.MetricFamily(nil), families...) + sort.Slice(sorted, func(i, j int) bool { + return sorted[i].GetName() < sorted[j].GetName() + }) + enc := expfmt.NewEncoder(w, expfmt.NewFormat(expfmt.TypeOpenMetrics)) + for _, f := range sorted { + if f == nil { + continue + } + if err := enc.Encode(f); err != nil { + return err + } + } + if closer, ok := enc.(expfmt.Closer); ok { + return closer.Close() + } + return nil +} + +func mergeSnapshots(snapshots []exporterSnapshot, extra []*dto.MetricFamily, cfgFor func(string) mergeConfig) []*dto.MetricFamily { + byName := map[string]*dto.MetricFamily{} + for _, f := range extra { + if f == nil || f.GetName() == "" { + continue + } + byName[f.GetName()] = f + } + for _, snap := range snapshots { + if len(snap.text) == 0 { + continue + } + families, err := parseMetricFamilies(snap.text) + if err != nil { + continue + } + cfg := cfgFor(snap.name) + for _, f := range families { + applyFamily(f, cfg) + existing, ok := byName[f.GetName()] + if !ok { + byName[f.GetName()] = f + continue + } + existing.Metric = append(existing.Metric, f.Metric...) + } + } + out := make([]*dto.MetricFamily, 0, len(byName)) + for _, f := range byName { + out = append(out, f) + } + return out +} + +func applyFamily(f *dto.MetricFamily, cfg mergeConfig) { + for _, m := range f.GetMetric() { + applyMetric(m, cfg) + } +} + +func applyMetric(m *dto.Metric, cfg mergeConfig) { + setLabel(m, labelExporter, cfg.exporterName) + if v, ok := getLabel(m, labelDriverType); ok { + setLabel(m, labelDriverType, remapDriverType(v, cfg.driverTypes)) + } + if m.Counter != nil { + m.Counter.Exemplar = filterExemplar(m.Counter.Exemplar, cfg.exemplarKeys) + } + if m.Histogram != nil { + for _, b := range m.Histogram.Bucket { + if b != nil { + b.Exemplar = filterExemplar(b.Exemplar, cfg.exemplarKeys) + } + } + if len(m.Histogram.Exemplars) > 0 { + filtered := make([]*dto.Exemplar, 0, len(m.Histogram.Exemplars)) + for _, ex := range m.Histogram.Exemplars { + if fe := filterExemplar(ex, cfg.exemplarKeys); fe != nil { + filtered = append(filtered, fe) + } + } + m.Histogram.Exemplars = filtered + } + } +} + +func remapDriverType(value string, allow map[string]struct{}) string { + if _, ok := allow[value]; ok { + return value + } + return driverTypeOther +} + +func getLabel(m *dto.Metric, name string) (string, bool) { + for _, lp := range m.GetLabel() { + if lp.GetName() == name { + return lp.GetValue(), true + } + } + return "", false +} + +func setLabel(m *dto.Metric, name, value string) { + for _, lp := range m.GetLabel() { + if lp.GetName() == name { + v := value + lp.Value = &v + return + } + } + m.Label = append(m.Label, labelPair(name, value)) +} + +func labelPair(name, value string) *dto.LabelPair { + n, v := name, value + return &dto.LabelPair{Name: &n, Value: &v} +} + +func filterExemplar(ex *dto.Exemplar, allow map[string]struct{}) *dto.Exemplar { + if ex == nil { + return nil + } + kept := make([]*dto.LabelPair, 0, len(ex.GetLabel())) + for _, lp := range ex.GetLabel() { + if _, ok := allow[lp.GetName()]; !ok { + continue + } + if strings.TrimSpace(lp.GetValue()) == "" { + continue + } + kept = append(kept, lp) + } + if len(kept) == 0 { + return nil + } + ex.Label = kept + return ex +} diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go new file mode 100644 index 000000000..5f6dfe70a --- /dev/null +++ b/controller/internal/service/metrics_merge_test.go @@ -0,0 +1,172 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "bytes" + "strings" + "testing" + + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" +) + +func TestApplyMetric_OverwritesExporterRemapsDriverTypeAndFiltersExemplars(t *testing.T) { + one := 1.0 + m := &dto.Metric{ + Label: []*dto.LabelPair{ + labelPair("exporter", "spoofed"), + labelPair("driver_type", "tuya"), + labelPair("operation", "on"), + }, + Counter: &dto.Counter{ + Value: &one, + Exemplar: &dto.Exemplar{ + Label: []*dto.LabelPair{ + labelPair("client", "ci"), + labelPair("lease_id", "abc"), + labelPair("trace_id", "drop-me"), + }, + }, + }, + } + applyMetric(m, mergeConfig{ + exporterName: "sidekick", + driverTypes: setToMap(DefaultDriverTypeEnum), + exemplarKeys: setToMap(DefaultExemplarKeys), + }) + + if got, _ := getLabel(m, "exporter"); got != "sidekick" { + t.Errorf("exporter label = %q, want sidekick", got) + } + if got, _ := getLabel(m, "driver_type"); got != "other" { + t.Errorf("driver_type = %q, want other", got) + } + ex := m.Counter.GetExemplar() + if ex == nil { + t.Fatal("expected exemplar to be kept") + } + got := map[string]string{} + for _, lp := range ex.GetLabel() { + got[lp.GetName()] = lp.GetValue() + } + if got["client"] != "ci" || got["lease_id"] != "abc" { + t.Errorf("exemplar labels = %v, want client=ci lease_id=abc", got) + } + if _, ok := got["trace_id"]; ok { + t.Errorf("trace_id should have been dropped, got %v", got) + } +} + +func TestApplyMetric_KeepsAllowlistedDriverType(t *testing.T) { + m := &dto.Metric{ + Label: []*dto.LabelPair{ + labelPair("driver_type", "power"), + }, + } + applyMetric(m, mergeConfig{ + exporterName: "exp-a", + driverTypes: setToMap(DefaultDriverTypeEnum), + exemplarKeys: setToMap(DefaultExemplarKeys), + }) + if got, _ := getLabel(m, "driver_type"); got != "power" { + t.Errorf("driver_type = %q, want power", got) + } + if got, _ := getLabel(m, "exporter"); got != "exp-a" { + t.Errorf("exporter = %q, want exp-a", got) + } +} + +func TestMergeSnapshots_CombinesExportersAndSkipsInvalidText(t *testing.T) { + cfg := func(name string) mergeConfig { + return mergeConfig{ + exporterName: name, + driverTypes: setToMap(DefaultDriverTypeEnum), + exemplarKeys: setToMap(DefaultExemplarKeys), + } + } + textA := []byte(`# TYPE jumpstarter_operations_total counter +# HELP jumpstarter_operations_total Total operations performed. +jumpstarter_operations_total{exporter="a",operation="on",result="success",driver_type="power"} 2.0 +# EOF +`) + textB := []byte(`# TYPE jumpstarter_operations_total counter +jumpstarter_operations_total{exporter="b",operation="off",result="success",driver_type="power"} 3.0 +# EOF +`) + families := mergeSnapshots([]exporterSnapshot{ + {name: "exp-a", text: textA}, + {name: "exp-b", text: textB}, + {name: "bad", text: []byte("not metrics")}, + }, nil, cfg) + + var ops *dto.MetricFamily + for _, f := range families { + if f.GetName() == "jumpstarter_operations_total" { + ops = f + break + } + } + if ops == nil { + t.Fatal("missing jumpstarter_operations_total") + } + if len(ops.Metric) != 2 { + t.Fatalf("got %d series, want 2 (invalid snapshot omitted)", len(ops.Metric)) + } + exporters := map[string]bool{} + for _, m := range ops.Metric { + name, _ := getLabel(m, "exporter") + exporters[name] = true + } + if !exporters["exp-a"] || !exporters["exp-b"] { + t.Errorf("exporters = %v, want exp-a and exp-b", exporters) + } +} + +func TestEncodeMetricFamilies_WritesOpenMetricsEOF(t *testing.T) { + name := "jumpstarter_scrape_timeouts_total" + help := "Exporter MetricsStream scrapes that exceeded scrapeTimeout." + metricType := dto.MetricType_COUNTER + zero := 0.0 + families := []*dto.MetricFamily{{ + Name: &name, + Help: &help, + Type: &metricType, + Metric: []*dto.Metric{{ + Counter: &dto.Counter{Value: &zero}, + }}, + }} + var buf bytes.Buffer + if err := encodeMetricFamilies(&buf, families); err != nil { + t.Fatalf("encode: %v", err) + } + body := buf.String() + if !strings.Contains(body, scrapeTimeoutsMetric) { + t.Errorf("encoded body missing %s:\n%s", scrapeTimeoutsMetric, body) + } + if !strings.Contains(body, "# EOF") { + t.Errorf("OpenMetrics body missing # EOF:\n%s", body) + } + dec := expfmt.NewDecoder(strings.NewReader(body), expfmt.NewFormat(expfmt.TypeOpenMetrics)) + mf := new(dto.MetricFamily) + if err := dec.Decode(mf); err != nil { + t.Fatalf("re-decode: %v", err) + } + if mf.GetName() != scrapeTimeoutsMetric { + t.Errorf("decoded name = %q", mf.GetName()) + } +} diff --git a/controller/internal/service/metrics_stream.go b/controller/internal/service/metrics_stream.go new file mode 100644 index 000000000..b16bb78ef --- /dev/null +++ b/controller/internal/service/metrics_stream.go @@ -0,0 +1,241 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "context" + "errors" + "sync" + "time" + + pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" + "sigs.k8s.io/controller-runtime/pkg/log" +) + +var errScrapeTimeout = errors.New("metrics scrape timeout") + +const defaultScrapeTimeout = 7 * time.Second + +// metricsConn is one authenticated MetricsStream. Scrapes are serialized +// because MetricsScrapeRequest has no scrape id (JEP-0013). +type metricsConn struct { + id exporterIdentity + scrapeMu sync.Mutex + mu sync.Mutex + send func(*pb.MetricsStreamResponse) error + // pending is non-nil only while a scrape is waiting for a response. + pending chan *pb.MetricsScrapeResponse + done <-chan struct{} + // started is closed when the Recv loop is running. + started chan struct{} +} + +func (s *TelemetryService) scrapeTimeoutDuration() time.Duration { + if s.ScrapeTimeout > 0 { + return s.ScrapeTimeout + } + return defaultScrapeTimeout +} + +func (s *TelemetryService) initMetricsState() { + s.stateMu.Lock() + defer s.stateMu.Unlock() + if s.conns == nil { + s.conns = make(map[string]*metricsConn) + } +} + +func (s *TelemetryService) registerConn(c *metricsConn) { + s.stateMu.Lock() + defer s.stateMu.Unlock() + if s.conns == nil { + s.conns = make(map[string]*metricsConn) + } + s.conns[c.id.key()] = c +} + +func (s *TelemetryService) unregisterConn(c *metricsConn) { + s.stateMu.Lock() + defer s.stateMu.Unlock() + if s.conns == nil { + return + } + if cur, ok := s.conns[c.id.key()]; ok && cur == c { + delete(s.conns, c.id.key()) + } +} + +func (s *TelemetryService) snapshotConns() []*metricsConn { + s.stateMu.Lock() + defer s.stateMu.Unlock() + out := make([]*metricsConn, 0, len(s.conns)) + for _, c := range s.conns { + out = append(out, c) + } + return out +} + +// MetricsStream reverse-scrapes one exporter. The first message must be +// MetricsRegister whose identity matches the bearer token. +func (s *TelemetryService) MetricsStream(stream pb.TelemetryService_MetricsStreamServer) error { + s.initMetricsState() + id, err := s.authenticateExporter(stream.Context()) + if err != nil { + return err + } + + first, err := stream.Recv() + if err != nil { + return err + } + reg := first.GetRegister() + if reg == nil { + return status.Error(codes.InvalidArgument, "first MetricsStream message must be MetricsRegister") + } + if reg.GetIdentity() != id.name { + return status.Errorf(codes.PermissionDenied, "register identity %q does not match authenticated exporter %q", reg.GetIdentity(), id.name) + } + + done := make(chan struct{}) + conn := &metricsConn{ + id: id, + send: stream.Send, + done: done, + started: make(chan struct{}), + } + s.registerConn(conn) + defer s.unregisterConn(conn) + + logger := log.FromContext(stream.Context()).WithName("telemetry").WithValues( + "exporter", id.name, + "namespace", id.namespace, + ) + logger.Info("MetricsStream registered") + + var closeDone sync.Once + finish := func() { closeDone.Do(func() { close(done) }) } + defer finish() + close(conn.started) + + for { + msg, err := stream.Recv() + if err != nil { + return err + } + resp := msg.GetScrapeResponse() + if resp == nil { + return status.Error(codes.InvalidArgument, "expected MetricsScrapeResponse after register") + } + conn.mu.Lock() + pending := conn.pending + conn.pending = nil + conn.mu.Unlock() + if pending == nil { + continue + } + select { + case pending <- resp: + case <-done: + return nil + case <-stream.Context().Done(): + return stream.Context().Err() + } + } +} + +func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) ([]byte, error) { + c.scrapeMu.Lock() + defer c.scrapeMu.Unlock() + + reply := make(chan *pb.MetricsScrapeResponse, 1) + c.mu.Lock() + c.pending = reply + c.mu.Unlock() + defer func() { + c.mu.Lock() + if c.pending == reply { + c.pending = nil + } + c.mu.Unlock() + }() + + err := c.send(&pb.MetricsStreamResponse{ + Msg: &pb.MetricsStreamResponse_ScrapeRequest{ + ScrapeRequest: &pb.MetricsScrapeRequest{}, + }, + }) + if err != nil { + return nil, err + } + + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case resp := <-reply: + if resp == nil { + return nil, errScrapeTimeout + } + return resp.GetMetricsText(), nil + case <-timer.C: + return nil, errScrapeTimeout + case <-ctx.Done(): + return nil, ctx.Err() + case <-c.done: + return nil, context.Canceled + } +} + +func (s *TelemetryService) fanoutScrapes(ctx context.Context) []exporterSnapshot { + s.initMetricsState() + s.initScrapeTimeouts() + timeout := s.scrapeTimeoutDuration() + conns := s.snapshotConns() + if len(conns) == 0 { + return nil + } + + ctx, cancel := context.WithTimeout(ctx, timeout) + defer cancel() + + var mu sync.Mutex + var snaps []exporterSnapshot + var wg sync.WaitGroup + for _, c := range conns { + wg.Add(1) + go func(c *metricsConn) { + defer wg.Done() + text, err := c.scrape(ctx, timeout) + if err != nil { + if errors.Is(err, errScrapeTimeout) || errors.Is(err, context.DeadlineExceeded) { + s.scrapeTimeouts.Inc() + } + log.FromContext(ctx).WithName("telemetry").V(1).Info("exporter scrape omitted", + "exporter", c.id.name, + "error", err.Error(), + ) + return + } + mu.Lock() + snaps = append(snaps, exporterSnapshot{name: c.id.name, text: text}) + mu.Unlock() + }(c) + } + wg.Wait() + return snaps +} diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go new file mode 100644 index 000000000..6f4a50cd0 --- /dev/null +++ b/controller/internal/service/metrics_stream_test.go @@ -0,0 +1,402 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "context" + "errors" + "io" + "net" + "net/http" + "strings" + "testing" + "time" + + pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" + "google.golang.org/grpc/test/bufconn" +) + +const bufconnSize = 1024 * 1024 + +func startTestHub(t *testing.T, timeout time.Duration) (*TelemetryService, pb.TelemetryServiceClient, string) { + t.Helper() + signer := testSigner(t) + svc := &TelemetryService{ + Signer: signer, + MetricsBindAddr: "127.0.0.1:0", + ScrapeTimeout: timeout, + DriverTypeEnum: DefaultDriverTypeEnum, + ExemplarKeys: DefaultExemplarKeys, + } + svc.grpcReady.Store(true) + + shutdown, err := svc.startMetricsHTTP() + if err != nil { + t.Fatalf("startMetricsHTTP: %v", err) + } + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = shutdown(ctx) + }) + + lis := bufconn.Listen(bufconnSize) + gs := grpc.NewServer() + pb.RegisterTelemetryServiceServer(gs, svc) + go func() { _ = gs.Serve(lis) }() + t.Cleanup(func() { + gs.Stop() + _ = lis.Close() + }) + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return lis.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + return svc, pb.NewTelemetryServiceClient(conn), svc.metricsAddr +} + +func exporterStreamCtx(t *testing.T, svc *TelemetryService, subject string) context.Context { + t.Helper() + token, err := svc.Signer.Token(subject) + if err != nil { + t.Fatalf("Token: %v", err) + } + return metadata.NewOutgoingContext( + context.Background(), + metadata.Pairs("authorization", "Bearer "+token), + ) +} + +func waitRegistered(t *testing.T, svc *TelemetryService, n int) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + conns := svc.snapshotConns() + if len(conns) != n { + time.Sleep(10 * time.Millisecond) + continue + } + ready := true + for _, c := range conns { + if c.started == nil { + ready = false + break + } + select { + case <-c.started: + default: + ready = false + } + if !ready { + break + } + } + if ready { + return + } + time.Sleep(10 * time.Millisecond) + } + t.Fatalf("timed out waiting for %d MetricsStream connections, have %d", n, len(svc.snapshotConns())) +} + +func httpGet(t *testing.T, url string) (int, string) { + t.Helper() + client := &http.Client{Timeout: 3 * time.Second} + var resp *http.Response + var lastErr error + for i := 0; i < 30; i++ { + resp, lastErr = client.Get(url) + if lastErr == nil { + break + } + time.Sleep(20 * time.Millisecond) + } + if lastErr != nil { + t.Fatalf("GET %s: %v", url, lastErr) + } + defer func() { _ = resp.Body.Close() }() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read body: %v", err) + } + return resp.StatusCode, string(body) +} + +func TestHealthzAndReadyz(t *testing.T) { + _, _, addr := startTestHub(t, 200*time.Millisecond) + code, body := httpGet(t, "http://"+addr+"/healthz") + if code != http.StatusOK || !strings.Contains(body, "ok") { + t.Fatalf("healthz status=%d body=%q", code, body) + } + code, body = httpGet(t, "http://"+addr+"/readyz") + if code != http.StatusOK || !strings.Contains(body, "ok") { + t.Fatalf("readyz status=%d body=%q", code, body) + } +} + +func TestReadyzNotReadyWhenGRPCDown(t *testing.T) { + svc := &TelemetryService{MetricsBindAddr: "127.0.0.1:0"} + shutdown, err := svc.startMetricsHTTP() + if err != nil { + t.Fatalf("startMetricsHTTP: %v", err) + } + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + _ = shutdown(ctx) + }) + code, _ := httpGet(t, "http://"+svc.metricsAddr+"/readyz") + if code != http.StatusServiceUnavailable { + t.Fatalf("readyz status=%d, want 503", code) + } +} + +func TestMetricsStream_RejectsIdentityMismatch(t *testing.T) { + svc, client, _ := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:alice:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "bob"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + _, err = stream.Recv() + if err == nil { + t.Fatal("expected PermissionDenied, got nil") + } + if status.Code(err) != codes.PermissionDenied { + t.Fatalf("code = %v, want PermissionDenied (%v)", status.Code(err), err) + } +} + +func TestMetricsStream_RejectsNonRegisterFirstMessage(t *testing.T) { + svc, client, _ := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:alice:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_ScrapeResponse{ + ScrapeResponse: &pb.MetricsScrapeResponse{MetricsText: []byte("# EOF\n")}, + }, + }); err != nil { + t.Fatalf("Send: %v", err) + } + _, err = stream.Recv() + if status.Code(err) != codes.InvalidArgument { + t.Fatalf("code = %v, want InvalidArgument (%v)", status.Code(err), err) + } +} + +func TestMetricsStream_RejectsNonExporterToken(t *testing.T) { + svc, client, _ := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "client:jumpstarter:ci:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "ci"}, + }, + }); err != nil { + t.Fatalf("Send: %v", err) + } + _, err = stream.Recv() + if status.Code(err) != codes.PermissionDenied { + t.Fatalf("code = %v, want PermissionDenied (%v)", status.Code(err), err) + } +} + +func TestMetricsConn_ScrapeTimeout(t *testing.T) { + done := make(chan struct{}) + c := &metricsConn{ + send: func(*pb.MetricsStreamResponse) error { return nil }, + done: done, + } + _, err := c.scrape(context.Background(), 40*time.Millisecond) + if !errors.Is(err, errScrapeTimeout) { + t.Fatalf("err = %v, want errScrapeTimeout", err) + } +} + +func TestFanout_TimeoutIncrementsCounterWithoutGRPC(t *testing.T) { + svc := &TelemetryService{ScrapeTimeout: 40 * time.Millisecond} + svc.initScrapeTimeouts() + done := make(chan struct{}) + c := &metricsConn{ + id: exporterIdentity{namespace: "jumpstarter", name: "slow"}, + send: func(*pb.MetricsStreamResponse) error { return nil }, + done: done, + } + svc.registerConn(c) + snaps := svc.fanoutScrapes(context.Background()) + if len(snaps) != 0 { + t.Fatalf("timed-out scrape must be omitted, got %d snapshots", len(snaps)) + } + mfs, err := svc.metricsRegistry.Gather() + if err != nil { + t.Fatalf("Gather: %v", err) + } + var value float64 + var found bool + for _, mf := range mfs { + if mf.GetName() == scrapeTimeoutsMetric || mf.GetName()+"_total" == scrapeTimeoutsMetric { + found = true + if len(mf.Metric) > 0 { + value = mf.Metric[0].GetCounter().GetValue() + } + } + } + if !found || value < 1 { + t.Fatalf("scrape timeout counter = %v found=%v, want >= 1", value, found) + } +} + +func TestFanout_TimeoutOmitsExporterAndIncrementsCounter(t *testing.T) { + svc, client, addr := startTestHub(t, 150*time.Millisecond) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:slow:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "slow"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + // Drain scrape requests so Send on the server does not block, but never reply. + go func() { + for { + if _, err := stream.Recv(); err != nil { + return + } + } + }() + waitRegistered(t, svc, 1) + + code, body := httpGet(t, "http://"+addr+"/metrics") + if code != http.StatusOK { + t.Fatalf("GET /metrics status=%d body=%s", code, body) + } + if strings.Contains(body, "jumpstarter_operations_total") { + t.Errorf("timed-out exporter metrics must be omitted, body:\n%s", body) + } + mfs, err := svc.metricsRegistry.Gather() + if err != nil { + t.Fatalf("Gather: %v", err) + } + var value float64 + var found bool + for _, mf := range mfs { + if mf.GetName() == scrapeTimeoutsMetric || mf.GetName()+"_total" == scrapeTimeoutsMetric { + found = true + if len(mf.Metric) > 0 { + value = mf.Metric[0].GetCounter().GetValue() + } + } + } + if !found || value < 1 { + t.Fatalf("scrape timeout counter = %v found=%v body:\n%s", value, found, body) + } +} + +func TestFanout_MergesOpenMetricsFromConnectedExporter(t *testing.T) { + svc, client, addr := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "sidekick"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + + snapshot := []byte(`# TYPE jumpstarter_operations_total counter +# HELP jumpstarter_operations_total Total operations performed. +jumpstarter_operations_total{exporter="spoofed",operation="on",result="success",driver_type="tuya"} 4.0 +# EOF +`) + errCh := make(chan error, 1) + go func() { + for { + msg, err := stream.Recv() + if err != nil { + errCh <- err + return + } + if msg.GetScrapeRequest() == nil { + continue + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_ScrapeResponse{ + ScrapeResponse: &pb.MetricsScrapeResponse{MetricsText: snapshot}, + }, + }); err != nil { + errCh <- err + return + } + } + }() + waitRegistered(t, svc, 1) + + code, body := httpGet(t, "http://"+addr+"/metrics") + if code != http.StatusOK { + t.Fatalf("GET /metrics status=%d body=%s", code, body) + } + if !strings.Contains(body, `exporter="sidekick"`) { + t.Errorf("authenticated exporter label missing:\n%s", body) + } + if strings.Contains(body, `exporter="spoofed"`) { + t.Errorf("spoofed exporter label must be overwritten:\n%s", body) + } + if !strings.Contains(body, `driver_type="other"`) { + t.Errorf("unknown driver_type must remap to other:\n%s", body) + } + select { + case err := <-errCh: + if err != nil && err != io.EOF && status.Code(err) != codes.Canceled && status.Code(err) != codes.Unavailable { + t.Fatalf("stream goroutine: %v", err) + } + default: + } +} diff --git a/controller/internal/service/telemetry_http.go b/controller/internal/service/telemetry_http.go new file mode 100644 index 000000000..6d12daff4 --- /dev/null +++ b/controller/internal/service/telemetry_http.go @@ -0,0 +1,135 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "bytes" + "context" + "net" + "net/http" + "time" + + "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "github.com/prometheus/common/expfmt" + ctrl "sigs.k8s.io/controller-runtime" +) + +func (s *TelemetryService) initScrapeTimeouts() { + s.stateMu.Lock() + defer s.stateMu.Unlock() + if s.metricsRegistry != nil { + return + } + s.metricsRegistry = prometheus.NewRegistry() + s.scrapeTimeouts = prometheus.NewCounter(prometheus.CounterOpts{ + Name: scrapeTimeoutsMetric, + Help: "Exporter MetricsStream scrapes that exceeded scrapeTimeout.", + }) + s.metricsRegistry.MustRegister(s.scrapeTimeouts) +} + +func metricsHTTPEnabled(addr string) bool { + return addr != "" && addr != "0" +} + +func (s *TelemetryService) startMetricsHTTP() (func(context.Context) error, error) { + addr := s.MetricsBindAddr + if !metricsHTTPEnabled(addr) { + return nil, nil + } + s.initMetricsState() + s.initScrapeTimeouts() + + ln, err := net.Listen("tcp", addr) + if err != nil { + return nil, err + } + s.metricsAddr = ln.Addr().String() + + mux := http.NewServeMux() + mux.HandleFunc("/metrics", s.handleMetrics) + mux.HandleFunc("/healthz", s.handleHealthz) + mux.HandleFunc("/readyz", s.handleReadyz) + + writeTimeout := s.scrapeTimeoutDuration() + 15*time.Second + if writeTimeout < 30*time.Second { + writeTimeout = 30 * time.Second + } + + srv := &http.Server{ + Handler: mux, + ReadHeaderTimeout: 10 * time.Second, + ReadTimeout: 30 * time.Second, + WriteTimeout: writeTimeout, + IdleTimeout: 5 * time.Minute, + } + go func() { + if err := srv.Serve(ln); err != nil && err != http.ErrServerClosed { + ctrl.Log.WithName("telemetry").Error(err, "metrics HTTP server stopped unexpectedly") + } + }() + ctrl.Log.WithName("telemetry").Info("Telemetry metrics HTTP listening", + "addr", s.metricsAddr, + ) + return srv.Shutdown, nil +} + +func (s *TelemetryService) handleHealthz(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok\n")) +} + +// handleReadyz is 200 once the gRPC listener is bound. +// JEP DD-7 also gates on Loki reachability and a connected exporter; Loki is +// Phase 3 PR D, and requiring an exporter would keep an empty lab unready +// (Prometheus could not scrape jumpstarter_scrape_timeouts_total). +func (s *TelemetryService) handleReadyz(w http.ResponseWriter, _ *http.Request) { + if !s.grpcReady.Load() { + http.Error(w, "grpc not ready\n", http.StatusServiceUnavailable) + return + } + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte("ok\n")) +} + +func (s *TelemetryService) handleMetrics(w http.ResponseWriter, r *http.Request) { + s.initScrapeTimeouts() + snaps := s.fanoutScrapes(r.Context()) + + var extra []*dto.MetricFamily + if s.metricsRegistry != nil { + gathered, err := s.metricsRegistry.Gather() + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + extra = gathered + } + + families := mergeSnapshots(snaps, extra, s.mergeConfigFor) + + var buf bytes.Buffer + if err := encodeMetricFamilies(&buf, families); err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", string(expfmt.NewFormat(expfmt.TypeOpenMetrics))) + w.WriteHeader(http.StatusOK) + _, _ = w.Write(buf.Bytes()) +} diff --git a/controller/internal/service/telemetry_identity.go b/controller/internal/service/telemetry_identity.go new file mode 100644 index 000000000..e1f23de4e --- /dev/null +++ b/controller/internal/service/telemetry_identity.go @@ -0,0 +1,62 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "context" + "strings" + + "github.com/jumpstarter-dev/jumpstarter/controller/internal/authentication" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/status" +) + +// exporterIdentity is the exporter CRD namespace/name claimed by a bearer token. +type exporterIdentity struct { + namespace string + name string +} + +func (id exporterIdentity) key() string { + return id.namespace + "/" + id.name +} + +func parseExporterSubject(subject string) (exporterIdentity, error) { + parts := strings.SplitN(subject, ":", 4) + if len(parts) != 4 || parts[0] != "exporter" { + return exporterIdentity{}, status.Errorf(codes.PermissionDenied, "token is not an exporter token") + } + if parts[1] == "" || parts[2] == "" { + return exporterIdentity{}, status.Errorf(codes.PermissionDenied, "token has incomplete exporter identity") + } + return exporterIdentity{namespace: parts[1], name: parts[2]}, nil +} + +func (s *TelemetryService) authenticateExporter(ctx context.Context) (exporterIdentity, error) { + token, err := authentication.BearerTokenFromContext(ctx) + if err != nil { + return exporterIdentity{}, err + } + if s.Signer == nil { + return exporterIdentity{}, status.Error(codes.Internal, "telemetry signer is not configured") + } + subject, err := s.Signer.ParseSubject(token) + if err != nil { + return exporterIdentity{}, status.Errorf(codes.Unauthenticated, "invalid token: %v", err) + } + return parseExporterSubject(subject) +} diff --git a/controller/internal/service/telemetry_service.go b/controller/internal/service/telemetry_service.go index 80844b807..e3c60bb77 100644 --- a/controller/internal/service/telemetry_service.go +++ b/controller/internal/service/telemetry_service.go @@ -22,17 +22,17 @@ import ( "fmt" "net" "strings" + "sync" + "sync/atomic" "time" "github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/recovery" - "github.com/jumpstarter-dev/jumpstarter/controller/internal/authentication" "github.com/jumpstarter-dev/jumpstarter/controller/internal/oidc" pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" + "github.com/prometheus/client_golang/prometheus" "google.golang.org/grpc" - "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials" "google.golang.org/grpc/reflection" - "google.golang.org/grpc/status" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/log" ) @@ -69,42 +69,47 @@ var reservedExtraFieldKeys = map[string]struct{}{ type TelemetryService struct { pb.UnimplementedTelemetryServiceServer - // BindAddr is the TCP address to listen on (e.g. ":9093"). + // BindAddr is the TCP address to listen on for gRPC (e.g. ":9093"). BindAddr string - // Signer is used to validate bearer tokens on every PushLogs call. - // Tokens are issued by the controller from the same CONTROLLER_KEY seed, - // so the telemetry binary can verify them locally without a k8s client. + // MetricsBindAddr is the TCP address for HTTP GET /metrics, /healthz, and + // /readyz. Empty or "0" disables the HTTP server (tests). Production + // default is :8080 — a dedicated port, not multiplexed onto gRPC :9093. + MetricsBindAddr string + + // ScrapeTimeout is the fan-out wait for MetricsStream responses (JEP default 7s). + ScrapeTimeout time.Duration + + // DriverTypeEnum allowlist; unknown driver_type values are remapped to "other". + DriverTypeEnum []string + + // ExemplarKeys allowlist applied on the telemetry merge path. + ExemplarKeys []string + + // Signer is used to validate bearer tokens on every PushLogs call and + // MetricsStream. Tokens are issued by the controller from the same + // CONTROLLER_KEY seed, so the telemetry binary can verify them locally + // without a k8s client. Signer *oidc.Signer + + stateMu sync.Mutex + conns map[string]*metricsConn + scrapeTimeouts prometheus.Counter + metricsRegistry *prometheus.Registry + metricsAddr string + grpcReady atomic.Bool } // PushLogs receives a batch of structured log entries and writes them via the // controller-runtime logger (structured JSON to stdout). // Future phase: forward to Loki push API. func (s *TelemetryService) PushLogs(ctx context.Context, req *pb.PushLogsRequest) (*pb.PushLogsResponse, error) { - token, err := authentication.BearerTokenFromContext(ctx) + id, err := s.authenticateExporter(ctx) if err != nil { return nil, err } - - // Validate token and extract the subject (format: exporter:namespace:name:uid). - subject, err := s.Signer.ParseSubject(token) - if err != nil { - return nil, status.Errorf(codes.Unauthenticated, "invalid token: %v", err) - } - - // Only exporter tokens are allowed to push logs. Any other validly-signed - // token (e.g. a client token) is rejected immediately so that the identity - // checks below always have a non-empty claimedName/claimedNamespace. - parts := strings.SplitN(subject, ":", 4) - if len(parts) != 4 || parts[0] != "exporter" { - return nil, status.Errorf(codes.PermissionDenied, "token is not an exporter token") - } - claimedNamespace := parts[1] - claimedName := parts[2] - if claimedNamespace == "" || claimedName == "" { - return nil, status.Errorf(codes.PermissionDenied, "token has incomplete exporter identity") - } + claimedNamespace := id.namespace + claimedName := id.name // Use context-based logger so tests can inject their own via logf.IntoContext. logger := log.FromContext(ctx).WithName("telemetry") @@ -260,9 +265,21 @@ func (s *TelemetryService) Start(ctx context.Context) error { return fmt.Errorf("telemetry: listen %s: %w", s.BindAddr, err) } + s.initMetricsState() + s.initScrapeTimeouts() + s.grpcReady.Store(true) + + httpShutdown, err := s.startMetricsHTTP() + if err != nil { + s.grpcReady.Store(false) + _ = lis.Close() + return fmt.Errorf("telemetry: metrics HTTP listen %s: %w", s.MetricsBindAddr, err) + } + srv := grpc.NewServer( grpc.Creds(creds), grpc.ChainUnaryInterceptor(recovery.UnaryServerInterceptor()), + grpc.ChainStreamInterceptor(recovery.StreamServerInterceptor()), ) pb.RegisterTelemetryServiceServer(srv, s) reflection.Register(srv) @@ -276,12 +293,24 @@ func (s *TelemetryService) Start(ctx context.Context) error { select { case <-ctx.Done(): + s.grpcReady.Store(false) + if httpShutdown != nil { + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = httpShutdown(shutdownCtx) + } srv.GracefulStop() if err := <-errCh; err != nil && !errors.Is(err, grpc.ErrServerStopped) { return err } return nil case err := <-errCh: + s.grpcReady.Store(false) + if httpShutdown != nil { + shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + _ = httpShutdown(shutdownCtx) + } srv.Stop() return err } diff --git a/protocol/proto/jumpstarter/v1/telemetry.proto b/protocol/proto/jumpstarter/v1/telemetry.proto index 200748201..fbbfd00ee 100644 --- a/protocol/proto/jumpstarter/v1/telemetry.proto +++ b/protocol/proto/jumpstarter/v1/telemetry.proto @@ -6,13 +6,44 @@ package jumpstarter.v1; import "google/protobuf/timestamp.proto"; -// A service that receives structured logs from exporters and clients. +// A service that reverse-scrapes exporter metrics and receives structured logs. // Implemented by jumpstarter-telemetry; not part of the controller. service TelemetryService { + // Persistent bidirectional stream: telemetry sends scrape requests, + // exporter responds with full metric snapshots (OpenMetrics text). + rpc MetricsStream(stream MetricsStreamRequest) returns (stream MetricsStreamResponse); + // Push structured log entries to the telemetry service for Loki ingest. rpc PushLogs(PushLogsRequest) returns (PushLogsResponse); } +// Exporter → Telemetry +message MetricsStreamRequest { + oneof msg { + MetricsRegister register = 1; // First message: identify this exporter. + MetricsScrapeResponse scrape_response = 2; // Subsequent: reply to a scrape. + } +} + +message MetricsRegister { + string identity = 1; // Exporter CRD name (verified against the auth token by the server). +} + +message MetricsScrapeResponse { + bytes metrics_text = 1; // generate_latest() OpenMetrics output. + google.protobuf.Timestamp timestamp = 2; +} + +// Telemetry → Exporter +message MetricsStreamResponse { + oneof msg { + MetricsScrapeRequest scrape_request = 1; + } +} + +// Empty request: "send your /metrics now". +message MetricsScrapeRequest {} + // Request to push log entries to the telemetry service. message PushLogsRequest { repeated LogEntry entries = 1; // Log entries to push. diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py index 30983ccc3..80e4bc2bc 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py @@ -25,24 +25,34 @@ from google.protobuf import timestamp_pb2 as google_dot_protobuf_dot_timestamp__pb2 -DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x1ejumpstarter/v1/telemetry.proto\x12\x0ejumpstarter.v1\x1a\x1fgoogle/protobuf/timestamp.proto"E\n\x0fPushLogsRequest\x122\n\x07entries\x18\x01 \x03(\x0b2\x18.jumpstarter.v1.LogEntryR\x07entries"H\n\x10PushLogsResponse\x12\x1a\n\x08accepted\x18\x01 \x01(\rR\x08accepted\x12\x18\n\x07dropped\x18\x02 \x01(\rR\x07dropped"\xe5\x03\n\x08LogEntry\x128\n\ttimestamp\x18\x01 \x01(\x0b2\x1a.google.protobuf.TimestampR\ttimestamp\x12\x1a\n\x08severity\x18\x02 \x01(\tR\x08severity\x12\x18\n\x07message\x18\x03 \x01(\tR\x07message\x12\x1c\n\tcomponent\x18\x04 \x01(\tR\tcomponent\x12\x1a\n\x08exporter\x18\x05 \x01(\tR\x08exporter\x12\x14\n\x05lease\x18\x06 \x01(\tR\x05lease\x12\x16\n\x06client\x18\x07 \x01(\tR\x06client\x12\x1c\n\toperation\x18\x08 \x01(\tR\toperation\x12\x16\n\x06result\x18\t \x01(\tR\x06result\x12\x1f\n\x0bdriver_type\x18\n \x01(\tR\ndriverType\x12L\n\x0cextra_fields\x18\x0b \x03(\x0b2).jumpstarter.v1.LogEntry.ExtraFieldsEntryR\x0bextraFields\x12\x1c\n\tnamespace\x18\x0c \x01(\tR\tnamespace\x1a>\n\x10ExtraFieldsEntry\x12\x10\n\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value:\x028\x012a\n\x10TelemetryService\x12M\n\x08PushLogs\x12\x1f.jumpstarter.v1.PushLogsRequest\x1a .jumpstarter.v1.PushLogsResponseB\xd1\x01\n\x12com.jumpstarter.v1B\x0eTelemetryProtoP\x01ZRgithub.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1\xa2\x02\x03JXX\xaa\x02\x0eJumpstarter.V1\xca\x02\x0eJumpstarter\\V1\xe2\x02\x1aJumpstarter\\V1\\GPBMetadata\xea\x02\x0fJumpstarter::V1b\x06proto3') +DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x1ejumpstarter/v1/telemetry.proto\x12\x0ejumpstarter.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xae\x01\n\x14MetricsStreamRequest\x12=\n\x08register\x18\x01 \x01(\x0b\x32\x1f.jumpstarter.v1.MetricsRegisterH\x00R\x08register\x12P\n\x0fscrape_response\x18\x02 \x01(\x0b\x32%.jumpstarter.v1.MetricsScrapeResponseH\x00R\x0escrapeResponseB\x05\n\x03msg\"-\n\x0fMetricsRegister\x12\x1a\n\x08identity\x18\x01 \x01(\tR\x08identity\"t\n\x15MetricsScrapeResponse\x12!\n\x0cmetrics_text\x18\x01 \x01(\x0cR\x0bmetricsText\x12\x38\n\ttimestamp\x18\x02 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\"m\n\x15MetricsStreamResponse\x12M\n\x0escrape_request\x18\x01 \x01(\x0b\x32$.jumpstarter.v1.MetricsScrapeRequestH\x00R\rscrapeRequestB\x05\n\x03msg\"\x16\n\x14MetricsScrapeRequest\"E\n\x0fPushLogsRequest\x12\x32\n\x07\x65ntries\x18\x01 \x03(\x0b\x32\x18.jumpstarter.v1.LogEntryR\x07\x65ntries\"H\n\x10PushLogsResponse\x12\x1a\n\x08\x61\x63\x63\x65pted\x18\x01 \x01(\rR\x08\x61\x63\x63\x65pted\x12\x18\n\x07\x64ropped\x18\x02 \x01(\rR\x07\x64ropped\"\xe5\x03\n\x08LogEntry\x12\x38\n\ttimestamp\x18\x01 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\x12\x1a\n\x08severity\x18\x02 \x01(\tR\x08severity\x12\x18\n\x07message\x18\x03 \x01(\tR\x07message\x12\x1c\n\tcomponent\x18\x04 \x01(\tR\tcomponent\x12\x1a\n\x08\x65xporter\x18\x05 \x01(\tR\x08\x65xporter\x12\x14\n\x05lease\x18\x06 \x01(\tR\x05lease\x12\x16\n\x06\x63lient\x18\x07 \x01(\tR\x06\x63lient\x12\x1c\n\toperation\x18\x08 \x01(\tR\toperation\x12\x16\n\x06result\x18\t \x01(\tR\x06result\x12\x1f\n\x0b\x64river_type\x18\n \x01(\tR\ndriverType\x12L\n\x0c\x65xtra_fields\x18\x0b \x03(\x0b\x32).jumpstarter.v1.LogEntry.ExtraFieldsEntryR\x0b\x65xtraFields\x12\x1c\n\tnamespace\x18\x0c \x01(\tR\tnamespace\x1a>\n\x10\x45xtraFieldsEntry\x12\x10\n\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value:\x02\x38\x01\x32\xc3\x01\n\x10TelemetryService\x12`\n\rMetricsStream\x12$.jumpstarter.v1.MetricsStreamRequest\x1a%.jumpstarter.v1.MetricsStreamResponse(\x01\x30\x01\x12M\n\x08PushLogs\x12\x1f.jumpstarter.v1.PushLogsRequest\x1a .jumpstarter.v1.PushLogsResponseB}\n\x12\x63om.jumpstarter.v1B\x0eTelemetryProtoP\x01\xa2\x02\x03JXX\xaa\x02\x0eJumpstarter.V1\xca\x02\x0eJumpstarter\\V1\xe2\x02\x1aJumpstarter\\V1\\GPBMetadata\xea\x02\x0fJumpstarter::V1b\x06proto3') _globals = globals() _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) _builder.BuildTopDescriptorsAndMessages(DESCRIPTOR, 'jumpstarter.v1.telemetry_pb2', _globals) if not _descriptor._USE_C_DESCRIPTORS: _globals['DESCRIPTOR']._loaded_options = None - _globals['DESCRIPTOR']._serialized_options = b'\n\022com.jumpstarter.v1B\016TelemetryProtoP\001ZRgithub.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1\242\002\003JXX\252\002\016Jumpstarter.V1\312\002\016Jumpstarter\\V1\342\002\032Jumpstarter\\V1\\GPBMetadata\352\002\017Jumpstarter::V1' + _globals['DESCRIPTOR']._serialized_options = b'\n\022com.jumpstarter.v1B\016TelemetryProtoP\001\242\002\003JXX\252\002\016Jumpstarter.V1\312\002\016Jumpstarter\\V1\342\002\032Jumpstarter\\V1\\GPBMetadata\352\002\017Jumpstarter::V1' _globals['_LOGENTRY_EXTRAFIELDSENTRY']._loaded_options = None _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_options = b'8\001' - _globals['_PUSHLOGSREQUEST']._serialized_start=83 - _globals['_PUSHLOGSREQUEST']._serialized_end=152 - _globals['_PUSHLOGSRESPONSE']._serialized_start=154 - _globals['_PUSHLOGSRESPONSE']._serialized_end=226 - _globals['_LOGENTRY']._serialized_start=229 - _globals['_LOGENTRY']._serialized_end=714 - _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_start=652 - _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_end=714 - _globals['_TELEMETRYSERVICE']._serialized_start=716 - _globals['_TELEMETRYSERVICE']._serialized_end=813 + _globals['_METRICSSTREAMREQUEST']._serialized_start=84 + _globals['_METRICSSTREAMREQUEST']._serialized_end=258 + _globals['_METRICSREGISTER']._serialized_start=260 + _globals['_METRICSREGISTER']._serialized_end=305 + _globals['_METRICSSCRAPERESPONSE']._serialized_start=307 + _globals['_METRICSSCRAPERESPONSE']._serialized_end=423 + _globals['_METRICSSTREAMRESPONSE']._serialized_start=425 + _globals['_METRICSSTREAMRESPONSE']._serialized_end=534 + _globals['_METRICSSCRAPEREQUEST']._serialized_start=536 + _globals['_METRICSSCRAPEREQUEST']._serialized_end=558 + _globals['_PUSHLOGSREQUEST']._serialized_start=560 + _globals['_PUSHLOGSREQUEST']._serialized_end=629 + _globals['_PUSHLOGSRESPONSE']._serialized_start=631 + _globals['_PUSHLOGSRESPONSE']._serialized_end=703 + _globals['_LOGENTRY']._serialized_start=706 + _globals['_LOGENTRY']._serialized_end=1191 + _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_start=1129 + _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_end=1191 + _globals['_TELEMETRYSERVICE']._serialized_start=1194 + _globals['_TELEMETRYSERVICE']._serialized_end=1389 # @@protoc_insertion_point(module_scope) diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi index 2f28d9f24..968f7ea75 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi @@ -19,6 +19,103 @@ else: DESCRIPTOR: google.protobuf.descriptor.FileDescriptor +@typing.final +class MetricsStreamRequest(google.protobuf.message.Message): + """Exporter → Telemetry""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + REGISTER_FIELD_NUMBER: builtins.int + SCRAPE_RESPONSE_FIELD_NUMBER: builtins.int + @property + def register(self) -> Global___MetricsRegister: + """First message: identify this exporter.""" + + @property + def scrape_response(self) -> Global___MetricsScrapeResponse: + """Subsequent: reply to a scrape.""" + + def __init__( + self, + *, + register: Global___MetricsRegister | None = ..., + scrape_response: Global___MetricsScrapeResponse | None = ..., + ) -> None: ... + def HasField(self, field_name: typing.Literal["msg", b"msg", "register", b"register", "scrape_response", b"scrape_response"]) -> builtins.bool: ... + def ClearField(self, field_name: typing.Literal["msg", b"msg", "register", b"register", "scrape_response", b"scrape_response"]) -> None: ... + def WhichOneof(self, oneof_group: typing.Literal["msg", b"msg"]) -> typing.Literal["register", "scrape_response"] | None: ... + +Global___MetricsStreamRequest: typing_extensions.TypeAlias = MetricsStreamRequest + +@typing.final +class MetricsRegister(google.protobuf.message.Message): + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + IDENTITY_FIELD_NUMBER: builtins.int + identity: builtins.str + """Exporter CRD name (verified against the auth token by the server).""" + def __init__( + self, + *, + identity: builtins.str = ..., + ) -> None: ... + def ClearField(self, field_name: typing.Literal["identity", b"identity"]) -> None: ... + +Global___MetricsRegister: typing_extensions.TypeAlias = MetricsRegister + +@typing.final +class MetricsScrapeResponse(google.protobuf.message.Message): + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + METRICS_TEXT_FIELD_NUMBER: builtins.int + TIMESTAMP_FIELD_NUMBER: builtins.int + metrics_text: builtins.bytes + """generate_latest() OpenMetrics output.""" + @property + def timestamp(self) -> google.protobuf.timestamp_pb2.Timestamp: ... + def __init__( + self, + *, + metrics_text: builtins.bytes = ..., + timestamp: google.protobuf.timestamp_pb2.Timestamp | None = ..., + ) -> None: ... + def HasField(self, field_name: typing.Literal["timestamp", b"timestamp"]) -> builtins.bool: ... + def ClearField(self, field_name: typing.Literal["metrics_text", b"metrics_text", "timestamp", b"timestamp"]) -> None: ... + +Global___MetricsScrapeResponse: typing_extensions.TypeAlias = MetricsScrapeResponse + +@typing.final +class MetricsStreamResponse(google.protobuf.message.Message): + """Telemetry → Exporter""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + SCRAPE_REQUEST_FIELD_NUMBER: builtins.int + @property + def scrape_request(self) -> Global___MetricsScrapeRequest: ... + def __init__( + self, + *, + scrape_request: Global___MetricsScrapeRequest | None = ..., + ) -> None: ... + def HasField(self, field_name: typing.Literal["msg", b"msg", "scrape_request", b"scrape_request"]) -> builtins.bool: ... + def ClearField(self, field_name: typing.Literal["msg", b"msg", "scrape_request", b"scrape_request"]) -> None: ... + def WhichOneof(self, oneof_group: typing.Literal["msg", b"msg"]) -> typing.Literal["scrape_request"] | None: ... + +Global___MetricsStreamResponse: typing_extensions.TypeAlias = MetricsStreamResponse + +@typing.final +class MetricsScrapeRequest(google.protobuf.message.Message): + """Empty request: "send your /metrics now".""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + def __init__( + self, + ) -> None: ... + +Global___MetricsScrapeRequest: typing_extensions.TypeAlias = MetricsScrapeRequest + @typing.final class PushLogsRequest(google.protobuf.message.Message): """Request to push log entries to the telemetry service.""" @@ -96,6 +193,7 @@ class LogEntry(google.protobuf.message.Message): RESULT_FIELD_NUMBER: builtins.int DRIVER_TYPE_FIELD_NUMBER: builtins.int EXTRA_FIELDS_FIELD_NUMBER: builtins.int + NAMESPACE_FIELD_NUMBER: builtins.int severity: builtins.str """Log severity: debug, info, warning, error, critical.""" message: builtins.str @@ -115,7 +213,9 @@ class LogEntry(google.protobuf.message.Message): driver_type: builtins.str """Log body: driver category (storage, power, network, etc.).""" namespace: builtins.str - """Loki stream label: Kubernetes namespace (bounded by cluster size).""" + """Capped at 16 entries, 64-char keys, 256-char values. + Loki stream label: Kubernetes namespace (bounded by cluster size). + """ @property def timestamp(self) -> google.protobuf.timestamp_pb2.Timestamp: """When the log was emitted.""" diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py index 7be5e24a0..e34c217b6 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py @@ -6,7 +6,7 @@ class TelemetryServiceStub: - """A service that receives structured logs from exporters and clients. + """A service that reverse-scrapes exporter metrics and receives structured logs. Implemented by jumpstarter-telemetry; not part of the controller. """ @@ -16,6 +16,11 @@ def __init__(self, channel): Args: channel: A grpc.Channel. """ + self.MetricsStream = channel.stream_stream( + '/jumpstarter.v1.TelemetryService/MetricsStream', + request_serializer=jumpstarter_dot_v1_dot_telemetry__pb2.MetricsStreamRequest.SerializeToString, + response_deserializer=jumpstarter_dot_v1_dot_telemetry__pb2.MetricsStreamResponse.FromString, + _registered_method=True) self.PushLogs = channel.unary_unary( '/jumpstarter.v1.TelemetryService/PushLogs', request_serializer=jumpstarter_dot_v1_dot_telemetry__pb2.PushLogsRequest.SerializeToString, @@ -24,10 +29,18 @@ def __init__(self, channel): class TelemetryServiceServicer: - """A service that receives structured logs from exporters and clients. + """A service that reverse-scrapes exporter metrics and receives structured logs. Implemented by jumpstarter-telemetry; not part of the controller. """ + def MetricsStream(self, request_iterator, context): + """Persistent bidirectional stream: telemetry sends scrape requests, + exporter responds with full metric snapshots (OpenMetrics text). + """ + context.set_code(grpc.StatusCode.UNIMPLEMENTED) + context.set_details('Method not implemented!') + raise NotImplementedError('Method not implemented!') + def PushLogs(self, request, context): """Push structured log entries to the telemetry service for Loki ingest. """ @@ -38,6 +51,11 @@ def PushLogs(self, request, context): def add_TelemetryServiceServicer_to_server(servicer, server): rpc_method_handlers = { + 'MetricsStream': grpc.stream_stream_rpc_method_handler( + servicer.MetricsStream, + request_deserializer=jumpstarter_dot_v1_dot_telemetry__pb2.MetricsStreamRequest.FromString, + response_serializer=jumpstarter_dot_v1_dot_telemetry__pb2.MetricsStreamResponse.SerializeToString, + ), 'PushLogs': grpc.unary_unary_rpc_method_handler( servicer.PushLogs, request_deserializer=jumpstarter_dot_v1_dot_telemetry__pb2.PushLogsRequest.FromString, @@ -52,10 +70,37 @@ def add_TelemetryServiceServicer_to_server(servicer, server): # This class is part of an EXPERIMENTAL API. class TelemetryService: - """A service that receives structured logs from exporters and clients. + """A service that reverse-scrapes exporter metrics and receives structured logs. Implemented by jumpstarter-telemetry; not part of the controller. """ + @staticmethod + def MetricsStream(request_iterator, + target, + options=(), + channel_credentials=None, + call_credentials=None, + insecure=False, + compression=None, + wait_for_ready=None, + timeout=None, + metadata=None): + return grpc.experimental.stream_stream( + request_iterator, + target, + '/jumpstarter.v1.TelemetryService/MetricsStream', + jumpstarter_dot_v1_dot_telemetry__pb2.MetricsStreamRequest.SerializeToString, + jumpstarter_dot_v1_dot_telemetry__pb2.MetricsStreamResponse.FromString, + options, + channel_credentials, + insecure, + call_credentials, + compression, + wait_for_ready, + timeout, + metadata, + _registered_method=True) + @staticmethod def PushLogs(request, target, diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi index dc163b903..d810b4378 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi @@ -25,6 +25,22 @@ class _ServicerContext(grpc.ServicerContext, grpc.aio.ServicerContext): # type: GRPC_GENERATED_VERSION: str GRPC_VERSION: str +_TelemetryServiceMetricsStreamType = typing_extensions.TypeVar( + '_TelemetryServiceMetricsStreamType', + grpc.StreamStreamMultiCallable[ + jumpstarter.v1.telemetry_pb2.MetricsStreamRequest, + jumpstarter.v1.telemetry_pb2.MetricsStreamResponse, + ], + grpc.aio.StreamStreamMultiCallable[ + jumpstarter.v1.telemetry_pb2.MetricsStreamRequest, + jumpstarter.v1.telemetry_pb2.MetricsStreamResponse, + ], + default=grpc.StreamStreamMultiCallable[ + jumpstarter.v1.telemetry_pb2.MetricsStreamRequest, + jumpstarter.v1.telemetry_pb2.MetricsStreamResponse, + ], +) + _TelemetryServicePushLogsType = typing_extensions.TypeVar( '_TelemetryServicePushLogsType', grpc.UnaryUnaryMultiCallable[ @@ -41,13 +57,17 @@ _TelemetryServicePushLogsType = typing_extensions.TypeVar( ], ) -class TelemetryServiceStub(typing.Generic[_TelemetryServicePushLogsType]): - """A service that receives structured logs from exporters and clients. +class TelemetryServiceStub(typing.Generic[_TelemetryServiceMetricsStreamType, _TelemetryServicePushLogsType]): + """A service that reverse-scrapes exporter metrics and receives structured logs. Implemented by jumpstarter-telemetry; not part of the controller. """ @typing.overload def __init__(self: TelemetryServiceStub[ + grpc.StreamStreamMultiCallable[ + jumpstarter.v1.telemetry_pb2.MetricsStreamRequest, + jumpstarter.v1.telemetry_pb2.MetricsStreamResponse, + ], grpc.UnaryUnaryMultiCallable[ jumpstarter.v1.telemetry_pb2.PushLogsRequest, jumpstarter.v1.telemetry_pb2.PushLogsResponse, @@ -56,16 +76,29 @@ class TelemetryServiceStub(typing.Generic[_TelemetryServicePushLogsType]): @typing.overload def __init__(self: TelemetryServiceStub[ + grpc.aio.StreamStreamMultiCallable[ + jumpstarter.v1.telemetry_pb2.MetricsStreamRequest, + jumpstarter.v1.telemetry_pb2.MetricsStreamResponse, + ], grpc.aio.UnaryUnaryMultiCallable[ jumpstarter.v1.telemetry_pb2.PushLogsRequest, jumpstarter.v1.telemetry_pb2.PushLogsResponse, ], ], channel: grpc.aio.Channel) -> None: ... + MetricsStream: _TelemetryServiceMetricsStreamType + """Persistent bidirectional stream: telemetry sends scrape requests, + exporter responds with full metric snapshots (OpenMetrics text). + """ + PushLogs: _TelemetryServicePushLogsType """Push structured log entries to the telemetry service for Loki ingest.""" TelemetryServiceAsyncStub: typing_extensions.TypeAlias = TelemetryServiceStub[ + grpc.aio.StreamStreamMultiCallable[ + jumpstarter.v1.telemetry_pb2.MetricsStreamRequest, + jumpstarter.v1.telemetry_pb2.MetricsStreamResponse, + ], grpc.aio.UnaryUnaryMultiCallable[ jumpstarter.v1.telemetry_pb2.PushLogsRequest, jumpstarter.v1.telemetry_pb2.PushLogsResponse, @@ -73,10 +106,20 @@ TelemetryServiceAsyncStub: typing_extensions.TypeAlias = TelemetryServiceStub[ ] class TelemetryServiceServicer(metaclass=abc.ABCMeta): - """A service that receives structured logs from exporters and clients. + """A service that reverse-scrapes exporter metrics and receives structured logs. Implemented by jumpstarter-telemetry; not part of the controller. """ + @abc.abstractmethod + def MetricsStream( + self, + request_iterator: _MaybeAsyncIterator[jumpstarter.v1.telemetry_pb2.MetricsStreamRequest], + context: _ServicerContext, + ) -> typing.Union[collections.abc.Iterator[jumpstarter.v1.telemetry_pb2.MetricsStreamResponse], collections.abc.AsyncIterator[jumpstarter.v1.telemetry_pb2.MetricsStreamResponse]]: + """Persistent bidirectional stream: telemetry sends scrape requests, + exporter responds with full metric snapshots (OpenMetrics text). + """ + @abc.abstractmethod def PushLogs( self, From 517b5ad72edc906febd5704525306e08cba8c402 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Tue, 1 Sep 2026 20:35:14 -0230 Subject: [PATCH 02/14] fix: surface MetricsStream OpenMetrics parse failures (JEP-0013 Phase 3) Stop silently dropping unparseable exporter snapshots. Log the exporter and error, and increment jumpstarter_metrics_parse_errors_total so reverse-scrape omissions are visible on the same /metrics response. Co-authored-by: Cursor --- controller/internal/service/metrics_merge.go | 9 +- .../internal/service/metrics_merge_test.go | 106 ++++++++++++++++-- .../internal/service/metrics_stream_test.go | 67 +++++++++++ controller/internal/service/telemetry_http.go | 22 +++- .../internal/service/telemetry_service.go | 1 + 5 files changed, 190 insertions(+), 15 deletions(-) diff --git a/controller/internal/service/metrics_merge.go b/controller/internal/service/metrics_merge.go index 378e54af3..484b4e27e 100644 --- a/controller/internal/service/metrics_merge.go +++ b/controller/internal/service/metrics_merge.go @@ -35,6 +35,10 @@ const ( // scrapeTimeoutsMetric is incremented once per exporter that does not // answer a MetricsStream scrape within scrapeTimeout (JEP-0013). scrapeTimeoutsMetric = "jumpstarter_scrape_timeouts_total" + + // metricsParseErrorsMetric is incremented once per exporter snapshot that + // MetricsStream delivered but OpenMetrics parse rejected (JEP-0013). + metricsParseErrorsMetric = "jumpstarter_metrics_parse_errors_total" ) // DefaultDriverTypeEnum is the JEP-0013 default allowlist for driver_type. @@ -119,7 +123,7 @@ func encodeMetricFamilies(w io.Writer, families []*dto.MetricFamily) error { return nil } -func mergeSnapshots(snapshots []exporterSnapshot, extra []*dto.MetricFamily, cfgFor func(string) mergeConfig) []*dto.MetricFamily { +func mergeSnapshots(snapshots []exporterSnapshot, extra []*dto.MetricFamily, cfgFor func(string) mergeConfig, onParseError func(exporter string, err error)) []*dto.MetricFamily { byName := map[string]*dto.MetricFamily{} for _, f := range extra { if f == nil || f.GetName() == "" { @@ -133,6 +137,9 @@ func mergeSnapshots(snapshots []exporterSnapshot, extra []*dto.MetricFamily, cfg } families, err := parseMetricFamilies(snap.text) if err != nil { + if onParseError != nil { + onParseError(snap.name, err) + } continue } cfg := cfgFor(snap.name) diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go index 5f6dfe70a..72bdbbbc4 100644 --- a/controller/internal/service/metrics_merge_test.go +++ b/controller/internal/service/metrics_merge_test.go @@ -18,6 +18,7 @@ package service import ( "bytes" + "errors" "strings" "testing" @@ -25,6 +26,24 @@ import ( "github.com/prometheus/common/expfmt" ) +func labeledCounterValue(t *testing.T, mfs []*dto.MetricFamily, name, label, want string) float64 { + t.Helper() + for _, mf := range mfs { + if mf.GetName() != name && mf.GetName()+"_total" != name { + continue + } + for _, m := range mf.Metric { + for _, lp := range m.GetLabel() { + if lp.GetName() == label && lp.GetValue() == want { + return m.GetCounter().GetValue() + } + } + } + } + t.Fatalf("%s{%s=%q} missing from gathered families", name, label, want) + return 0 +} + func TestApplyMetric_OverwritesExporterRemapsDriverTypeAndFiltersExemplars(t *testing.T) { one := 1.0 m := &dto.Metric{ @@ -91,14 +110,32 @@ func TestApplyMetric_KeepsAllowlistedDriverType(t *testing.T) { } } -func TestMergeSnapshots_CombinesExportersAndSkipsInvalidText(t *testing.T) { - cfg := func(name string) mergeConfig { - return mergeConfig{ - exporterName: name, - driverTypes: setToMap(DefaultDriverTypeEnum), - exemplarKeys: setToMap(DefaultExemplarKeys), - } +// pythonOpenMetricsWithExemplar is the prometheus_client OpenMetrics form +// that prometheus/common v0.62's text fallback cannot parse (it treats '#' as +// a timestamp). This is the body exporters send after driver operations. +const pythonOpenMetricsWithExemplar = `# TYPE jumpstarter_operation_duration_seconds histogram +jumpstarter_operation_duration_seconds_bucket{exporter="sidekick",le="0.005",operation="on",result="success",driver_type="power"} 1.0 # {lease_id="lease-1"} 0.00262 1788299670.149 +jumpstarter_operation_duration_seconds_sum{exporter="sidekick",operation="on",result="success",driver_type="power"} 0.00262 +jumpstarter_operation_duration_seconds_count{exporter="sidekick",operation="on",result="success",driver_type="power"} 1.0 +# EOF +` + +func testMergeCfg(name string) mergeConfig { + return mergeConfig{ + exporterName: name, + driverTypes: setToMap(DefaultDriverTypeEnum), + exemplarKeys: setToMap(DefaultExemplarKeys), + } +} + +func TestParseMetricFamilies_OpenMetricsExemplarSuffixFails(t *testing.T) { + _, err := parseMetricFamilies([]byte(pythonOpenMetricsWithExemplar)) + if err == nil { + t.Fatal("expected parse error for OpenMetrics exemplar suffix") } +} + +func TestMergeSnapshots_CombinesExportersAndReportsInvalidText(t *testing.T) { textA := []byte(`# TYPE jumpstarter_operations_total counter # HELP jumpstarter_operations_total Total operations performed. jumpstarter_operations_total{exporter="a",operation="on",result="success",driver_type="power"} 2.0 @@ -108,11 +145,18 @@ jumpstarter_operations_total{exporter="a",operation="on",result="success",driver jumpstarter_operations_total{exporter="b",operation="off",result="success",driver_type="power"} 3.0 # EOF `) + type parseErrCall struct { + exporter string + err error + } + var calls []parseErrCall families := mergeSnapshots([]exporterSnapshot{ {name: "exp-a", text: textA}, {name: "exp-b", text: textB}, {name: "bad", text: []byte("not metrics")}, - }, nil, cfg) + }, nil, testMergeCfg, func(exporter string, err error) { + calls = append(calls, parseErrCall{exporter: exporter, err: err}) + }) var ops *dto.MetricFamily for _, f := range families { @@ -135,6 +179,52 @@ jumpstarter_operations_total{exporter="b",operation="off",result="success",drive if !exporters["exp-a"] || !exporters["exp-b"] { t.Errorf("exporters = %v, want exp-a and exp-b", exporters) } + if len(calls) != 1 { + t.Fatalf("parse-error callbacks = %d, want 1", len(calls)) + } + if calls[0].exporter != "bad" { + t.Errorf("parse-error exporter = %q, want bad", calls[0].exporter) + } + if calls[0].err == nil { + t.Fatal("parse-error callback err is nil") + } +} + +func TestMergeSnapshots_ReportsOpenMetricsExemplarParseError(t *testing.T) { + var gotExporter string + var gotErr error + families := mergeSnapshots([]exporterSnapshot{ + {name: "sidekick", text: []byte(pythonOpenMetricsWithExemplar)}, + }, nil, testMergeCfg, func(exporter string, err error) { + gotExporter = exporter + gotErr = err + }) + for _, f := range families { + if strings.HasPrefix(f.GetName(), "jumpstarter_operation_duration_seconds") { + t.Fatalf("unparseable snapshot must be omitted, got family %s", f.GetName()) + } + } + if gotExporter != "sidekick" { + t.Errorf("parse-error exporter = %q, want sidekick", gotExporter) + } + if gotErr == nil { + t.Fatal("parse-error callback err is nil") + } +} + +func TestRecordMetricsParseError_IncrementsLabeledCounter(t *testing.T) { + svc := &TelemetryService{} + svc.initScrapeTimeouts() + svc.recordMetricsParseError("sidekick", errors.New("parse failed")) + + mfs, err := svc.metricsRegistry.Gather() + if err != nil { + t.Fatalf("Gather: %v", err) + } + value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, labelExporter, "sidekick") + if value != 1 { + t.Fatalf("%s{exporter=sidekick} = %v, want 1", metricsParseErrorsMetric, value) + } } func TestEncodeMetricFamilies_WritesOpenMetricsEOF(t *testing.T) { diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index 6f4a50cd0..6aed15c36 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -400,3 +400,70 @@ jumpstarter_operations_total{exporter="spoofed",operation="on",result="success", default: } } + +func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testing.T) { + svc, client, addr := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "sidekick"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + + errCh := make(chan error, 1) + go func() { + for { + msg, err := stream.Recv() + if err != nil { + errCh <- err + return + } + if msg.GetScrapeRequest() == nil { + continue + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_ScrapeResponse{ + ScrapeResponse: &pb.MetricsScrapeResponse{MetricsText: []byte(pythonOpenMetricsWithExemplar)}, + }, + }); err != nil { + errCh <- err + return + } + } + }() + waitRegistered(t, svc, 1) + + code, body := httpGet(t, "http://"+addr+"/metrics") + if code != http.StatusOK { + t.Fatalf("GET /metrics status=%d body=%s", code, body) + } + if strings.Contains(body, "jumpstarter_operation_duration_seconds") { + t.Errorf("unparseable exporter snapshot must be omitted, body:\n%s", body) + } + if !strings.Contains(body, metricsParseErrorsMetric) || !strings.Contains(body, `exporter="sidekick"`) { + t.Errorf("parse-error counter missing from same /metrics response:\n%s", body) + } + + mfs, err := svc.metricsRegistry.Gather() + if err != nil { + t.Fatalf("Gather: %v", err) + } + value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, labelExporter, "sidekick") + if value < 1 { + t.Fatalf("%s{exporter=sidekick} = %v, want >= 1 body:\n%s", metricsParseErrorsMetric, value, body) + } + + select { + case err := <-errCh: + if err != nil && err != io.EOF && status.Code(err) != codes.Canceled && status.Code(err) != codes.Unavailable { + t.Fatalf("stream goroutine: %v", err) + } + default: + } +} diff --git a/controller/internal/service/telemetry_http.go b/controller/internal/service/telemetry_http.go index 6d12daff4..ecd403019 100644 --- a/controller/internal/service/telemetry_http.go +++ b/controller/internal/service/telemetry_http.go @@ -24,7 +24,6 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" - dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" ctrl "sigs.k8s.io/controller-runtime" ) @@ -40,7 +39,18 @@ func (s *TelemetryService) initScrapeTimeouts() { Name: scrapeTimeoutsMetric, Help: "Exporter MetricsStream scrapes that exceeded scrapeTimeout.", }) - s.metricsRegistry.MustRegister(s.scrapeTimeouts) + s.parseErrors = prometheus.NewCounterVec(prometheus.CounterOpts{ + Name: metricsParseErrorsMetric, + Help: "Exporter MetricsStream snapshots omitted because OpenMetrics parse failed.", + }, []string{labelExporter}) + s.metricsRegistry.MustRegister(s.scrapeTimeouts, s.parseErrors) +} + +func (s *TelemetryService) recordMetricsParseError(exporter string, err error) { + if s.parseErrors != nil { + s.parseErrors.WithLabelValues(exporter).Inc() + } + ctrl.Log.WithName("telemetry").Error(err, "exporter metrics snapshot omitted", logFieldExporter, exporter) } func metricsHTTPEnabled(addr string) bool { @@ -110,19 +120,19 @@ func (s *TelemetryService) handleReadyz(w http.ResponseWriter, _ *http.Request) func (s *TelemetryService) handleMetrics(w http.ResponseWriter, r *http.Request) { s.initScrapeTimeouts() snaps := s.fanoutScrapes(r.Context()) + // Merge exporter snapshots first so parse failures increment parseErrors + // before hub metrics are gathered into the same /metrics response. + families := mergeSnapshots(snaps, nil, s.mergeConfigFor, s.recordMetricsParseError) - var extra []*dto.MetricFamily if s.metricsRegistry != nil { gathered, err := s.metricsRegistry.Gather() if err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) return } - extra = gathered + families = mergeSnapshots(nil, append(gathered, families...), s.mergeConfigFor, nil) } - families := mergeSnapshots(snaps, extra, s.mergeConfigFor) - var buf bytes.Buffer if err := encodeMetricFamilies(&buf, families); err != nil { http.Error(w, err.Error(), http.StatusInternalServerError) diff --git a/controller/internal/service/telemetry_service.go b/controller/internal/service/telemetry_service.go index e3c60bb77..b2be84936 100644 --- a/controller/internal/service/telemetry_service.go +++ b/controller/internal/service/telemetry_service.go @@ -95,6 +95,7 @@ type TelemetryService struct { stateMu sync.Mutex conns map[string]*metricsConn scrapeTimeouts prometheus.Counter + parseErrors *prometheus.CounterVec metricsRegistry *prometheus.Registry metricsAddr string grpcReady atomic.Bool From a94b7a959175107995c89fce558aaf6d1378acff Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Thu, 3 Sep 2026 09:59:33 -0230 Subject: [PATCH 03/14] fix: unparseable becomes unparsable as per typo check Signed-off-by: Roddie Kieley --- controller/internal/service/metrics_merge_test.go | 2 +- controller/internal/service/metrics_stream_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go index 72bdbbbc4..984612040 100644 --- a/controller/internal/service/metrics_merge_test.go +++ b/controller/internal/service/metrics_merge_test.go @@ -201,7 +201,7 @@ func TestMergeSnapshots_ReportsOpenMetricsExemplarParseError(t *testing.T) { }) for _, f := range families { if strings.HasPrefix(f.GetName(), "jumpstarter_operation_duration_seconds") { - t.Fatalf("unparseable snapshot must be omitted, got family %s", f.GetName()) + t.Fatalf("unparsable snapshot must be omitted, got family %s", f.GetName()) } } if gotExporter != "sidekick" { diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index 6aed15c36..ed2f1356f 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -444,7 +444,7 @@ func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testin t.Fatalf("GET /metrics status=%d body=%s", code, body) } if strings.Contains(body, "jumpstarter_operation_duration_seconds") { - t.Errorf("unparseable exporter snapshot must be omitted, body:\n%s", body) + t.Errorf("unparsable exporter snapshot must be omitted, body:\n%s", body) } if !strings.Contains(body, metricsParseErrorsMetric) || !strings.Contains(body, `exporter="sidekick"`) { t.Errorf("parse-error counter missing from same /metrics response:\n%s", body) From 0f2a623b091346f952de17de896f9445d37ba09a Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 09:55:34 -0230 Subject: [PATCH 04/14] feat: add MetricsFamily DD-3 sidecar for MetricsStream (JEP-0013) prometheus/common cannot parse OpenMetrics exemplar suffixes, so exporters send structured registry families for the hub to merge instead of relying on metrics_text. Co-authored-by: Cursor --- .../protocol/jumpstarter/v1/telemetry.pb.go | 464 ++++++++++++++++-- .../jumpstarter/v1/telemetry_grpc.pb.go | 4 +- .../internal/service/metrics_families.go | 326 ++++++++++++ .../internal/service/metrics_families_test.go | 250 ++++++++++ controller/internal/service/metrics_merge.go | 24 +- controller/internal/service/metrics_stream.go | 12 +- .../internal/service/metrics_stream_test.go | 92 ++++ protocol/proto/jumpstarter/v1/telemetry.proto | 48 +- .../jumpstarter/v1/telemetry_pb2.py | 44 +- .../jumpstarter/v1/telemetry_pb2.pyi | 173 ++++++- .../jumpstarter/v1/telemetry_pb2_grpc.py | 2 +- .../jumpstarter/v1/telemetry_pb2_grpc.pyi | 4 +- .../jumpstarter/metrics/__init__.py | 3 + .../jumpstarter/metrics/families.py | 126 +++++ .../jumpstarter/metrics/metrics_test.py | 43 ++ 15 files changed, 1525 insertions(+), 90 deletions(-) create mode 100644 controller/internal/service/metrics_families.go create mode 100644 controller/internal/service/metrics_families_test.go create mode 100644 python/packages/jumpstarter/jumpstarter/metrics/families.py diff --git a/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go b/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go index ee0ac7352..092ba99ed 100644 --- a/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go +++ b/controller/internal/protocol/jumpstarter/v1/telemetry.pb.go @@ -2,7 +2,7 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.11 +// protoc-gen-go v1.36.12 // protoc (unknown) // source: jumpstarter/v1/telemetry.proto @@ -24,6 +24,65 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) +// Prometheus metric type for a MetricsFamily. +type MetricsType int32 + +const ( + MetricsType_METRICS_TYPE_UNSPECIFIED MetricsType = 0 // Unknown or unset type. + MetricsType_METRICS_TYPE_COUNTER MetricsType = 1 // Monotonic counter. + MetricsType_METRICS_TYPE_GAUGE MetricsType = 2 // Gauge. + MetricsType_METRICS_TYPE_HISTOGRAM MetricsType = 3 // Histogram (bucket / sum / count samples). + MetricsType_METRICS_TYPE_SUMMARY MetricsType = 4 // Summary. + MetricsType_METRICS_TYPE_UNTYPED MetricsType = 5 // Untyped / unknown. +) + +// Enum value maps for MetricsType. +var ( + MetricsType_name = map[int32]string{ + 0: "METRICS_TYPE_UNSPECIFIED", + 1: "METRICS_TYPE_COUNTER", + 2: "METRICS_TYPE_GAUGE", + 3: "METRICS_TYPE_HISTOGRAM", + 4: "METRICS_TYPE_SUMMARY", + 5: "METRICS_TYPE_UNTYPED", + } + MetricsType_value = map[string]int32{ + "METRICS_TYPE_UNSPECIFIED": 0, + "METRICS_TYPE_COUNTER": 1, + "METRICS_TYPE_GAUGE": 2, + "METRICS_TYPE_HISTOGRAM": 3, + "METRICS_TYPE_SUMMARY": 4, + "METRICS_TYPE_UNTYPED": 5, + } +) + +func (x MetricsType) Enum() *MetricsType { + p := new(MetricsType) + *p = x + return p +} + +func (x MetricsType) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (MetricsType) Descriptor() protoreflect.EnumDescriptor { + return file_jumpstarter_v1_telemetry_proto_enumTypes[0].Descriptor() +} + +func (MetricsType) Type() protoreflect.EnumType { + return &file_jumpstarter_v1_telemetry_proto_enumTypes[0] +} + +func (x MetricsType) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use MetricsType.Descriptor instead. +func (MetricsType) EnumDescriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{0} +} + // Exporter → Telemetry type MetricsStreamRequest struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -152,9 +211,12 @@ func (x *MetricsRegister) GetIdentity() string { } type MetricsScrapeResponse struct { - state protoimpl.MessageState `protogen:"open.v1"` - MetricsText []byte `protobuf:"bytes,1,opt,name=metrics_text,json=metricsText,proto3" json:"metrics_text,omitempty"` // generate_latest() OpenMetrics output. - Timestamp *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=timestamp,proto3" json:"timestamp,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + MetricsText []byte `protobuf:"bytes,1,opt,name=metrics_text,json=metricsText,proto3" json:"metrics_text,omitempty"` // Optional generate_latest() OpenMetrics text (legacy / debug). + Timestamp *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=timestamp,proto3" json:"timestamp,omitempty"` // When the snapshot was taken. + // DD-3 sidecar: in-memory registry dump so the hub can merge and filter + // exemplars without parsing OpenMetrics text (prometheus/common cannot). + Families []*MetricsFamily `protobuf:"bytes,3,rep,name=families,proto3" json:"families,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -203,6 +265,265 @@ func (x *MetricsScrapeResponse) GetTimestamp() *timestamppb.Timestamp { return nil } +func (x *MetricsScrapeResponse) GetFamilies() []*MetricsFamily { + if x != nil { + return x.Families + } + return nil +} + +// One label name/value pair on a sample or exemplar. +type MetricsLabel struct { + state protoimpl.MessageState `protogen:"open.v1"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // Label name. + Value string `protobuf:"bytes,2,opt,name=value,proto3" json:"value,omitempty"` // Label value. + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsLabel) Reset() { + *x = MetricsLabel{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[3] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsLabel) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsLabel) ProtoMessage() {} + +func (x *MetricsLabel) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[3] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsLabel.ProtoReflect.Descriptor instead. +func (*MetricsLabel) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{3} +} + +func (x *MetricsLabel) GetName() string { + if x != nil { + return x.Name + } + return "" +} + +func (x *MetricsLabel) GetValue() string { + if x != nil { + return x.Value + } + return "" +} + +// OpenMetrics exemplar attached to a sample (JEP-0013 allowlist applied on the hub). +type MetricsExemplar struct { + state protoimpl.MessageState `protogen:"open.v1"` + Labels []*MetricsLabel `protobuf:"bytes,1,rep,name=labels,proto3" json:"labels,omitempty"` // Exemplar labels (client, lease_id, …). + Value float64 `protobuf:"fixed64,2,opt,name=value,proto3" json:"value,omitempty"` // Exemplar observation value. + Timestamp *timestamppb.Timestamp `protobuf:"bytes,3,opt,name=timestamp,proto3" json:"timestamp,omitempty"` // Optional exemplar timestamp. + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsExemplar) Reset() { + *x = MetricsExemplar{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[4] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsExemplar) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsExemplar) ProtoMessage() {} + +func (x *MetricsExemplar) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[4] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsExemplar.ProtoReflect.Descriptor instead. +func (*MetricsExemplar) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{4} +} + +func (x *MetricsExemplar) GetLabels() []*MetricsLabel { + if x != nil { + return x.Labels + } + return nil +} + +func (x *MetricsExemplar) GetValue() float64 { + if x != nil { + return x.Value + } + return 0 +} + +func (x *MetricsExemplar) GetTimestamp() *timestamppb.Timestamp { + if x != nil { + return x.Timestamp + } + return nil +} + +// One collected sample from prometheus_client (including histogram _bucket/_sum/_count). +type MetricsSample struct { + state protoimpl.MessageState `protogen:"open.v1"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // Sample name (may include _bucket, _sum, _count). + Labels []*MetricsLabel `protobuf:"bytes,2,rep,name=labels,proto3" json:"labels,omitempty"` // Sample labels (histogram buckets include le). + Value float64 `protobuf:"fixed64,3,opt,name=value,proto3" json:"value,omitempty"` // Sample value. + Exemplar *MetricsExemplar `protobuf:"bytes,4,opt,name=exemplar,proto3" json:"exemplar,omitempty"` // Optional exemplar for this sample. + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsSample) Reset() { + *x = MetricsSample{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[5] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsSample) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsSample) ProtoMessage() {} + +func (x *MetricsSample) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[5] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsSample.ProtoReflect.Descriptor instead. +func (*MetricsSample) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{5} +} + +func (x *MetricsSample) GetName() string { + if x != nil { + return x.Name + } + return "" +} + +func (x *MetricsSample) GetLabels() []*MetricsLabel { + if x != nil { + return x.Labels + } + return nil +} + +func (x *MetricsSample) GetValue() float64 { + if x != nil { + return x.Value + } + return 0 +} + +func (x *MetricsSample) GetExemplar() *MetricsExemplar { + if x != nil { + return x.Exemplar + } + return nil +} + +// One metric family from CollectorRegistry.collect(). +type MetricsFamily struct { + state protoimpl.MessageState `protogen:"open.v1"` + Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // Family name (counters include _total; no _bucket/_sum/_count). + Help string `protobuf:"bytes,2,opt,name=help,proto3" json:"help,omitempty"` // Help text. + Type MetricsType `protobuf:"varint,3,opt,name=type,proto3,enum=jumpstarter.v1.MetricsType" json:"type,omitempty"` // Family type. + Samples []*MetricsSample `protobuf:"bytes,4,rep,name=samples,proto3" json:"samples,omitempty"` // Collected samples, including exemplars. + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache +} + +func (x *MetricsFamily) Reset() { + *x = MetricsFamily{} + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[6] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) +} + +func (x *MetricsFamily) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*MetricsFamily) ProtoMessage() {} + +func (x *MetricsFamily) ProtoReflect() protoreflect.Message { + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[6] + if x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use MetricsFamily.ProtoReflect.Descriptor instead. +func (*MetricsFamily) Descriptor() ([]byte, []int) { + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{6} +} + +func (x *MetricsFamily) GetName() string { + if x != nil { + return x.Name + } + return "" +} + +func (x *MetricsFamily) GetHelp() string { + if x != nil { + return x.Help + } + return "" +} + +func (x *MetricsFamily) GetType() MetricsType { + if x != nil { + return x.Type + } + return MetricsType_METRICS_TYPE_UNSPECIFIED +} + +func (x *MetricsFamily) GetSamples() []*MetricsSample { + if x != nil { + return x.Samples + } + return nil +} + // Telemetry → Exporter type MetricsStreamResponse struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -216,7 +537,7 @@ type MetricsStreamResponse struct { func (x *MetricsStreamResponse) Reset() { *x = MetricsStreamResponse{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[3] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[7] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -228,7 +549,7 @@ func (x *MetricsStreamResponse) String() string { func (*MetricsStreamResponse) ProtoMessage() {} func (x *MetricsStreamResponse) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[3] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[7] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -241,7 +562,7 @@ func (x *MetricsStreamResponse) ProtoReflect() protoreflect.Message { // Deprecated: Use MetricsStreamResponse.ProtoReflect.Descriptor instead. func (*MetricsStreamResponse) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{3} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{7} } func (x *MetricsStreamResponse) GetMsg() isMetricsStreamResponse_Msg { @@ -279,7 +600,7 @@ type MetricsScrapeRequest struct { func (x *MetricsScrapeRequest) Reset() { *x = MetricsScrapeRequest{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[4] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[8] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -291,7 +612,7 @@ func (x *MetricsScrapeRequest) String() string { func (*MetricsScrapeRequest) ProtoMessage() {} func (x *MetricsScrapeRequest) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[4] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[8] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -304,7 +625,7 @@ func (x *MetricsScrapeRequest) ProtoReflect() protoreflect.Message { // Deprecated: Use MetricsScrapeRequest.ProtoReflect.Descriptor instead. func (*MetricsScrapeRequest) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{4} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{8} } // Request to push log entries to the telemetry service. @@ -317,7 +638,7 @@ type PushLogsRequest struct { func (x *PushLogsRequest) Reset() { *x = PushLogsRequest{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[5] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[9] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -329,7 +650,7 @@ func (x *PushLogsRequest) String() string { func (*PushLogsRequest) ProtoMessage() {} func (x *PushLogsRequest) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[5] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[9] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -342,7 +663,7 @@ func (x *PushLogsRequest) ProtoReflect() protoreflect.Message { // Deprecated: Use PushLogsRequest.ProtoReflect.Descriptor instead. func (*PushLogsRequest) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{5} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{9} } func (x *PushLogsRequest) GetEntries() []*LogEntry { @@ -363,7 +684,7 @@ type PushLogsResponse struct { func (x *PushLogsResponse) Reset() { *x = PushLogsResponse{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[6] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[10] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -375,7 +696,7 @@ func (x *PushLogsResponse) String() string { func (*PushLogsResponse) ProtoMessage() {} func (x *PushLogsResponse) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[6] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[10] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -388,7 +709,7 @@ func (x *PushLogsResponse) ProtoReflect() protoreflect.Message { // Deprecated: Use PushLogsResponse.ProtoReflect.Descriptor instead. func (*PushLogsResponse) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{6} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{10} } func (x *PushLogsResponse) GetAccepted() uint32 { @@ -428,7 +749,7 @@ type LogEntry struct { func (x *LogEntry) Reset() { *x = LogEntry{} - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[7] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[11] ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) ms.StoreMessageInfo(mi) } @@ -440,7 +761,7 @@ func (x *LogEntry) String() string { func (*LogEntry) ProtoMessage() {} func (x *LogEntry) ProtoReflect() protoreflect.Message { - mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[7] + mi := &file_jumpstarter_v1_telemetry_proto_msgTypes[11] if x != nil { ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) if ms.LoadMessageInfo() == nil { @@ -453,7 +774,7 @@ func (x *LogEntry) ProtoReflect() protoreflect.Message { // Deprecated: Use LogEntry.ProtoReflect.Descriptor instead. func (*LogEntry) Descriptor() ([]byte, []int) { - return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{7} + return file_jumpstarter_v1_telemetry_proto_rawDescGZIP(), []int{11} } func (x *LogEntry) GetTimestamp() *timestamppb.Timestamp { @@ -550,10 +871,28 @@ const file_jumpstarter_v1_telemetry_proto_rawDesc = "" + "\x0fscrape_response\x18\x02 \x01(\v2%.jumpstarter.v1.MetricsScrapeResponseH\x00R\x0escrapeResponseB\x05\n" + "\x03msg\"-\n" + "\x0fMetricsRegister\x12\x1a\n" + - "\bidentity\x18\x01 \x01(\tR\bidentity\"t\n" + + "\bidentity\x18\x01 \x01(\tR\bidentity\"\xaf\x01\n" + "\x15MetricsScrapeResponse\x12!\n" + "\fmetrics_text\x18\x01 \x01(\fR\vmetricsText\x128\n" + - "\ttimestamp\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\ttimestamp\"m\n" + + "\ttimestamp\x18\x02 \x01(\v2\x1a.google.protobuf.TimestampR\ttimestamp\x129\n" + + "\bfamilies\x18\x03 \x03(\v2\x1d.jumpstarter.v1.MetricsFamilyR\bfamilies\"8\n" + + "\fMetricsLabel\x12\x12\n" + + "\x04name\x18\x01 \x01(\tR\x04name\x12\x14\n" + + "\x05value\x18\x02 \x01(\tR\x05value\"\x97\x01\n" + + "\x0fMetricsExemplar\x124\n" + + "\x06labels\x18\x01 \x03(\v2\x1c.jumpstarter.v1.MetricsLabelR\x06labels\x12\x14\n" + + "\x05value\x18\x02 \x01(\x01R\x05value\x128\n" + + "\ttimestamp\x18\x03 \x01(\v2\x1a.google.protobuf.TimestampR\ttimestamp\"\xac\x01\n" + + "\rMetricsSample\x12\x12\n" + + "\x04name\x18\x01 \x01(\tR\x04name\x124\n" + + "\x06labels\x18\x02 \x03(\v2\x1c.jumpstarter.v1.MetricsLabelR\x06labels\x12\x14\n" + + "\x05value\x18\x03 \x01(\x01R\x05value\x12;\n" + + "\bexemplar\x18\x04 \x01(\v2\x1f.jumpstarter.v1.MetricsExemplarR\bexemplar\"\xa1\x01\n" + + "\rMetricsFamily\x12\x12\n" + + "\x04name\x18\x01 \x01(\tR\x04name\x12\x12\n" + + "\x04help\x18\x02 \x01(\tR\x04help\x12/\n" + + "\x04type\x18\x03 \x01(\x0e2\x1b.jumpstarter.v1.MetricsTypeR\x04type\x127\n" + + "\asamples\x18\x04 \x03(\v2\x1d.jumpstarter.v1.MetricsSampleR\asamples\"m\n" + "\x15MetricsStreamResponse\x12M\n" + "\x0escrape_request\x18\x01 \x01(\v2$.jumpstarter.v1.MetricsScrapeRequestH\x00R\rscrapeRequestB\x05\n" + "\x03msg\"\x16\n" + @@ -580,7 +919,14 @@ const file_jumpstarter_v1_telemetry_proto_rawDesc = "" + "\tnamespace\x18\f \x01(\tR\tnamespace\x1a>\n" + "\x10ExtraFieldsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x012\xc3\x01\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01*\xad\x01\n" + + "\vMetricsType\x12\x1c\n" + + "\x18METRICS_TYPE_UNSPECIFIED\x10\x00\x12\x18\n" + + "\x14METRICS_TYPE_COUNTER\x10\x01\x12\x16\n" + + "\x12METRICS_TYPE_GAUGE\x10\x02\x12\x1a\n" + + "\x16METRICS_TYPE_HISTOGRAM\x10\x03\x12\x18\n" + + "\x14METRICS_TYPE_SUMMARY\x10\x04\x12\x18\n" + + "\x14METRICS_TYPE_UNTYPED\x10\x052\xc3\x01\n" + "\x10TelemetryService\x12`\n" + "\rMetricsStream\x12$.jumpstarter.v1.MetricsStreamRequest\x1a%.jumpstarter.v1.MetricsStreamResponse(\x010\x01\x12M\n" + "\bPushLogs\x12\x1f.jumpstarter.v1.PushLogsRequest\x1a .jumpstarter.v1.PushLogsResponseB\xdf\x01\n" + @@ -598,36 +944,49 @@ func file_jumpstarter_v1_telemetry_proto_rawDescGZIP() []byte { return file_jumpstarter_v1_telemetry_proto_rawDescData } -var file_jumpstarter_v1_telemetry_proto_msgTypes = make([]protoimpl.MessageInfo, 9) +var file_jumpstarter_v1_telemetry_proto_enumTypes = make([]protoimpl.EnumInfo, 1) +var file_jumpstarter_v1_telemetry_proto_msgTypes = make([]protoimpl.MessageInfo, 13) var file_jumpstarter_v1_telemetry_proto_goTypes = []any{ - (*MetricsStreamRequest)(nil), // 0: jumpstarter.v1.MetricsStreamRequest - (*MetricsRegister)(nil), // 1: jumpstarter.v1.MetricsRegister - (*MetricsScrapeResponse)(nil), // 2: jumpstarter.v1.MetricsScrapeResponse - (*MetricsStreamResponse)(nil), // 3: jumpstarter.v1.MetricsStreamResponse - (*MetricsScrapeRequest)(nil), // 4: jumpstarter.v1.MetricsScrapeRequest - (*PushLogsRequest)(nil), // 5: jumpstarter.v1.PushLogsRequest - (*PushLogsResponse)(nil), // 6: jumpstarter.v1.PushLogsResponse - (*LogEntry)(nil), // 7: jumpstarter.v1.LogEntry - nil, // 8: jumpstarter.v1.LogEntry.ExtraFieldsEntry - (*timestamppb.Timestamp)(nil), // 9: google.protobuf.Timestamp + (MetricsType)(0), // 0: jumpstarter.v1.MetricsType + (*MetricsStreamRequest)(nil), // 1: jumpstarter.v1.MetricsStreamRequest + (*MetricsRegister)(nil), // 2: jumpstarter.v1.MetricsRegister + (*MetricsScrapeResponse)(nil), // 3: jumpstarter.v1.MetricsScrapeResponse + (*MetricsLabel)(nil), // 4: jumpstarter.v1.MetricsLabel + (*MetricsExemplar)(nil), // 5: jumpstarter.v1.MetricsExemplar + (*MetricsSample)(nil), // 6: jumpstarter.v1.MetricsSample + (*MetricsFamily)(nil), // 7: jumpstarter.v1.MetricsFamily + (*MetricsStreamResponse)(nil), // 8: jumpstarter.v1.MetricsStreamResponse + (*MetricsScrapeRequest)(nil), // 9: jumpstarter.v1.MetricsScrapeRequest + (*PushLogsRequest)(nil), // 10: jumpstarter.v1.PushLogsRequest + (*PushLogsResponse)(nil), // 11: jumpstarter.v1.PushLogsResponse + (*LogEntry)(nil), // 12: jumpstarter.v1.LogEntry + nil, // 13: jumpstarter.v1.LogEntry.ExtraFieldsEntry + (*timestamppb.Timestamp)(nil), // 14: google.protobuf.Timestamp } var file_jumpstarter_v1_telemetry_proto_depIdxs = []int32{ - 1, // 0: jumpstarter.v1.MetricsStreamRequest.register:type_name -> jumpstarter.v1.MetricsRegister - 2, // 1: jumpstarter.v1.MetricsStreamRequest.scrape_response:type_name -> jumpstarter.v1.MetricsScrapeResponse - 9, // 2: jumpstarter.v1.MetricsScrapeResponse.timestamp:type_name -> google.protobuf.Timestamp - 4, // 3: jumpstarter.v1.MetricsStreamResponse.scrape_request:type_name -> jumpstarter.v1.MetricsScrapeRequest - 7, // 4: jumpstarter.v1.PushLogsRequest.entries:type_name -> jumpstarter.v1.LogEntry - 9, // 5: jumpstarter.v1.LogEntry.timestamp:type_name -> google.protobuf.Timestamp - 8, // 6: jumpstarter.v1.LogEntry.extra_fields:type_name -> jumpstarter.v1.LogEntry.ExtraFieldsEntry - 0, // 7: jumpstarter.v1.TelemetryService.MetricsStream:input_type -> jumpstarter.v1.MetricsStreamRequest - 5, // 8: jumpstarter.v1.TelemetryService.PushLogs:input_type -> jumpstarter.v1.PushLogsRequest - 3, // 9: jumpstarter.v1.TelemetryService.MetricsStream:output_type -> jumpstarter.v1.MetricsStreamResponse - 6, // 10: jumpstarter.v1.TelemetryService.PushLogs:output_type -> jumpstarter.v1.PushLogsResponse - 9, // [9:11] is the sub-list for method output_type - 7, // [7:9] is the sub-list for method input_type - 7, // [7:7] is the sub-list for extension type_name - 7, // [7:7] is the sub-list for extension extendee - 0, // [0:7] is the sub-list for field type_name + 2, // 0: jumpstarter.v1.MetricsStreamRequest.register:type_name -> jumpstarter.v1.MetricsRegister + 3, // 1: jumpstarter.v1.MetricsStreamRequest.scrape_response:type_name -> jumpstarter.v1.MetricsScrapeResponse + 14, // 2: jumpstarter.v1.MetricsScrapeResponse.timestamp:type_name -> google.protobuf.Timestamp + 7, // 3: jumpstarter.v1.MetricsScrapeResponse.families:type_name -> jumpstarter.v1.MetricsFamily + 4, // 4: jumpstarter.v1.MetricsExemplar.labels:type_name -> jumpstarter.v1.MetricsLabel + 14, // 5: jumpstarter.v1.MetricsExemplar.timestamp:type_name -> google.protobuf.Timestamp + 4, // 6: jumpstarter.v1.MetricsSample.labels:type_name -> jumpstarter.v1.MetricsLabel + 5, // 7: jumpstarter.v1.MetricsSample.exemplar:type_name -> jumpstarter.v1.MetricsExemplar + 0, // 8: jumpstarter.v1.MetricsFamily.type:type_name -> jumpstarter.v1.MetricsType + 6, // 9: jumpstarter.v1.MetricsFamily.samples:type_name -> jumpstarter.v1.MetricsSample + 9, // 10: jumpstarter.v1.MetricsStreamResponse.scrape_request:type_name -> jumpstarter.v1.MetricsScrapeRequest + 12, // 11: jumpstarter.v1.PushLogsRequest.entries:type_name -> jumpstarter.v1.LogEntry + 14, // 12: jumpstarter.v1.LogEntry.timestamp:type_name -> google.protobuf.Timestamp + 13, // 13: jumpstarter.v1.LogEntry.extra_fields:type_name -> jumpstarter.v1.LogEntry.ExtraFieldsEntry + 1, // 14: jumpstarter.v1.TelemetryService.MetricsStream:input_type -> jumpstarter.v1.MetricsStreamRequest + 10, // 15: jumpstarter.v1.TelemetryService.PushLogs:input_type -> jumpstarter.v1.PushLogsRequest + 8, // 16: jumpstarter.v1.TelemetryService.MetricsStream:output_type -> jumpstarter.v1.MetricsStreamResponse + 11, // 17: jumpstarter.v1.TelemetryService.PushLogs:output_type -> jumpstarter.v1.PushLogsResponse + 16, // [16:18] is the sub-list for method output_type + 14, // [14:16] is the sub-list for method input_type + 14, // [14:14] is the sub-list for extension type_name + 14, // [14:14] is the sub-list for extension extendee + 0, // [0:14] is the sub-list for field type_name } func init() { file_jumpstarter_v1_telemetry_proto_init() } @@ -639,7 +998,7 @@ func file_jumpstarter_v1_telemetry_proto_init() { (*MetricsStreamRequest_Register)(nil), (*MetricsStreamRequest_ScrapeResponse)(nil), } - file_jumpstarter_v1_telemetry_proto_msgTypes[3].OneofWrappers = []any{ + file_jumpstarter_v1_telemetry_proto_msgTypes[7].OneofWrappers = []any{ (*MetricsStreamResponse_ScrapeRequest)(nil), } type x struct{} @@ -647,13 +1006,14 @@ func file_jumpstarter_v1_telemetry_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_jumpstarter_v1_telemetry_proto_rawDesc), len(file_jumpstarter_v1_telemetry_proto_rawDesc)), - NumEnums: 0, - NumMessages: 9, + NumEnums: 1, + NumMessages: 13, NumExtensions: 0, NumServices: 1, }, GoTypes: file_jumpstarter_v1_telemetry_proto_goTypes, DependencyIndexes: file_jumpstarter_v1_telemetry_proto_depIdxs, + EnumInfos: file_jumpstarter_v1_telemetry_proto_enumTypes, MessageInfos: file_jumpstarter_v1_telemetry_proto_msgTypes, }.Build() File_jumpstarter_v1_telemetry_proto = out.File diff --git a/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go b/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go index d5db4a9a7..66d5cb256 100644 --- a/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go +++ b/controller/internal/protocol/jumpstarter/v1/telemetry_grpc.pb.go @@ -33,7 +33,7 @@ const ( // Implemented by jumpstarter-telemetry; not part of the controller. type TelemetryServiceClient interface { // Persistent bidirectional stream: telemetry sends scrape requests, - // exporter responds with full metric snapshots (OpenMetrics text). + // exporter responds with metric snapshots (structured families plus optional OpenMetrics text). MetricsStream(ctx context.Context, opts ...grpc.CallOption) (grpc.BidiStreamingClient[MetricsStreamRequest, MetricsStreamResponse], error) // Push structured log entries to the telemetry service for Loki ingest. PushLogs(ctx context.Context, in *PushLogsRequest, opts ...grpc.CallOption) (*PushLogsResponse, error) @@ -78,7 +78,7 @@ func (c *telemetryServiceClient) PushLogs(ctx context.Context, in *PushLogsReque // Implemented by jumpstarter-telemetry; not part of the controller. type TelemetryServiceServer interface { // Persistent bidirectional stream: telemetry sends scrape requests, - // exporter responds with full metric snapshots (OpenMetrics text). + // exporter responds with metric snapshots (structured families plus optional OpenMetrics text). MetricsStream(grpc.BidiStreamingServer[MetricsStreamRequest, MetricsStreamResponse]) error // Push structured log entries to the telemetry service for Loki ingest. PushLogs(context.Context, *PushLogsRequest) (*PushLogsResponse, error) diff --git a/controller/internal/service/metrics_families.go b/controller/internal/service/metrics_families.go new file mode 100644 index 000000000..be5963166 --- /dev/null +++ b/controller/internal/service/metrics_families.go @@ -0,0 +1,326 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "fmt" + "math" + "sort" + "strconv" + "strings" + + pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" + dto "github.com/prometheus/client_model/go" + "google.golang.org/protobuf/types/known/timestamppb" +) + +func dtoFromProtoFamilies(in []*pb.MetricsFamily) ([]*dto.MetricFamily, error) { + out := make([]*dto.MetricFamily, 0, len(in)) + for _, fam := range in { + if fam == nil || fam.GetName() == "" { + continue + } + converted, err := dtoFromProtoFamily(fam) + if err != nil { + return nil, fmt.Errorf("family %s: %w", fam.GetName(), err) + } + if converted == nil || len(converted.Metric) == 0 { + continue + } + out = append(out, converted) + } + return out, nil +} + +func canonicalMetricFamilyName(fam *pb.MetricsFamily) string { + name := fam.GetName() + if fam.GetType() == pb.MetricsType_METRICS_TYPE_COUNTER && name != "" && !strings.HasSuffix(name, "_total") { + // prometheus_client OpenMetrics collect() omits _total from the family + // name. The hub encoder would otherwise emit TYPE unknown, and merge + // would not join those series with text-path snapshots. + return name + "_total" + } + return name +} + +func dtoFromProtoFamily(fam *pb.MetricsFamily) (*dto.MetricFamily, error) { + name := canonicalMetricFamilyName(fam) + help := fam.GetHelp() + mf := &dto.MetricFamily{ + Name: &name, + Help: &help, + } + switch fam.GetType() { + case pb.MetricsType_METRICS_TYPE_COUNTER: + t := dto.MetricType_COUNTER + mf.Type = &t + for _, sample := range fam.GetSamples() { + if skipCreatedSample(sample.GetName()) { + continue + } + v := sample.GetValue() + mf.Metric = append(mf.Metric, &dto.Metric{ + Label: dtoLabels(sample.GetLabels()), + Counter: &dto.Counter{ + Value: &v, + Exemplar: dtoExemplar(sample.GetExemplar()), + }, + }) + } + case pb.MetricsType_METRICS_TYPE_GAUGE: + t := dto.MetricType_GAUGE + mf.Type = &t + for _, sample := range fam.GetSamples() { + v := sample.GetValue() + mf.Metric = append(mf.Metric, &dto.Metric{ + Label: dtoLabels(sample.GetLabels()), + Gauge: &dto.Gauge{Value: &v}, + }) + } + case pb.MetricsType_METRICS_TYPE_UNTYPED, pb.MetricsType_METRICS_TYPE_UNSPECIFIED: + t := dto.MetricType_UNTYPED + mf.Type = &t + for _, sample := range fam.GetSamples() { + v := sample.GetValue() + mf.Metric = append(mf.Metric, &dto.Metric{ + Label: dtoLabels(sample.GetLabels()), + Untyped: &dto.Untyped{Value: &v}, + }) + } + case pb.MetricsType_METRICS_TYPE_HISTOGRAM: + t := dto.MetricType_HISTOGRAM + mf.Type = &t + metrics, err := dtoHistogramMetrics(name, fam.GetSamples()) + if err != nil { + return nil, err + } + mf.Metric = metrics + case pb.MetricsType_METRICS_TYPE_SUMMARY: + t := dto.MetricType_SUMMARY + mf.Type = &t + metrics, err := dtoSummaryMetrics(name, fam.GetSamples()) + if err != nil { + return nil, err + } + mf.Metric = metrics + default: + return nil, fmt.Errorf("unsupported metric type %s", fam.GetType()) + } + return mf, nil +} + +func skipCreatedSample(sample string) bool { + return strings.HasSuffix(sample, "_created") +} + +func dtoLabels(in []*pb.MetricsLabel) []*dto.LabelPair { + if len(in) == 0 { + return nil + } + out := make([]*dto.LabelPair, 0, len(in)) + for _, lp := range in { + if lp == nil || lp.GetName() == "" { + continue + } + out = append(out, labelPair(lp.GetName(), lp.GetValue())) + } + sort.Slice(out, func(i, j int) bool { return out[i].GetName() < out[j].GetName() }) + return out +} + +func dtoExemplar(ex *pb.MetricsExemplar) *dto.Exemplar { + if ex == nil { + return nil + } + if len(ex.GetLabels()) == 0 && ex.GetValue() == 0 && ex.GetTimestamp() == nil { + return nil + } + v := ex.GetValue() + out := &dto.Exemplar{ + Label: dtoLabels(ex.GetLabels()), + Value: &v, + } + if ts := ex.GetTimestamp(); ts != nil && ts.IsValid() { + out.Timestamp = timestamppb.New(ts.AsTime()) + } + return out +} + +type histAgg struct { + labels []*dto.LabelPair + count uint64 + sum float64 + buckets []*dto.Bucket +} + +func dtoHistogramMetrics(family string, samples []*pb.MetricsSample) ([]*dto.Metric, error) { + grouped := map[string]*histAgg{} + order := make([]string, 0) + for _, sample := range samples { + if sample == nil { + continue + } + sname := sample.GetName() + if skipCreatedSample(sname) { + continue + } + labels := dtoLabels(sample.GetLabels()) + base, le, isBucket := splitLeLabel(labels) + key := labelsKey(base) + agg, ok := grouped[key] + if !ok { + agg = &histAgg{labels: base} + grouped[key] = agg + order = append(order, key) + } + switch { + case sname == family+"_count": + agg.count = uint64(sample.GetValue()) + case sname == family+"_sum": + agg.sum = sample.GetValue() + case isBucket || sname == family+"_bucket": + bound, err := parseLe(le) + if err != nil { + return nil, fmt.Errorf("bucket le %q: %w", le, err) + } + count := uint64(sample.GetValue()) + agg.buckets = append(agg.buckets, &dto.Bucket{ + CumulativeCount: &count, + UpperBound: &bound, + Exemplar: dtoExemplar(sample.GetExemplar()), + }) + default: + // Ignore native-histogram or unknown suffixes. + } + } + out := make([]*dto.Metric, 0, len(order)) + for _, key := range order { + agg := grouped[key] + sort.Slice(agg.buckets, func(i, j int) bool { + return agg.buckets[i].GetUpperBound() < agg.buckets[j].GetUpperBound() + }) + out = append(out, &dto.Metric{ + Label: agg.labels, + Histogram: &dto.Histogram{ + SampleCount: &agg.count, + SampleSum: &agg.sum, + Bucket: agg.buckets, + }, + }) + } + return out, nil +} + +func dtoSummaryMetrics(family string, samples []*pb.MetricsSample) ([]*dto.Metric, error) { + type sumAgg struct { + labels []*dto.LabelPair + count uint64 + sum float64 + quantiles []*dto.Quantile + } + grouped := map[string]*sumAgg{} + order := make([]string, 0) + for _, sample := range samples { + if sample == nil || skipCreatedSample(sample.GetName()) { + continue + } + labels := dtoLabels(sample.GetLabels()) + base, qv, isQuantile := splitQuantileLabel(labels) + key := labelsKey(base) + agg, ok := grouped[key] + if !ok { + agg = &sumAgg{labels: base} + grouped[key] = agg + order = append(order, key) + } + switch sample.GetName() { + case family + "_count": + agg.count = uint64(sample.GetValue()) + case family + "_sum": + agg.sum = sample.GetValue() + default: + if isQuantile { + q := qv + v := sample.GetValue() + agg.quantiles = append(agg.quantiles, &dto.Quantile{Quantile: &q, Value: &v}) + } + } + } + out := make([]*dto.Metric, 0, len(order)) + for _, key := range order { + agg := grouped[key] + out = append(out, &dto.Metric{ + Label: agg.labels, + Summary: &dto.Summary{ + SampleCount: &agg.count, + SampleSum: &agg.sum, + Quantile: agg.quantiles, + }, + }) + } + return out, nil +} + +func splitLeLabel(labels []*dto.LabelPair) (base []*dto.LabelPair, le string, ok bool) { + base = make([]*dto.LabelPair, 0, len(labels)) + for _, lp := range labels { + if lp.GetName() == "le" { + le = lp.GetValue() + ok = true + continue + } + base = append(base, lp) + } + return base, le, ok +} + +func splitQuantileLabel(labels []*dto.LabelPair) (base []*dto.LabelPair, q float64, ok bool) { + base = make([]*dto.LabelPair, 0, len(labels)) + for _, lp := range labels { + if lp.GetName() == "quantile" { + v, err := strconv.ParseFloat(lp.GetValue(), 64) + if err == nil { + q = v + ok = true + } + continue + } + base = append(base, lp) + } + return base, q, ok +} + +func parseLe(le string) (float64, error) { + switch le { + case "+Inf", "Inf", "inf": + return math.Inf(1), nil + case "-Inf": + return math.Inf(-1), nil + } + return strconv.ParseFloat(le, 64) +} + +func labelsKey(labels []*dto.LabelPair) string { + var b strings.Builder + for _, lp := range labels { + b.WriteString(lp.GetName()) + b.WriteByte('=') + b.WriteString(lp.GetValue()) + b.WriteByte(0) + } + return b.String() +} diff --git a/controller/internal/service/metrics_families_test.go b/controller/internal/service/metrics_families_test.go new file mode 100644 index 000000000..ce9307b64 --- /dev/null +++ b/controller/internal/service/metrics_families_test.go @@ -0,0 +1,250 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "math" + "testing" + + pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" + dto "github.com/prometheus/client_model/go" +) + +func pbL(kv ...string) []*pb.MetricsLabel { + out := make([]*pb.MetricsLabel, 0, len(kv)/2) + for i := 0; i+1 < len(kv); i += 2 { + out = append(out, &pb.MetricsLabel{Name: kv[i], Value: kv[i+1]}) + } + return out +} + +func TestDtoFromProtoFamilies_CounterAndHistogramExemplars(t *testing.T) { + families, err := dtoFromProtoFamilies([]*pb.MetricsFamily{ + { + Name: "jumpstarter_operations_total", + Help: "Total operations performed.", + Type: pb.MetricsType_METRICS_TYPE_COUNTER, + Samples: []*pb.MetricsSample{{ + Name: "jumpstarter_operations_total", + Labels: pbL("exporter", "sidekick", "operation", "on", "result", "success", "driver_type", "power"), + Value: 2, + Exemplar: &pb.MetricsExemplar{ + Labels: pbL("client", "ci-bot", "lease_id", "lease-1"), + Value: 1, + }, + }}, + }, + { + Name: "jumpstarter_operation_duration_seconds", + Help: "Duration of each operation.", + Type: pb.MetricsType_METRICS_TYPE_HISTOGRAM, + Samples: []*pb.MetricsSample{ + { + Name: "jumpstarter_operation_duration_seconds_bucket", + Labels: pbL("exporter", "sidekick", "le", "0.005", "operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + Exemplar: &pb.MetricsExemplar{ + Labels: pbL("lease_id", "lease-1"), + Value: 0.00262, + }, + }, + { + Name: "jumpstarter_operation_duration_seconds_bucket", + Labels: pbL("exporter", "sidekick", "le", "+Inf", "operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + }, + { + Name: "jumpstarter_operation_duration_seconds_sum", + Labels: pbL("exporter", "sidekick", "operation", "on", "result", "success", "driver_type", "power"), + Value: 0.00262, + }, + { + Name: "jumpstarter_operation_duration_seconds_count", + Labels: pbL("exporter", "sidekick", "operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + }, + }, + }, + }) + if err != nil { + t.Fatalf("dtoFromProtoFamilies: %v", err) + } + + var ops, dur *dto.MetricFamily + for _, f := range families { + switch f.GetName() { + case "jumpstarter_operations_total": + ops = f + case "jumpstarter_operation_duration_seconds": + dur = f + } + } + if ops == nil || len(ops.Metric) != 1 { + t.Fatalf("counter family = %+v", ops) + } + ex := ops.Metric[0].GetCounter().GetExemplar() + if ex == nil { + t.Fatal("counter exemplar missing") + } + got := map[string]string{} + for _, lp := range ex.GetLabel() { + got[lp.GetName()] = lp.GetValue() + } + if got["client"] != "ci-bot" || got["lease_id"] != "lease-1" { + t.Errorf("counter exemplar = %v", got) + } + + if dur == nil || len(dur.Metric) != 1 { + t.Fatalf("histogram family = %+v", dur) + } + h := dur.Metric[0].GetHistogram() + if h.GetSampleCount() != 1 { + t.Errorf("sample_count = %d", h.GetSampleCount()) + } + if len(h.GetBucket()) != 2 { + t.Fatalf("buckets = %d, want 2", len(h.GetBucket())) + } + var infBound bool + var bucketEx *dto.Exemplar + for _, b := range h.GetBucket() { + if math.IsInf(b.GetUpperBound(), 1) { + infBound = true + } + if b.GetExemplar() != nil { + bucketEx = b.GetExemplar() + } + } + if !infBound { + t.Error("missing +Inf bucket") + } + if bucketEx == nil { + t.Fatal("histogram bucket exemplar missing") + } + foundLease := false + for _, lp := range bucketEx.GetLabel() { + if lp.GetName() == "lease_id" && lp.GetValue() == "lease-1" { + foundLease = true + } + } + if !foundLease { + t.Errorf("bucket exemplar labels = %v", bucketEx.GetLabel()) + } +} + +func TestDtoFromProtoFamilies_CanonicalizesOpenMetricsCounterName(t *testing.T) { + families, err := dtoFromProtoFamilies([]*pb.MetricsFamily{{ + Name: "jumpstarter_operations", + Help: "Total operations performed.", + Type: pb.MetricsType_METRICS_TYPE_COUNTER, + Samples: []*pb.MetricsSample{{ + Name: "jumpstarter_operations_total", + Labels: pbL("operation", "on"), + Value: 1, + Exemplar: &pb.MetricsExemplar{ + Labels: pbL("lease_id", "lease-1"), + Value: 1, + }, + }, { + Name: "jumpstarter_operations_created", + Labels: pbL("operation", "on"), + Value: 1.789e9, + }}, + }}) + if err != nil { + t.Fatalf("dtoFromProtoFamilies: %v", err) + } + if len(families) != 1 { + t.Fatalf("families = %d, want 1", len(families)) + } + if families[0].GetName() != "jumpstarter_operations_total" { + t.Errorf("name = %q, want jumpstarter_operations_total", families[0].GetName()) + } + if len(families[0].Metric) != 1 { + t.Fatalf("created sample should be dropped, got %d metrics", len(families[0].Metric)) + } + if families[0].Metric[0].GetCounter().GetExemplar() == nil { + t.Fatal("exemplar dropped during canonicalize") + } +} + +func TestMergeSnapshots_PrefersFamiliesOverUnparseableText(t *testing.T) { + var calls int + families := mergeSnapshots([]exporterSnapshot{{ + name: "sidekick", + text: []byte(pythonOpenMetricsWithExemplar), + families: []*pb.MetricsFamily{{ + Name: "jumpstarter_operation_duration_seconds", + Help: "Duration of each operation.", + Type: pb.MetricsType_METRICS_TYPE_HISTOGRAM, + Samples: []*pb.MetricsSample{ + { + Name: "jumpstarter_operation_duration_seconds_bucket", + Labels: pbL("le", "0.005", "operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + Exemplar: &pb.MetricsExemplar{ + Labels: pbL("lease_id", "lease-1", "trace_id", "drop-me"), + Value: 0.00262, + }, + }, + { + Name: "jumpstarter_operation_duration_seconds_sum", + Labels: pbL("operation", "on", "result", "success", "driver_type", "power"), + Value: 0.00262, + }, + { + Name: "jumpstarter_operation_duration_seconds_count", + Labels: pbL("operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + }, + }, + }}, + }}, nil, testMergeCfg, func(string, error) { calls++ }) + + if calls != 0 { + t.Fatalf("parse-error callbacks = %d, want 0 when families sidecar is present", calls) + } + var dur *dto.MetricFamily + for _, f := range families { + if f.GetName() == "jumpstarter_operation_duration_seconds" { + dur = f + } + } + if dur == nil || len(dur.Metric) != 1 { + t.Fatalf("expected merged histogram, got %+v", families) + } + if got, _ := getLabel(dur.Metric[0], "exporter"); got != "sidekick" { + t.Errorf("exporter = %q, want sidekick", got) + } + ex := dur.Metric[0].GetHistogram().GetBucket()[0].GetExemplar() + if ex == nil { + t.Fatal("allowlisted exemplar dropped") + } + for _, lp := range ex.GetLabel() { + if lp.GetName() == "trace_id" { + t.Errorf("trace_id should have been filtered, got %v", ex.GetLabel()) + } + } + foundLease := false + for _, lp := range ex.GetLabel() { + if lp.GetName() == "lease_id" && lp.GetValue() == "lease-1" { + foundLease = true + } + } + if !foundLease { + t.Errorf("lease_id missing from filtered exemplar: %v", ex.GetLabel()) + } +} diff --git a/controller/internal/service/metrics_merge.go b/controller/internal/service/metrics_merge.go index 484b4e27e..a15ae6383 100644 --- a/controller/internal/service/metrics_merge.go +++ b/controller/internal/service/metrics_merge.go @@ -23,6 +23,7 @@ import ( "sort" "strings" + pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" dto "github.com/prometheus/client_model/go" "github.com/prometheus/common/expfmt" ) @@ -56,8 +57,9 @@ type mergeConfig struct { } type exporterSnapshot struct { - name string - text []byte + name string + text []byte + families []*pb.MetricsFamily } func setToMap(values []string) map[string]struct{} { @@ -103,6 +105,16 @@ func parseMetricFamilies(text []byte) ([]*dto.MetricFamily, error) { } } +func familiesFromSnapshot(snap exporterSnapshot) ([]*dto.MetricFamily, error) { + if len(snap.families) > 0 { + return dtoFromProtoFamilies(snap.families) + } + if len(snap.text) == 0 { + return nil, nil + } + return parseMetricFamilies(snap.text) +} + func encodeMetricFamilies(w io.Writer, families []*dto.MetricFamily) error { sorted := append([]*dto.MetricFamily(nil), families...) sort.Slice(sorted, func(i, j int) bool { @@ -132,16 +144,16 @@ func mergeSnapshots(snapshots []exporterSnapshot, extra []*dto.MetricFamily, cfg byName[f.GetName()] = f } for _, snap := range snapshots { - if len(snap.text) == 0 { - continue - } - families, err := parseMetricFamilies(snap.text) + families, err := familiesFromSnapshot(snap) if err != nil { if onParseError != nil { onParseError(snap.name, err) } continue } + if len(families) == 0 { + continue + } cfg := cfgFor(snap.name) for _, f := range families { applyFamily(f, cfg) diff --git a/controller/internal/service/metrics_stream.go b/controller/internal/service/metrics_stream.go index b16bb78ef..145b1665e 100644 --- a/controller/internal/service/metrics_stream.go +++ b/controller/internal/service/metrics_stream.go @@ -159,7 +159,7 @@ func (s *TelemetryService) MetricsStream(stream pb.TelemetryService_MetricsStrea } } -func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) ([]byte, error) { +func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) (*pb.MetricsScrapeResponse, error) { c.scrapeMu.Lock() defer c.scrapeMu.Unlock() @@ -191,7 +191,7 @@ func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) ([]byte if resp == nil { return nil, errScrapeTimeout } - return resp.GetMetricsText(), nil + return resp, nil case <-timer.C: return nil, errScrapeTimeout case <-ctx.Done(): @@ -220,7 +220,7 @@ func (s *TelemetryService) fanoutScrapes(ctx context.Context) []exporterSnapshot wg.Add(1) go func(c *metricsConn) { defer wg.Done() - text, err := c.scrape(ctx, timeout) + resp, err := c.scrape(ctx, timeout) if err != nil { if errors.Is(err, errScrapeTimeout) || errors.Is(err, context.DeadlineExceeded) { s.scrapeTimeouts.Inc() @@ -232,7 +232,11 @@ func (s *TelemetryService) fanoutScrapes(ctx context.Context) []exporterSnapshot return } mu.Lock() - snaps = append(snaps, exporterSnapshot{name: c.id.name, text: text}) + snaps = append(snaps, exporterSnapshot{ + name: c.id.name, + text: resp.GetMetricsText(), + families: resp.GetFamilies(), + }) mu.Unlock() }(c) } diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index ed2f1356f..9fcd9f33f 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -467,3 +467,95 @@ func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testin default: } } + +func TestFanout_StructuredFamiliesKeepHistogramExemplars(t *testing.T) { + svc, client, addr := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "sidekick"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + + errCh := make(chan error, 1) + go func() { + for { + msg, err := stream.Recv() + if err != nil { + errCh <- err + return + } + if msg.GetScrapeRequest() == nil { + continue + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_ScrapeResponse{ + ScrapeResponse: &pb.MetricsScrapeResponse{ + MetricsText: []byte(pythonOpenMetricsWithExemplar), + Families: []*pb.MetricsFamily{{ + Name: "jumpstarter_operation_duration_seconds", + Help: "Duration of each operation.", + Type: pb.MetricsType_METRICS_TYPE_HISTOGRAM, + Samples: []*pb.MetricsSample{ + { + Name: "jumpstarter_operation_duration_seconds_bucket", + Labels: pbL("le", "0.005", "operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + Exemplar: &pb.MetricsExemplar{ + Labels: pbL("lease_id", "lease-1"), + Value: 0.00262, + }, + }, + { + Name: "jumpstarter_operation_duration_seconds_sum", + Labels: pbL("operation", "on", "result", "success", "driver_type", "power"), + Value: 0.00262, + }, + { + Name: "jumpstarter_operation_duration_seconds_count", + Labels: pbL("operation", "on", "result", "success", "driver_type", "power"), + Value: 1, + }, + }, + }}, + }, + }, + }); err != nil { + errCh <- err + return + } + } + }() + waitRegistered(t, svc, 1) + + code, body := httpGet(t, "http://"+addr+"/metrics") + if code != http.StatusOK { + t.Fatalf("GET /metrics status=%d body=%s", code, body) + } + if !strings.Contains(body, "jumpstarter_operation_duration_seconds") { + t.Errorf("structured histogram missing from /metrics:\n%s", body) + } + if !strings.Contains(body, `exporter="sidekick"`) { + t.Errorf("authenticated exporter label missing:\n%s", body) + } + if !strings.Contains(body, `lease_id="lease-1"`) { + t.Errorf("histogram exemplar missing from merged /metrics:\n%s", body) + } + if strings.Contains(body, `jumpstarter_metrics_parse_errors_total{exporter="sidekick"}`) { + t.Errorf("parse-error counter must not increment when families sidecar is used:\n%s", body) + } + + select { + case err := <-errCh: + if err != nil && err != io.EOF && status.Code(err) != codes.Canceled && status.Code(err) != codes.Unavailable { + t.Fatalf("stream goroutine: %v", err) + } + default: + } +} diff --git a/protocol/proto/jumpstarter/v1/telemetry.proto b/protocol/proto/jumpstarter/v1/telemetry.proto index fbbfd00ee..1c2f27159 100644 --- a/protocol/proto/jumpstarter/v1/telemetry.proto +++ b/protocol/proto/jumpstarter/v1/telemetry.proto @@ -10,7 +10,7 @@ import "google/protobuf/timestamp.proto"; // Implemented by jumpstarter-telemetry; not part of the controller. service TelemetryService { // Persistent bidirectional stream: telemetry sends scrape requests, - // exporter responds with full metric snapshots (OpenMetrics text). + // exporter responds with metric snapshots (structured families plus optional OpenMetrics text). rpc MetricsStream(stream MetricsStreamRequest) returns (stream MetricsStreamResponse); // Push structured log entries to the telemetry service for Loki ingest. @@ -30,8 +30,50 @@ message MetricsRegister { } message MetricsScrapeResponse { - bytes metrics_text = 1; // generate_latest() OpenMetrics output. - google.protobuf.Timestamp timestamp = 2; + bytes metrics_text = 1; // Optional generate_latest() OpenMetrics text (legacy / debug). + google.protobuf.Timestamp timestamp = 2; // When the snapshot was taken. + // DD-3 sidecar: in-memory registry dump so the hub can merge and filter + // exemplars without parsing OpenMetrics text (prometheus/common cannot). + repeated MetricsFamily families = 3; +} + +// Prometheus metric type for a MetricsFamily. +enum MetricsType { + METRICS_TYPE_UNSPECIFIED = 0; // Unknown or unset type. + METRICS_TYPE_COUNTER = 1; // Monotonic counter. + METRICS_TYPE_GAUGE = 2; // Gauge. + METRICS_TYPE_HISTOGRAM = 3; // Histogram (bucket / sum / count samples). + METRICS_TYPE_SUMMARY = 4; // Summary. + METRICS_TYPE_UNTYPED = 5; // Untyped / unknown. +} + +// One label name/value pair on a sample or exemplar. +message MetricsLabel { + string name = 1; // Label name. + string value = 2; // Label value. +} + +// OpenMetrics exemplar attached to a sample (JEP-0013 allowlist applied on the hub). +message MetricsExemplar { + repeated MetricsLabel labels = 1; // Exemplar labels (client, lease_id, …). + double value = 2; // Exemplar observation value. + google.protobuf.Timestamp timestamp = 3; // Optional exemplar timestamp. +} + +// One collected sample from prometheus_client (including histogram _bucket/_sum/_count). +message MetricsSample { + string name = 1; // Sample name (may include _bucket, _sum, _count). + repeated MetricsLabel labels = 2; // Sample labels (histogram buckets include le). + double value = 3; // Sample value. + MetricsExemplar exemplar = 4; // Optional exemplar for this sample. +} + +// One metric family from CollectorRegistry.collect(). +message MetricsFamily { + string name = 1; // Family name (counters include _total; no _bucket/_sum/_count). + string help = 2; // Help text. + MetricsType type = 3; // Family type. + repeated MetricsSample samples = 4; // Collected samples, including exemplars. } // Telemetry → Exporter diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py index 80e4bc2bc..47e48e2cd 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.py @@ -25,7 +25,7 @@ from google.protobuf import timestamp_pb2 as google_dot_protobuf_dot_timestamp__pb2 -DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x1ejumpstarter/v1/telemetry.proto\x12\x0ejumpstarter.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xae\x01\n\x14MetricsStreamRequest\x12=\n\x08register\x18\x01 \x01(\x0b\x32\x1f.jumpstarter.v1.MetricsRegisterH\x00R\x08register\x12P\n\x0fscrape_response\x18\x02 \x01(\x0b\x32%.jumpstarter.v1.MetricsScrapeResponseH\x00R\x0escrapeResponseB\x05\n\x03msg\"-\n\x0fMetricsRegister\x12\x1a\n\x08identity\x18\x01 \x01(\tR\x08identity\"t\n\x15MetricsScrapeResponse\x12!\n\x0cmetrics_text\x18\x01 \x01(\x0cR\x0bmetricsText\x12\x38\n\ttimestamp\x18\x02 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\"m\n\x15MetricsStreamResponse\x12M\n\x0escrape_request\x18\x01 \x01(\x0b\x32$.jumpstarter.v1.MetricsScrapeRequestH\x00R\rscrapeRequestB\x05\n\x03msg\"\x16\n\x14MetricsScrapeRequest\"E\n\x0fPushLogsRequest\x12\x32\n\x07\x65ntries\x18\x01 \x03(\x0b\x32\x18.jumpstarter.v1.LogEntryR\x07\x65ntries\"H\n\x10PushLogsResponse\x12\x1a\n\x08\x61\x63\x63\x65pted\x18\x01 \x01(\rR\x08\x61\x63\x63\x65pted\x12\x18\n\x07\x64ropped\x18\x02 \x01(\rR\x07\x64ropped\"\xe5\x03\n\x08LogEntry\x12\x38\n\ttimestamp\x18\x01 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\x12\x1a\n\x08severity\x18\x02 \x01(\tR\x08severity\x12\x18\n\x07message\x18\x03 \x01(\tR\x07message\x12\x1c\n\tcomponent\x18\x04 \x01(\tR\tcomponent\x12\x1a\n\x08\x65xporter\x18\x05 \x01(\tR\x08\x65xporter\x12\x14\n\x05lease\x18\x06 \x01(\tR\x05lease\x12\x16\n\x06\x63lient\x18\x07 \x01(\tR\x06\x63lient\x12\x1c\n\toperation\x18\x08 \x01(\tR\toperation\x12\x16\n\x06result\x18\t \x01(\tR\x06result\x12\x1f\n\x0b\x64river_type\x18\n \x01(\tR\ndriverType\x12L\n\x0c\x65xtra_fields\x18\x0b \x03(\x0b\x32).jumpstarter.v1.LogEntry.ExtraFieldsEntryR\x0b\x65xtraFields\x12\x1c\n\tnamespace\x18\x0c \x01(\tR\tnamespace\x1a>\n\x10\x45xtraFieldsEntry\x12\x10\n\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value:\x02\x38\x01\x32\xc3\x01\n\x10TelemetryService\x12`\n\rMetricsStream\x12$.jumpstarter.v1.MetricsStreamRequest\x1a%.jumpstarter.v1.MetricsStreamResponse(\x01\x30\x01\x12M\n\x08PushLogs\x12\x1f.jumpstarter.v1.PushLogsRequest\x1a .jumpstarter.v1.PushLogsResponseB}\n\x12\x63om.jumpstarter.v1B\x0eTelemetryProtoP\x01\xa2\x02\x03JXX\xaa\x02\x0eJumpstarter.V1\xca\x02\x0eJumpstarter\\V1\xe2\x02\x1aJumpstarter\\V1\\GPBMetadata\xea\x02\x0fJumpstarter::V1b\x06proto3') +DESCRIPTOR = _descriptor_pool.Default().AddSerializedFile(b'\n\x1ejumpstarter/v1/telemetry.proto\x12\x0ejumpstarter.v1\x1a\x1fgoogle/protobuf/timestamp.proto\"\xae\x01\n\x14MetricsStreamRequest\x12=\n\x08register\x18\x01 \x01(\x0b\x32\x1f.jumpstarter.v1.MetricsRegisterH\x00R\x08register\x12P\n\x0fscrape_response\x18\x02 \x01(\x0b\x32%.jumpstarter.v1.MetricsScrapeResponseH\x00R\x0escrapeResponseB\x05\n\x03msg\"-\n\x0fMetricsRegister\x12\x1a\n\x08identity\x18\x01 \x01(\tR\x08identity\"\xaf\x01\n\x15MetricsScrapeResponse\x12!\n\x0cmetrics_text\x18\x01 \x01(\x0cR\x0bmetricsText\x12\x38\n\ttimestamp\x18\x02 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\x12\x39\n\x08\x66\x61milies\x18\x03 \x03(\x0b\x32\x1d.jumpstarter.v1.MetricsFamilyR\x08\x66\x61milies\"8\n\x0cMetricsLabel\x12\x12\n\x04name\x18\x01 \x01(\tR\x04name\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value\"\x97\x01\n\x0fMetricsExemplar\x12\x34\n\x06labels\x18\x01 \x03(\x0b\x32\x1c.jumpstarter.v1.MetricsLabelR\x06labels\x12\x14\n\x05value\x18\x02 \x01(\x01R\x05value\x12\x38\n\ttimestamp\x18\x03 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\"\xac\x01\n\rMetricsSample\x12\x12\n\x04name\x18\x01 \x01(\tR\x04name\x12\x34\n\x06labels\x18\x02 \x03(\x0b\x32\x1c.jumpstarter.v1.MetricsLabelR\x06labels\x12\x14\n\x05value\x18\x03 \x01(\x01R\x05value\x12;\n\x08\x65xemplar\x18\x04 \x01(\x0b\x32\x1f.jumpstarter.v1.MetricsExemplarR\x08\x65xemplar\"\xa1\x01\n\rMetricsFamily\x12\x12\n\x04name\x18\x01 \x01(\tR\x04name\x12\x12\n\x04help\x18\x02 \x01(\tR\x04help\x12/\n\x04type\x18\x03 \x01(\x0e\x32\x1b.jumpstarter.v1.MetricsTypeR\x04type\x12\x37\n\x07samples\x18\x04 \x03(\x0b\x32\x1d.jumpstarter.v1.MetricsSampleR\x07samples\"m\n\x15MetricsStreamResponse\x12M\n\x0escrape_request\x18\x01 \x01(\x0b\x32$.jumpstarter.v1.MetricsScrapeRequestH\x00R\rscrapeRequestB\x05\n\x03msg\"\x16\n\x14MetricsScrapeRequest\"E\n\x0fPushLogsRequest\x12\x32\n\x07\x65ntries\x18\x01 \x03(\x0b\x32\x18.jumpstarter.v1.LogEntryR\x07\x65ntries\"H\n\x10PushLogsResponse\x12\x1a\n\x08\x61\x63\x63\x65pted\x18\x01 \x01(\rR\x08\x61\x63\x63\x65pted\x12\x18\n\x07\x64ropped\x18\x02 \x01(\rR\x07\x64ropped\"\xe5\x03\n\x08LogEntry\x12\x38\n\ttimestamp\x18\x01 \x01(\x0b\x32\x1a.google.protobuf.TimestampR\ttimestamp\x12\x1a\n\x08severity\x18\x02 \x01(\tR\x08severity\x12\x18\n\x07message\x18\x03 \x01(\tR\x07message\x12\x1c\n\tcomponent\x18\x04 \x01(\tR\tcomponent\x12\x1a\n\x08\x65xporter\x18\x05 \x01(\tR\x08\x65xporter\x12\x14\n\x05lease\x18\x06 \x01(\tR\x05lease\x12\x16\n\x06\x63lient\x18\x07 \x01(\tR\x06\x63lient\x12\x1c\n\toperation\x18\x08 \x01(\tR\toperation\x12\x16\n\x06result\x18\t \x01(\tR\x06result\x12\x1f\n\x0b\x64river_type\x18\n \x01(\tR\ndriverType\x12L\n\x0c\x65xtra_fields\x18\x0b \x03(\x0b\x32).jumpstarter.v1.LogEntry.ExtraFieldsEntryR\x0b\x65xtraFields\x12\x1c\n\tnamespace\x18\x0c \x01(\tR\tnamespace\x1a>\n\x10\x45xtraFieldsEntry\x12\x10\n\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n\x05value\x18\x02 \x01(\tR\x05value:\x02\x38\x01*\xad\x01\n\x0bMetricsType\x12\x1c\n\x18METRICS_TYPE_UNSPECIFIED\x10\x00\x12\x18\n\x14METRICS_TYPE_COUNTER\x10\x01\x12\x16\n\x12METRICS_TYPE_GAUGE\x10\x02\x12\x1a\n\x16METRICS_TYPE_HISTOGRAM\x10\x03\x12\x18\n\x14METRICS_TYPE_SUMMARY\x10\x04\x12\x18\n\x14METRICS_TYPE_UNTYPED\x10\x05\x32\xc3\x01\n\x10TelemetryService\x12`\n\rMetricsStream\x12$.jumpstarter.v1.MetricsStreamRequest\x1a%.jumpstarter.v1.MetricsStreamResponse(\x01\x30\x01\x12M\n\x08PushLogs\x12\x1f.jumpstarter.v1.PushLogsRequest\x1a .jumpstarter.v1.PushLogsResponseB}\n\x12\x63om.jumpstarter.v1B\x0eTelemetryProtoP\x01\xa2\x02\x03JXX\xaa\x02\x0eJumpstarter.V1\xca\x02\x0eJumpstarter\\V1\xe2\x02\x1aJumpstarter\\V1\\GPBMetadata\xea\x02\x0fJumpstarter::V1b\x06proto3') _globals = globals() _builder.BuildMessageAndEnumDescriptors(DESCRIPTOR, _globals) @@ -35,24 +35,34 @@ _globals['DESCRIPTOR']._serialized_options = b'\n\022com.jumpstarter.v1B\016TelemetryProtoP\001\242\002\003JXX\252\002\016Jumpstarter.V1\312\002\016Jumpstarter\\V1\342\002\032Jumpstarter\\V1\\GPBMetadata\352\002\017Jumpstarter::V1' _globals['_LOGENTRY_EXTRAFIELDSENTRY']._loaded_options = None _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_options = b'8\001' + _globals['_METRICSTYPE']._serialized_start=1805 + _globals['_METRICSTYPE']._serialized_end=1978 _globals['_METRICSSTREAMREQUEST']._serialized_start=84 _globals['_METRICSSTREAMREQUEST']._serialized_end=258 _globals['_METRICSREGISTER']._serialized_start=260 _globals['_METRICSREGISTER']._serialized_end=305 - _globals['_METRICSSCRAPERESPONSE']._serialized_start=307 - _globals['_METRICSSCRAPERESPONSE']._serialized_end=423 - _globals['_METRICSSTREAMRESPONSE']._serialized_start=425 - _globals['_METRICSSTREAMRESPONSE']._serialized_end=534 - _globals['_METRICSSCRAPEREQUEST']._serialized_start=536 - _globals['_METRICSSCRAPEREQUEST']._serialized_end=558 - _globals['_PUSHLOGSREQUEST']._serialized_start=560 - _globals['_PUSHLOGSREQUEST']._serialized_end=629 - _globals['_PUSHLOGSRESPONSE']._serialized_start=631 - _globals['_PUSHLOGSRESPONSE']._serialized_end=703 - _globals['_LOGENTRY']._serialized_start=706 - _globals['_LOGENTRY']._serialized_end=1191 - _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_start=1129 - _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_end=1191 - _globals['_TELEMETRYSERVICE']._serialized_start=1194 - _globals['_TELEMETRYSERVICE']._serialized_end=1389 + _globals['_METRICSSCRAPERESPONSE']._serialized_start=308 + _globals['_METRICSSCRAPERESPONSE']._serialized_end=483 + _globals['_METRICSLABEL']._serialized_start=485 + _globals['_METRICSLABEL']._serialized_end=541 + _globals['_METRICSEXEMPLAR']._serialized_start=544 + _globals['_METRICSEXEMPLAR']._serialized_end=695 + _globals['_METRICSSAMPLE']._serialized_start=698 + _globals['_METRICSSAMPLE']._serialized_end=870 + _globals['_METRICSFAMILY']._serialized_start=873 + _globals['_METRICSFAMILY']._serialized_end=1034 + _globals['_METRICSSTREAMRESPONSE']._serialized_start=1036 + _globals['_METRICSSTREAMRESPONSE']._serialized_end=1145 + _globals['_METRICSSCRAPEREQUEST']._serialized_start=1147 + _globals['_METRICSSCRAPEREQUEST']._serialized_end=1169 + _globals['_PUSHLOGSREQUEST']._serialized_start=1171 + _globals['_PUSHLOGSREQUEST']._serialized_end=1240 + _globals['_PUSHLOGSRESPONSE']._serialized_start=1242 + _globals['_PUSHLOGSRESPONSE']._serialized_end=1314 + _globals['_LOGENTRY']._serialized_start=1317 + _globals['_LOGENTRY']._serialized_end=1802 + _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_start=1740 + _globals['_LOGENTRY_EXTRAFIELDSENTRY']._serialized_end=1802 + _globals['_TELEMETRYSERVICE']._serialized_start=1981 + _globals['_TELEMETRYSERVICE']._serialized_end=2176 # @@protoc_insertion_point(module_scope) diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi index 968f7ea75..09e7c3294 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2.pyi @@ -7,6 +7,7 @@ import builtins import collections.abc import google.protobuf.descriptor import google.protobuf.internal.containers +import google.protobuf.internal.enum_type_wrapper import google.protobuf.message import google.protobuf.timestamp_pb2 import sys @@ -19,6 +20,42 @@ else: DESCRIPTOR: google.protobuf.descriptor.FileDescriptor +class _MetricsType: + ValueType = typing.NewType("ValueType", builtins.int) + V: typing_extensions.TypeAlias = ValueType + +class _MetricsTypeEnumTypeWrapper(google.protobuf.internal.enum_type_wrapper._EnumTypeWrapper[_MetricsType.ValueType], builtins.type): + DESCRIPTOR: google.protobuf.descriptor.EnumDescriptor + METRICS_TYPE_UNSPECIFIED: _MetricsType.ValueType # 0 + """Unknown or unset type.""" + METRICS_TYPE_COUNTER: _MetricsType.ValueType # 1 + """Monotonic counter.""" + METRICS_TYPE_GAUGE: _MetricsType.ValueType # 2 + """Gauge.""" + METRICS_TYPE_HISTOGRAM: _MetricsType.ValueType # 3 + """Histogram (bucket / sum / count samples).""" + METRICS_TYPE_SUMMARY: _MetricsType.ValueType # 4 + """Summary.""" + METRICS_TYPE_UNTYPED: _MetricsType.ValueType # 5 + """Untyped / unknown.""" + +class MetricsType(_MetricsType, metaclass=_MetricsTypeEnumTypeWrapper): + """Prometheus metric type for a MetricsFamily.""" + +METRICS_TYPE_UNSPECIFIED: MetricsType.ValueType # 0 +"""Unknown or unset type.""" +METRICS_TYPE_COUNTER: MetricsType.ValueType # 1 +"""Monotonic counter.""" +METRICS_TYPE_GAUGE: MetricsType.ValueType # 2 +"""Gauge.""" +METRICS_TYPE_HISTOGRAM: MetricsType.ValueType # 3 +"""Histogram (bucket / sum / count samples).""" +METRICS_TYPE_SUMMARY: MetricsType.ValueType # 4 +"""Summary.""" +METRICS_TYPE_UNTYPED: MetricsType.ValueType # 5 +"""Untyped / unknown.""" +Global___MetricsType: typing_extensions.TypeAlias = MetricsType + @typing.final class MetricsStreamRequest(google.protobuf.message.Message): """Exporter → Telemetry""" @@ -69,21 +106,151 @@ class MetricsScrapeResponse(google.protobuf.message.Message): METRICS_TEXT_FIELD_NUMBER: builtins.int TIMESTAMP_FIELD_NUMBER: builtins.int + FAMILIES_FIELD_NUMBER: builtins.int metrics_text: builtins.bytes - """generate_latest() OpenMetrics output.""" + """Optional generate_latest() OpenMetrics text (legacy / debug).""" + @property + def timestamp(self) -> google.protobuf.timestamp_pb2.Timestamp: + """When the snapshot was taken.""" + @property - def timestamp(self) -> google.protobuf.timestamp_pb2.Timestamp: ... + def families(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[Global___MetricsFamily]: + """DD-3 sidecar: in-memory registry dump so the hub can merge and filter + exemplars without parsing OpenMetrics text (prometheus/common cannot). + """ + def __init__( self, *, metrics_text: builtins.bytes = ..., timestamp: google.protobuf.timestamp_pb2.Timestamp | None = ..., + families: collections.abc.Iterable[Global___MetricsFamily] | None = ..., ) -> None: ... def HasField(self, field_name: typing.Literal["timestamp", b"timestamp"]) -> builtins.bool: ... - def ClearField(self, field_name: typing.Literal["metrics_text", b"metrics_text", "timestamp", b"timestamp"]) -> None: ... + def ClearField(self, field_name: typing.Literal["families", b"families", "metrics_text", b"metrics_text", "timestamp", b"timestamp"]) -> None: ... Global___MetricsScrapeResponse: typing_extensions.TypeAlias = MetricsScrapeResponse +@typing.final +class MetricsLabel(google.protobuf.message.Message): + """One label name/value pair on a sample or exemplar.""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + NAME_FIELD_NUMBER: builtins.int + VALUE_FIELD_NUMBER: builtins.int + name: builtins.str + """Label name.""" + value: builtins.str + """Label value.""" + def __init__( + self, + *, + name: builtins.str = ..., + value: builtins.str = ..., + ) -> None: ... + def ClearField(self, field_name: typing.Literal["name", b"name", "value", b"value"]) -> None: ... + +Global___MetricsLabel: typing_extensions.TypeAlias = MetricsLabel + +@typing.final +class MetricsExemplar(google.protobuf.message.Message): + """OpenMetrics exemplar attached to a sample (JEP-0013 allowlist applied on the hub).""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + LABELS_FIELD_NUMBER: builtins.int + VALUE_FIELD_NUMBER: builtins.int + TIMESTAMP_FIELD_NUMBER: builtins.int + value: builtins.float + """Exemplar observation value.""" + @property + def labels(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[Global___MetricsLabel]: + """Exemplar labels (client, lease_id, …).""" + + @property + def timestamp(self) -> google.protobuf.timestamp_pb2.Timestamp: + """Optional exemplar timestamp.""" + + def __init__( + self, + *, + labels: collections.abc.Iterable[Global___MetricsLabel] | None = ..., + value: builtins.float = ..., + timestamp: google.protobuf.timestamp_pb2.Timestamp | None = ..., + ) -> None: ... + def HasField(self, field_name: typing.Literal["timestamp", b"timestamp"]) -> builtins.bool: ... + def ClearField(self, field_name: typing.Literal["labels", b"labels", "timestamp", b"timestamp", "value", b"value"]) -> None: ... + +Global___MetricsExemplar: typing_extensions.TypeAlias = MetricsExemplar + +@typing.final +class MetricsSample(google.protobuf.message.Message): + """One collected sample from prometheus_client (including histogram _bucket/_sum/_count).""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + NAME_FIELD_NUMBER: builtins.int + LABELS_FIELD_NUMBER: builtins.int + VALUE_FIELD_NUMBER: builtins.int + EXEMPLAR_FIELD_NUMBER: builtins.int + name: builtins.str + """Sample name (may include _bucket, _sum, _count).""" + value: builtins.float + """Sample value.""" + @property + def labels(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[Global___MetricsLabel]: + """Sample labels (histogram buckets include le).""" + + @property + def exemplar(self) -> Global___MetricsExemplar: + """Optional exemplar for this sample.""" + + def __init__( + self, + *, + name: builtins.str = ..., + labels: collections.abc.Iterable[Global___MetricsLabel] | None = ..., + value: builtins.float = ..., + exemplar: Global___MetricsExemplar | None = ..., + ) -> None: ... + def HasField(self, field_name: typing.Literal["exemplar", b"exemplar"]) -> builtins.bool: ... + def ClearField(self, field_name: typing.Literal["exemplar", b"exemplar", "labels", b"labels", "name", b"name", "value", b"value"]) -> None: ... + +Global___MetricsSample: typing_extensions.TypeAlias = MetricsSample + +@typing.final +class MetricsFamily(google.protobuf.message.Message): + """One metric family from CollectorRegistry.collect().""" + + DESCRIPTOR: google.protobuf.descriptor.Descriptor + + NAME_FIELD_NUMBER: builtins.int + HELP_FIELD_NUMBER: builtins.int + TYPE_FIELD_NUMBER: builtins.int + SAMPLES_FIELD_NUMBER: builtins.int + name: builtins.str + """Family name (counters include _total; no _bucket/_sum/_count).""" + help: builtins.str + """Help text.""" + type: Global___MetricsType.ValueType + """Family type.""" + @property + def samples(self) -> google.protobuf.internal.containers.RepeatedCompositeFieldContainer[Global___MetricsSample]: + """Collected samples, including exemplars.""" + + def __init__( + self, + *, + name: builtins.str = ..., + help: builtins.str = ..., + type: Global___MetricsType.ValueType = ..., + samples: collections.abc.Iterable[Global___MetricsSample] | None = ..., + ) -> None: ... + def ClearField(self, field_name: typing.Literal["help", b"help", "name", b"name", "samples", b"samples", "type", b"type"]) -> None: ... + +Global___MetricsFamily: typing_extensions.TypeAlias = MetricsFamily + @typing.final class MetricsStreamResponse(google.protobuf.message.Message): """Telemetry → Exporter""" diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py index e34c217b6..96cf6e536 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.py @@ -35,7 +35,7 @@ class TelemetryServiceServicer: def MetricsStream(self, request_iterator, context): """Persistent bidirectional stream: telemetry sends scrape requests, - exporter responds with full metric snapshots (OpenMetrics text). + exporter responds with metric snapshots (structured families plus optional OpenMetrics text). """ context.set_code(grpc.StatusCode.UNIMPLEMENTED) context.set_details('Method not implemented!') diff --git a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi index d810b4378..bbb96205b 100644 --- a/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi +++ b/python/packages/jumpstarter-protocol/jumpstarter_protocol/jumpstarter/v1/telemetry_pb2_grpc.pyi @@ -88,7 +88,7 @@ class TelemetryServiceStub(typing.Generic[_TelemetryServiceMetricsStreamType, _T MetricsStream: _TelemetryServiceMetricsStreamType """Persistent bidirectional stream: telemetry sends scrape requests, - exporter responds with full metric snapshots (OpenMetrics text). + exporter responds with metric snapshots (structured families plus optional OpenMetrics text). """ PushLogs: _TelemetryServicePushLogsType @@ -117,7 +117,7 @@ class TelemetryServiceServicer(metaclass=abc.ABCMeta): context: _ServicerContext, ) -> typing.Union[collections.abc.Iterator[jumpstarter.v1.telemetry_pb2.MetricsStreamResponse], collections.abc.AsyncIterator[jumpstarter.v1.telemetry_pb2.MetricsStreamResponse]]: """Persistent bidirectional stream: telemetry sends scrape requests, - exporter responds with full metric snapshots (OpenMetrics text). + exporter responds with metric snapshots (structured families plus optional OpenMetrics text). """ @abc.abstractmethod diff --git a/python/packages/jumpstarter/jumpstarter/metrics/__init__.py b/python/packages/jumpstarter/jumpstarter/metrics/__init__.py index 56041b452..f86d0fa9b 100644 --- a/python/packages/jumpstarter/jumpstarter/metrics/__init__.py +++ b/python/packages/jumpstarter/jumpstarter/metrics/__init__.py @@ -1,5 +1,6 @@ """Exporter-local Prometheus metrics.""" +from .families import families_from_collector, scrape_response_from_registry from .registry import ( DEFAULT_EXEMPLAR_KEYS, MetricsRegistry, @@ -10,6 +11,8 @@ __all__ = [ "DEFAULT_EXEMPLAR_KEYS", "MetricsRegistry", + "families_from_collector", "get_registry", + "scrape_response_from_registry", "start_metrics_server", ] diff --git a/python/packages/jumpstarter/jumpstarter/metrics/families.py b/python/packages/jumpstarter/jumpstarter/metrics/families.py new file mode 100644 index 000000000..07af6a0aa --- /dev/null +++ b/python/packages/jumpstarter/jumpstarter/metrics/families.py @@ -0,0 +1,126 @@ +"""Encode a CollectorRegistry into MetricsFamily protos (JEP-0013 DD-3 sidecar). + +The hub merges these families instead of parsing OpenMetrics text, which +prometheus/common cannot do when exemplars are present. +""" + +from __future__ import annotations + +from typing import Any + +from jumpstarter_protocol import telemetry_pb2 +from prometheus_client import CollectorRegistry + +from .registry import MetricsRegistry + +_TYPE_MAP = { + "counter": telemetry_pb2.METRICS_TYPE_COUNTER, + "gauge": telemetry_pb2.METRICS_TYPE_GAUGE, + "histogram": telemetry_pb2.METRICS_TYPE_HISTOGRAM, + "gaugehistogram": telemetry_pb2.METRICS_TYPE_HISTOGRAM, + "summary": telemetry_pb2.METRICS_TYPE_SUMMARY, + "untyped": telemetry_pb2.METRICS_TYPE_UNTYPED, + "unknown": telemetry_pb2.METRICS_TYPE_UNTYPED, +} + + +def _family_name(metric: Any) -> str: + """Prometheus family name, including ``_total`` for counters. + + ``prometheus_client`` OpenMetrics ``collect()`` strips ``_total`` from the + family name. The hub OpenMetrics encoder treats a counter without that + suffix as ``unknown``, and text-path snapshots already use the ``_total`` + name, so mixed exporters would otherwise fail to merge. + """ + name = metric.name + if metric.type == "counter" and not name.endswith("_total"): + return f"{name}_total" + return name + + +def _set_unix_timestamp(dest: Any, ts: float) -> None: + seconds = int(ts) + nanos = int(round((ts - seconds) * 1_000_000_000)) + if nanos >= 1_000_000_000: + seconds += 1 + nanos -= 1_000_000_000 + if nanos < 0: + seconds -= 1 + nanos += 1_000_000_000 + dest.seconds = seconds + dest.nanos = nanos + + +def _copy_labels(dest: Any, labels: Any) -> None: + if not labels: + return + for name, val in labels.items(): + if name is None or val is None: + continue + lp = dest.add() + lp.name = str(name) + lp.value = str(val) + + +def _exemplar_proto(raw: Any) -> telemetry_pb2.MetricsExemplar | None: + if not raw: + return None + labels = getattr(raw, "labels", None) + value = getattr(raw, "value", None) + timestamp = getattr(raw, "timestamp", None) + if not labels and value is None: + return None + ex = telemetry_pb2.MetricsExemplar() + if value is not None: + ex.value = float(value) + _copy_labels(ex.labels, labels) + if timestamp is not None: + try: + _set_unix_timestamp(ex.timestamp, float(timestamp)) + except (TypeError, ValueError): + pass + if not ex.labels and ex.value == 0 and not ex.HasField("timestamp"): + return None + return ex + + +def families_from_collector(registry: CollectorRegistry) -> list[telemetry_pb2.MetricsFamily]: + """Dump ``registry.collect()`` into proto families, including exemplars.""" + out: list[telemetry_pb2.MetricsFamily] = [] + for metric in registry.collect(): + fam = telemetry_pb2.MetricsFamily( + name=_family_name(metric), + help=metric.documentation or "", + type=_TYPE_MAP.get(metric.type, telemetry_pb2.METRICS_TYPE_UNSPECIFIED), + ) + for sample in metric.samples: + if sample.name.endswith("_created"): + continue + ps = fam.samples.add() + ps.name = sample.name + ps.value = float(sample.value) + _copy_labels(ps.labels, sample.labels) + ex = _exemplar_proto(getattr(sample, "exemplar", None)) + if ex is not None: + ps.exemplar.CopyFrom(ex) + if fam.samples: + out.append(fam) + return out + + +def scrape_response_from_registry( + registry: MetricsRegistry, + *, + include_text: bool = True, +) -> telemetry_pb2.MetricsScrapeResponse: + """Build a MetricsScrapeResponse from the in-memory registry. + + Structured ``families`` are the scrape payload. ``metrics_text`` remains + optional OpenMetrics for lab/debug and older hubs. + """ + resp = telemetry_pb2.MetricsScrapeResponse() + resp.timestamp.GetCurrentTime() + resp.families.extend(families_from_collector(registry.collector_registry)) + if include_text: + resp.metrics_text = registry.generate_latest() + return resp diff --git a/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py b/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py index 9d6c0e5c1..efb4ea53a 100644 --- a/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py +++ b/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py @@ -200,6 +200,49 @@ def test_exemplars_include_client_and_lease_id(): ) +def test_scrape_response_families_carry_exemplars_without_text_parse(): + from jumpstarter_protocol import telemetry_pb2 + + from jumpstarter.metrics.families import scrape_response_from_registry + + reg = get_registry() + reg.record_operation( + exporter="lab-01", + operation="on", + result="success", + driver_type="power", + duration_seconds=0.01, + exemplars={"client": "ci-bot", "lease_id": "lease-xyz"}, + ) + resp = scrape_response_from_registry(reg, include_text=True) + assert resp.metrics_text + names = {fam.name: fam for fam in resp.families} + assert "jumpstarter_operations_total" in names + assert "jumpstarter_operation_duration_seconds" in names + + counter = names["jumpstarter_operations_total"] + assert counter.type == telemetry_pb2.METRICS_TYPE_COUNTER + assert counter.samples + ex_labels = {lp.name: lp.value for lp in counter.samples[0].exemplar.labels} + assert ex_labels["client"] == "ci-bot" + assert ex_labels["lease_id"] == "lease-xyz" + + hist = names["jumpstarter_operation_duration_seconds"] + buckets = [s for s in hist.samples if s.name.endswith("_bucket")] + assert buckets + bucket_ex = next((s.exemplar for s in buckets if s.HasField("exemplar") and s.exemplar.labels), None) + assert bucket_ex is not None + assert {lp.name: lp.value for lp in bucket_ex.labels}["lease_id"] == "lease-xyz" + assert not any(s.name.endswith("_created") for fam in resp.families for s in fam.samples) + + textless = scrape_response_from_registry(reg, include_text=False) + assert not textless.metrics_text + assert {fam.name for fam in textless.families} >= { + "jumpstarter_operations_total", + "jumpstarter_operation_duration_seconds", + } + + def test_metrics_http_endpoint_serves_prometheus_text(): reg = get_registry() reg.set_active_sessions(exporter="lab-01", value=2) From 6b04471c5ce1a67f940a78cef3f058aef8abb515 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 10:24:08 -0230 Subject: [PATCH 05/14] fix: coalesce MetricsStream scrapes and bound gRPC shutdown Overlapping /metrics requests now share one reverse-scrape, and GracefulStop falls back to Stop after 5s so a stuck exporter stream cannot hang rollout. Co-authored-by: Cursor --- controller/internal/service/metrics_stream.go | 18 ++- .../internal/service/metrics_stream_test.go | 151 ++++++++++++++++++ .../internal/service/telemetry_service.go | 34 +++- 3 files changed, 201 insertions(+), 2 deletions(-) diff --git a/controller/internal/service/metrics_stream.go b/controller/internal/service/metrics_stream.go index 145b1665e..fb6ec6bd2 100644 --- a/controller/internal/service/metrics_stream.go +++ b/controller/internal/service/metrics_stream.go @@ -32,6 +32,9 @@ var errScrapeTimeout = errors.New("metrics scrape timeout") const defaultScrapeTimeout = 7 * time.Second +// scrapeFlightKey is the singleflight key for overlapping /metrics fan-outs. +const scrapeFlightKey = "metricsstream-fanout" + // metricsConn is one authenticated MetricsStream. Scrapes are serialized // because MetricsScrapeRequest has no scrape id (JEP-0013). type metricsConn struct { @@ -202,6 +205,19 @@ func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) (*pb.Me } func (s *TelemetryService) fanoutScrapes(ctx context.Context) []exporterSnapshot { + ch := s.scrapeGroup.DoChan(scrapeFlightKey, func() (any, error) { + return s.doFanoutScrapes(), nil + }) + select { + case r := <-ch: + snaps, _ := r.Val.([]exporterSnapshot) + return snaps + case <-ctx.Done(): + return nil + } +} + +func (s *TelemetryService) doFanoutScrapes() []exporterSnapshot { s.initMetricsState() s.initScrapeTimeouts() timeout := s.scrapeTimeoutDuration() @@ -210,7 +226,7 @@ func (s *TelemetryService) fanoutScrapes(ctx context.Context) []exporterSnapshot return nil } - ctx, cancel := context.WithTimeout(ctx, timeout) + ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() var mu sync.Mutex diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index 9fcd9f33f..dcc74f7af 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -23,6 +23,8 @@ import ( "net" "net/http" "strings" + "sync" + "sync/atomic" "testing" "time" @@ -559,3 +561,152 @@ func TestFanout_StructuredFamiliesKeepHistogramExemplars(t *testing.T) { default: } } + +func TestFanout_CoalescesConcurrentHTTPScrapes(t *testing.T) { + svc, client, addr := startTestHub(t, 2*time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "sidekick"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + + var scrapes atomic.Int32 + release := make(chan struct{}) + errCh := make(chan error, 1) + go func() { + for { + msg, err := stream.Recv() + if err != nil { + errCh <- err + return + } + if msg.GetScrapeRequest() == nil { + continue + } + scrapes.Add(1) + <-release + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_ScrapeResponse{ + ScrapeResponse: &pb.MetricsScrapeResponse{ + MetricsText: []byte(`# TYPE jumpstarter_active_sessions gauge +jumpstarter_active_sessions{exporter="sidekick"} 1 +# EOF +`), + }, + }, + }); err != nil { + errCh <- err + return + } + } + }() + waitRegistered(t, svc, 1) + + const n = 8 + var wg sync.WaitGroup + codes := make([]int, n) + errs := make([]error, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + client := &http.Client{Timeout: 5 * time.Second} + resp, err := client.Get("http://" + addr + "/metrics") + if err != nil { + errs[i] = err + return + } + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + codes[i] = resp.StatusCode + }(i) + } + + deadline := time.Now().Add(2 * time.Second) + for scrapes.Load() == 0 && time.Now().Before(deadline) { + time.Sleep(5 * time.Millisecond) + } + if scrapes.Load() == 0 { + t.Fatal("expected a reverse-scrape to start") + } + time.Sleep(50 * time.Millisecond) + close(release) + wg.Wait() + + if got := scrapes.Load(); got != 1 { + t.Fatalf("scrape requests = %d, want 1 coalesced fan-out", got) + } + for i := 0; i < n; i++ { + if errs[i] != nil { + t.Errorf("GET %d: %v", i, errs[i]) + continue + } + if codes[i] != http.StatusOK { + t.Errorf("GET %d status=%d, want 200", i, codes[i]) + } + } +} + +func TestStopGRPCServer_IdleCompletesQuickly(t *testing.T) { + lis := bufconn.Listen(bufconnSize) + gs := grpc.NewServer() + go func() { _ = gs.Serve(lis) }() + t.Cleanup(func() { gs.Stop(); _ = lis.Close() }) + + start := time.Now() + stopGRPCServer(gs, time.Second) + if elapsed := time.Since(start); elapsed > 300*time.Millisecond { + t.Fatalf("idle GracefulStop took %v", elapsed) + } +} + +func TestStopGRPCServer_UnblocksStuckMetricsStream(t *testing.T) { + svc := &TelemetryService{Signer: testSigner(t)} + lis := bufconn.Listen(bufconnSize) + gs := grpc.NewServer() + pb.RegisterTelemetryServiceServer(gs, svc) + go func() { _ = gs.Serve(lis) }() + + conn, err := grpc.NewClient("passthrough:///bufnet", + grpc.WithContextDialer(func(ctx context.Context, _ string) (net.Conn, error) { + return lis.DialContext(ctx) + }), + grpc.WithTransportCredentials(insecure.NewCredentials()), + ) + if err != nil { + t.Fatalf("grpc.NewClient: %v", err) + } + t.Cleanup(func() { _ = conn.Close() }) + + client := pb.NewTelemetryServiceClient(conn) + stream, err := client.MetricsStream(exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1")) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "sidekick"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + waitRegistered(t, svc, 1) + + const grace = 150 * time.Millisecond + start := time.Now() + stopGRPCServer(gs, grace) + elapsed := time.Since(start) + if elapsed < grace/2 { + t.Fatalf("stop returned in %v; stuck stream should wait for the grace deadline", elapsed) + } + if elapsed > time.Second { + t.Fatalf("stopGRPCServer hung for %v", elapsed) + } +} diff --git a/controller/internal/service/telemetry_service.go b/controller/internal/service/telemetry_service.go index b2be84936..f8c2a0ac8 100644 --- a/controller/internal/service/telemetry_service.go +++ b/controller/internal/service/telemetry_service.go @@ -30,6 +30,7 @@ import ( "github.com/jumpstarter-dev/jumpstarter/controller/internal/oidc" pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" "github.com/prometheus/client_golang/prometheus" + "golang.org/x/sync/singleflight" "google.golang.org/grpc" "google.golang.org/grpc/credentials" "google.golang.org/grpc/reflection" @@ -46,6 +47,10 @@ const ( const maxEntriesPerBatch = 500 +// grpcGracefulStopTimeout bounds GracefulStop so a stuck MetricsStream cannot +// hang process shutdown (HTTP metrics shutdown uses the same 5s budget). +const grpcGracefulStopTimeout = 5 * time.Second + // logFieldExporter is the structured log field key carrying the exporter name. const logFieldExporter = "exporter" @@ -99,6 +104,8 @@ type TelemetryService struct { metricsRegistry *prometheus.Registry metricsAddr string grpcReady atomic.Bool + // scrapeGroup coalesces overlapping /metrics reverse-scrapes. + scrapeGroup singleflight.Group } // PushLogs receives a batch of structured log entries and writes them via the @@ -300,7 +307,7 @@ func (s *TelemetryService) Start(ctx context.Context) error { defer cancel() _ = httpShutdown(shutdownCtx) } - srv.GracefulStop() + stopGRPCServer(srv, grpcGracefulStopTimeout) if err := <-errCh; err != nil && !errors.Is(err, grpc.ErrServerStopped) { return err } @@ -316,3 +323,28 @@ func (s *TelemetryService) Start(ctx context.Context) error { return err } } + +// stopGRPCServer runs GracefulStop with a deadline, then Stop, so long-lived +// MetricsStream RPCs cannot block process termination. +func stopGRPCServer(srv *grpc.Server, timeout time.Duration) { + if srv == nil { + return + } + if timeout <= 0 { + srv.Stop() + return + } + done := make(chan struct{}) + go func() { + srv.GracefulStop() + close(done) + }() + timer := time.NewTimer(timeout) + defer timer.Stop() + select { + case <-done: + case <-timer.C: + srv.Stop() + <-done + } +} From 18723c40785cf8fe82e06041641a2ee40629f52d Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 12:43:54 -0230 Subject: [PATCH 06/14] test: cover MetricsStream scrape, unregister, and merge fallbacks Add the unit tests requested on PR #1060 for send errors, in-flight cancellation, metricsHTTPEnabled, replacement connections, and empty allowlist defaults. Co-authored-by: Cursor --- .../internal/service/metrics_merge_test.go | 76 +++++++++++++++++++ .../internal/service/metrics_stream_test.go | 64 ++++++++++++++++ .../internal/service/telemetry_http_test.go | 36 +++++++++ 3 files changed, 176 insertions(+) create mode 100644 controller/internal/service/telemetry_http_test.go diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go index 984612040..828c02905 100644 --- a/controller/internal/service/metrics_merge_test.go +++ b/controller/internal/service/metrics_merge_test.go @@ -260,3 +260,79 @@ func TestEncodeMetricFamilies_WritesOpenMetricsEOF(t *testing.T) { t.Errorf("decoded name = %q", mf.GetName()) } } + +func TestMergeConfigFor_EmptyAllowlistsUseDefaults(t *testing.T) { + svc := &TelemetryService{} + cfg := svc.mergeConfigFor("sidekick") + if cfg.exporterName != "sidekick" { + t.Errorf("exporterName = %q, want sidekick", cfg.exporterName) + } + for _, dt := range DefaultDriverTypeEnum { + if _, ok := cfg.driverTypes[dt]; !ok { + t.Errorf("default driver type %q missing", dt) + } + } + for _, key := range DefaultExemplarKeys { + if _, ok := cfg.exemplarKeys[key]; !ok { + t.Errorf("default exemplar key %q missing", key) + } + } + + m := &dto.Metric{ + Label: []*dto.LabelPair{labelPair("driver_type", "tuya")}, + Counter: &dto.Counter{ + Exemplar: &dto.Exemplar{ + Label: []*dto.LabelPair{ + labelPair("client", "ci"), + labelPair("trace_id", "drop-me"), + }, + }, + }, + } + applyMetric(m, cfg) + if got, _ := getLabel(m, "driver_type"); got != "other" { + t.Errorf("unknown driver_type = %q, want other", got) + } + ex := m.Counter.GetExemplar() + if ex == nil { + t.Fatal("allowlisted exemplar dropped") + } + got := map[string]string{} + for _, lp := range ex.GetLabel() { + got[lp.GetName()] = lp.GetValue() + } + if got["client"] != "ci" { + t.Errorf("exemplar labels = %v, want client kept", got) + } + if _, ok := got["trace_id"]; ok { + t.Errorf("trace_id should have been filtered, got %v", got) + } +} + +func TestMergeConfigFor_EmptyTypesUsesDefaultDriverEnum(t *testing.T) { + svc := &TelemetryService{ExemplarKeys: []string{"client"}} + cfg := svc.mergeConfigFor("exp-a") + if _, ok := cfg.driverTypes["power"]; !ok { + t.Fatal("empty DriverTypeEnum should fall back to DefaultDriverTypeEnum") + } + if _, ok := cfg.exemplarKeys["client"]; !ok { + t.Fatal("configured exemplar key client missing") + } + if _, ok := cfg.exemplarKeys["lease_id"]; ok { + t.Fatal("custom ExemplarKeys should not include unset lease_id") + } +} + +func TestMergeConfigFor_EmptyKeysUsesDefaultExemplarKeys(t *testing.T) { + svc := &TelemetryService{DriverTypeEnum: []string{"power"}} + cfg := svc.mergeConfigFor("exp-a") + if _, ok := cfg.exemplarKeys["lease_id"]; !ok { + t.Fatal("empty ExemplarKeys should fall back to DefaultExemplarKeys") + } + if _, ok := cfg.driverTypes["power"]; !ok { + t.Fatal("configured driver type power missing") + } + if _, ok := cfg.driverTypes["storage"]; ok { + t.Fatal("custom DriverTypeEnum should not include unset storage") + } +} diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index dcc74f7af..c3c9aff95 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -255,6 +255,70 @@ func TestMetricsConn_ScrapeTimeout(t *testing.T) { } } +func TestMetricsConn_ScrapeReturnsSendError(t *testing.T) { + sentinel := errors.New("send failed") + c := &metricsConn{ + send: func(*pb.MetricsStreamResponse) error { return sentinel }, + done: make(chan struct{}), + } + _, err := c.scrape(context.Background(), time.Second) + if !errors.Is(err, sentinel) { + t.Fatalf("scrape error = %v, want sentinel %v", err, sentinel) + } +} + +func TestMetricsConn_ScrapeCancelsWhenDoneClosed(t *testing.T) { + started := make(chan struct{}) + unblockSend := make(chan struct{}) + done := make(chan struct{}) + c := &metricsConn{ + send: func(*pb.MetricsStreamResponse) error { + close(started) + <-unblockSend + return nil + }, + done: done, + } + errCh := make(chan error, 1) + go func() { + _, err := c.scrape(context.Background(), 5*time.Second) + errCh <- err + }() + select { + case <-started: + case <-time.After(2 * time.Second): + t.Fatal("scrape did not call send") + } + close(done) + close(unblockSend) + select { + case err := <-errCh: + if !errors.Is(err, context.Canceled) { + t.Fatalf("scrape error = %v, want context.Canceled", err) + } + case <-time.After(2 * time.Second): + t.Fatal("scrape did not return after done closed") + } +} + +func TestUnregisterConn_DoesNotDropReplacement(t *testing.T) { + svc := &TelemetryService{} + svc.initMetricsState() + id := exporterIdentity{namespace: "jumpstarter", name: "sidekick"} + first := &metricsConn{id: id} + second := &metricsConn{id: id} + svc.registerConn(first) + svc.registerConn(second) + svc.unregisterConn(first) + got := svc.snapshotConns() + if len(got) != 1 { + t.Fatalf("conns = %d, want 1 remaining replacement", len(got)) + } + if got[0] != second { + t.Fatal("unregisterConn dropped the replacement connection") + } +} + func TestFanout_TimeoutIncrementsCounterWithoutGRPC(t *testing.T) { svc := &TelemetryService{ScrapeTimeout: 40 * time.Millisecond} svc.initScrapeTimeouts() diff --git a/controller/internal/service/telemetry_http_test.go b/controller/internal/service/telemetry_http_test.go new file mode 100644 index 000000000..261e90591 --- /dev/null +++ b/controller/internal/service/telemetry_http_test.go @@ -0,0 +1,36 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import "testing" + +func TestMetricsHTTPEnabled(t *testing.T) { + t.Parallel() + cases := []struct { + addr string + want bool + }{ + {addr: "", want: false}, + {addr: "0", want: false}, + {addr: ":8080", want: true}, + } + for _, tc := range cases { + if got := metricsHTTPEnabled(tc.addr); got != tc.want { + t.Errorf("metricsHTTPEnabled(%q) = %v, want %v", tc.addr, got, tc.want) + } + } +} From 76b700285958741b913272ea0d05799eca125960 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 13:01:10 -0230 Subject: [PATCH 07/14] fix: keep hub metric families when exporter names collide Append gathered hub families last so scrape_timeouts and parse_errors cannot be replaced by an exporter snapshot of the same name. Co-authored-by: Cursor --- .../internal/service/metrics_merge_test.go | 38 ++++++++++ .../internal/service/metrics_stream_test.go | 71 +++++++++++++++++++ controller/internal/service/telemetry_http.go | 4 +- 3 files changed, 112 insertions(+), 1 deletion(-) diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go index 828c02905..3cd45a91a 100644 --- a/controller/internal/service/metrics_merge_test.go +++ b/controller/internal/service/metrics_merge_test.go @@ -261,6 +261,44 @@ func TestEncodeMetricFamilies_WritesOpenMetricsEOF(t *testing.T) { } } +func TestMergeSnapshots_LaterExtraFamilyReplacesEarlier(t *testing.T) { + name := scrapeTimeoutsMetric + helpExp := "spoofed by exporter" + helpHub := "Exporter MetricsStream scrapes that exceeded scrapeTimeout." + metricType := dto.MetricType_COUNTER + expVal, hubVal := 999.0, 1.0 + exporterFam := &dto.MetricFamily{ + Name: &name, + Help: &helpExp, + Type: &metricType, + Metric: []*dto.Metric{{ + Label: []*dto.LabelPair{labelPair(labelExporter, "sidekick")}, + Counter: &dto.Counter{Value: &expVal}, + }}, + } + hubFam := &dto.MetricFamily{ + Name: &name, + Help: &helpHub, + Type: &metricType, + Metric: []*dto.Metric{{ + Counter: &dto.Counter{Value: &hubVal}, + }}, + } + out := mergeSnapshots(nil, append([]*dto.MetricFamily{exporterFam}, hubFam), testMergeCfg, nil) + if len(out) != 1 { + t.Fatalf("families = %d, want 1", len(out)) + } + if out[0].GetHelp() != helpHub { + t.Errorf("help = %q, want hub help", out[0].GetHelp()) + } + if len(out[0].Metric) != 1 || out[0].Metric[0].GetCounter().GetValue() != hubVal { + t.Errorf("hub series replaced, got %+v", out[0].Metric) + } + if _, ok := getLabel(out[0].Metric[0], labelExporter); ok { + t.Errorf("exporter label leaked into hub family: %+v", out[0].Metric[0].GetLabel()) + } +} + func TestMergeConfigFor_EmptyAllowlistsUseDefaults(t *testing.T) { svc := &TelemetryService{} cfg := svc.mergeConfigFor("sidekick") diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index c3c9aff95..a0c6333da 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -467,6 +467,77 @@ jumpstarter_operations_total{exporter="spoofed",operation="on",result="success", } } +func TestFanout_HubMetricsWinOverExporterNameCollision(t *testing.T) { + svc, client, addr := startTestHub(t, time.Second) + ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") + stream, err := client.MetricsStream(ctx) + if err != nil { + t.Fatalf("MetricsStream: %v", err) + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_Register{ + Register: &pb.MetricsRegister{Identity: "sidekick"}, + }, + }); err != nil { + t.Fatalf("Send register: %v", err) + } + + snapshot := []byte(`# TYPE jumpstarter_scrape_timeouts_total counter +# HELP jumpstarter_scrape_timeouts_total spoofed by exporter +jumpstarter_scrape_timeouts_total{exporter="sidekick"} 999 +# TYPE jumpstarter_operations_total counter +jumpstarter_operations_total{operation="on",result="success",driver_type="power"} 4.0 +# EOF +`) + errCh := make(chan error, 1) + go func() { + for { + msg, err := stream.Recv() + if err != nil { + errCh <- err + return + } + if msg.GetScrapeRequest() == nil { + continue + } + if err := stream.Send(&pb.MetricsStreamRequest{ + Msg: &pb.MetricsStreamRequest_ScrapeResponse{ + ScrapeResponse: &pb.MetricsScrapeResponse{MetricsText: snapshot}, + }, + }); err != nil { + errCh <- err + return + } + } + }() + waitRegistered(t, svc, 1) + + code, body := httpGet(t, "http://"+addr+"/metrics") + if code != http.StatusOK { + t.Fatalf("GET /metrics status=%d body=%s", code, body) + } + if !strings.Contains(body, "jumpstarter_operations_total") { + t.Errorf("non-colliding exporter series missing:\n%s", body) + } + if strings.Contains(body, `jumpstarter_scrape_timeouts_total{exporter="sidekick"}`) { + t.Errorf("exporter must not overwrite hub scrape-timeout family:\n%s", body) + } + if strings.Contains(body, "spoofed by exporter") { + t.Errorf("exporter HELP text overwrote hub metric:\n%s", body) + } + if !strings.Contains(body, scrapeTimeoutsMetric) { + t.Errorf("hub scrape-timeout metric missing:\n%s", body) + } + + select { + case err := <-errCh: + if err != nil && err != io.EOF && status.Code(err) != codes.Canceled && status.Code(err) != codes.Unavailable { + t.Fatalf("stream goroutine: %v", err) + } + default: + } +} + func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testing.T) { svc, client, addr := startTestHub(t, time.Second) ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") diff --git a/controller/internal/service/telemetry_http.go b/controller/internal/service/telemetry_http.go index ecd403019..5f1d64367 100644 --- a/controller/internal/service/telemetry_http.go +++ b/controller/internal/service/telemetry_http.go @@ -130,7 +130,9 @@ func (s *TelemetryService) handleMetrics(w http.ResponseWriter, r *http.Request) http.Error(w, err.Error(), http.StatusInternalServerError) return } - families = mergeSnapshots(nil, append(gathered, families...), s.mergeConfigFor, nil) + // Hub families last: mergeSnapshots last-wins on extra, so exporters + // cannot replace scrape_timeouts / parse_errors (or any other hub series). + families = mergeSnapshots(nil, append(families, gathered...), s.mergeConfigFor, nil) } var buf bytes.Buffer From a18c52b6411c89827f8decf8c4e5a78a055124bf Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 13:12:38 -0230 Subject: [PATCH 08/14] fix: label scrape timeouts by exporter Track jumpstarter_scrape_timeouts_total as a CounterVec and drop reserved hub family names from exporter snapshots so unused vec series cannot be spoofed. Co-authored-by: Cursor --- controller/internal/service/metrics_merge.go | 23 ++++++++++++ .../internal/service/metrics_merge_test.go | 14 ++++++++ controller/internal/service/metrics_stream.go | 4 ++- .../internal/service/metrics_stream_test.go | 36 +++++-------------- controller/internal/service/telemetry_http.go | 11 +++--- .../internal/service/telemetry_service.go | 2 +- 6 files changed, 56 insertions(+), 34 deletions(-) diff --git a/controller/internal/service/metrics_merge.go b/controller/internal/service/metrics_merge.go index a15ae6383..4d5139324 100644 --- a/controller/internal/service/metrics_merge.go +++ b/controller/internal/service/metrics_merge.go @@ -42,6 +42,11 @@ const ( metricsParseErrorsMetric = "jumpstarter_metrics_parse_errors_total" ) +// hubMetricNames are reserved telemetry-hub series. Exporter snapshots of the +// same name are dropped so a CounterVec with no observations yet cannot be +// replaced by a spoofed family. +var hubMetricNames = []string{scrapeTimeoutsMetric, metricsParseErrorsMetric} + // DefaultDriverTypeEnum is the JEP-0013 default allowlist for driver_type. var DefaultDriverTypeEnum = []string{ "power", "storage", "network", "serial", "console", "video", "composite", @@ -172,6 +177,24 @@ func mergeSnapshots(snapshots []exporterSnapshot, extra []*dto.MetricFamily, cfg return out } +func dropHubMetricFamilies(families []*dto.MetricFamily) []*dto.MetricFamily { + if len(families) == 0 { + return families + } + skip := setToMap(hubMetricNames) + out := make([]*dto.MetricFamily, 0, len(families)) + for _, f := range families { + if f == nil { + continue + } + if _, reserved := skip[f.GetName()]; reserved { + continue + } + out = append(out, f) + } + return out +} + func applyFamily(f *dto.MetricFamily, cfg mergeConfig) { for _, m := range f.GetMetric() { applyMetric(m, cfg) diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go index 3cd45a91a..ce23d0cbf 100644 --- a/controller/internal/service/metrics_merge_test.go +++ b/controller/internal/service/metrics_merge_test.go @@ -299,6 +299,20 @@ func TestMergeSnapshots_LaterExtraFamilyReplacesEarlier(t *testing.T) { } } +func TestDropHubMetricFamilies_RemovesReservedNames(t *testing.T) { + timeout := scrapeTimeoutsMetric + ops := "jumpstarter_operations_total" + metricType := dto.MetricType_COUNTER + one := 1.0 + kept := dropHubMetricFamilies([]*dto.MetricFamily{ + {Name: &timeout, Type: &metricType, Metric: []*dto.Metric{{Counter: &dto.Counter{Value: &one}}}}, + {Name: &ops, Type: &metricType, Metric: []*dto.Metric{{Counter: &dto.Counter{Value: &one}}}}, + }) + if len(kept) != 1 || kept[0].GetName() != ops { + t.Fatalf("kept %+v, want only %s", kept, ops) + } +} + func TestMergeConfigFor_EmptyAllowlistsUseDefaults(t *testing.T) { svc := &TelemetryService{} cfg := svc.mergeConfigFor("sidekick") diff --git a/controller/internal/service/metrics_stream.go b/controller/internal/service/metrics_stream.go index fb6ec6bd2..4bad45191 100644 --- a/controller/internal/service/metrics_stream.go +++ b/controller/internal/service/metrics_stream.go @@ -239,7 +239,9 @@ func (s *TelemetryService) doFanoutScrapes() []exporterSnapshot { resp, err := c.scrape(ctx, timeout) if err != nil { if errors.Is(err, errScrapeTimeout) || errors.Is(err, context.DeadlineExceeded) { - s.scrapeTimeouts.Inc() + if s.scrapeTimeouts != nil { + s.scrapeTimeouts.WithLabelValues(c.id.name).Inc() + } } log.FromContext(ctx).WithName("telemetry").V(1).Info("exporter scrape omitted", "exporter", c.id.name, diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index a0c6333da..a42e04ab4 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -337,18 +337,9 @@ func TestFanout_TimeoutIncrementsCounterWithoutGRPC(t *testing.T) { if err != nil { t.Fatalf("Gather: %v", err) } - var value float64 - var found bool - for _, mf := range mfs { - if mf.GetName() == scrapeTimeoutsMetric || mf.GetName()+"_total" == scrapeTimeoutsMetric { - found = true - if len(mf.Metric) > 0 { - value = mf.Metric[0].GetCounter().GetValue() - } - } - } - if !found || value < 1 { - t.Fatalf("scrape timeout counter = %v found=%v, want >= 1", value, found) + value := labeledCounterValue(t, mfs, scrapeTimeoutsMetric, labelExporter, "slow") + if value != 1 { + t.Fatalf("%s{exporter=slow} = %v, want 1", scrapeTimeoutsMetric, value) } } @@ -383,22 +374,16 @@ func TestFanout_TimeoutOmitsExporterAndIncrementsCounter(t *testing.T) { if strings.Contains(body, "jumpstarter_operations_total") { t.Errorf("timed-out exporter metrics must be omitted, body:\n%s", body) } + if !strings.Contains(body, `jumpstarter_scrape_timeouts_total{exporter="slow"}`) { + t.Errorf("hub timeout counter missing exporter label:\n%s", body) + } mfs, err := svc.metricsRegistry.Gather() if err != nil { t.Fatalf("Gather: %v", err) } - var value float64 - var found bool - for _, mf := range mfs { - if mf.GetName() == scrapeTimeoutsMetric || mf.GetName()+"_total" == scrapeTimeoutsMetric { - found = true - if len(mf.Metric) > 0 { - value = mf.Metric[0].GetCounter().GetValue() - } - } - } - if !found || value < 1 { - t.Fatalf("scrape timeout counter = %v found=%v body:\n%s", value, found, body) + value := labeledCounterValue(t, mfs, scrapeTimeoutsMetric, labelExporter, "slow") + if value != 1 { + t.Fatalf("%s{exporter=slow} = %v, want 1 body:\n%s", scrapeTimeoutsMetric, value, body) } } @@ -525,9 +510,6 @@ jumpstarter_operations_total{operation="on",result="success",driver_type="power" if strings.Contains(body, "spoofed by exporter") { t.Errorf("exporter HELP text overwrote hub metric:\n%s", body) } - if !strings.Contains(body, scrapeTimeoutsMetric) { - t.Errorf("hub scrape-timeout metric missing:\n%s", body) - } select { case err := <-errCh: diff --git a/controller/internal/service/telemetry_http.go b/controller/internal/service/telemetry_http.go index 5f1d64367..ffd95bd98 100644 --- a/controller/internal/service/telemetry_http.go +++ b/controller/internal/service/telemetry_http.go @@ -35,10 +35,10 @@ func (s *TelemetryService) initScrapeTimeouts() { return } s.metricsRegistry = prometheus.NewRegistry() - s.scrapeTimeouts = prometheus.NewCounter(prometheus.CounterOpts{ + s.scrapeTimeouts = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: scrapeTimeoutsMetric, Help: "Exporter MetricsStream scrapes that exceeded scrapeTimeout.", - }) + }, []string{labelExporter}) s.parseErrors = prometheus.NewCounterVec(prometheus.CounterOpts{ Name: metricsParseErrorsMetric, Help: "Exporter MetricsStream snapshots omitted because OpenMetrics parse failed.", @@ -130,9 +130,10 @@ func (s *TelemetryService) handleMetrics(w http.ResponseWriter, r *http.Request) http.Error(w, err.Error(), http.StatusInternalServerError) return } - // Hub families last: mergeSnapshots last-wins on extra, so exporters - // cannot replace scrape_timeouts / parse_errors (or any other hub series). - families = mergeSnapshots(nil, append(families, gathered...), s.mergeConfigFor, nil) + // Drop reserved hub names from exporter snapshots, then append gathered + // hub families last. CounterVec series are omitted until first Inc, so + // last-wins alone cannot protect unused hub names from spoofing. + families = mergeSnapshots(nil, append(dropHubMetricFamilies(families), gathered...), s.mergeConfigFor, nil) } var buf bytes.Buffer diff --git a/controller/internal/service/telemetry_service.go b/controller/internal/service/telemetry_service.go index f8c2a0ac8..3f1634fd9 100644 --- a/controller/internal/service/telemetry_service.go +++ b/controller/internal/service/telemetry_service.go @@ -99,7 +99,7 @@ type TelemetryService struct { stateMu sync.Mutex conns map[string]*metricsConn - scrapeTimeouts prometheus.Counter + scrapeTimeouts *prometheus.CounterVec parseErrors *prometheus.CounterVec metricsRegistry *prometheus.Registry metricsAddr string From 6e630e2bf6e79960d5d14069fb7c582da08b0ab6 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 13:23:52 -0230 Subject: [PATCH 09/14] fix: time out MetricsStream scrapes when Send blocks stream.Send has no context and can stall if the exporter is not reading, so honor timeout and cancel without waiting for Send, and refuse a second Send while one is still in flight. Co-authored-by: Cursor --- controller/internal/service/metrics_stream.go | 59 +++++---- .../internal/service/metrics_stream_test.go | 113 +++++++++++++++++- 2 files changed, 149 insertions(+), 23 deletions(-) diff --git a/controller/internal/service/metrics_stream.go b/controller/internal/service/metrics_stream.go index 4bad45191..87dda0d12 100644 --- a/controller/internal/service/metrics_stream.go +++ b/controller/internal/service/metrics_stream.go @@ -40,8 +40,12 @@ const scrapeFlightKey = "metricsstream-fanout" type metricsConn struct { id exporterIdentity scrapeMu sync.Mutex - mu sync.Mutex - send func(*pb.MetricsStreamResponse) error + // sendMu is held for the lifetime of one stream.Send. A scrape that + // times out while Send is blocked must not start another Send: gRPC + // streams are not concurrent-safe for Send. + sendMu sync.Mutex + mu sync.Mutex + send func(*pb.MetricsStreamResponse) error // pending is non-nil only while a scrape is waiting for a response. pending chan *pb.MetricsScrapeResponse done <-chan struct{} @@ -166,6 +170,13 @@ func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) (*pb.Me c.scrapeMu.Lock() defer c.scrapeMu.Unlock() + // stream.Send has no context and can block indefinitely if the exporter + // is not reading. Run it in the background so timeout, ctx, and conn + // teardown can still finish the scrape. + if !c.sendMu.TryLock() { + return nil, errScrapeTimeout + } + reply := make(chan *pb.MetricsScrapeResponse, 1) c.mu.Lock() c.pending = reply @@ -178,29 +189,37 @@ func (c *metricsConn) scrape(ctx context.Context, timeout time.Duration) (*pb.Me c.mu.Unlock() }() - err := c.send(&pb.MetricsStreamResponse{ - Msg: &pb.MetricsStreamResponse_ScrapeRequest{ - ScrapeRequest: &pb.MetricsScrapeRequest{}, - }, - }) - if err != nil { - return nil, err - } + sendErr := make(chan error, 1) + go func() { + defer c.sendMu.Unlock() + sendErr <- c.send(&pb.MetricsStreamResponse{ + Msg: &pb.MetricsStreamResponse_ScrapeRequest{ + ScrapeRequest: &pb.MetricsScrapeRequest{}, + }, + }) + }() timer := time.NewTimer(timeout) defer timer.Stop() - select { - case resp := <-reply: - if resp == nil { + for { + select { + case err := <-sendErr: + if err != nil { + return nil, err + } + // Request was written; keep waiting for the scrape response. + case resp := <-reply: + if resp == nil { + return nil, errScrapeTimeout + } + return resp, nil + case <-timer.C: return nil, errScrapeTimeout + case <-ctx.Done(): + return nil, ctx.Err() + case <-c.done: + return nil, context.Canceled } - return resp, nil - case <-timer.C: - return nil, errScrapeTimeout - case <-ctx.Done(): - return nil, ctx.Err() - case <-c.done: - return nil, context.Canceled } } diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index a42e04ab4..b399d6387 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -267,6 +267,113 @@ func TestMetricsConn_ScrapeReturnsSendError(t *testing.T) { } } +func TestMetricsConn_ScrapeTimesOutWhenSendBlocks(t *testing.T) { + unblock := make(chan struct{}) + c := &metricsConn{ + send: func(*pb.MetricsStreamResponse) error { + <-unblock + return nil + }, + done: make(chan struct{}), + } + t.Cleanup(func() { close(unblock) }) + + errCh := make(chan error, 1) + go func() { + _, err := c.scrape(context.Background(), 40*time.Millisecond) + errCh <- err + }() + select { + case err := <-errCh: + if !errors.Is(err, errScrapeTimeout) { + t.Fatalf("scrape error = %v, want errScrapeTimeout", err) + } + case <-time.After(300 * time.Millisecond): + t.Fatal("scrape blocked on send past scrapeTimeout") + } +} + +func TestMetricsConn_ScrapeCancelsWhenSendBlocks(t *testing.T) { + unblock := make(chan struct{}) + started := make(chan struct{}) + c := &metricsConn{ + send: func(*pb.MetricsStreamResponse) error { + close(started) + <-unblock + return nil + }, + done: make(chan struct{}), + } + t.Cleanup(func() { close(unblock) }) + + ctx, cancel := context.WithCancel(context.Background()) + errCh := make(chan error, 1) + go func() { + _, err := c.scrape(ctx, 5*time.Second) + errCh <- err + }() + select { + case <-started: + case <-time.After(2 * time.Second): + t.Fatal("scrape did not call send") + } + cancel() + select { + case err := <-errCh: + if !errors.Is(err, context.Canceled) { + t.Fatalf("scrape error = %v, want context.Canceled", err) + } + case <-time.After(300 * time.Millisecond): + t.Fatal("scrape blocked on send after ctx cancel") + } +} + +func TestMetricsConn_ScrapeDoesNotSendWhilePriorSendBlocked(t *testing.T) { + var sends atomic.Int32 + started := make(chan struct{}, 1) + unblock := make(chan struct{}) + c := &metricsConn{ + send: func(*pb.MetricsStreamResponse) error { + sends.Add(1) + select { + case started <- struct{}{}: + default: + } + <-unblock + return nil + }, + done: make(chan struct{}), + } + t.Cleanup(func() { close(unblock) }) + + first := make(chan error, 1) + go func() { + _, err := c.scrape(context.Background(), 40*time.Millisecond) + first <- err + }() + select { + case <-started: + case <-time.After(2 * time.Second): + t.Fatal("scrape did not call send") + } + select { + case err := <-first: + if !errors.Is(err, errScrapeTimeout) { + t.Fatalf("first scrape error = %v, want errScrapeTimeout", err) + } + case <-time.After(300 * time.Millisecond): + t.Fatal("first scrape blocked on send past scrapeTimeout") + } + + _, err := c.scrape(context.Background(), 40*time.Millisecond) + if !errors.Is(err, errScrapeTimeout) { + t.Fatalf("second scrape error = %v, want errScrapeTimeout", err) + } + if n := sends.Load(); n != 1 { + t.Fatalf("send calls = %d, want 1 (no concurrent stream.Send)", n) + } +} + func TestMetricsConn_ScrapeCancelsWhenDoneClosed(t *testing.T) { started := make(chan struct{}) unblockSend := make(chan struct{}) @@ -290,15 +397,15 @@ func TestMetricsConn_ScrapeCancelsWhenDoneClosed(t *testing.T) { t.Fatal("scrape did not call send") } close(done) - close(unblockSend) select { case err := <-errCh: if !errors.Is(err, context.Canceled) { t.Fatalf("scrape error = %v, want context.Canceled", err) } - case <-time.After(2 * time.Second): - t.Fatal("scrape did not return after done closed") + case <-time.After(300 * time.Millisecond): + t.Fatal("scrape blocked on send after done closed") } + close(unblockSend) } func TestUnregisterConn_DoesNotDropReplacement(t *testing.T) { From 127c0e078b0d3fa3316eecb353f2794063ac2aa4 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 13:31:10 -0230 Subject: [PATCH 10/14] fix: keep jwt parse errors out of telemetry Unauthenticated status Wrapping ParseSubject in the gRPC message told clients why verification failed (expired vs bad signature vs malformed). Return a generic invalid token and log the parse error server-side. Co-authored-by: Cursor --- .../internal/service/telemetry_identity.go | 7 +- .../service/telemetry_identity_test.go | 82 +++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 controller/internal/service/telemetry_identity_test.go diff --git a/controller/internal/service/telemetry_identity.go b/controller/internal/service/telemetry_identity.go index e1f23de4e..b01be74ca 100644 --- a/controller/internal/service/telemetry_identity.go +++ b/controller/internal/service/telemetry_identity.go @@ -23,6 +23,7 @@ import ( "github.com/jumpstarter-dev/jumpstarter/controller/internal/authentication" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "sigs.k8s.io/controller-runtime/pkg/log" ) // exporterIdentity is the exporter CRD namespace/name claimed by a bearer token. @@ -56,7 +57,11 @@ func (s *TelemetryService) authenticateExporter(ctx context.Context) (exporterId } subject, err := s.Signer.ParseSubject(token) if err != nil { - return exporterIdentity{}, status.Errorf(codes.Unauthenticated, "invalid token: %v", err) + // Do not wrap the jwt parse error into the gRPC status: clients + // would learn why verification failed (expired vs bad signature vs + // malformed). RouterService likewise returns a generic client message. + log.FromContext(ctx).V(1).Info("telemetry token parse failed", "error", err.Error()) + return exporterIdentity{}, status.Error(codes.Unauthenticated, "invalid token") } return parseExporterSubject(subject) } diff --git a/controller/internal/service/telemetry_identity_test.go b/controller/internal/service/telemetry_identity_test.go new file mode 100644 index 000000000..f4abe2578 --- /dev/null +++ b/controller/internal/service/telemetry_identity_test.go @@ -0,0 +1,82 @@ +/* +Copyright 2026. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package service + +import ( + "context" + "strings" + "testing" + "time" + + "github.com/jumpstarter-dev/jumpstarter/controller/internal/oidc" + "google.golang.org/grpc/codes" + "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" +) + +func TestAuthenticateExporter_InvalidTokenDoesNotLeakParseError(t *testing.T) { + hub := testSigner(t) + expired := testSigner(t) + expired.SetTokenLifetime(-time.Hour) + expiredTok, err := expired.Token("exporter:jumpstarter:alice:uid1") + if err != nil { + t.Fatalf("expired token: %v", err) + } + other, err := oidc.NewSignerFromSeed([]byte("other-seed"), hub.Issuer(), hub.Audience()) + if err != nil { + t.Fatalf("other signer: %v", err) + } + foreignTok, err := other.Token("exporter:jumpstarter:alice:uid1") + if err != nil { + t.Fatalf("foreign token: %v", err) + } + + cases := []struct { + name string + token string + leak string + }{ + {name: "malformed", token: "not-a-real-token", leak: "malformed"}, + {name: "expired", token: expiredTok, leak: "expired"}, + {name: "wrong key", token: foreignTok, leak: "signature"}, + } + + svc := &TelemetryService{Signer: hub} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctx := metadata.NewIncomingContext( + context.Background(), + metadata.Pairs("authorization", "Bearer "+tc.token), + ) + _, err := svc.authenticateExporter(ctx) + if err == nil { + t.Fatal("expected Unauthenticated") + } + st := status.Convert(err) + if st.Code() != codes.Unauthenticated { + t.Fatalf("code = %v, want Unauthenticated (%v)", st.Code(), err) + } + msg := st.Message() + if msg != "invalid token" { + t.Fatalf("client message = %q, want %q (must not wrap jwt parse error)", msg, "invalid token") + } + if strings.Contains(strings.ToLower(msg), tc.leak) { + t.Fatalf("client message %q leaked jwt detail %q", msg, tc.leak) + } + }) + } +} From b98a6739b831ed11dda5b45d6323c78eef884a66 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Fri, 11 Sep 2026 13:44:09 -0230 Subject: [PATCH 11/14] test: require parse-error counter to increment exactly once value < 1 would miss a double-increment on jumpstarter_metrics_parse_errors_total. Co-authored-by: Cursor --- controller/internal/service/metrics_stream_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index b399d6387..c81dd4c73 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -681,8 +681,8 @@ func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testin t.Fatalf("Gather: %v", err) } value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, labelExporter, "sidekick") - if value < 1 { - t.Fatalf("%s{exporter=sidekick} = %v, want >= 1 body:\n%s", metricsParseErrorsMetric, value, body) + if value != 1 { + t.Fatalf("%s{exporter=sidekick} = %v, want 1 body:\n%s", metricsParseErrorsMetric, value, body) } select { From 66dea8fe9cefa03454d7845bc4c2fb370661bec7 Mon Sep 17 00:00:00 2001 From: Roddie Kieley Date: Sat, 12 Sep 2026 09:29:44 -0230 Subject: [PATCH 12/14] fix: reject invalid structured metric counts and normalize info families Malformed summary quantiles and non-integral histogram counts were silently truncated. Map prometheus_client info and stateset collectors to gauges so the sidecar matches OpenMetrics names and types. Co-authored-by: Cursor --- .../internal/service/metrics_families.go | 42 ++++++-- .../internal/service/metrics_families_test.go | 95 +++++++++++++++++++ .../jumpstarter/metrics/families.py | 7 ++ .../jumpstarter/metrics/metrics_test.py | 23 +++++ 4 files changed, 157 insertions(+), 10 deletions(-) diff --git a/controller/internal/service/metrics_families.go b/controller/internal/service/metrics_families.go index be5963166..61684e872 100644 --- a/controller/internal/service/metrics_families.go +++ b/controller/internal/service/metrics_families.go @@ -189,7 +189,11 @@ func dtoHistogramMetrics(family string, samples []*pb.MetricsSample) ([]*dto.Met } switch { case sname == family+"_count": - agg.count = uint64(sample.GetValue()) + count, err := uint64Count(sample.GetValue()) + if err != nil { + return nil, err + } + agg.count = count case sname == family+"_sum": agg.sum = sample.GetValue() case isBucket || sname == family+"_bucket": @@ -197,7 +201,10 @@ func dtoHistogramMetrics(family string, samples []*pb.MetricsSample) ([]*dto.Met if err != nil { return nil, fmt.Errorf("bucket le %q: %w", le, err) } - count := uint64(sample.GetValue()) + count, err := uint64Count(sample.GetValue()) + if err != nil { + return nil, err + } agg.buckets = append(agg.buckets, &dto.Bucket{ CumulativeCount: &count, UpperBound: &bound, @@ -239,7 +246,10 @@ func dtoSummaryMetrics(family string, samples []*pb.MetricsSample) ([]*dto.Metri continue } labels := dtoLabels(sample.GetLabels()) - base, qv, isQuantile := splitQuantileLabel(labels) + base, qv, isQuantile, err := splitQuantileLabel(labels) + if err != nil { + return nil, err + } key := labelsKey(base) agg, ok := grouped[key] if !ok { @@ -249,7 +259,11 @@ func dtoSummaryMetrics(family string, samples []*pb.MetricsSample) ([]*dto.Metri } switch sample.GetName() { case family + "_count": - agg.count = uint64(sample.GetValue()) + count, err := uint64Count(sample.GetValue()) + if err != nil { + return nil, err + } + agg.count = count case family + "_sum": agg.sum = sample.GetValue() default: @@ -288,20 +302,28 @@ func splitLeLabel(labels []*dto.LabelPair) (base []*dto.LabelPair, le string, ok return base, le, ok } -func splitQuantileLabel(labels []*dto.LabelPair) (base []*dto.LabelPair, q float64, ok bool) { +func splitQuantileLabel(labels []*dto.LabelPair) (base []*dto.LabelPair, q float64, ok bool, err error) { base = make([]*dto.LabelPair, 0, len(labels)) for _, lp := range labels { if lp.GetName() == "quantile" { - v, err := strconv.ParseFloat(lp.GetValue(), 64) - if err == nil { - q = v - ok = true + v, perr := strconv.ParseFloat(lp.GetValue(), 64) + if perr != nil { + return nil, 0, false, fmt.Errorf("quantile %q: %w", lp.GetValue(), perr) } + q = v + ok = true continue } base = append(base, lp) } - return base, q, ok + return base, q, ok, nil +} + +func uint64Count(v float64) (uint64, error) { + if math.IsNaN(v) || math.IsInf(v, 0) || v < 0 || v != math.Trunc(v) { + return 0, fmt.Errorf("invalid count %v", v) + } + return uint64(v), nil } func parseLe(le string) (float64, error) { diff --git a/controller/internal/service/metrics_families_test.go b/controller/internal/service/metrics_families_test.go index ce9307b64..47569b884 100644 --- a/controller/internal/service/metrics_families_test.go +++ b/controller/internal/service/metrics_families_test.go @@ -18,6 +18,7 @@ package service import ( "math" + "strings" "testing" pb "github.com/jumpstarter-dev/jumpstarter/controller/internal/protocol/jumpstarter/v1" @@ -248,3 +249,97 @@ func TestMergeSnapshots_PrefersFamiliesOverUnparseableText(t *testing.T) { t.Errorf("lease_id missing from filtered exemplar: %v", ex.GetLabel()) } } + +func TestDtoFromProtoFamilies_RejectsMalformedSummaryQuantile(t *testing.T) { + _, err := dtoFromProtoFamilies([]*pb.MetricsFamily{{ + Name: "rpc_latency_seconds", + Help: "RPC latency.", + Type: pb.MetricsType_METRICS_TYPE_SUMMARY, + Samples: []*pb.MetricsSample{ + { + Name: "rpc_latency_seconds", + Labels: pbL("quantile", "not-a-number"), + Value: 0.2, + }, + { + Name: "rpc_latency_seconds_count", + Value: 10, + }, + { + Name: "rpc_latency_seconds_sum", + Value: 1.5, + }, + }, + }}) + if err == nil { + t.Fatal("expected error for non-numeric quantile label") + } +} + +func TestDtoFromProtoFamilies_RejectsInvalidCounts(t *testing.T) { + cases := []struct { + name string + family *pb.MetricsFamily + wantErr string + }{ + { + name: "histogram negative count", + family: &pb.MetricsFamily{ + Name: "op_duration_seconds", + Type: pb.MetricsType_METRICS_TYPE_HISTOGRAM, + Samples: []*pb.MetricsSample{ + {Name: "op_duration_seconds_count", Value: -1}, + {Name: "op_duration_seconds_sum", Value: 0.1}, + }, + }, + wantErr: "count", + }, + { + name: "histogram fractional count", + family: &pb.MetricsFamily{ + Name: "op_duration_seconds", + Type: pb.MetricsType_METRICS_TYPE_HISTOGRAM, + Samples: []*pb.MetricsSample{ + {Name: "op_duration_seconds_count", Value: 1.5}, + {Name: "op_duration_seconds_sum", Value: 0.1}, + }, + }, + wantErr: "count", + }, + { + name: "histogram nan bucket", + family: &pb.MetricsFamily{ + Name: "op_duration_seconds", + Type: pb.MetricsType_METRICS_TYPE_HISTOGRAM, + Samples: []*pb.MetricsSample{ + {Name: "op_duration_seconds_bucket", Labels: pbL("le", "+Inf"), Value: math.NaN()}, + {Name: "op_duration_seconds_count", Value: 1}, + }, + }, + wantErr: "count", + }, + { + name: "summary infinite count", + family: &pb.MetricsFamily{ + Name: "rpc_latency_seconds", + Type: pb.MetricsType_METRICS_TYPE_SUMMARY, + Samples: []*pb.MetricsSample{ + {Name: "rpc_latency_seconds_count", Value: math.Inf(1)}, + {Name: "rpc_latency_seconds_sum", Value: 1}, + }, + }, + wantErr: "count", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := dtoFromProtoFamilies([]*pb.MetricsFamily{tc.family}) + if err == nil { + t.Fatal("expected error for invalid count") + } + if !strings.Contains(err.Error(), tc.wantErr) { + t.Fatalf("error %q, want substring %q", err, tc.wantErr) + } + }) + } +} diff --git a/python/packages/jumpstarter/jumpstarter/metrics/families.py b/python/packages/jumpstarter/jumpstarter/metrics/families.py index 07af6a0aa..608924231 100644 --- a/python/packages/jumpstarter/jumpstarter/metrics/families.py +++ b/python/packages/jumpstarter/jumpstarter/metrics/families.py @@ -18,6 +18,8 @@ "gauge": telemetry_pb2.METRICS_TYPE_GAUGE, "histogram": telemetry_pb2.METRICS_TYPE_HISTOGRAM, "gaugehistogram": telemetry_pb2.METRICS_TYPE_HISTOGRAM, + "info": telemetry_pb2.METRICS_TYPE_GAUGE, + "stateset": telemetry_pb2.METRICS_TYPE_GAUGE, "summary": telemetry_pb2.METRICS_TYPE_SUMMARY, "untyped": telemetry_pb2.METRICS_TYPE_UNTYPED, "unknown": telemetry_pb2.METRICS_TYPE_UNTYPED, @@ -31,10 +33,15 @@ def _family_name(metric: Any) -> str: family name. The hub OpenMetrics encoder treats a counter without that suffix as ``unknown``, and text-path snapshots already use the ``_total`` name, so mixed exporters would otherwise fail to merge. + + ``Info`` collectors use type ``info`` and a family name without ``_info``; + OpenMetrics appends that suffix, so the sidecar does the same. """ name = metric.name if metric.type == "counter" and not name.endswith("_total"): return f"{name}_total" + if metric.type == "info" and not name.endswith("_info"): + return f"{name}_info" return name diff --git a/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py b/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py index efb4ea53a..c9ccf04cd 100644 --- a/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py +++ b/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py @@ -243,6 +243,29 @@ def test_scrape_response_families_carry_exemplars_without_text_parse(): } +def test_families_from_collector_normalizes_info_and_enum(): + from prometheus_client import CollectorRegistry, Enum, Info + from jumpstarter_protocol import telemetry_pb2 + + from jumpstarter.metrics.families import families_from_collector + + registry = CollectorRegistry() + Info("jumpstarter_build", "Build metadata", registry=registry).info({"version": "9"}) + Enum( + "jumpstarter_lease", + "Lease state", + states=["free", "held"], + registry=registry, + ).state("held") + + fams = {fam.name: fam for fam in families_from_collector(registry)} + build = fams["jumpstarter_build_info"] + assert build.type == telemetry_pb2.METRICS_TYPE_GAUGE + lease = fams["jumpstarter_lease"] + assert lease.type == telemetry_pb2.METRICS_TYPE_GAUGE + assert "jumpstarter_lease_state" not in fams + + def test_metrics_http_endpoint_serves_prometheus_text(): reg = get_registry() reg.set_active_sessions(exporter="lab-01", value=2) From 7b61f5c47e06f15a60977024eb84d2fb50beb30a Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Pelayo Date: Tue, 22 Sep 2026 11:03:31 +0200 Subject: [PATCH 13/14] fix: address lint-go, lint-python, and typos CI failures - drop constant params flagged by unparam (labeledCounterValue label, waitRegistered n) - rename Unparseable test names per typos - sort import block in metrics_test.py per ruff I001 Signed-off-by: Miguel Angel Ajo Pelayo --- .../internal/service/metrics_families_test.go | 2 +- .../internal/service/metrics_merge_test.go | 8 +++--- .../internal/service/metrics_stream_test.go | 28 +++++++++---------- .../jumpstarter/metrics/metrics_test.py | 2 +- 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/controller/internal/service/metrics_families_test.go b/controller/internal/service/metrics_families_test.go index 47569b884..470edd6df 100644 --- a/controller/internal/service/metrics_families_test.go +++ b/controller/internal/service/metrics_families_test.go @@ -182,7 +182,7 @@ func TestDtoFromProtoFamilies_CanonicalizesOpenMetricsCounterName(t *testing.T) } } -func TestMergeSnapshots_PrefersFamiliesOverUnparseableText(t *testing.T) { +func TestMergeSnapshots_PrefersFamiliesOverUnparsableText(t *testing.T) { var calls int families := mergeSnapshots([]exporterSnapshot{{ name: "sidekick", diff --git a/controller/internal/service/metrics_merge_test.go b/controller/internal/service/metrics_merge_test.go index ce23d0cbf..171ce7052 100644 --- a/controller/internal/service/metrics_merge_test.go +++ b/controller/internal/service/metrics_merge_test.go @@ -26,7 +26,7 @@ import ( "github.com/prometheus/common/expfmt" ) -func labeledCounterValue(t *testing.T, mfs []*dto.MetricFamily, name, label, want string) float64 { +func labeledCounterValue(t *testing.T, mfs []*dto.MetricFamily, name, want string) float64 { t.Helper() for _, mf := range mfs { if mf.GetName() != name && mf.GetName()+"_total" != name { @@ -34,13 +34,13 @@ func labeledCounterValue(t *testing.T, mfs []*dto.MetricFamily, name, label, wan } for _, m := range mf.Metric { for _, lp := range m.GetLabel() { - if lp.GetName() == label && lp.GetValue() == want { + if lp.GetName() == labelExporter && lp.GetValue() == want { return m.GetCounter().GetValue() } } } } - t.Fatalf("%s{%s=%q} missing from gathered families", name, label, want) + t.Fatalf("%s{%s=%q} missing from gathered families", name, labelExporter, want) return 0 } @@ -221,7 +221,7 @@ func TestRecordMetricsParseError_IncrementsLabeledCounter(t *testing.T) { if err != nil { t.Fatalf("Gather: %v", err) } - value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, labelExporter, "sidekick") + value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, "sidekick") if value != 1 { t.Fatalf("%s{exporter=sidekick} = %v, want 1", metricsParseErrorsMetric, value) } diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index c81dd4c73..80f56533c 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -96,12 +96,12 @@ func exporterStreamCtx(t *testing.T, svc *TelemetryService, subject string) cont ) } -func waitRegistered(t *testing.T, svc *TelemetryService, n int) { +func waitRegistered(t *testing.T, svc *TelemetryService) { t.Helper() deadline := time.Now().Add(2 * time.Second) for time.Now().Before(deadline) { conns := svc.snapshotConns() - if len(conns) != n { + if len(conns) != 1 { time.Sleep(10 * time.Millisecond) continue } @@ -125,7 +125,7 @@ func waitRegistered(t *testing.T, svc *TelemetryService, n int) { } time.Sleep(10 * time.Millisecond) } - t.Fatalf("timed out waiting for %d MetricsStream connections, have %d", n, len(svc.snapshotConns())) + t.Fatalf("timed out waiting for MetricsStream connection, have %d", len(svc.snapshotConns())) } func httpGet(t *testing.T, url string) (int, string) { @@ -444,7 +444,7 @@ func TestFanout_TimeoutIncrementsCounterWithoutGRPC(t *testing.T) { if err != nil { t.Fatalf("Gather: %v", err) } - value := labeledCounterValue(t, mfs, scrapeTimeoutsMetric, labelExporter, "slow") + value := labeledCounterValue(t, mfs, scrapeTimeoutsMetric, "slow") if value != 1 { t.Fatalf("%s{exporter=slow} = %v, want 1", scrapeTimeoutsMetric, value) } @@ -472,7 +472,7 @@ func TestFanout_TimeoutOmitsExporterAndIncrementsCounter(t *testing.T) { } } }() - waitRegistered(t, svc, 1) + waitRegistered(t, svc) code, body := httpGet(t, "http://"+addr+"/metrics") if code != http.StatusOK { @@ -488,7 +488,7 @@ func TestFanout_TimeoutOmitsExporterAndIncrementsCounter(t *testing.T) { if err != nil { t.Fatalf("Gather: %v", err) } - value := labeledCounterValue(t, mfs, scrapeTimeoutsMetric, labelExporter, "slow") + value := labeledCounterValue(t, mfs, scrapeTimeoutsMetric, "slow") if value != 1 { t.Fatalf("%s{exporter=slow} = %v, want 1 body:\n%s", scrapeTimeoutsMetric, value, body) } @@ -535,7 +535,7 @@ jumpstarter_operations_total{exporter="spoofed",operation="on",result="success", } } }() - waitRegistered(t, svc, 1) + waitRegistered(t, svc) code, body := httpGet(t, "http://"+addr+"/metrics") if code != http.StatusOK { @@ -602,7 +602,7 @@ jumpstarter_operations_total{operation="on",result="success",driver_type="power" } } }() - waitRegistered(t, svc, 1) + waitRegistered(t, svc) code, body := httpGet(t, "http://"+addr+"/metrics") if code != http.StatusOK { @@ -627,7 +627,7 @@ jumpstarter_operations_total{operation="on",result="success",driver_type="power" } } -func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testing.T) { +func TestFanout_UnparsableSnapshotIncrementsParseErrorsOnSameResponse(t *testing.T) { svc, client, addr := startTestHub(t, time.Second) ctx := exporterStreamCtx(t, svc, "exporter:jumpstarter:sidekick:uid1") stream, err := client.MetricsStream(ctx) @@ -663,7 +663,7 @@ func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testin } } }() - waitRegistered(t, svc, 1) + waitRegistered(t, svc) code, body := httpGet(t, "http://"+addr+"/metrics") if code != http.StatusOK { @@ -680,7 +680,7 @@ func TestFanout_UnparseableSnapshotIncrementsParseErrorsOnSameResponse(t *testin if err != nil { t.Fatalf("Gather: %v", err) } - value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, labelExporter, "sidekick") + value := labeledCounterValue(t, mfs, metricsParseErrorsMetric, "sidekick") if value != 1 { t.Fatalf("%s{exporter=sidekick} = %v, want 1 body:\n%s", metricsParseErrorsMetric, value, body) } @@ -758,7 +758,7 @@ func TestFanout_StructuredFamiliesKeepHistogramExemplars(t *testing.T) { } } }() - waitRegistered(t, svc, 1) + waitRegistered(t, svc) code, body := httpGet(t, "http://"+addr+"/metrics") if code != http.StatusOK { @@ -831,7 +831,7 @@ jumpstarter_active_sessions{exporter="sidekick"} 1 } } }() - waitRegistered(t, svc, 1) + waitRegistered(t, svc) const n = 8 var wg sync.WaitGroup @@ -921,7 +921,7 @@ func TestStopGRPCServer_UnblocksStuckMetricsStream(t *testing.T) { }); err != nil { t.Fatalf("Send register: %v", err) } - waitRegistered(t, svc, 1) + waitRegistered(t, svc) const grace = 150 * time.Millisecond start := time.Now() diff --git a/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py b/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py index c9ccf04cd..f468866c0 100644 --- a/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py +++ b/python/packages/jumpstarter/jumpstarter/metrics/metrics_test.py @@ -244,8 +244,8 @@ def test_scrape_response_families_carry_exemplars_without_text_parse(): def test_families_from_collector_normalizes_info_and_enum(): - from prometheus_client import CollectorRegistry, Enum, Info from jumpstarter_protocol import telemetry_pb2 + from prometheus_client import CollectorRegistry, Enum, Info from jumpstarter.metrics.families import families_from_collector From 6fc31ee2a9921a9695983338017b51ec95f07f58 Mon Sep 17 00:00:00 2001 From: Miguel Angel Ajo Pelayo Date: Tue, 22 Sep 2026 12:57:30 +0200 Subject: [PATCH 14/14] fix: apply go fix modernizations to metrics stream and telemetry HTTP --- controller/internal/service/metrics_stream_test.go | 6 +++--- controller/internal/service/telemetry_http.go | 5 +---- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/controller/internal/service/metrics_stream_test.go b/controller/internal/service/metrics_stream_test.go index 80f56533c..f5750b089 100644 --- a/controller/internal/service/metrics_stream_test.go +++ b/controller/internal/service/metrics_stream_test.go @@ -133,7 +133,7 @@ func httpGet(t *testing.T, url string) (int, string) { client := &http.Client{Timeout: 3 * time.Second} var resp *http.Response var lastErr error - for i := 0; i < 30; i++ { + for range 30 { resp, lastErr = client.Get(url) if lastErr == nil { break @@ -837,7 +837,7 @@ jumpstarter_active_sessions{exporter="sidekick"} 1 var wg sync.WaitGroup codes := make([]int, n) errs := make([]error, n) - for i := 0; i < n; i++ { + for i := range n { wg.Add(1) go func(i int) { defer wg.Done() @@ -867,7 +867,7 @@ jumpstarter_active_sessions{exporter="sidekick"} 1 if got := scrapes.Load(); got != 1 { t.Fatalf("scrape requests = %d, want 1 coalesced fan-out", got) } - for i := 0; i < n; i++ { + for i := range n { if errs[i] != nil { t.Errorf("GET %d: %v", i, errs[i]) continue diff --git a/controller/internal/service/telemetry_http.go b/controller/internal/service/telemetry_http.go index ffd95bd98..ad167f35a 100644 --- a/controller/internal/service/telemetry_http.go +++ b/controller/internal/service/telemetry_http.go @@ -76,10 +76,7 @@ func (s *TelemetryService) startMetricsHTTP() (func(context.Context) error, erro mux.HandleFunc("/healthz", s.handleHealthz) mux.HandleFunc("/readyz", s.handleReadyz) - writeTimeout := s.scrapeTimeoutDuration() + 15*time.Second - if writeTimeout < 30*time.Second { - writeTimeout = 30 * time.Second - } + writeTimeout := max(s.scrapeTimeoutDuration()+15*time.Second, 30*time.Second) srv := &http.Server{ Handler: mux,