review changes

Author: vaind

packages/replay/metrics/README.md

@@ -2,9 +2,9 @@
 
 Evaluates Replay impact on website performance by running a web app in Chromium via Playwright and collecting various metrics.
 
-The general idea is to run a web app without Sentry Replay and then run the same app again with Replay included.
-For both scenarios, we collect some metrics (CPU, memory, vitals) and later compare them and post as a comment in a PR.
-Changes in the collected, compared to previous runs from the main branch, should be evaluated on case-by-case basis when preparing and reviewing the PR.
+The general idea is to run a web app without Sentry Replay and then run the same app again with Sentry and another one with Sentry+Replay included.
+For the three scenarios, we collect some metrics (CPU, memory, vitals) and later compare them and post as a comment in a PR.
+Changes in the metrics, compared to previous runs from the main branch, should be evaluated on case-by-case basis when preparing and reviewing the PR.
 
 ## Resources
 

packages/replay/metrics/src/results/metrics-stats.ts

@@ -2,7 +2,7 @@ import * as ss from 'simple-statistics'
 
 import { Metrics } from '../collector';
 
-export type NumberProvider = (metrics: Metrics) => number;
+export type NumberProvider = (metrics: Metrics) => number | undefined;
 export type AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => number | undefined;
 
 export class MetricsStats {
@@ -13,20 +13,24 @@ export class MetricsStats {
   static memoryMax: NumberProvider = metrics => ss.max(Array.from(metrics.memory.snapshots.values()));
 
   static mean: AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => {
-    const numbers = MetricsStats._filteredValues(items.map(dataProvider));
+    const numbers = MetricsStats._filteredValues(MetricsStats._collect(items, dataProvider));
     return numbers.length > 0 ? ss.mean(numbers) : undefined;
   }
 
   static max: AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => {
-    const numbers = MetricsStats._filteredValues(items.map(dataProvider));
+    const numbers = MetricsStats._filteredValues(MetricsStats._collect(items, dataProvider));
     return numbers.length > 0 ? ss.max(numbers) : undefined;
   }
 
   static stddev: AnalyticsFunction = (items: Metrics[], dataProvider: NumberProvider) => {
-    const numbers = MetricsStats._filteredValues(items.map(dataProvider));
+    const numbers = MetricsStats._filteredValues(MetricsStats._collect(items, dataProvider));
     return numbers.length > 0 ? ss.standardDeviation(numbers) : undefined;
   }
 
+  private static _collect(items: Metrics[], dataProvider: NumberProvider): number[] {
+    return items.map(dataProvider).filter(v => v != undefined && !Number.isNaN(v)) as number[];
+  }
+
   // See https://en.wikipedia.org/wiki/Interquartile_range#Outliers for details on filtering.
   private static _filteredValues(numbers: number[]): number[] {
     numbers.sort((a, b) => a - b)

packages/replay/metrics/src/vitals/cls.ts

@@ -34,7 +34,7 @@ class CLS {
     }`);
   }
 
-  public async collect(): Promise<number> {
+  public async collect(): Promise<number | undefined> {
     const result = await this._page.evaluate('window.cumulativeLayoutShiftScore');
     return result as number;
   }

packages/replay/metrics/src/vitals/fid.ts

@@ -28,7 +28,7 @@ class FID {
     }`);
   }
 
-  public async collect(): Promise<number> {
+  public async collect(): Promise<number | undefined> {
     const result = await this._page.evaluate('window.firstInputDelay');
     return result as number;
   }

packages/replay/metrics/src/vitals/index.ts

@@ -7,7 +7,7 @@ import { LCP } from './lcp.js';
 export { WebVitals, WebVitalsCollector };
 
 class WebVitals {
-  constructor(public lcp: number, public cls: number, public fid: number) { }
+  constructor(public lcp: number | undefined, public cls: number | undefined, public fid: number | undefined) { }
 
   public static fromJSON(data: Partial<WebVitals>): WebVitals {
     return new WebVitals(data.lcp as number, data.cls as number, data.fid as number);

packages/replay/metrics/src/vitals/lcp.ts

@@ -28,7 +28,7 @@ class LCP {
     }`);
   }
 
-  public async collect(): Promise<number> {
+  public async collect(): Promise<number | undefined> {
     const result = await this._page.evaluate('window.largestContentfulPaint');
     return result as number;
   }