Change WebRTC metrics to be request/response from the Engine (#410)

* Add in a Metrics request/response handler

Signed-off-by: Paul Tagliamonte <paul@kittycad.io>

* Update @kittycad/lib to 0.0.37

Signed-off-by: Paul Tagliamonte <paul@kittycad.io>

* Fix up type issues

Signed-off-by: Paul Tagliamonte <paul@kittycad.io>

* yarn fmt

* Remove VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS

Signed-off-by: Paul Tagliamonte <paul@kittycad.io>

---------

Signed-off-by: Paul Tagliamonte <paul@kittycad.io>
Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

.env.development

@@ -3,5 +3,4 @@ VITE_KC_API_BASE_URL=https://api.dev.kittycad.io
 VITE_KC_SITE_BASE_URL=https://dev.kittycad.io
 VITE_KC_SKIP_AUTH=false
 VITE_KC_CONNECTION_TIMEOUT_MS=5000
-VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS=0
 VITE_KC_SENTRY_DSN=

.env.production

@@ -3,5 +3,4 @@ VITE_KC_API_BASE_URL=https://api.kittycad.io
 VITE_KC_SITE_BASE_URL=https://kittycad.io
 VITE_KC_SKIP_AUTH=false
 VITE_KC_CONNECTION_TIMEOUT_MS=15000
-VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS=30000
 VITE_KC_SENTRY_DSN=https://a814f2f66734989a90367f48feee28ca@o1042111.ingest.sentry.io/4505789425844224

package.json

@@ -10,7 +10,7 @@
     "@fortawesome/react-fontawesome": "^0.2.0",
     "@headlessui/react": "^1.7.13",
     "@headlessui/tailwindcss": "^0.2.0",
-    "@kittycad/lib": "^0.0.36",
+    "@kittycad/lib": "^0.0.37",
     "@lezer/javascript": "^1.4.7",
     "@open-rpc/client-js": "^1.8.1",
     "@react-hook/resize-observer": "^1.2.6",

src/env.ts

@@ -8,8 +8,6 @@ export const VITE_KC_API_WS_MODELING_URL = import.meta.env
   .VITE_KC_API_WS_MODELING_URL
 export const VITE_KC_API_BASE_URL = import.meta.env.VITE_KC_API_BASE_URL
 export const VITE_KC_SITE_BASE_URL = import.meta.env.VITE_KC_SITE_BASE_URL
-export const VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS = import.meta.env
-  .VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS
 export const VITE_KC_CONNECTION_TIMEOUT_MS = import.meta.env
   .VITE_KC_CONNECTION_TIMEOUT_MS
 export const VITE_KC_SENTRY_DSN = import.meta.env.VITE_KC_SENTRY_DSN

src/lang/std/engineConnection.ts

@@ -1,10 +1,6 @@
 import { SourceRange } from 'lang/executor'
 import { Selections } from 'useStore'
-import {
+import { VITE_KC_API_WS_MODELING_URL, VITE_KC_CONNECTION_TIMEOUT_MS } from 'env'
-  VITE_KC_API_WS_MODELING_URL,
-  VITE_KC_CONNECTION_TIMEOUT_MS,
-  VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS,
-} from 'env'
 import { Models } from '@kittycad/lib'
 import { exportSave } from 'lib/exportSave'
 import { v4 as uuidv4 } from 'uuid'
@@ -39,6 +35,8 @@ interface NewTrackArgs {
 
 type WebSocketResponse = Models['OkWebSocketResponseData_type']
 
+type ClientMetrics = Models['ClientMetrics_type']
+
 // EngineConnection encapsulates the connection(s) to the Engine
 // for the EngineCommandManager; namely, the underlying WebSocket
 // and WebRTC connections.
@@ -58,6 +56,9 @@ export class EngineConnection {
   private onClose: (engineConnection: EngineConnection) => void
   private onNewTrack: (track: NewTrackArgs) => void
 
+  // TODO: actual type is ClientMetrics
+  private webrtcStatsCollector?: () => Promise<ClientMetrics>
+
   constructor({
     url,
     token,
@@ -339,6 +340,17 @@ export class EngineConnection {
             })
           })
           .catch(console.log)
+      } else if (resp.type === 'metrics_request') {
+        if (this.webrtcStatsCollector === undefined) {
+          // TODO: Error message here?
+          return
+        }
+        this.webrtcStatsCollector().then((client_metrics) => {
+          this.send({
+            type: 'metrics_response',
+            metrics: client_metrics,
+          })
+        })
       }
 
       // TODO(paultag): This ought to be both controllable, as well as something
@@ -370,127 +382,58 @@ export class EngineConnection {
         })
       }
 
