From 9f3f78c8ad656bf5aa5490df703b81051b6559db Mon Sep 17 00:00:00 2001 From: Scott Leggett Date: Fri, 16 Aug 2024 11:07:04 +0800 Subject: [PATCH] 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. --- internal/sshportalapi/sshportal.go | 11 +++---- internal/sshserver/authhandler.go | 17 ++++------ internal/sshserver/sessionhandler.go | 11 +++---- internal/sshtoken/authhandler.go | 13 +++----- internal/sshtoken/sessionhandler.go | 48 ++++++++++++++++------------ 5 files changed, 47 insertions(+), 53 deletions(-) diff --git a/internal/sshportalapi/sshportal.go b/internal/sshportalapi/sshportal.go index aaf7fc11..5263e7be 100644 --- 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 index 99b0632f..a2f00944 100644 --- 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 index da208296..d530ac18 100644 --- 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 index a5273bd9..31b297b0 100644 --- 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 index fc018b37..13663434 100644 --- 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) } } }