Redo how Spans are used from the Engine (#359)

* Redo how Spans are used from the Engine

I don't like all the Sentry-specific stuff we've got to work around, and
I want to add a bunch more spans and more cleanly end the transaction.

This isn't generic enough to pull out of this code (yet?), but we
clearly need some class of abstraction due to the highly async pattern
in the WebRTC code.

I want to add in more tags, but there are a lot of events we need to
wait on. I'd like to hook into the <video> 'play' eventListener, but
it's hard to do from all the way down in the Engine.

Signed-off-by: Paul R. Tagliamonte <paul@kittycad.io>
This commit is contained in:
Paul Tagliamonte
2023-08-31 12:59:46 -04:00
committed by GitHub
parent afcf820bdd
commit 4837c52908

View File

@ -27,16 +27,6 @@ export interface SourceRangeMap {
[key: string]: SourceRange [key: string]: SourceRange
} }
interface SelectionsArgs {
id: string
type: Selections['codeBasedSelections'][number]['type']
}
interface CursorSelectionsArgs {
otherSelections: Selections['otherSelections']
idBasedSelections: { type: string; id: string }[]
}
interface NewTrackArgs { interface NewTrackArgs {
conn: EngineConnection conn: EngineConnection
mediaStream: MediaStream mediaStream: MediaStream
@ -128,17 +118,40 @@ export class EngineConnection {
// Information on the connect transaction // Information on the connect transaction
class SpanPromise {
span: Sentry.Span
promise: Promise<void>
resolve?: (v: void) => void
constructor(span: Sentry.Span) {
this.span = span
this.promise = new Promise((resolve) => {
this.resolve = (v: void) => {
// here we're going to invoke finish before resolving the
// promise so that a `.then()` will order strictly after
// all spans have -- for sure -- been resolved, rather than
// doing a `then` on this promise.
this.span.finish()
resolve(v)
}
})
}
}
let webrtcMediaTransaction: Sentry.Transaction let webrtcMediaTransaction: Sentry.Transaction
let websocketSpan: Sentry.Span let websocketSpan: SpanPromise
let mediaTrackSpan: Sentry.Span let mediaTrackSpan: SpanPromise
let dataChannelSpan: Sentry.Span let dataChannelSpan: SpanPromise
let handshakeSpan: Sentry.Span let handshakeSpan: SpanPromise
let iceSpan: SpanPromise
if (this.shouldTrace()) { if (this.shouldTrace()) {
webrtcMediaTransaction = Sentry.startTransaction({ webrtcMediaTransaction = Sentry.startTransaction({
name: 'webrtc-media', name: 'webrtc-media',
}) })
websocketSpan = webrtcMediaTransaction.startChild({ op: 'websocket' }) websocketSpan = new SpanPromise(
webrtcMediaTransaction.startChild({ op: 'websocket' })
)
} }
this.websocket = new WebSocket(this.url, []) this.websocket = new WebSocket(this.url, [])
@ -155,17 +168,36 @@ export class EngineConnection {
this.websocket.addEventListener('open', (event) => { this.websocket.addEventListener('open', (event) => {
if (this.shouldTrace()) { if (this.shouldTrace()) {
// websocketSpan.setStatus(SpanStatus.OK) websocketSpan.resolve?.()
websocketSpan.finish()
handshakeSpan = webrtcMediaTransaction.startChild({ op: 'handshake' }) handshakeSpan = new SpanPromise(
dataChannelSpan = webrtcMediaTransaction.startChild({ webrtcMediaTransaction.startChild({ op: 'handshake' })
)
iceSpan = new SpanPromise(
webrtcMediaTransaction.startChild({ op: 'ice' })
)
dataChannelSpan = new SpanPromise(
webrtcMediaTransaction.startChild({
op: 'data-channel', op: 'data-channel',
}) })
mediaTrackSpan = webrtcMediaTransaction.startChild({ )
mediaTrackSpan = new SpanPromise(
webrtcMediaTransaction.startChild({
op: 'media-track', op: 'media-track',
}) })
)
} }
Promise.all([
handshakeSpan.promise,
iceSpan.promise,
dataChannelSpan.promise,
mediaTrackSpan.promise,
]).then(() => {
console.log('All spans finished, reporting')
webrtcMediaTransaction?.finish()
})
this.onWebsocketOpen(this) this.onWebsocketOpen(this)
}) })
@ -233,7 +265,7 @@ export class EngineConnection {
// When both ends have a local and remote SDP, we've been able to // When both ends have a local and remote SDP, we've been able to
// set up successfully. We'll still need to find the right ICE // set up successfully. We'll still need to find the right ICE
// servers, but this is hand-shook. // servers, but this is hand-shook.
handshakeSpan.finish() handshakeSpan.resolve?.()
} }
} }
} else if (resp.type === 'trickle_ice') { } else if (resp.type === 'trickle_ice') {
@ -264,9 +296,9 @@ export class EngineConnection {
// PeerConnection and waiting for events to fire our callbacks. // PeerConnection and waiting for events to fire our callbacks.
this.pc.addEventListener('connectionstatechange', (event) => { this.pc.addEventListener('connectionstatechange', (event) => {
// if (this.pc?.iceConnectionState === 'disconnected') { if (this.pc?.iceConnectionState === 'connected') {
// this.close() iceSpan.resolve?.()
// } }
}) })
this.pc.addEventListener('icecandidate', (event) => { this.pc.addEventListener('icecandidate', (event) => {
@ -316,13 +348,16 @@ export class EngineConnection {
}) })
this.pc.addEventListener('track', (event) => { this.pc.addEventListener('track', (event) => {
console.log('received track', event)
const mediaStream = event.streams[0] const mediaStream = event.streams[0]
if (this.shouldTrace()) { if (this.shouldTrace()) {
mediaStream.getVideoTracks()[0].addEventListener('unmute', () => { let mediaStreamTrack = mediaStream.getVideoTracks()[0]
mediaTrackSpan.finish() mediaStreamTrack.addEventListener('unmute', () => {
webrtcMediaTransaction.finish() // let settings = mediaStreamTrack.getSettings()
// mediaTrackSpan.span.setTag("fps", settings.frameRate)
// mediaTrackSpan.span.setTag("width", settings.width)
// mediaTrackSpan.span.setTag("height", settings.height)
mediaTrackSpan.resolve?.()
}) })
} }
@ -339,8 +374,6 @@ export class EngineConnection {
return return
} }
console.log('Reporting statistics')
// Use the WebRTC Statistics API to collect statistical information // Use the WebRTC Statistics API to collect statistical information
// about the WebRTC connection we're using to report to Sentry. // about the WebRTC connection we're using to report to Sentry.
mediaStream.getVideoTracks().forEach((videoTrack) => { mediaStream.getVideoTracks().forEach((videoTrack) => {
@ -468,7 +501,7 @@ export class EngineConnection {
this.unreliableDataChannel.addEventListener('open', (event) => { this.unreliableDataChannel.addEventListener('open', (event) => {
console.log('unreliable data channel opened', event) console.log('unreliable data channel opened', event)
if (this.shouldTrace()) { if (this.shouldTrace()) {
dataChannelSpan.finish() dataChannelSpan.resolve?.()
} }
this.onDataChannelOpen(this) this.onDataChannelOpen(this)