fix: avoid unnecessary zero metrics

If the prometheus.Counter variables are global, they are propagated to all binaries - even those where it made no sense. For example sshportalapi metrics in the ssh-portal binary. Instead initialise these only on callback init so that they only appear in binaries where they are actually used.
uselagoon · Aug 16, 2024 · 9f3f78c · 9f3f78c
1 parent 3807745
commit 9f3f78c
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 53 deletions.
diff --git a/internal/sshportalapi/sshportal.go b/internal/sshportalapi/sshportal.go
@@ -40,13 +40,6 @@ func (q SSHAccessQuery) LogValue() slog.Value {
 	)
 }
 
-var (
-	requestsCounter = promauto.NewCounter(prometheus.CounterOpts{
-		Name: "sshportalapi_requests_total",
-		Help: "The total number of ssh-portal-api requests received",
-	})
-)
-
 func sshportal(
 	ctx context.Context,
 	log *slog.Logger,
@@ -55,6 +48,10 @@ func sshportal(
 	l LagoonDBService,
 	k KeycloakService,
 ) nats.Handler {
+	requestsCounter := promauto.NewCounter(prometheus.CounterOpts{
+		Name: "sshportalapi_requests_total",
+		Help: "The total number of ssh-portal-api requests received",
+	})
 	return func(_, replySubject string, query *SSHAccessQuery) {
 		var realmRoles, userGroups []string
 		// set up tracing and update metrics

diff --git a/internal/sshserver/authhandler.go b/internal/sshserver/authhandler.go
@@ -23,25 +23,22 @@ const (
 	sshFingerprint
 )
 
-var (
+const (
 	natsTimeout = 8 * time.Second
 )
 
-var (
-	authAttemptsTotal = promauto.NewCounter(prometheus.CounterOpts{
+// pubKeyAuth returns a ssh.PublicKeyHandler which queries the remote
+// ssh-portal-api for Lagoon SSH authorization.
+func pubKeyAuth(log *slog.Logger, nc *nats.EncodedConn,
+	c *k8s.Client) ssh.PublicKeyHandler {
+	authAttemptsTotal := promauto.NewCounter(prometheus.CounterOpts{
 		Name: "sshportal_authentication_attempts_total",
 		Help: "The total number of ssh-portal authentication attempts",
 	})
-	authSuccessTotal = promauto.NewCounter(prometheus.CounterOpts{
+	authSuccessTotal := promauto.NewCounter(prometheus.CounterOpts{
 		Name: "sshportal_authentication_success_total",
 		Help: "The total number of successful ssh-portal authentications",
 	})
-)
-
-// pubKeyAuth returns a ssh.PublicKeyHandler which queries the remote
-// ssh-portal-api for Lagoon SSH authorization.
-func pubKeyAuth(log *slog.Logger, nc *nats.EncodedConn,
-	c *k8s.Client) ssh.PublicKeyHandler {
 	return func(ctx ssh.Context, key ssh.PublicKey) bool {
 		authAttemptsTotal.Inc()
 		log := log.With(slog.String("sessionID", ctx.SessionID()))

diff --git a/internal/sshserver/sessionhandler.go b/internal/sshserver/sessionhandler.go
@@ -23,13 +23,6 @@ type K8SAPIService interface {
 	Logs(context.Context, string, string, string, bool, int64, io.ReadWriter) error
 }
 
-var (
-	sessionTotal = promauto.NewCounter(prometheus.CounterOpts{
-		Name: "sshportal_sessions_total",
-		Help: "The total number of ssh-portal sessions started",
-	})
-)
-
 // authCtxValues extracts the context values set by the authhandler.
 func authCtxValues(ctx ssh.Context) (int, string, int, string, string, error) {
 	var ok bool
@@ -91,6 +84,10 @@ func getSSHIntent(sftp bool, cmd []string) []string {
 // There is no support for a built-in sftp server.
 func sessionHandler(log *slog.Logger, c K8SAPIService,
 	sftp, logAccessEnabled bool) ssh.Handler {
+	sessionTotal := promauto.NewCounter(prometheus.CounterOpts{
+		Name: "sshportal_sessions_total",
+		Help: "The total number of ssh-portal sessions started",
+	})
 	return func(s ssh.Session) {
 		sessionTotal.Inc()
 		ctx := s.Context()

diff --git a/internal/sshtoken/authhandler.go b/internal/sshtoken/authhandler.go
@@ -18,20 +18,17 @@ const (
 	userUUID ctxKey = iota
 )
 
-var (
-	authnAttemptsTotal = promauto.NewCounter(prometheus.CounterOpts{
+// pubKeyAuth returns a ssh.PublicKeyHandler which accepts any key which
+// matches a user, and the associated user UUID to the ssh context.
+func pubKeyAuth(log *slog.Logger, ldb LagoonDBService) ssh.PublicKeyHandler {
+	authnAttemptsTotal := promauto.NewCounter(prometheus.CounterOpts{
 		Name: "sshtoken_authentication_attempts_total",
 		Help: "The total number of ssh-token authentication attempts",
 	})
-	authnSuccessTotal = promauto.NewCounter(prometheus.CounterOpts{
+	authnSuccessTotal := promauto.NewCounter(prometheus.CounterOpts{
 		Name: "sshtoken_authentication_success_total",
 		Help: "The total number of successful ssh-token authentications",
 	})
-)
-
-// pubKeyAuth returns a ssh.PublicKeyHandler which accepts any key which
-// matches a user, and the associated user UUID to the ssh context.
-func pubKeyAuth(log *slog.Logger, ldb LagoonDBService) ssh.PublicKeyHandler {
 	return func(ctx ssh.Context, key ssh.PublicKey) bool {
 		authnAttemptsTotal.Inc()
 		log := log.With(slog.String("sessionID", ctx.SessionID()))

diff --git a/internal/sshtoken/sessionhandler.go b/internal/sshtoken/sessionhandler.go
@@ -29,25 +29,15 @@ type KeycloakUserInfoService interface {
 	UserRolesAndGroups(context.Context, *uuid.UUID) ([]string, []string, error)
 }
 
-var (
-	sessionTotal = promauto.NewCounter(prometheus.CounterOpts{
-		Name: "sshtoken_sessions_total",
-		Help: "The total number of ssh-token sessions started",
-	})
-	tokensGeneratedTotal = promauto.NewCounter(prometheus.CounterOpts{
-		Name: "sshtoken_tokens_generated_total",
-		Help: "The total number of ssh-token user access tokens generated",
-	})
-	redirectsTotal = promauto.NewCounter(prometheus.CounterOpts{
-		Name: "sshtoken_redirects_total",
-		Help: "The total number of ssh redirect responses served",
-	})
-)
-
 // tokenSession returns a bare access token or full access token response based
 // on the user ID
-func tokenSession(s ssh.Session, log *slog.Logger,
-	keycloakToken KeycloakTokenService, uid *uuid.UUID) {
+func tokenSession(
+	s ssh.Session,
+	log *slog.Logger,
+	keycloakToken KeycloakTokenService,
+	uid *uuid.UUID,
+	tokensGeneratedTotal prometheus.Counter,
+) {
 	// valid commands:
 	// - grant: returns a full access token response as per
 	//   https://www.rfc-editor.org/rfc/rfc6749#section-4.1.4
@@ -132,6 +122,7 @@ func redirectSession(
 	keycloakUserInfo KeycloakUserInfoService,
 	ldb LagoonDBService,
 	uid *uuid.UUID,
+	redirectsTotal prometheus.Counter,
 ) {
 	ctx := s.Context()
 	// get the user roles and groups
@@ -250,10 +241,25 @@ func redirectSession(
 
 // sessionHandler returns a ssh.Handler which writes a Lagoon access token to
 // the session stream and then closes the connection.
-func sessionHandler(log *slog.Logger, p *rbac.Permission,
+func sessionHandler(
+	log *slog.Logger,
+	p *rbac.Permission,
 	keycloakToken KeycloakTokenService,
 	keycloakPermission KeycloakUserInfoService,
-	ldb LagoonDBService) ssh.Handler {
+	ldb LagoonDBService,
+) ssh.Handler {
+	sessionTotal := promauto.NewCounter(prometheus.CounterOpts{
+		Name: "sshtoken_sessions_total",
+		Help: "The total number of ssh-token sessions started",
+	})
+	tokensGeneratedTotal := promauto.NewCounter(prometheus.CounterOpts{
+		Name: "sshtoken_tokens_generated_total",
+		Help: "The total number of ssh-token user access tokens generated",
+	})
+	redirectsTotal := promauto.NewCounter(prometheus.CounterOpts{
+		Name: "sshtoken_redirects_total",
+		Help: "The total number of ssh redirect responses served",
+	})
 	return func(s ssh.Session) {
 		sessionTotal.Inc()
 		// extract required info from the session context
@@ -272,9 +278,9 @@ func sessionHandler(log *slog.Logger, p *rbac.Permission,
 		}
 		log = log.With(slog.String("userUUID", uid.String()))
 		if s.User() == "lagoon" {
-			tokenSession(s, log, keycloakToken, uid)
+			tokenSession(s, log, keycloakToken, uid, tokensGeneratedTotal)
 		} else {
-			redirectSession(s, log, p, keycloakPermission, ldb, uid)
+			redirectSession(s, log, p, keycloakPermission, ldb, uid, redirectsTotal)
 		}
 	}
 }