From 10a757e3db1b7d34dd35181deaf617688995308e Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Tue, 11 Nov 2025 13:50:36 -0500 Subject: [PATCH 1/6] Introduce metrics.Registry and use it --- lib/metrics/registry.go | 122 +++++++++++++++++++++++++++++++++++++ lib/msgraph/client.go | 17 +++--- lib/msgraph/metrics.go | 18 +++--- lib/services/reconciler.go | 20 +++--- 4 files changed, 154 insertions(+), 23 deletions(-) create mode 100644 lib/metrics/registry.go diff --git a/lib/metrics/registry.go b/lib/metrics/registry.go new file mode 100644 index 0000000000000..9451b3f33a0d0 --- /dev/null +++ b/lib/metrics/registry.go @@ -0,0 +1,122 @@ +/* + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +package metrics + +import ( + "errors" + + "github.com/prometheus/client_golang/prometheus" +) + +// Registry is a Teleport metrics registry. Wrapping the [prometheus.Registerer] +// allows us to propagate additional information such as: +// - the metric namespace (`teleport`, `teleport_bot`, `teleport_plugins`) +// - an optional subsystem +// +// This should be passed alongside the logger. +type Registry struct { + prometheus.Registerer + + namespace string + subsystem string +} + +// Namespace returns the namespace that should be used by metrics registered +// in this Registry. Common namespaces are "teleport", "tbot", and +// "teleport_plugins". +func (r *Registry) Namespace() string { + return r.namespace +} + +// Subsystem is the subsystem base that should be used by metrics registered in +// this Registry. Subsystem parts can be added with WrapWithSubsystem. +func (r *Registry) Subsystem() string { + return r.subsystem +} + +// Wrap wraps a Registry by adding a component to its subsystem. +// This should be used before passing a registry to a sub-component. +// Example usage: +// +// rootReg := prometheus.NewRegistry() +// process.AddGatherer(rootReg) +// reg, err := NewRegistry(rootReg, "teleport_plugins", "") +// go runFooService(ctx, log, reg.Wrap("foo")) +// go runBarService(ctx, log, reg.Wrap("bar")) +func (r *Registry) Wrap(subsystem string) *Registry { + newReg := &Registry{ + Registerer: r.Registerer, + namespace: r.namespace, + subsystem: r.subsystem + "_" + subsystem, + } + return newReg +} + +// NewRegistry creates a new Registry wrapping a prometheus registry. +// This should only be called when starting the service management routines such +// as: service.NewTeleport(), tbot.New(), or the hosted plugin manager. +// Services and sub-services should take the registry as a parameter, like they +// already do for the logger. +// Example usage: +// +// rootReg := prometheus.NewRegistry() +// process.AddGatherer(rootReg) +// reg, err := NewRegistry(rootReg, "teleport_plugins", "") +// go runFooService(ctx, log, reg.Wrap("foo")) +// go runBarService(ctx, log, reg.Wrap("bar")) +func NewRegistry(reg prometheus.Registerer, namespace, subsystem string) (*Registry, error) { + if reg == nil { + return nil, errors.New("nil prometheus.Registerer (this is a bug)") + } + if namespace == "" { + return nil, errors.New("namespace is required (this is a bug)") + } + return &Registry{ + Registerer: reg, + namespace: namespace, + subsystem: subsystem, + }, nil +} + +// BlackHole returns a Registry that doesn't register metrics. +// This can be used in tests, or to provide backward compatibility when a nil +// Registry is passed. +func BlackHole() *Registry { + return &Registry{ + Registerer: blackHoleRegistry{}, + namespace: "blackhole", + subsystem: "", + } +} + +type blackHoleRegistry struct{} + +// Register implements [prometheus.Registerer]. +func (b blackHoleRegistry) Register(collector prometheus.Collector) error { + return nil +} + +// MustRegister implements [prometheus.Registerer]. +func (b blackHoleRegistry) MustRegister(collector ...prometheus.Collector) { +} + +// Unregister implements [prometheus.Registerer]. +func (b blackHoleRegistry) Unregister(collector prometheus.Collector) bool { + return true +} diff --git a/lib/msgraph/client.go b/lib/msgraph/client.go index 081d6b28da239..4d1666a1ec8a0 100644 --- a/lib/msgraph/client.go +++ b/lib/msgraph/client.go @@ -37,13 +37,13 @@ import ( "github.com/google/uuid" "github.com/gravitational/trace" "github.com/jonboulle/clockwork" - "github.com/prometheus/client_golang/prometheus" "github.com/gravitational/teleport" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/utils/retryutils" "github.com/gravitational/teleport/lib/defaults" + "github.com/gravitational/teleport/lib/metrics" "github.com/gravitational/teleport/lib/utils" ) @@ -101,7 +101,7 @@ type Config struct { Logger *slog.Logger // MetricsRegistry configures where metrics should be registered. // When nil, metrics are created but not registered. - MetricsRegistry prometheus.Registerer + MetricsRegistry *metrics.Registry } // SetDefaults sets the default values for optional fields. @@ -124,6 +124,9 @@ func (cfg *Config) SetDefaults() { if cfg.Logger == nil { cfg.Logger = slog.With(teleport.ComponentKey, "msgraph") } + if cfg.MetricsRegistry == nil { + cfg.MetricsRegistry = metrics.BlackHole() + } } // Validate checks that required fields are set. @@ -162,12 +165,10 @@ func NewClient(cfg Config) (*Client, error) { return nil, trace.Wrap(err) } - metrics := newMetrics() + m := newMetrics(cfg.MetricsRegistry) // gracefully handle not being given a metric registry - if cfg.MetricsRegistry != nil { - if err := metrics.register(cfg.MetricsRegistry); err != nil { - return nil, trace.Wrap(err, "registering metrics") - } + if err := m.register(cfg.MetricsRegistry); err != nil { + cfg.Logger.ErrorContext(context.Background(), "Failed to register metrics.", "error", err) } return &Client{ @@ -178,7 +179,7 @@ func NewClient(cfg Config) (*Client, error) { baseURL: base.JoinPath(graphVersion), pageSize: cfg.PageSize, logger: cfg.Logger, - metrics: metrics, + metrics: m, }, nil } diff --git a/lib/msgraph/metrics.go b/lib/msgraph/metrics.go index 2c762ea17ff18..9e29f952a1c81 100644 --- a/lib/msgraph/metrics.go +++ b/lib/msgraph/metrics.go @@ -22,7 +22,7 @@ import ( "github.com/gravitational/trace" "github.com/prometheus/client_golang/prometheus" - "github.com/gravitational/teleport" + "github.com/gravitational/teleport/lib/metrics" ) type clientMetrics struct { @@ -34,22 +34,26 @@ type clientMetrics struct { } const ( - metricSubsystem = "msgraph" metricsLabelStatus = "status" metricsLabelsMethod = "method" ) -func newMetrics() *clientMetrics { +func newMetrics(reg *metrics.Registry) *clientMetrics { + var namespace, subsystem string + if reg != nil { + namespace = reg.Namespace() + subsystem = reg.Subsystem() + } return &clientMetrics{ requestTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ - Namespace: teleport.MetricNamespace, - Subsystem: metricSubsystem, + Namespace: namespace, + Subsystem: subsystem, Name: "request_total", Help: "Total number of requests made to MS Graph", }, []string{metricsLabelsMethod, metricsLabelStatus}), requestDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Namespace: teleport.MetricNamespace, - Subsystem: metricSubsystem, + Namespace: namespace, + Subsystem: subsystem, Name: "request_duration_seconds", Help: "Request to MS Graph duration in seconds.", }, []string{metricsLabelsMethod}), diff --git a/lib/services/reconciler.go b/lib/services/reconciler.go index 26bdc7170b7d3..f0e5122cc51b8 100644 --- a/lib/services/reconciler.go +++ b/lib/services/reconciler.go @@ -28,6 +28,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/lib/metrics" logutils "github.com/gravitational/teleport/lib/utils/log" ) @@ -105,7 +106,7 @@ func (c *GenericReconcilerConfig[K, T]) CheckAndSetDefaults() error { var err error // If we are not given metrics, we create our own so we don't // panic when trying to increment/observe. - c.Metrics, err = NewReconcilerMetrics("unknown") + c.Metrics, err = NewReconcilerMetrics(metrics.BlackHole().Wrap("unknown")) if err != nil { return trace.Wrap(err) } @@ -136,20 +137,23 @@ const ( // The caller is responsible for registering them into an appropriate registry. // The same ReconcilerMetrics can be used across different reconcilers. // The metrics subsystem cannot be empty. -func NewReconcilerMetrics(subsystem string) (*ReconcilerMetrics, error) { - if subsystem == "" { - return nil, trace.BadParameter("missing reconciler metric subsystem (this is a bug)") +func NewReconcilerMetrics(reg *metrics.Registry) (*ReconcilerMetrics, error) { + if reg == nil { + return nil, trace.BadParameter("missing metrics registry (this is a bug)") + } + if reg.Subsystem() == "" { + return nil, trace.BadParameter("missing metrics subsystem (this is a bug)") } return &ReconcilerMetrics{ reconciliationTotal: prometheus.NewCounterVec(prometheus.CounterOpts{ - Namespace: teleport.MetricNamespace, - Subsystem: subsystem, + Namespace: reg.Namespace(), + Subsystem: reg.Subsystem(), Name: "reconciliation_total", Help: "Total number of individual resource reconciliations.", }, []string{metricLabelKind, metricLabelOperation, metricLabelResult}), reconciliationDuration: prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Namespace: teleport.MetricNamespace, - Subsystem: subsystem, + Namespace: reg.Namespace(), + Subsystem: reg.Subsystem(), Name: "reconciliation_duration_seconds", Help: "The duration of individual resource reconciliation in seconds.", }, []string{metricLabelKind, metricLabelOperation}), From fbdea7a1ba3131ce8efdacd1d2af41eceb3c0c04 Mon Sep 17 00:00:00 2001 From: Hugo Shaka Date: Tue, 11 Nov 2025 15:42:24 -0500 Subject: [PATCH 2/6] Update lib/metrics/registry.go Co-authored-by: rosstimothy <39066650+rosstimothy@users.noreply.github.com> --- lib/metrics/registry.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/metrics/registry.go b/lib/metrics/registry.go index 9451b3f33a0d0..4e8d460e4e7a5 100644 --- a/lib/metrics/registry.go +++ b/lib/metrics/registry.go @@ -24,12 +24,12 @@ import ( "github.com/prometheus/client_golang/prometheus" ) -// Registry is a Teleport metrics registry. Wrapping the [prometheus.Registerer] -// allows us to propagate additional information such as: +// Registry is a [prometheus.Registerer] for a Teleport process that +// allows propagating additional information such as: // - the metric namespace (`teleport`, `teleport_bot`, `teleport_plugins`) // - an optional subsystem // -// This should be passed alongside the logger. +// This should be passed anywhere that needs to register a metric. type Registry struct { prometheus.Registerer From 0abe5981b513183a435aa888ec12e8c772374f7b Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Tue, 11 Nov 2025 15:44:23 -0500 Subject: [PATCH 3/6] BlackHole -> BlackHoleRegistry --- lib/metrics/registry.go | 4 ++-- lib/msgraph/client.go | 2 +- lib/services/reconciler.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/metrics/registry.go b/lib/metrics/registry.go index 4e8d460e4e7a5..ecaec9edbaefe 100644 --- a/lib/metrics/registry.go +++ b/lib/metrics/registry.go @@ -94,10 +94,10 @@ func NewRegistry(reg prometheus.Registerer, namespace, subsystem string) (*Regis }, nil } -// BlackHole returns a Registry that doesn't register metrics. +// BlackHoleRegistry returns a Registry that doesn't register metrics. // This can be used in tests, or to provide backward compatibility when a nil // Registry is passed. -func BlackHole() *Registry { +func BlackHoleRegistry() *Registry { return &Registry{ Registerer: blackHoleRegistry{}, namespace: "blackhole", diff --git a/lib/msgraph/client.go b/lib/msgraph/client.go index 4d1666a1ec8a0..c50eaa8229a43 100644 --- a/lib/msgraph/client.go +++ b/lib/msgraph/client.go @@ -125,7 +125,7 @@ func (cfg *Config) SetDefaults() { cfg.Logger = slog.With(teleport.ComponentKey, "msgraph") } if cfg.MetricsRegistry == nil { - cfg.MetricsRegistry = metrics.BlackHole() + cfg.MetricsRegistry = metrics.BlackHoleRegistry() } } diff --git a/lib/services/reconciler.go b/lib/services/reconciler.go index f0e5122cc51b8..621baacd4bc35 100644 --- a/lib/services/reconciler.go +++ b/lib/services/reconciler.go @@ -106,7 +106,7 @@ func (c *GenericReconcilerConfig[K, T]) CheckAndSetDefaults() error { var err error // If we are not given metrics, we create our own so we don't // panic when trying to increment/observe. - c.Metrics, err = NewReconcilerMetrics(metrics.BlackHole().Wrap("unknown")) + c.Metrics, err = NewReconcilerMetrics(metrics.BlackHoleRegistry().Wrap("unknown")) if err != nil { return trace.Wrap(err) } From 4e9b58cbcae7756fbb8facc842cd6ae9fd02b93f Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Tue, 11 Nov 2025 15:50:22 -0500 Subject: [PATCH 4/6] merge lib/metrics and lib/observability/metrics --- lib/auth/middleware.go | 3 +- lib/msgraph/client.go | 2 +- lib/msgraph/metrics.go | 2 +- lib/{ => observability}/metrics/gatherers.go | 0 lib/observability/metrics/grpc/grpc.go | 47 +++++++++++++++++++ lib/observability/metrics/prometheus.go | 42 ----------------- lib/{ => observability}/metrics/registry.go | 0 lib/service/connect.go | 3 +- lib/service/service.go | 2 +- lib/service/service_test.go | 2 +- lib/services/reconciler.go | 2 +- .../services/workloadidentity/workload_api.go | 3 +- lib/tbot/tbot.go | 3 +- 13 files changed, 60 insertions(+), 51 deletions(-) rename lib/{ => observability}/metrics/gatherers.go (100%) create mode 100644 lib/observability/metrics/grpc/grpc.go rename lib/{ => observability}/metrics/registry.go (100%) diff --git a/lib/auth/middleware.go b/lib/auth/middleware.go index d4d22a28a5186..3fab9846c99b3 100644 --- a/lib/auth/middleware.go +++ b/lib/auth/middleware.go @@ -52,6 +52,7 @@ import ( "github.com/gravitational/teleport/lib/limiter" "github.com/gravitational/teleport/lib/multiplexer" "github.com/gravitational/teleport/lib/observability/metrics" + "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/utils" logutils "github.com/gravitational/teleport/lib/utils/log" ) @@ -150,7 +151,7 @@ func NewTLSServer(ctx context.Context, cfg TLSServerConfig) (*TLSServer, error) } // sets up gRPC metrics interceptor - grpcMetrics := metrics.CreateGRPCServerMetrics(cfg.Metrics.GRPCServerLatency, prometheus.Labels{teleport.TagServer: "teleport-auth"}) + grpcMetrics := grpcmetrics.CreateGRPCServerMetrics(cfg.Metrics.GRPCServerLatency, prometheus.Labels{teleport.TagServer: "teleport-auth"}) err = metrics.RegisterPrometheusCollectors(grpcMetrics) if err != nil { return nil, trace.Wrap(err) diff --git a/lib/msgraph/client.go b/lib/msgraph/client.go index c50eaa8229a43..d9de73d04b5c1 100644 --- a/lib/msgraph/client.go +++ b/lib/msgraph/client.go @@ -43,7 +43,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/api/utils/retryutils" "github.com/gravitational/teleport/lib/defaults" - "github.com/gravitational/teleport/lib/metrics" + "github.com/gravitational/teleport/lib/observability/metrics" "github.com/gravitational/teleport/lib/utils" ) diff --git a/lib/msgraph/metrics.go b/lib/msgraph/metrics.go index 9e29f952a1c81..d7bca2ee9f223 100644 --- a/lib/msgraph/metrics.go +++ b/lib/msgraph/metrics.go @@ -22,7 +22,7 @@ import ( "github.com/gravitational/trace" "github.com/prometheus/client_golang/prometheus" - "github.com/gravitational/teleport/lib/metrics" + "github.com/gravitational/teleport/lib/observability/metrics" ) type clientMetrics struct { diff --git a/lib/metrics/gatherers.go b/lib/observability/metrics/gatherers.go similarity index 100% rename from lib/metrics/gatherers.go rename to lib/observability/metrics/gatherers.go diff --git a/lib/observability/metrics/grpc/grpc.go b/lib/observability/metrics/grpc/grpc.go new file mode 100644 index 0000000000000..21c1fde229688 --- /dev/null +++ b/lib/observability/metrics/grpc/grpc.go @@ -0,0 +1,47 @@ +package grpcmetrics + +import ( + prometheus2 "github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus" + "github.com/prometheus/client_golang/prometheus" +) + +// CreateGRPCServerMetrics creates server gRPC metrics configuration that is to be registered and used by the caller +// in an openmetrics unary and/or stream interceptor +func CreateGRPCServerMetrics( + latencyEnabled bool, labels prometheus.Labels, +) *prometheus2.ServerMetrics { + serverOpts := []prometheus2.ServerMetricsOption{ + prometheus2.WithServerCounterOptions(prometheus2.WithConstLabels(labels)), + } + if latencyEnabled { + histOpts := grpcHistogramOpts(labels) + serverOpts = append( + serverOpts, prometheus2.WithServerHandlingTimeHistogram(histOpts...), + ) + } + return prometheus2.NewServerMetrics(serverOpts...) +} + +// CreateGRPCClientMetrics creates client gRPC metrics configuration that is to be registered and used by the caller +// in an openmetrics unary and/or stream interceptor +func CreateGRPCClientMetrics( + latencyEnabled bool, labels prometheus.Labels, +) *prometheus2.ClientMetrics { + clientOpts := []prometheus2.ClientMetricsOption{ + prometheus2.WithClientCounterOptions(prometheus2.WithConstLabels(labels)), + } + if latencyEnabled { + histOpts := grpcHistogramOpts(labels) + clientOpts = append( + clientOpts, prometheus2.WithClientHandlingTimeHistogram(histOpts...), + ) + } + return prometheus2.NewClientMetrics(clientOpts...) +} + +func grpcHistogramOpts(labels prometheus.Labels) []prometheus2.HistogramOption { + return []prometheus2.HistogramOption{ + prometheus2.WithHistogramBuckets(prometheus.ExponentialBuckets(0.001, 2, 16)), + prometheus2.WithHistogramConstLabels(labels), + } +} diff --git a/lib/observability/metrics/prometheus.go b/lib/observability/metrics/prometheus.go index d190c7ca1f5dc..440a4078c696c 100644 --- a/lib/observability/metrics/prometheus.go +++ b/lib/observability/metrics/prometheus.go @@ -23,7 +23,6 @@ import ( "runtime" "github.com/gravitational/trace" - grpcprom "github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus" "github.com/prometheus/client_golang/prometheus" "github.com/gravitational/teleport" @@ -70,44 +69,3 @@ func BuildCollector() prometheus.Collector { func() float64 { return 1 }, ) } - -// CreateGRPCServerMetrics creates server gRPC metrics configuration that is to be registered and used by the caller -// in an openmetrics unary and/or stream interceptor -func CreateGRPCServerMetrics( - latencyEnabled bool, labels prometheus.Labels, -) *grpcprom.ServerMetrics { - serverOpts := []grpcprom.ServerMetricsOption{ - grpcprom.WithServerCounterOptions(grpcprom.WithConstLabels(labels)), - } - if latencyEnabled { - histOpts := grpcHistogramOpts(labels) - serverOpts = append( - serverOpts, grpcprom.WithServerHandlingTimeHistogram(histOpts...), - ) - } - return grpcprom.NewServerMetrics(serverOpts...) -} - -// CreateGRPCClientMetrics creates client gRPC metrics configuration that is to be registered and used by the caller -// in an openmetrics unary and/or stream interceptor -func CreateGRPCClientMetrics( - latencyEnabled bool, labels prometheus.Labels, -) *grpcprom.ClientMetrics { - clientOpts := []grpcprom.ClientMetricsOption{ - grpcprom.WithClientCounterOptions(grpcprom.WithConstLabels(labels)), - } - if latencyEnabled { - histOpts := grpcHistogramOpts(labels) - clientOpts = append( - clientOpts, grpcprom.WithClientHandlingTimeHistogram(histOpts...), - ) - } - return grpcprom.NewClientMetrics(clientOpts...) -} - -func grpcHistogramOpts(labels prometheus.Labels) []grpcprom.HistogramOption { - return []grpcprom.HistogramOption{ - grpcprom.WithHistogramBuckets(prometheus.ExponentialBuckets(0.001, 2, 16)), - grpcprom.WithHistogramConstLabels(labels), - } -} diff --git a/lib/metrics/registry.go b/lib/observability/metrics/registry.go similarity index 100% rename from lib/metrics/registry.go rename to lib/observability/metrics/registry.go diff --git a/lib/service/connect.go b/lib/service/connect.go index 1815691d1594f..6eef5db40c224 100644 --- a/lib/service/connect.go +++ b/lib/service/connect.go @@ -57,6 +57,7 @@ import ( "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/join/joinclient" "github.com/gravitational/teleport/lib/observability/metrics" + "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/openssh" "github.com/gravitational/teleport/lib/reversetunnelclient" servicebreaker "github.com/gravitational/teleport/lib/service/breaker" @@ -1507,7 +1508,7 @@ func (process *TeleportProcess) newClientDirect(authServers []utils.NetAddr, tls var dialOpts []grpc.DialOption if role == types.RoleProxy { - grpcMetrics := metrics.CreateGRPCClientMetrics(process.Config.Metrics.GRPCClientLatency, prometheus.Labels{teleport.TagClient: "teleport-proxy"}) + grpcMetrics := grpcmetrics.CreateGRPCClientMetrics(process.Config.Metrics.GRPCClientLatency, prometheus.Labels{teleport.TagClient: "teleport-proxy"}) if err := metrics.RegisterPrometheusCollectors(grpcMetrics); err != nil { return nil, nil, trace.Wrap(err) } diff --git a/lib/service/service.go b/lib/service/service.go index ff5d024075a3b..8652583700629 100644 --- a/lib/service/service.go +++ b/lib/service/service.go @@ -143,9 +143,9 @@ import ( kubeproxy "github.com/gravitational/teleport/lib/kube/proxy" "github.com/gravitational/teleport/lib/labels" "github.com/gravitational/teleport/lib/limiter" - "github.com/gravitational/teleport/lib/metrics" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/multiplexer" + "github.com/gravitational/teleport/lib/observability/metrics" "github.com/gravitational/teleport/lib/observability/tracing" "github.com/gravitational/teleport/lib/openssh" "github.com/gravitational/teleport/lib/pam" diff --git a/lib/service/service_test.go b/lib/service/service_test.go index bc71c1d103dac..6229a0436744f 100644 --- a/lib/service/service_test.go +++ b/lib/service/service_test.go @@ -70,10 +70,10 @@ import ( "github.com/gravitational/teleport/lib/events/athena" "github.com/gravitational/teleport/lib/integrations/externalauditstorage" "github.com/gravitational/teleport/lib/limiter" - "github.com/gravitational/teleport/lib/metrics" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/modules/modulestest" "github.com/gravitational/teleport/lib/multiplexer" + "github.com/gravitational/teleport/lib/observability/metrics" "github.com/gravitational/teleport/lib/reversetunnelclient" "github.com/gravitational/teleport/lib/service/servicecfg" "github.com/gravitational/teleport/lib/services" diff --git a/lib/services/reconciler.go b/lib/services/reconciler.go index 621baacd4bc35..8d40d86535075 100644 --- a/lib/services/reconciler.go +++ b/lib/services/reconciler.go @@ -28,7 +28,7 @@ import ( "github.com/gravitational/teleport" "github.com/gravitational/teleport/api/types" - "github.com/gravitational/teleport/lib/metrics" + "github.com/gravitational/teleport/lib/observability/metrics" logutils "github.com/gravitational/teleport/lib/utils/log" ) diff --git a/lib/tbot/services/workloadidentity/workload_api.go b/lib/tbot/services/workloadidentity/workload_api.go index 23a26f00d936c..10278cf6d626b 100644 --- a/lib/tbot/services/workloadidentity/workload_api.go +++ b/lib/tbot/services/workloadidentity/workload_api.go @@ -46,6 +46,7 @@ import ( apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/observability/metrics" + "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/tbot/bot" "github.com/gravitational/teleport/lib/tbot/client" "github.com/gravitational/teleport/lib/tbot/internal" @@ -168,7 +169,7 @@ func (s *WorkloadAPIService) Run(ctx context.Context) error { defer s.client.Close() s.log.DebugContext(ctx, "Completed pre-run initialization") - srvMetrics := metrics.CreateGRPCServerMetrics( + srvMetrics := grpcmetrics.CreateGRPCServerMetrics( true, prometheus.Labels{ teleport.TagServer: "tbot-workload-identity-api", }, diff --git a/lib/tbot/tbot.go b/lib/tbot/tbot.go index f8f9a628a19d4..882ebaf8eb0d9 100644 --- a/lib/tbot/tbot.go +++ b/lib/tbot/tbot.go @@ -35,6 +35,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/observability/metrics" + "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/tbot/bot" "github.com/gravitational/teleport/lib/tbot/bot/connection" "github.com/gravitational/teleport/lib/tbot/config" @@ -57,7 +58,7 @@ import ( var tracer = otel.Tracer("github.com/gravitational/teleport/lib/tbot") -var clientMetrics = metrics.CreateGRPCClientMetrics( +var clientMetrics = grpcmetrics.CreateGRPCClientMetrics( false, prometheus.Labels{}, ) From d9eb007ada1f0691d6a6df1701799119fb31e385 Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Tue, 11 Nov 2025 16:13:03 -0500 Subject: [PATCH 5/6] lint --- lib/auth/middleware.go | 2 +- lib/observability/metrics/grpc/grpc.go | 18 ++++++++++++++++++ lib/service/connect.go | 2 +- .../services/workloadidentity/workload_api.go | 2 +- lib/tbot/tbot.go | 2 +- 5 files changed, 22 insertions(+), 4 deletions(-) diff --git a/lib/auth/middleware.go b/lib/auth/middleware.go index 3fab9846c99b3..6b34f7a8e716d 100644 --- a/lib/auth/middleware.go +++ b/lib/auth/middleware.go @@ -52,7 +52,7 @@ import ( "github.com/gravitational/teleport/lib/limiter" "github.com/gravitational/teleport/lib/multiplexer" "github.com/gravitational/teleport/lib/observability/metrics" - "github.com/gravitational/teleport/lib/observability/metrics/grpc" + grpcmetrics "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/utils" logutils "github.com/gravitational/teleport/lib/utils/log" ) diff --git a/lib/observability/metrics/grpc/grpc.go b/lib/observability/metrics/grpc/grpc.go index 21c1fde229688..b7121035e43b4 100644 --- a/lib/observability/metrics/grpc/grpc.go +++ b/lib/observability/metrics/grpc/grpc.go @@ -1,3 +1,21 @@ +/* + * Teleport + * Copyright (C) 2025 Gravitational, Inc. + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + package grpcmetrics import ( diff --git a/lib/service/connect.go b/lib/service/connect.go index 6eef5db40c224..2f1b938eef36f 100644 --- a/lib/service/connect.go +++ b/lib/service/connect.go @@ -57,7 +57,7 @@ import ( "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/join/joinclient" "github.com/gravitational/teleport/lib/observability/metrics" - "github.com/gravitational/teleport/lib/observability/metrics/grpc" + grpcmetrics "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/openssh" "github.com/gravitational/teleport/lib/reversetunnelclient" servicebreaker "github.com/gravitational/teleport/lib/service/breaker" diff --git a/lib/tbot/services/workloadidentity/workload_api.go b/lib/tbot/services/workloadidentity/workload_api.go index 10278cf6d626b..2dc7bacb98cf9 100644 --- a/lib/tbot/services/workloadidentity/workload_api.go +++ b/lib/tbot/services/workloadidentity/workload_api.go @@ -46,7 +46,7 @@ import ( apiclient "github.com/gravitational/teleport/api/client" "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/observability/metrics" - "github.com/gravitational/teleport/lib/observability/metrics/grpc" + grpcmetrics "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/tbot/bot" "github.com/gravitational/teleport/lib/tbot/client" "github.com/gravitational/teleport/lib/tbot/internal" diff --git a/lib/tbot/tbot.go b/lib/tbot/tbot.go index 882ebaf8eb0d9..0229419b00954 100644 --- a/lib/tbot/tbot.go +++ b/lib/tbot/tbot.go @@ -35,7 +35,7 @@ import ( "github.com/gravitational/teleport/api/types" "github.com/gravitational/teleport/lib/modules" "github.com/gravitational/teleport/lib/observability/metrics" - "github.com/gravitational/teleport/lib/observability/metrics/grpc" + grpcmetrics "github.com/gravitational/teleport/lib/observability/metrics/grpc" "github.com/gravitational/teleport/lib/tbot/bot" "github.com/gravitational/teleport/lib/tbot/bot/connection" "github.com/gravitational/teleport/lib/tbot/config" From a24bba394eb3aeddc2a83497a83f0bf4661772d5 Mon Sep 17 00:00:00 2001 From: hugoShaka Date: Wed, 12 Nov 2025 10:47:58 -0500 Subject: [PATCH 6/6] address noah's feedback --- lib/msgraph/client.go | 2 +- lib/observability/metrics/grpc/grpc.go | 30 +++++++++++++------------- lib/observability/metrics/registry.go | 16 +++++++------- lib/services/reconciler.go | 2 +- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/lib/msgraph/client.go b/lib/msgraph/client.go index d9de73d04b5c1..4f74ed1531ce3 100644 --- a/lib/msgraph/client.go +++ b/lib/msgraph/client.go @@ -125,7 +125,7 @@ func (cfg *Config) SetDefaults() { cfg.Logger = slog.With(teleport.ComponentKey, "msgraph") } if cfg.MetricsRegistry == nil { - cfg.MetricsRegistry = metrics.BlackHoleRegistry() + cfg.MetricsRegistry = metrics.NoopRegistry() } } diff --git a/lib/observability/metrics/grpc/grpc.go b/lib/observability/metrics/grpc/grpc.go index b7121035e43b4..426b7c30f988c 100644 --- a/lib/observability/metrics/grpc/grpc.go +++ b/lib/observability/metrics/grpc/grpc.go @@ -19,7 +19,7 @@ package grpcmetrics import ( - prometheus2 "github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus" + grpcprom "github.com/grpc-ecosystem/go-grpc-middleware/providers/prometheus" "github.com/prometheus/client_golang/prometheus" ) @@ -27,39 +27,39 @@ import ( // in an openmetrics unary and/or stream interceptor func CreateGRPCServerMetrics( latencyEnabled bool, labels prometheus.Labels, -) *prometheus2.ServerMetrics { - serverOpts := []prometheus2.ServerMetricsOption{ - prometheus2.WithServerCounterOptions(prometheus2.WithConstLabels(labels)), +) *grpcprom.ServerMetrics { + serverOpts := []grpcprom.ServerMetricsOption{ + grpcprom.WithServerCounterOptions(grpcprom.WithConstLabels(labels)), } if latencyEnabled { histOpts := grpcHistogramOpts(labels) serverOpts = append( - serverOpts, prometheus2.WithServerHandlingTimeHistogram(histOpts...), + serverOpts, grpcprom.WithServerHandlingTimeHistogram(histOpts...), ) } - return prometheus2.NewServerMetrics(serverOpts...) + return grpcprom.NewServerMetrics(serverOpts...) } // CreateGRPCClientMetrics creates client gRPC metrics configuration that is to be registered and used by the caller // in an openmetrics unary and/or stream interceptor func CreateGRPCClientMetrics( latencyEnabled bool, labels prometheus.Labels, -) *prometheus2.ClientMetrics { - clientOpts := []prometheus2.ClientMetricsOption{ - prometheus2.WithClientCounterOptions(prometheus2.WithConstLabels(labels)), +) *grpcprom.ClientMetrics { + clientOpts := []grpcprom.ClientMetricsOption{ + grpcprom.WithClientCounterOptions(grpcprom.WithConstLabels(labels)), } if latencyEnabled { histOpts := grpcHistogramOpts(labels) clientOpts = append( - clientOpts, prometheus2.WithClientHandlingTimeHistogram(histOpts...), + clientOpts, grpcprom.WithClientHandlingTimeHistogram(histOpts...), ) } - return prometheus2.NewClientMetrics(clientOpts...) + return grpcprom.NewClientMetrics(clientOpts...) } -func grpcHistogramOpts(labels prometheus.Labels) []prometheus2.HistogramOption { - return []prometheus2.HistogramOption{ - prometheus2.WithHistogramBuckets(prometheus.ExponentialBuckets(0.001, 2, 16)), - prometheus2.WithHistogramConstLabels(labels), +func grpcHistogramOpts(labels prometheus.Labels) []grpcprom.HistogramOption { + return []grpcprom.HistogramOption{ + grpcprom.WithHistogramBuckets(prometheus.ExponentialBuckets(0.001, 2, 16)), + grpcprom.WithHistogramConstLabels(labels), } } diff --git a/lib/observability/metrics/registry.go b/lib/observability/metrics/registry.go index ecaec9edbaefe..d329ab9876212 100644 --- a/lib/observability/metrics/registry.go +++ b/lib/observability/metrics/registry.go @@ -94,29 +94,29 @@ func NewRegistry(reg prometheus.Registerer, namespace, subsystem string) (*Regis }, nil } -// BlackHoleRegistry returns a Registry that doesn't register metrics. +// NoopRegistry returns a Registry that doesn't register metrics. // This can be used in tests, or to provide backward compatibility when a nil // Registry is passed. -func BlackHoleRegistry() *Registry { +func NoopRegistry() *Registry { return &Registry{ - Registerer: blackHoleRegistry{}, - namespace: "blackhole", + Registerer: noopRegistry{}, + namespace: "noop", subsystem: "", } } -type blackHoleRegistry struct{} +type noopRegistry struct{} // Register implements [prometheus.Registerer]. -func (b blackHoleRegistry) Register(collector prometheus.Collector) error { +func (b noopRegistry) Register(collector prometheus.Collector) error { return nil } // MustRegister implements [prometheus.Registerer]. -func (b blackHoleRegistry) MustRegister(collector ...prometheus.Collector) { +func (b noopRegistry) MustRegister(collector ...prometheus.Collector) { } // Unregister implements [prometheus.Registerer]. -func (b blackHoleRegistry) Unregister(collector prometheus.Collector) bool { +func (b noopRegistry) Unregister(collector prometheus.Collector) bool { return true } diff --git a/lib/services/reconciler.go b/lib/services/reconciler.go index 8d40d86535075..a7cae8d16f3db 100644 --- a/lib/services/reconciler.go +++ b/lib/services/reconciler.go @@ -106,7 +106,7 @@ func (c *GenericReconcilerConfig[K, T]) CheckAndSetDefaults() error { var err error // If we are not given metrics, we create our own so we don't // panic when trying to increment/observe. - c.Metrics, err = NewReconcilerMetrics(metrics.BlackHoleRegistry().Wrap("unknown")) + c.Metrics, err = NewReconcilerMetrics(metrics.NoopRegistry().Wrap("unknown")) if err != nil { return trace.Wrap(err) }