-      // Set up the background thread to keep an eye on statistical
+      this.webrtcStatsCollector = (): Promise<ClientMetrics> => {
-      // information about the WebRTC media stream from the server to
+        return new Promise((resolve, reject) => {
-      // us. We'll also eventually want more global statistical information,
+          if (mediaStream.getVideoTracks().length !== 1) {
-      // but this will give us a baseline.
+            reject(new Error('too many video tracks to report'))
-      if (parseInt(VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS) !== 0) {
-        setInterval(() => {
-          if (this.pc === undefined) {
-            return
-          }
-          if (!this.shouldTrace()) {
             return
           }
 
-          // Use the WebRTC Statistics API to collect statistical information
+          let videoTrack = mediaStream.getVideoTracks()[0]
-          // about the WebRTC connection we're using to report to Sentry.
-          mediaStream.getVideoTracks().forEach((videoTrack) => {
-            let trackStats = new Map<string, any>()
           this.pc?.getStats(videoTrack).then((videoTrackStats) => {
-              // Sentry only allows 10 metrics per transaction. We're going
+            // TODO(paultag): this needs type information from the KittyCAD typescript
-              // to have to pick carefully here, eventually send like a prom
+            // library once it's updated
-              // file or something to the peer.
+            let client_metrics: ClientMetrics = {
+              rtc_frames_decoded: 0,
+              rtc_frames_dropped: 0,
+              rtc_frames_received: 0,
+              rtc_frames_per_second: 0,
+              rtc_freeze_count: 0,
+              rtc_jitter_sec: 0.0,
+              rtc_keyframes_decoded: 0,
+              rtc_total_freezes_duration_sec: 0.0,
+            }
+
+            // TODO(paultag): Since we can technically have multiple WebRTC
+            // video tracks (even if the Server doesn't at the moment), we
+            // ought to send stats for every video track(?), and add the stream
+            // ID into it.  This raises the cardinality of collected metrics
+            // when/if we do, but for now, just report the one stream.
 
-              const transaction = Sentry.startTransaction({
-                name: 'webrtc-stats',
-              })
             videoTrackStats.forEach((videoTrackReport) => {
               if (videoTrackReport.type === 'inbound-rtp') {
-                  // RTC Stream Info
+                client_metrics.rtc_frames_decoded =
-                  // transaction.setMeasurement(
+                  videoTrackReport.framesDecoded
-                  //   'mediaStreamTrack.framesDecoded',
+                client_metrics.rtc_frames_dropped =
-                  //   videoTrackReport.framesDecoded,
+                  videoTrackReport.framesDropped
-                  //   'frame'
+                client_metrics.rtc_frames_received =
-                  // )
+                  videoTrackReport.framesReceived
-                  transaction.setMeasurement(
+                client_metrics.rtc_frames_per_second =
-                    'rtcFramesDropped',
+                  videoTrackReport.framesPerSecond || 0
-                    videoTrackReport.framesDropped,
+                client_metrics.rtc_freeze_count = videoTrackReport.freezeCount
-                    ''
+                client_metrics.rtc_jitter_sec = videoTrackReport.jitter
-                  )
+                client_metrics.rtc_keyframes_decoded =
-                  // transaction.setMeasurement(
+                  videoTrackReport.keyFramesDecoded
-                  //   'mediaStreamTrack.framesReceived',
+                client_metrics.rtc_total_freezes_duration_sec =
-                  //   videoTrackReport.framesReceived,
+                  videoTrackReport.totalFreezesDuration
-                  //   'frame'
-                  // )
-                  transaction.setMeasurement(
-                    'rtcFramesPerSecond',
-                    videoTrackReport.framesPerSecond,
-                    'fps'
-                  )
-                  transaction.setMeasurement(
-                    'rtcFreezeCount',
-                    videoTrackReport.freezeCount,
-                    ''
-                  )
-                  transaction.setMeasurement(
-                    'rtcJitter',
-                    videoTrackReport.jitter,
-                    'second'
-                  )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.jitterBufferDelay',
-                  //   videoTrackReport.jitterBufferDelay,
-                  //   ''
-                  // )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.jitterBufferEmittedCount',
-                  //   videoTrackReport.jitterBufferEmittedCount,
-                  //   ''
-                  // )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.jitterBufferMinimumDelay',
-                  //   videoTrackReport.jitterBufferMinimumDelay,
-                  //   ''
-                  // )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.jitterBufferTargetDelay',
-                  //   videoTrackReport.jitterBufferTargetDelay,
-                  //   ''
-                  // )
-                  transaction.setMeasurement(
-                    'rtcKeyFramesDecoded',
-                    videoTrackReport.keyFramesDecoded,
-                    ''
-                  )
-                  transaction.setMeasurement(
-                    'rtcTotalFreezesDuration',
-                    videoTrackReport.totalFreezesDuration,
-                    'second'
-                  )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.totalInterFrameDelay',
-                  //   videoTrackReport.totalInterFrameDelay,
-                  //   ''
-                  // )
-                  transaction.setMeasurement(
-                    'rtcTotalPausesDuration',
-                    videoTrackReport.totalPausesDuration,
-                    'second'
-                  )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.totalProcessingDelay',
-                  //   videoTrackReport.totalProcessingDelay,
-                  //   'second'
-                  // )
               } else if (videoTrackReport.type === 'transport') {
-                  // // Bytes i/o
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.bytesReceived',
                 // videoTrackReport.bytesReceived,
-                  //   'byte'
-                  // )
-                  // transaction.setMeasurement(
-                  //   'mediaStreamTrack.bytesSent',
                 // videoTrackReport.bytesSent,
-                  //   'byte'
-                  // )
               }
             })
-              transaction?.finish()
+            resolve(client_metrics)
           })
         })
-        }, VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS)
       }
 
       this.onNewTrack({
@@ -499,10 +442,6 @@ export class EngineConnection {
       })
     })
 
-    // During startup, we'll track the time from `connect` being called
-    // until the 'done' event fires.
-    let connectionStarted = new Date()
-
     this.pc.addEventListener('datachannel', (event) => {
       this.unreliableDataChannel = event.channel
 
@@ -546,6 +485,7 @@ export class EngineConnection {
     this.websocket = undefined
     this.pc = undefined
     this.unreliableDataChannel = undefined
+    this.webrtcStatsCollector = undefined
 
     this.onClose(this)
     this.ready = false

yarn.lock

@@ -1530,10 +1530,10 @@
   resolved "https://registry.yarnpkg.com/@juggle/resize-observer/-/resize-observer-3.4.0.tgz#08d6c5e20cf7e4cc02fd181c4b0c225cd31dbb60"
   integrity sha512-dfLbk+PwWvFzSxwk3n5ySL0hfBog779o8h68wK/7/APo/7cgyWp5jcXockbxdk5kFRkbeXWm4Fbi9FrdN381sA==
 
-"@kittycad/lib@^0.0.36":
+"@kittycad/lib@^0.0.37":
-  version "0.0.36"
+  version "0.0.37"
-  resolved "https://registry.yarnpkg.com/@kittycad/lib/-/lib-0.0.36.tgz#7b9676c975bc629f227d41897b38e7d73280db71"
+  resolved "https://registry.yarnpkg.com/@kittycad/lib/-/lib-0.0.37.tgz#ec4f6c4fb5d06402a19339f3374036b6582d2265"
-  integrity sha512-4bVXTaIzpSRuJAuLbAD/CWWTns7H/IxogPj0827n8mwXDkj+65EBCNXhJGWRkMG2CeTVJVk1LSWKlaHE+ToxGA==
+  integrity sha512-P8p9FeLV79/0Lfd0RioBta1drzhmpROnU4YV38+zsAA4LhibQCTjeekRkxVvHztGumPxz9pPsAeeLJyuz2RWKQ==
   dependencies:
     node-fetch "3.3.2"
     openapi-types "^12.0.0"