From cf9eeae2de928f70b7caac74b41d0ded0a554194 Mon Sep 17 00:00:00 2001 From: Victor Elias Date: Tue, 14 Nov 2023 18:27:43 -0300 Subject: [PATCH 1/5] media/metrics: Avoid reporting preloadTime as 0 --- packages/core/src/media/metrics/metrics.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/core/src/media/metrics/metrics.ts b/packages/core/src/media/metrics/metrics.ts index b915e816..e12f50c1 100644 --- a/packages/core/src/media/metrics/metrics.ts +++ b/packages/core/src/media/metrics/metrics.ts @@ -3,7 +3,7 @@ import { MediaControllerStore } from '../controller'; import { MimeType } from '../mime'; type RawMetrics = { - preloadTime: number; + preloadTime?: number; ttff: number; firstPlayback: number; @@ -178,8 +178,8 @@ function isInIframe() { } export class MetricsStatus { - requestedPlayTime = 0; - firstFrameTime = 0; + requestedPlayTime?: number; + firstFrameTime?: number; retryCount = 0; connected = false; @@ -233,7 +233,6 @@ export class MetricsStatus { sourceUrl: currentState?.src?.src ?? null, playerHeight: null, playerWidth: null, - preloadTime: 0, timeStalled: 0, timeUnpaused: 0, timeWaiting: 0, @@ -248,7 +247,7 @@ export class MetricsStatus { }; this.destroy = store.subscribe((state, prevState) => { - if (this.requestedPlayTime === 0 && state._playLastTime !== 0) { + if (this.requestedPlayTime === undefined && state._playLastTime !== 0) { this.requestedPlayTime = Math.max(state._playLastTime - bootMs, 0); } @@ -335,10 +334,11 @@ export class MetricsStatus { offset: this.store.getState().playbackOffsetMs ?? 0, // this is the amount of time that a video has had to preload content, from boot until play was requested - preloadTime: Math.max(this.requestedPlayTime, 0), + preloadTime: this.requestedPlayTime, // time from when the first `play` event is emitted and the first progress update ttff: - this.firstFrameTime > 0 && this.requestedPlayTime > 0 + this.firstFrameTime !== undefined && + this.requestedPlayTime !== undefined ? Math.max(this.firstFrameTime - this.requestedPlayTime, 0) : 0, }; @@ -542,7 +542,7 @@ export function addMediaMetricsToStore( if ( state.progress !== prevState.progress && - metricsStatus.getFirstFrameTime() === 0 + metricsStatus.getFirstFrameTime() === undefined ) { metricsStatus.setFirstFrameTime(); } From 244646431f8d6754b30b2e40b557798072134de7 Mon Sep 17 00:00:00 2001 From: Chase Adams Date: Tue, 14 Nov 2023 15:03:17 -0700 Subject: [PATCH 2/5] Revert "media/metrics: Avoid reporting preloadTime as 0" This reverts commit cf9eeae2de928f70b7caac74b41d0ded0a554194. --- packages/core/src/media/metrics/metrics.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/core/src/media/metrics/metrics.ts b/packages/core/src/media/metrics/metrics.ts index e12f50c1..b915e816 100644 --- a/packages/core/src/media/metrics/metrics.ts +++ b/packages/core/src/media/metrics/metrics.ts @@ -3,7 +3,7 @@ import { MediaControllerStore } from '../controller'; import { MimeType } from '../mime'; type RawMetrics = { - preloadTime?: number; + preloadTime: number; ttff: number; firstPlayback: number; @@ -178,8 +178,8 @@ function isInIframe() { } export class MetricsStatus { - requestedPlayTime?: number; - firstFrameTime?: number; + requestedPlayTime = 0; + firstFrameTime = 0; retryCount = 0; connected = false; @@ -233,6 +233,7 @@ export class MetricsStatus { sourceUrl: currentState?.src?.src ?? null, playerHeight: null, playerWidth: null, + preloadTime: 0, timeStalled: 0, timeUnpaused: 0, timeWaiting: 0, @@ -247,7 +248,7 @@ export class MetricsStatus { }; this.destroy = store.subscribe((state, prevState) => { - if (this.requestedPlayTime === undefined && state._playLastTime !== 0) { + if (this.requestedPlayTime === 0 && state._playLastTime !== 0) { this.requestedPlayTime = Math.max(state._playLastTime - bootMs, 0); } @@ -334,11 +335,10 @@ export class MetricsStatus { offset: this.store.getState().playbackOffsetMs ?? 0, // this is the amount of time that a video has had to preload content, from boot until play was requested - preloadTime: this.requestedPlayTime, + preloadTime: Math.max(this.requestedPlayTime, 0), // time from when the first `play` event is emitted and the first progress update ttff: - this.firstFrameTime !== undefined && - this.requestedPlayTime !== undefined + this.firstFrameTime > 0 && this.requestedPlayTime > 0 ? Math.max(this.firstFrameTime - this.requestedPlayTime, 0) : 0, }; @@ -542,7 +542,7 @@ export function addMediaMetricsToStore( if ( state.progress !== prevState.progress && - metricsStatus.getFirstFrameTime() === undefined + metricsStatus.getFirstFrameTime() === 0 ) { metricsStatus.setFirstFrameTime(); } From 9dc282dab2f33bc2b9bba6b331f370897f973aa0 Mon Sep 17 00:00:00 2001 From: Chase Adams Date: Tue, 14 Nov 2023 15:24:32 -0700 Subject: [PATCH 3/5] fix: fix metrics to only send valid values --- packages/core/src/media/metrics/metrics.ts | 69 ++++++++++++---------- packages/core/src/version.ts | 6 +- 2 files changed, 42 insertions(+), 33 deletions(-) diff --git a/packages/core/src/media/metrics/metrics.ts b/packages/core/src/media/metrics/metrics.ts index b915e816..931ec694 100644 --- a/packages/core/src/media/metrics/metrics.ts +++ b/packages/core/src/media/metrics/metrics.ts @@ -3,19 +3,19 @@ import { MediaControllerStore } from '../controller'; import { MimeType } from '../mime'; type RawMetrics = { - preloadTime: number; - ttff: number; - firstPlayback: number; + preloadTime: number | null; + ttff: number | null; + firstPlayback: number | null; - nWaiting: number; + nWaiting: number | null; timeWaiting: number; - nStalled: number; + nStalled: number | null; timeStalled: number; timeUnpaused: number; - nError: number; + nError: number | null; lastError?: string; videoHeight: number | null; @@ -29,7 +29,7 @@ type RawMetrics = { sourceType: MimeType | 'unknown'; - offset: number; + offset: number | null; pageUrl: string; sourceUrl: string | null; @@ -178,8 +178,8 @@ function isInIframe() { } export class MetricsStatus { - requestedPlayTime = 0; - firstFrameTime = 0; + requestedPlayTime: number | null = null; + firstFrameTime: number | null = null; retryCount = 0; connected = false; @@ -221,11 +221,11 @@ export class MetricsStatus { ? 'preload-metadata' : 'standard', duration: null, - firstPlayback: 0, - nError: 0, - nStalled: 0, - nWaiting: 0, - offset: 0, + firstPlayback: null, + nError: null, + nStalled: null, + nWaiting: null, + offset: null, pageUrl, playbackScore: null, player: `${playerPrefix}-${version}`, @@ -233,11 +233,11 @@ export class MetricsStatus { sourceUrl: currentState?.src?.src ?? null, playerHeight: null, playerWidth: null, - preloadTime: 0, + preloadTime: null, timeStalled: 0, timeUnpaused: 0, timeWaiting: 0, - ttff: 0, + ttff: null, uid: currentState.viewerId, userAgent: String(currentState?.device?.userAgent ?? '').replace( /\\|"/gm, @@ -248,7 +248,7 @@ export class MetricsStatus { }; this.destroy = store.subscribe((state, prevState) => { - if (this.requestedPlayTime === 0 && state._playLastTime !== 0) { + if (this.requestedPlayTime === null && state._playLastTime !== 0) { this.requestedPlayTime = Math.max(state._playLastTime - bootMs, 0); } @@ -293,7 +293,7 @@ export class MetricsStatus { } addError(error: string) { - this.currentMetrics.nError++; + this.currentMetrics.nError = (this.currentMetrics.nError ?? 0) + 1; this.currentMetrics.lastError = error; } @@ -319,11 +319,11 @@ export class MetricsStatus { getMetrics() { const currentMetrics: RawMetrics = { ...this.currentMetrics, - playerHeight: this.store.getState().size?.container?.height ?? null, - playerWidth: this.store.getState().size?.container?.width ?? null, - videoWidth: this.store.getState().size?.media?.width ?? null, - videoHeight: this.store.getState().size?.media?.height ?? null, - duration: this.store.getState().duration, + playerHeight: this.store.getState().size?.container?.height || null, + playerWidth: this.store.getState().size?.container?.width || null, + videoWidth: this.store.getState().size?.media?.width || null, + videoHeight: this.store.getState().size?.media?.height || null, + duration: this.store.getState().duration || null, nWaiting: this.timeWaiting.getCountStarts(), nStalled: this.timeStalled.getCountStarts(), @@ -332,15 +332,18 @@ export class MetricsStatus { timeStalled: this.timeStalled.getTotalTime(), timeUnpaused: this.timeUnpaused.getTotalTime(), - offset: this.store.getState().playbackOffsetMs ?? 0, + offset: this.store.getState().playbackOffsetMs || null, // this is the amount of time that a video has had to preload content, from boot until play was requested - preloadTime: Math.max(this.requestedPlayTime, 0), + preloadTime: this.requestedPlayTime, // time from when the first `play` event is emitted and the first progress update ttff: - this.firstFrameTime > 0 && this.requestedPlayTime > 0 + this.firstFrameTime && + this.requestedPlayTime && + this.firstFrameTime > 0 && + this.requestedPlayTime > 0 ? Math.max(this.firstFrameTime - this.requestedPlayTime, 0) - : 0, + : null, }; const previousMetrics = this.previousMetrics; @@ -426,7 +429,13 @@ export function addMediaMetricsToStore( let key: keyof RawMetrics; for (key in metrics.current) { const val = metrics.current[key]; - if (val !== metrics?.previous?.[key]) { + + const shouldSendValue = + typeof val === 'number' + ? Number.isFinite(val) && !Number.isNaN(val) && val >= 0 + : Boolean(val); + + if (shouldSendValue && val !== metrics?.previous?.[key]) { (d[key] as typeof val) = val; } } @@ -535,14 +544,14 @@ export function addMediaMetricsToStore( const destroyTtffListener = store.subscribe((state, prevState) => { if ( state.playing !== prevState.playing && - metricsStatus.getFirstPlayback() === 0 + metricsStatus.getFirstPlayback() === null ) { metricsStatus.setFirstPlayback(); } if ( state.progress !== prevState.progress && - metricsStatus.getFirstFrameTime() === 0 + metricsStatus.getFirstFrameTime() === null ) { metricsStatus.setFirstFrameTime(); } diff --git a/packages/core/src/version.ts b/packages/core/src/version.ts index cf278f1e..08efea02 100644 --- a/packages/core/src/version.ts +++ b/packages/core/src/version.ts @@ -1,6 +1,6 @@ -const core = `@livepeer/core@2.0.1`; -const react = `@livepeer/react@3.0.1`; -const reactNative = `@livepeer/react-native@2.0.1`; +const core = `@livepeer/core@2.0.3`; +const react = `@livepeer/react@3.0.3`; +const reactNative = `@livepeer/react-native@2.0.3`; export const version = { core, From 5d2f290f02b39fc0ee32ee857f4b650f9680aeb8 Mon Sep 17 00:00:00 2001 From: Chase Adams Date: Tue, 14 Nov 2023 15:27:53 -0700 Subject: [PATCH 4/5] chore: changeset --- .changeset/chatty-moons-exercise.md | 9 +++++++++ 1 file changed, 9 insertions(+) create mode 100644 .changeset/chatty-moons-exercise.md diff --git a/.changeset/chatty-moons-exercise.md b/.changeset/chatty-moons-exercise.md new file mode 100644 index 00000000..09233105 --- /dev/null +++ b/.changeset/chatty-moons-exercise.md @@ -0,0 +1,9 @@ +--- +'@livepeer/core': patch +'@livepeer/core-react': patch +'@livepeer/core-web': patch +'@livepeer/react': patch +'@livepeer/react-native': patch +--- + +**Fix:** fixed metrics to only send values when they are defined, to avoid filtering on the backend. From da5c76d5ef4abae6a4222da96200f44af1846a17 Mon Sep 17 00:00:00 2001 From: Chase Adams Date: Tue, 14 Nov 2023 15:32:17 -0700 Subject: [PATCH 5/5] test: fix tests --- .../src/media/browser/metrics.test.ts | 220 +++++++++--------- 1 file changed, 110 insertions(+), 110 deletions(-) diff --git a/packages/core-web/src/media/browser/metrics.test.ts b/packages/core-web/src/media/browser/metrics.test.ts index 4b577a00..cc8afd72 100644 --- a/packages/core-web/src/media/browser/metrics.test.ts +++ b/packages/core-web/src/media/browser/metrics.test.ts @@ -25,32 +25,32 @@ describe('addMediaMetrics', () => { } expect(metricsSnapshot?.current).toMatchInlineSnapshot(` - { - "autoplay": "standard", - "duration": 0, - "firstPlayback": 0, - "nError": 0, - "nStalled": 0, - "nWaiting": 0, - "offset": 0, - "pageUrl": "http://localhost:3000/", - "playbackScore": null, - "player": "hls-1", - "playerHeight": null, - "playerWidth": null, - "preloadTime": 0, - "sourceType": "unknown", - "sourceUrl": null, - "timeStalled": 0, - "timeUnpaused": 0, - "timeWaiting": 0, - "ttff": 0, - "uid": "", - "userAgent": "UA", - "videoHeight": null, - "videoWidth": null, - } - `); + { + "autoplay": "standard", + "duration": null, + "firstPlayback": null, + "nError": null, + "nStalled": 0, + "nWaiting": 0, + "offset": null, + "pageUrl": "http://localhost:3000/", + "playbackScore": null, + "player": "hls-1", + "playerHeight": null, + "playerWidth": null, + "preloadTime": null, + "sourceType": "unknown", + "sourceUrl": null, + "timeStalled": 0, + "timeUnpaused": 0, + "timeWaiting": 0, + "ttff": null, + "uid": "", + "userAgent": "UA", + "videoHeight": null, + "videoWidth": null, + } + `); }); it('should update time unpaused and first playback', async () => { @@ -72,24 +72,24 @@ describe('addMediaMetrics', () => { expect(metricsSnapshot?.current).toMatchInlineSnapshot(` { "autoplay": "standard", - "duration": 0, - "firstPlayback": 0, - "nError": 0, + "duration": null, + "firstPlayback": null, + "nError": null, "nStalled": 0, "nWaiting": 0, - "offset": 0, + "offset": null, "pageUrl": "http://localhost:3000/", "playbackScore": null, "player": "hls-1", "playerHeight": null, "playerWidth": null, - "preloadTime": 0, + "preloadTime": null, "sourceType": "unknown", "sourceUrl": null, "timeStalled": 0, "timeUnpaused": 0, "timeWaiting": 0, - "ttff": 0, + "ttff": null, "uid": "", "userAgent": "UA", "videoHeight": null, @@ -115,32 +115,32 @@ describe('addMediaMetrics', () => { } expect(metricsSnapshot?.current).toMatchInlineSnapshot(` - { - "autoplay": "standard", - "duration": 0, - "firstPlayback": 0, - "nError": 0, - "nStalled": 0, - "nWaiting": 1, - "offset": 0, - "pageUrl": "http://localhost:3000/", - "playbackScore": null, - "player": "hls-1", - "playerHeight": null, - "playerWidth": null, - "preloadTime": 0, - "sourceType": "unknown", - "sourceUrl": null, - "timeStalled": 0, - "timeUnpaused": 0, - "timeWaiting": 1000, - "ttff": 0, - "uid": "", - "userAgent": "UA", - "videoHeight": null, - "videoWidth": null, - } - `); + { + "autoplay": "standard", + "duration": null, + "firstPlayback": null, + "nError": null, + "nStalled": 0, + "nWaiting": 1, + "offset": null, + "pageUrl": "http://localhost:3000/", + "playbackScore": null, + "player": "hls-1", + "playerHeight": null, + "playerWidth": null, + "preloadTime": null, + "sourceType": "unknown", + "sourceUrl": null, + "timeStalled": 0, + "timeUnpaused": 0, + "timeWaiting": 1000, + "ttff": null, + "uid": "", + "userAgent": "UA", + "videoHeight": null, + "videoWidth": null, + } + `); }); it('should update time stalled and stalled count', async () => { @@ -162,32 +162,32 @@ describe('addMediaMetrics', () => { } expect(metricsSnapshot?.current).toMatchInlineSnapshot(` - { - "autoplay": "standard", - "duration": 0, - "firstPlayback": 0, - "nError": 0, - "nStalled": 1, - "nWaiting": 0, - "offset": 0, - "pageUrl": "http://localhost:3000/", - "playbackScore": null, - "player": "hls-1", - "playerHeight": null, - "playerWidth": null, - "preloadTime": 0, - "sourceType": "unknown", - "sourceUrl": null, - "timeStalled": 1000, - "timeUnpaused": 0, - "timeWaiting": 0, - "ttff": 0, - "uid": "", - "userAgent": "UA", - "videoHeight": null, - "videoWidth": null, - } - `); + { + "autoplay": "standard", + "duration": null, + "firstPlayback": null, + "nError": null, + "nStalled": 1, + "nWaiting": 0, + "offset": null, + "pageUrl": "http://localhost:3000/", + "playbackScore": null, + "player": "hls-1", + "playerHeight": null, + "playerWidth": null, + "preloadTime": null, + "sourceType": "unknown", + "sourceUrl": null, + "timeStalled": 1000, + "timeUnpaused": 0, + "timeWaiting": 0, + "ttff": null, + "uid": "", + "userAgent": "UA", + "videoHeight": null, + "videoWidth": null, + } + `); }); }); @@ -209,31 +209,31 @@ describe('addMediaMetrics', () => { } expect(metricsSnapshot?.current).toMatchInlineSnapshot(` - { - "autoplay": "standard", - "duration": 0, - "firstPlayback": 0, - "nError": 0, - "nStalled": 0, - "nWaiting": 0, - "offset": 0, - "pageUrl": "http://localhost:3000/", - "playbackScore": null, - "player": "hls-1", - "playerHeight": null, - "playerWidth": null, - "preloadTime": 0, - "sourceType": "unknown", - "sourceUrl": null, - "timeStalled": 0, - "timeUnpaused": 0, - "timeWaiting": 0, - "ttff": 0, - "uid": "", - "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.101 Safari/537.36", - "videoHeight": null, - "videoWidth": null, - } - `); + { + "autoplay": "standard", + "duration": null, + "firstPlayback": null, + "nError": null, + "nStalled": 0, + "nWaiting": 0, + "offset": null, + "pageUrl": "http://localhost:3000/", + "playbackScore": null, + "player": "hls-1", + "playerHeight": null, + "playerWidth": null, + "preloadTime": null, + "sourceType": "unknown", + "sourceUrl": null, + "timeStalled": 0, + "timeUnpaused": 0, + "timeWaiting": 0, + "ttff": null, + "uid": "", + "userAgent": "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.101 Safari/537.36", + "videoHeight": null, + "videoWidth": null, + } + `); }); });