[MM-64244] Add websocket disconnect reason metric (#31032)

We've recently spent some effort improving websocket reconnection logic. With this commit, I've augmented the websocket reconnect metric to include a disconnect reason. This will help us measure the impact of these changes in production.

server/channels/api4/websocket.go

@@ -5,6 +5,7 @@ package api4
 
 import (
 	"net/http"
+	"strconv"
 
 	"github.com/gorilla/websocket"
 
@@ -15,11 +16,39 @@ import (
 )
 
 const (
-	connectionIDParam   = "connection_id"
-	sequenceNumberParam = "sequence_number"
-	postedAckParam      = "posted_ack"
+	connectionIDParam      = "connection_id"
+	sequenceNumberParam    = "sequence_number"
+	postedAckParam         = "posted_ack"
+	disconnectErrCodeParam = "disconnect_err_code"
+
+	clientPingTimeoutErrCode      = 4000
+	clientSequenceMismatchErrCode = 4001
 )
 
+// validateDisconnectErrCode ensures the specified disconnect error code
+// is a valid websocket close code
+func validateDisconnectErrCode(errCode string) bool {
+	if errCode == "" {
+		return false
+	}
+
+	// Ensure the disconnect code is a standard close code
+	code, err := strconv.Atoi(errCode)
+	if err != nil {
+		return false
+	}
+
+	// We only support the standard close codes between
+	// 1000 and 1016, and a few custom application codes
+	if (code < 1000 || code > 1016) &&
+		code != clientPingTimeoutErrCode &&
+		code != clientSequenceMismatchErrCode {
+		return false
+	}
+
+	return true
+}
+
 func (api *API) InitWebSocket() {
 	// Optionally supports a trailing slash
 	api.BaseRoutes.APIRoot.Handle("/{websocket:websocket(?:\\/)?}", api.APIHandlerTrustRequester(connectWebSocket)).Methods(http.MethodGet)
@@ -53,6 +82,12 @@ func connectWebSocket(c *Context, w http.ResponseWriter, r *http.Request) {
 		RemoteAddress: c.AppContext.IPAddress(),
 		XForwardedFor: c.AppContext.XForwardedFor(),
 	}
+
+	disconnectErrCode := r.URL.Query().Get(disconnectErrCodeParam)
+	if codeValid := validateDisconnectErrCode(disconnectErrCode); codeValid {
+		cfg.DisconnectErrCode = disconnectErrCode
+	}
+
 	// The WebSocket upgrade request coming from mobile is missing the
 	// user agent so we need to fallback on the session's metadata.
 	if c.AppContext.Session().IsMobileApp() {

server/channels/api4/websocket_test.go

@@ -465,3 +465,74 @@ func TestWebSocketUpgrade(t *testing.T) {
 	require.NoError(t, th.TestLogger.Flush())
 	testlib.AssertLog(t, buffer, mlog.LvlDebug.Name, "URL Blocked because of CORS. Url: ")
 }
+
+func TestValidateDisconnectErrCode(t *testing.T) {
+	testCases := []struct {
+		name    string
+		errCode string
+		valid   bool
+	}{
+		{
+			name:    "empty string",
+			errCode: "",
+			valid:   false,
+		},
+		{
+			name:    "non-numeric string",
+			errCode: "not-a-number",
+			valid:   false,
+		},
+		{
+			name:    "valid standard close code - 1000",
+			errCode: "1000",
+			valid:   true,
+		},
+		{
+			name:    "valid standard close code - 1001",
+			errCode: "1001",
+			valid:   true,
+		},
+		{
+			name:    "valid standard close code - 1015",
+			errCode: "1015",
+			valid:   true,
+		},
+		{
+			name:    "valid standard close code - 1016",
+			errCode: "1016",
+			valid:   true,
+		},
+		{
+			name:    "out of range (too low)",
+			errCode: "999",
+			valid:   false,
+		},
+		{
+			name:    "out of range (too high)",
+			errCode: "1017",
+			valid:   false,
+		},
+		{
+			name:    "valid custom code - client ping timeout",
+			errCode: "4000",
+			valid:   true,
+		},
+		{
+			name:    "valid custom code - client sequence mismatch",
+			errCode: "4001",
+			valid:   true,
+		},
+		{
+			name:    "invalid custom code",
+			errCode: "5000",
+			valid:   false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			result := validateDisconnectErrCode(tc.errCode)
+			require.Equal(t, tc.valid, result)
+		})
+	}
+}

server/channels/app/platform/web_conn.go

@@ -62,17 +62,18 @@ type pluginWSPostedHook struct {
 }
 
 type WebConnConfig struct {
-	WebSocket     *websocket.Conn
-	Session       model.Session
-	TFunc         i18n.TranslateFunc
-	Locale        string
-	ConnectionID  string
-	Active        bool
-	ReuseCount    int
-	OriginClient  string
-	PostedAck     bool
-	RemoteAddress string
-	XForwardedFor string
+	WebSocket         *websocket.Conn
+	Session           model.Session
+	TFunc             i18n.TranslateFunc
+	Locale            string
+	ConnectionID      string
+	Active            bool
+	ReuseCount        int
+	OriginClient      string
+	PostedAck         bool
+	RemoteAddress     string
+	XForwardedFor     string
+	DisconnectErrCode string
 
 	// These aren't necessary to be exported to api layer.
 	sequence         int64
@@ -85,16 +86,17 @@ type WebConnConfig struct {
 // It contains all the necessary state to manage sending/receiving data to/from
 // a websocket.
 type WebConn struct {
-	sessionExpiresAt int64 // This should stay at the top for 64-bit alignment of 64-bit words accessed atomically
-	Platform         *PlatformService
-	Suite            SuiteIFace
-	HookRunner       HookRunner
-	WebSocket        *websocket.Conn
-	T                i18n.TranslateFunc
-	Locale           string
-	Sequence         int64
-	UserId           string
-	PostedAck        bool
+	sessionExpiresAt  int64 // This should stay at the top for 64-bit alignment of 64-bit words accessed atomically
+	Platform          *PlatformService
+	Suite             SuiteIFace
+	HookRunner        HookRunner
+	WebSocket         *websocket.Conn
+	T                 i18n.TranslateFunc
+	Locale            string
+	Sequence          int64
+	UserId            string
+	PostedAck         bool
+	DisconnectErrCode string
 
 	allChannelMembers         map[string]string
 	lastAllChannelMembersTime int64
@@ -246,6 +248,7 @@ func (ps *PlatformService) NewWebConn(cfg *WebConnConfig, suite SuiteIFace, runn
 		T:                  cfg.TFunc,
 		Locale:             cfg.Locale,
 		PostedAck:          cfg.PostedAck,
+		DisconnectErrCode:  cfg.DisconnectErrCode,
 		reuseCount:         cfg.ReuseCount,
 		endWritePump:       make(chan struct{}),
 		pumpFinished:       make(chan struct{}),
@@ -523,7 +526,7 @@ func (wc *WebConn) writePump() {
 				return
 			}
 			if m := wc.Platform.metricsIFace; m != nil {
-				m.IncrementWebsocketReconnectEvent(reconnectFound)
+				m.IncrementWebsocketReconnectEventWithDisconnectErrCode(reconnectFound, wc.DisconnectErrCode)
 			}
 		} else if wc.hasMsgLoss() {
 			// If the seq number is not in dead queue, but it was supposed to be,
@@ -541,11 +544,11 @@ func (wc *WebConn) writePump() {
 				return
 			}
 			if m := wc.Platform.metricsIFace; m != nil {
-				m.IncrementWebsocketReconnectEvent(reconnectNotFound)
+				m.IncrementWebsocketReconnectEventWithDisconnectErrCode(reconnectNotFound, wc.DisconnectErrCode)
 			}
 		} else {
 			if m := wc.Platform.metricsIFace; m != nil {
-				m.IncrementWebsocketReconnectEvent(reconnectLossless)
+				m.IncrementWebsocketReconnectEventWithDisconnectErrCode(reconnectLossless, wc.DisconnectErrCode)
 			}
 		}
 	}

server/einterfaces/metrics.go

@@ -48,7 +48,7 @@ type MetricsInterface interface {
 	DecrementWebSocketBroadcastBufferSize(hub string, amount float64)
 	IncrementWebSocketBroadcastUsersRegistered(hub string, amount float64)
 	DecrementWebSocketBroadcastUsersRegistered(hub string, amount float64)
-	IncrementWebsocketReconnectEvent(eventType string)
+	IncrementWebsocketReconnectEventWithDisconnectErrCode(eventType string, disconnectErrCode string)
 
 	IncrementHTTPWebSockets(originClient string)
 	DecrementHTTPWebSockets(originClient string)

server/einterfaces/mocks/MetricsInterface.go

@@ -298,9 +298,9 @@ func (_m *MetricsInterface) IncrementWebsocketEvent(eventType model.WebsocketEve
 	_m.Called(eventType)
 }
 
-// IncrementWebsocketReconnectEvent provides a mock function with given fields: eventType
-func (_m *MetricsInterface) IncrementWebsocketReconnectEvent(eventType string) {
-	_m.Called(eventType)
+// IncrementWebsocketReconnectEventWithDisconnectErrCode provides a mock function with given fields: eventType, disconnectErrCode
+func (_m *MetricsInterface) IncrementWebsocketReconnectEventWithDisconnectErrCode(eventType string, disconnectErrCode string) {
+	_m.Called(eventType, disconnectErrCode)
 }
 
 // ObserveAPIEndpointDuration provides a mock function with given fields: endpoint, method, statusCode, originClient, pageLoadContext, elapsed

server/enterprise/metrics/metrics.go

@@ -726,7 +726,7 @@ func New(ps *platform.PlatformService, driver, dataSource string) *MetricsInterf
 			Help:        "Total number of websocket reconnect attempts",
 			ConstLabels: additionalLabels,
 		},
-		[]string{"type"},
+		[]string{"type", "disconnect_err_code"},
 	)
 	m.Registry.MustRegister(m.WebSocketReconnectCounter)
 
@@ -1822,8 +1822,14 @@ func (mi *MetricsInterfaceImpl) IncrementWebsocketEvent(eventType model.Websocke
 	mi.WebsocketEventCounters.With(prometheus.Labels{"type": string(eventType)}).Inc()
 }
 
-func (mi *MetricsInterfaceImpl) IncrementWebsocketReconnectEvent(eventType string) {
-	mi.WebSocketReconnectCounter.With(prometheus.Labels{"type": eventType}).Inc()
+func (mi *MetricsInterfaceImpl) IncrementWebsocketReconnectEventWithDisconnectErrCode(eventType string, disconnectErrCode string) {
+	if disconnectErrCode == "" {
+		disconnectErrCode = "unknown"
+	}
+	mi.WebSocketReconnectCounter.With(prometheus.Labels{
+		"type":                eventType,
+		"disconnect_err_code": disconnectErrCode,
+	}).Inc()
 }
 
 func (mi *MetricsInterfaceImpl) IncrementWebSocketBroadcastBufferSize(hub string, amount float64) {