diff --git a/.env.development b/.env.development index fcbe12541..9bf3d60b3 100644 --- a/.env.development +++ b/.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= diff --git a/.env.production b/.env.production index 9bc03d506..5a8121f4f 100644 --- a/.env.production +++ b/.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 diff --git a/package.json b/package.json index a724ab9a1..17195b5c3 100644 --- a/package.json +++ b/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", diff --git a/src/env.ts b/src/env.ts index 37efa0c21..0efb2c156 100644 --- a/src/env.ts +++ b/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 diff --git a/src/lang/std/engineConnection.ts b/src/lang/std/engineConnection.ts index 8104f9d3a..8d7afed81 100644 --- a/src/lang/std/engineConnection.ts +++ b/src/lang/std/engineConnection.ts @@ -1,10 +1,6 @@ import { SourceRange } from 'lang/executor' import { Selections } from 'useStore' -import { - VITE_KC_API_WS_MODELING_URL, - VITE_KC_CONNECTION_TIMEOUT_MS, - VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS, -} from 'env' +import { VITE_KC_API_WS_MODELING_URL, VITE_KC_CONNECTION_TIMEOUT_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 + 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 - // information about the WebRTC media stream from the server to - // us. We'll also eventually want more global statistical information, - // but this will give us a baseline. - if (parseInt(VITE_KC_CONNECTION_WEBRTC_REPORT_STATS_MS) !== 0) { - setInterval(() => { - if (this.pc === undefined) { - return - } - if (!this.shouldTrace()) { + this.webrtcStatsCollector = (): Promise => { + return new Promise((resolve, reject) => { + if (mediaStream.getVideoTracks().length !== 1) { + reject(new Error('too many video tracks to report')) return } - // Use the WebRTC Statistics API to collect statistical information - // about the WebRTC connection we're using to report to Sentry. - mediaStream.getVideoTracks().forEach((videoTrack) => { - let trackStats = new Map() - this.pc?.getStats(videoTrack).then((videoTrackStats) => { - // Sentry only allows 10 metrics per transaction. We're going - // to have to pick carefully here, eventually send like a prom - // file or something to the peer. + let videoTrack = mediaStream.getVideoTracks()[0] + this.pc?.getStats(videoTrack).then((videoTrackStats) => { + // TODO(paultag): this needs type information from the KittyCAD typescript + // library once it's updated + 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, + } - const transaction = Sentry.startTransaction({ - name: 'webrtc-stats', - }) - videoTrackStats.forEach((videoTrackReport) => { - if (videoTrackReport.type === 'inbound-rtp') { - // RTC Stream Info - // transaction.setMeasurement( - // 'mediaStreamTrack.framesDecoded', - // videoTrackReport.framesDecoded, - // 'frame' - // ) - transaction.setMeasurement( - 'rtcFramesDropped', - videoTrackReport.framesDropped, - '' - ) - // transaction.setMeasurement( - // 'mediaStreamTrack.framesReceived', - // videoTrackReport.framesReceived, - // '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() + // 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. + + videoTrackStats.forEach((videoTrackReport) => { + if (videoTrackReport.type === 'inbound-rtp') { + client_metrics.rtc_frames_decoded = + videoTrackReport.framesDecoded + client_metrics.rtc_frames_dropped = + videoTrackReport.framesDropped + client_metrics.rtc_frames_received = + videoTrackReport.framesReceived + client_metrics.rtc_frames_per_second = + videoTrackReport.framesPerSecond || 0 + client_metrics.rtc_freeze_count = videoTrackReport.freezeCount + client_metrics.rtc_jitter_sec = videoTrackReport.jitter + client_metrics.rtc_keyframes_decoded = + videoTrackReport.keyFramesDecoded + client_metrics.rtc_total_freezes_duration_sec = + videoTrackReport.totalFreezesDuration + } else if (videoTrackReport.type === 'transport') { + // videoTrackReport.bytesReceived, + // videoTrackReport.bytesSent, + } }) + 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 diff --git a/yarn.lock b/yarn.lock index 9487c18cc..0158ab167 100644 --- a/yarn.lock +++ b/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": - version "0.0.36" - resolved "https://registry.yarnpkg.com/@kittycad/lib/-/lib-0.0.36.tgz#7b9676c975bc629f227d41897b38e7d73280db71" - integrity sha512-4bVXTaIzpSRuJAuLbAD/CWWTns7H/IxogPj0827n8mwXDkj+65EBCNXhJGWRkMG2CeTVJVk1LSWKlaHE+ToxGA== +"@kittycad/lib@^0.0.37": + version "0.0.37" + resolved "https://registry.yarnpkg.com/@kittycad/lib/-/lib-0.0.37.tgz#ec4f6c4fb5d06402a19339f3374036b6582d2265" + integrity sha512-P8p9FeLV79/0Lfd0RioBta1drzhmpROnU4YV38+zsAA4LhibQCTjeekRkxVvHztGumPxz9pPsAeeLJyuz2RWKQ== dependencies: node-fetch "3.3.2" openapi-types "^12.0.0"