[MM-57698] Implement client ICE candidate pairs metric (#803)

* Implement client ICE candidate pairs metric * Update calls-common
mattermost · Jul 26, 2024 · afed918 · afed918
1 parent a7dabb0
commit afed918
Show file tree

Hide file tree

Showing 14 changed files with 520 additions and 32 deletions.
diff --git a/server/interfaces/interfaces.go b/server/interfaces/interfaces.go
@@ -7,10 +7,14 @@ import (
 	"database/sql"
 	"net/http"
 
-	"github.com/mattermost/mattermost/server/public/model"
-	"github.com/mattermost/morph/models"
+	"github.com/mattermost/mattermost-plugin-calls/server/public"
+
 	rtcd "github.com/mattermost/rtcd/service"
 	"github.com/mattermost/rtcd/service/rtc"
+
+	"github.com/mattermost/mattermost/server/public/model"
+
+	"github.com/mattermost/morph/models"
 )
 
 type Metrics interface {
@@ -31,6 +35,7 @@ type Metrics interface {
 	ObserveAppHandlersTime(handler string, elapsed float64)
 	ObserveStoreMethodsTime(method string, elapsed float64)
 	RegisterDBMetrics(db *sql.DB, name string)
+	IncClientICECandidatePairs(p public.ClientICECandidatePairMetricPayload)
 }
 
 type StoreMetrics interface {

diff --git a/server/mocks/github.com/mattermost/mattermost-plugin-calls/server/interfaces/mock_Metrics.go b/server/mocks/github.com/mattermost/mattermost-plugin-calls/server/interfaces/mock_Metrics.go
diff --git a/server/performance/metrics.go b/server/performance/metrics.go
@@ -7,6 +7,8 @@ import (
 	"database/sql"
 	"net/http"
 
+	"github.com/mattermost/mattermost-plugin-calls/server/public"
+
 	"github.com/mattermost/rtcd/service/perf"
 	"github.com/mattermost/rtcd/service/rtc"
 
@@ -22,6 +24,7 @@ const (
 	metricsSubSystemApp     = "app"
 	metricsSubSystemStore   = "store"
 	metricsSubSystemJobs    = "jobs"
+	metricsSubSystemClient  = "client"
 )
 
 type DBStore interface {
@@ -48,6 +51,8 @@ type Metrics struct {
 	LiveCaptionsWindowDroppedCounter       prometheus.Counter
 	LiveCaptionsTranscriberBufFullCounter  prometheus.Counter
 	LiveCaptionsPktPayloadChBufFullCounter prometheus.Counter
+
+	ClientICECandidatePairsCounter *prometheus.CounterVec
 }
 
 func NewMetrics() *Metrics {
@@ -193,6 +198,17 @@ func NewMetrics() *Metrics {
 	)
 	m.registry.MustRegister(m.StoreMethodsTimeHistograms)
 
+	m.ClientICECandidatePairsCounter = prometheus.NewCounterVec(
+		prometheus.CounterOpts{
+			Namespace: metricsNamespace,
+			Subsystem: metricsSubSystemClient,
+			Name:      "ice_candidate_pairs_total",
+			Help:      "Total number of client-sent ICE candidate pairs",
+		},
+		[]string{"state", "local_type", "local_protocol", "remote_type", "remote_protocol"},
+	)
+	m.registry.MustRegister(m.ClientICECandidatePairsCounter)
+
 	m.rtcMetrics = perf.NewMetrics(metricsNamespace, m.registry)
 
 	return &m
@@ -265,3 +281,13 @@ func (m *Metrics) ObserveAppHandlersTime(handler string, elapsed float64) {
 func (m *Metrics) ObserveStoreMethodsTime(method string, elapsed float64) {
 	m.StoreMethodsTimeHistograms.With(prometheus.Labels{"method": method}).Observe(elapsed)
 }
+
+func (m *Metrics) IncClientICECandidatePairs(p public.ClientICECandidatePairMetricPayload) {
+	m.ClientICECandidatePairsCounter.With(prometheus.Labels{
+		"state":           p.State,
+		"local_type":      p.Local.Type,
+		"local_protocol":  p.Local.Protocol,
+		"remote_type":     p.Remote.Type,
+		"remote_protocol": p.Remote.Protocol,
+	}).Inc()
+}
diff --git a/server/public/job.go b/server/public/job.go
@@ -65,19 +65,6 @@ type CaptionMsg struct {
 	NewAudioLenMs float64 `json:"new_audio_len_ms"`
 }
 
-type MetricName string
-
-const (
-	MetricLiveCaptionsWindowDropped       MetricName = "live_captions_window_dropped"
-	MetricLiveCaptionsTranscriberBufFull  MetricName = "live_captions_transcriber_buf_full"
-	MetricLiveCaptionsPktPayloadChBufFull MetricName = "live_captions_pktPayloadCh_buf_full"
-)
-
-type MetricMsg struct {
-	SessionID  string     `json:"session_id"`
-	MetricName MetricName `json:"metric_name"`
-}
-
 type TranscribingJobInfo struct {
 	// Transcribing job ID
 	JobID string

diff --git a/server/public/metrics.go b/server/public/metrics.go
@@ -0,0 +1,65 @@
+package public
+
+import (
+	"fmt"
+)
+
+type MetricName string
+
+const (
+	MetricLiveCaptionsWindowDropped       MetricName = "live_captions_window_dropped"
+	MetricLiveCaptionsTranscriberBufFull  MetricName = "live_captions_transcriber_buf_full"
+	MetricLiveCaptionsPktPayloadChBufFull MetricName = "live_captions_pktPayloadCh_buf_full"
+
+	MetricClientICECandidatePair MetricName = "client_ice_candidate_pair"
+)
+
+type MetricMsg struct {
+	SessionID  string     `json:"session_id"`
+	MetricName MetricName `json:"metric_name"`
+}
+
+type ICECandidateInfo struct {
+	Type     string `json:"type"`
+	Protocol string `json:"protocol"`
+}
+
+func (i ICECandidateInfo) IsValid() error {
+	switch i.Type {
+	case "host", "srflx", "prflx", "relay":
+	default:
+		return fmt.Errorf("invalid type %q", i.Type)
+	}
+
+	switch i.Protocol {
+	case "udp", "tcp":
+	default:
+		return fmt.Errorf("invalid protocol %q", i.Protocol)
+	}
+
+	return nil
+}
+
+type ClientICECandidatePairMetricPayload struct {
+	State  string           `json:"state"`
+	Local  ICECandidateInfo `json:"local"`
+	Remote ICECandidateInfo `json:"remote"`
+}
+
+func (c ClientICECandidatePairMetricPayload) IsValid() error {
+	switch c.State {
+	case "succeeded", "waiting", "in-progress", "froze", "failed":
+	default:
+		return fmt.Errorf("invalid state %q", c.State)
+	}
+
+	if err := c.Local.IsValid(); err != nil {
+		return fmt.Errorf("invalid local candidate info: %w", err)
+	}
+
+	if err := c.Remote.IsValid(); err != nil {
+		return fmt.Errorf("invalid remote candidate info: %w", err)
+	}
+
+	return nil
+}
diff --git a/server/public/metrics_test.go b/server/public/metrics_test.go
@@ -0,0 +1,156 @@
+package public
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestICECandidateInfoIsValid(t *testing.T) {
+	tcs := []struct {
+		name string
+		info ICECandidateInfo
+		err  string
+	}{
+		{
+			name: "missing type",
+			info: ICECandidateInfo{
+				Protocol: "udp",
+			},
+			err: `invalid type ""`,
+		},
+		{
+			name: "missing protocol",
+			info: ICECandidateInfo{
+				Type: "host",
+			},
+			err: `invalid protocol ""`,
+		},
+		{
+			name: "invalid type",
+			info: ICECandidateInfo{
+				Type:     "invalid",
+				Protocol: "udp",
+			},
+			err: `invalid type "invalid"`,
+		},
+		{
+			name: "invalid protocol",
+			info: ICECandidateInfo{
+				Type:     "host",
+				Protocol: "invalid",
+			},
+			err: `invalid protocol "invalid"`,
+		},
+		{
+			name: "valid, udp",
+			info: ICECandidateInfo{
+				Type:     "host",
+				Protocol: "udp",
+			},
+		},
+		{
+			name: "valid, tcp",
+			info: ICECandidateInfo{
+				Type:     "host",
+				Protocol: "tcp",
+			},
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.info.IsValid()
+			if tc.err == "" {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tc.err)
+			}
+		})
+	}
+}
+
+func TestClientICECandidatePairMetricPayloadIsValid(t *testing.T) {
+	tcs := []struct {
+		name string
+		info ClientICECandidatePairMetricPayload
+		err  string
+	}{
+		{
+			name: "missing state",
+			info: ClientICECandidatePairMetricPayload{
+				Local: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+				Remote: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+			},
+			err: `invalid state ""`,
+		},
+		{
+			name: "invalid state",
+			info: ClientICECandidatePairMetricPayload{
+				State: "invalid",
+				Local: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+				Remote: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+			},
+			err: `invalid state "invalid"`,
+		},
+		{
+			name: "missing local",
+			info: ClientICECandidatePairMetricPayload{
+				State: "succeeded",
+				Remote: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+			},
+			err: `invalid local candidate info: invalid type ""`,
+		},
+		{
+			name: "missing remote",
+			info: ClientICECandidatePairMetricPayload{
+				State: "succeeded",
+				Local: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+			},
+			err: `invalid remote candidate info: invalid type ""`,
+		},
+		{
+			name: "valid",
+			info: ClientICECandidatePairMetricPayload{
+				State: "failed",
+				Local: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+				Remote: ICECandidateInfo{
+					Type:     "host",
+					Protocol: "udp",
+				},
+			},
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.name, func(t *testing.T) {
+			err := tc.info.IsValid()
+			if tc.err == "" {
+				require.NoError(t, err)
+			} else {
+				require.EqualError(t, err, tc.err)
+			}
+		})
+	}
+}