Merge branch '798-logger-lint' into 'main'

Lint fixes for logger metrics and pktline packages

Closes #798

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/1155

Merged-by: Igor Drozdov <[email protected]>
Approved-by: James Fargher <[email protected]>
Approved-by: Igor Drozdov <[email protected]>
Reviewed-by: James Fargher <[email protected]>
Co-authored-by: Archish <[email protected]>

Author: 2 people

internal/logger/logger.go

@@ -1,3 +1,4 @@
+// Package logger provides logging configuration utilities for the gitlab-shell
 package logger
 
 import (
@@ -52,7 +53,7 @@ func buildOpts(cfg *config.Config) []log.LoggerOption {
 // opened for writing.
 func Configure(cfg *config.Config) io.Closer {
 	var closer io.Closer = io.NopCloser(nil)
-	err := fmt.Errorf("No logfile specified")
+	err := fmt.Errorf("no logfile specified")
 
 	if cfg.LogFile != "" {
 		closer, err = log.Initialize(buildOpts(cfg)...)
@@ -66,7 +67,7 @@ func Configure(cfg *config.Config) io.Closer {
 			syslogLogger.Print(msg)
 		} else {
 			msg := fmt.Sprintf("%s: Unable to configure logging: %v, %v\n", progName, err.Error(), syslogLoggerErr.Error())
-			fmt.Fprintf(os.Stderr, msg)
+			fmt.Fprintln(os.Stderr, msg)
 		}
 
 		cfg.LogFile = "/dev/null"

internal/metrics/metrics.go

@@ -1,3 +1,4 @@
+// Package metrics provides Prometheus metrics for monitoring gitlab-shell components.
 package metrics
 
 import (
@@ -34,6 +35,7 @@ const (
 )
 
 var (
+	// SshdSessionDuration is a histogram of latencies for connections to gitlab-shell sshd.
 	SshdSessionDuration = promauto.NewHistogram(
 		prometheus.HistogramOpts{
 			Namespace: namespace,
@@ -48,6 +50,7 @@ var (
 		},
 	)
 
+	// SshdSessionEstablishedDuration is a histogram of latencies until session established to gitlab-shell sshd.
 	SshdSessionEstablishedDuration = promauto.NewHistogram(
 		prometheus.HistogramOpts{
 			Namespace: namespace,
@@ -62,6 +65,7 @@ var (
 		},
 	)
 
+	// SshdConnectionsInFlight is a gauge of connections currently being served by gitlab-shell sshd.
 	SshdConnectionsInFlight = promauto.NewGauge(
 		prometheus.GaugeOpts{
 			Namespace: namespace,
@@ -71,6 +75,7 @@ var (
 		},
 	)
 
+	// SshdHitMaxSessions is the number of times the concurrent sessions limit was hit in gitlab-shell sshd.
 	SshdHitMaxSessions = promauto.NewCounter(
 		prometheus.CounterOpts{
 			Namespace: namespace,
@@ -80,20 +85,23 @@ var (
 		},
 	)
 
+	// SliSshdSessionsTotal is the number of SSH sessions that have been established.
 	SliSshdSessionsTotal = promauto.NewCounter(
 		prometheus.CounterOpts{
 			Name: sliSshdSessionsTotalName,
 			Help: "Number of SSH sessions that have been established",
 		},
 	)
 
+	// SliSshdSessionsErrorsTotal is the number of SSH sessions that have failed.
 	SliSshdSessionsErrorsTotal = promauto.NewCounter(
 		prometheus.CounterOpts{
 			Name: sliSshdSessionsErrorsTotalName,
 			Help: "Number of SSH sessions that have failed",
 		},
 	)
 
+	// GitalyConnectionsTotal is a counter for the number of Gitaly connections that have been established,
 	GitalyConnectionsTotal = promauto.NewCounterVec(
 		prometheus.CounterOpts{
 			Namespace: namespace,
@@ -147,13 +155,15 @@ var (
 		},
 	)
 
+	// LfsHTTPConnectionsTotal is the number of LFS over HTTP connections that have been established.
 	LfsHTTPConnectionsTotal = promauto.NewCounter(
 		prometheus.CounterOpts{
 			Name: lfsHTTPConnectionsTotalName,
 			Help: "Number of LFS over HTTP connections that have been established",
 		},
 	)
 
+	// LfsSSHConnectionsTotal is the number of LFS over SSH connections that have been established.
 	LfsSSHConnectionsTotal = promauto.NewCounter(
 		prometheus.CounterOpts{
 			Name: lfsSSHConnectionsTotalName,
@@ -162,6 +172,7 @@ var (
 	)
 )
 
+// NewRoundTripper wraps an http.RoundTripper to instrument it with Prometheus metrics.
 func NewRoundTripper(next http.RoundTripper) promhttp.RoundTripperFunc {
 	rt := next
 

internal/pktline/pktline.go

@@ -1,3 +1,4 @@
+// Package pktline provides utility functions for working with the Git pkt-line format.
 package pktline
 
 // Utility functions for working with the Git pkt-line format. See
@@ -27,6 +28,7 @@ func NewScanner(r io.Reader) *bufio.Scanner {
 	return scanner
 }
 
+// IsRefRemoval checks if the packet represents a reference removal.
 func IsRefRemoval(pkt []byte) bool {
 	return branchRemovalPktRegexp.Match(pkt)
 }

support/lint_last_known_acceptable.txt

@@ -291,14 +291,6 @@ internal/gitlabnet/lfstransfer/client.go:214:1: exported: exported method Client
 internal/gitlabnet/lfstransfer/client.go:271:1: exported: exported method Client.Unlock should have comment or be unexported (revive)
 internal/gitlabnet/lfstransfer/client.go:321:1: exported: exported method Client.ListLocksVerify should have comment or be unexported (revive)
 internal/gitlabnet/personalaccesstoken/client_test.go:30:5: go-require: do not use require in http handlers (testifylint)
-internal/logger/logger.go:1:1: package-comments: should have a package comment (revive)
-internal/logger/logger.go:55:9: ST1005: error strings should not be capitalized (stylecheck)
-internal/logger/logger.go:69:27: printf: non-constant format string in call to fmt.Fprintf (govet)
-internal/metrics/metrics.go:1:1: package-comments: should have a package comment (revive)
-internal/metrics/metrics.go:37:2: exported: exported var SshdSessionDuration should have comment or be unexported (revive)
-internal/metrics/metrics.go:165:1: exported: exported function NewRoundTripper should have comment or be unexported (revive)
-internal/pktline/pktline.go:1:1: package-comments: should have a package comment (revive)
-internal/pktline/pktline.go:30:1: exported: exported function IsRefRemoval should have comment or be unexported (revive)
 internal/sshd/gssapi_unsupported.go:11:1: exported: exported function NewGSSAPIServer should have comment or be unexported (revive)
 internal/sshd/gssapi_unsupported.go:19:6: exported: exported type OSGSSAPIServer should have comment or be unexported (revive)
 internal/sshd/gssapi_unsupported.go:23:1: exported: exported method OSGSSAPIServer.AcceptSecContext should have comment or be unexported (revive)