addressing feedback

this commit ensures that metrics are properly accounted for

Signed-off-by: Imran Pochi <[email protected]>

Author: ipochi

pkg/server/tunnel.go

@@ -80,7 +80,7 @@ func (t *Tunnel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	if err != nil {
 		klog.ErrorS(err, "no tunnels available")
 		conn.Write([]byte(fmt.Sprintf("HTTP/1.1 500 Internal Server Error\r\nContent-Type: text/plain\r\n\r\ncurrently no tunnels available: %v", err)))
-		// conn.Close() is handled by the top-level defer
+		// The hijacked connection will be closed by the closeOnce defer.
 		return
 	}
 	closed := make(chan struct{})
@@ -104,13 +104,27 @@ func (t *Tunnel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 	// This defer acts as a safeguard to ensure we clean up the pending dial
 	// if the connection is never successfully established.
 	established := false
+	// Track if the frontend client explicitly closed the connection.
+	var frontendDidClose bool
 	defer func() {
 		if !established {
-			if t.Server.PendingDial.Remove(random) != nil {
-				// This metric is observed only when the frontend closes the connection.
-				// Other failure reasons are observed elsewhere.
+			// Always attempt to remove the pending dial.
+			// This is safe even if another goroutine already removed it.
+			removedConn := t.Server.PendingDial.Remove(random)
+
+			// If the frontend explicitly closed the connection, we record that failure reason.
+			// This is more accurate than inferring the reason from whether Remove() found an entry.
+			if frontendDidClose {
 				metrics.Metrics.ObserveDialFailure(metrics.DialFailureFrontendClose)
+			} else if removedConn != nil {
+				// If the frontend did *not* close, but we *did* remove the pending dial,
+				// it implies we hit a timeout or backend context cancellation before
+				// the agent could respond. We can attribute this to a backend failure.
+				metrics.Metrics.ObserveDialFailure(metrics.DialFailureBackendClose)
 			}
+			// If removedConn is nil and frontendDidClose is false, it means another
+			// goroutine (e.g., processing a DIAL_RSP) already handled the cleanup,
+			// so we don't need to do anything here.
 		}
 	}()
 
@@ -143,6 +157,7 @@ func (t *Tunnel) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		klog.V(3).InfoS("Connection established, sent 200 OK", "host", r.Host, "agentID", connection.agentID, "connectionID", connection.connectID)
 
 	case <-closed: // Connection was closed by the client before being established
+		frontendDidClose = true
 		klog.V(2).InfoS("Frontend connection closed before being established", "host", r.Host, "dialID", connection.dialID, "agentID", connection.agentID)
 		// The deferred cleanup will run when we return here.
 		return