Address PR comments

mtojek · mtojek · commit 9b0e31a033bc · 2023-05-19T11:14:37.000+02:00
diff --git a/agent/agentssh/agentssh_internal_test.go b/agent/agentssh/agentssh_internal_test.go
@@ -58,10 +58,11 @@ func Test_sessionStart_orphan(t *testing.T) {
 	done := make(chan struct{})
 	go func() {
 		defer close(done)
+
+		m := metricsForSession(s.metrics.sessions, "ssh")
 		// we don't really care what the error is here.  In the larger scenario,
 		// the client has disconnected, so we can't return any error information
 		// to them.
-		m := metricsForSession(s.metrics.sessions, "ssh")
 		_ = s.startPTYSession(sess, m, cmd, ptyInfo, windowSize)
 	}()
 
diff --git a/agent/agentssh/metrics.go b/agent/agentssh/metrics.go
@@ -20,6 +20,7 @@ type sshServerMetrics struct {
 
 	// X11
 	x11SocketDirError  prometheus.Counter
+	x11HostnameError   prometheus.Counter
 	x11XauthorityError prometheus.Counter
 
 	sessions sessionMetrics
@@ -86,6 +87,11 @@ func newSSHServerMetrics(registerer prometheus.Registerer) *sshServerMetrics {
 	})
 	registerer.MustRegister(sftpServerError)
 
+	x11HostnameError := prometheus.NewCounter(prometheus.CounterOpts{
+		Namespace: "agent", Subsystem: "ssh_server", Name: "x11_hostname_error",
+	})
+	registerer.MustRegister(x11HostnameError)
+
 	x11SocketDirError := prometheus.NewCounter(prometheus.CounterOpts{
 		Namespace: "agent", Subsystem: "ssh_server", Name: "x11_socket_dir_error",
 	})
@@ -106,6 +112,7 @@ func newSSHServerMetrics(registerer prometheus.Registerer) *sshServerMetrics {
 		x11Callback:                   x11Callback,
 		sftpHandler:                   sftpHandler,
 		sftpServerError:               sftpServerError,
+		x11HostnameError:              x11HostnameError,
 		x11SocketDirError:             x11SocketDirError,
 		x11XauthorityError:            x11XauthorityError,
 
diff --git a/cli/agent.go b/cli/agent.go
@@ -387,11 +387,10 @@ func urlPort(u string) (int, error) {
 }
 
 func prometheusMetricsHandler(prometheusRegistry *prometheus.Registry, logger slog.Logger) http.Handler {
-	// We don't have any other internal metrics so far, so it's safe to expose metrics this way.
-	// Based on: https://github.com/tailscale/tailscale/blob/280255acae604796a1113861f5a84e6fa2dc6121/ipn/localapi/localapi.go#L489
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", "text/plain")
 
+		// Based on: https://github.com/tailscale/tailscale/blob/280255acae604796a1113861f5a84e6fa2dc6121/ipn/localapi/localapi.go#L489
 		clientmetric.WritePrometheusExpositionFormat(w)
 
 		metricFamilies, err := prometheusRegistry.Gather()
