rework logic and rename functions to make the metrics sampling code easier to grok

Author: cemms1

dotcom-rendering/src/components/Metrics.importable.tsx

@@ -24,7 +24,7 @@ type Props = {
 
 const sampling = 1 / 100;
 /** defining this here allows to share this with other metrics */
-const willRecordCoreWebVitals = Math.random() < sampling;
+const isInSample = Math.random() < sampling;
 
 // For these tests switch off sampling and collect metrics for 100% of views
 const clientSideTestsToForceMetrics: ABTest[] = [
@@ -88,9 +88,8 @@ export const Metrics = ({ commercialMetricsEnabled, tests }: Props) => {
 
 	const userInServerSideTest = Object.keys(tests).length > 0;
 
-	const shouldBypassSampling = useCallback(
+	const bypassSampling = useCallback(
 		(api: ABTestAPI) =>
-			willRecordCoreWebVitals ||
 			userInServerSideTest ||
 			clientSideTestsToForceMetrics.some((test) =>
 				api.runnableTest(test),
@@ -105,7 +104,7 @@ export const Metrics = ({ commercialMetricsEnabled, tests }: Props) => {
 			if (isUndefined(isDev)) return;
 			if (isUndefined(pageViewId)) return;
 
-			const bypassSampling = shouldBypassSampling(abTestApi);
+			const recordMetrics = isInSample || bypassSampling(abTestApi);
 
 			/**
 			 * We rely on `bypassSampling` rather than the built-in sampling,
@@ -121,11 +120,11 @@ export const Metrics = ({ commercialMetricsEnabled, tests }: Props) => {
 				team: 'dotcom',
 			});
 
-			if (bypassSampling || isDev) {
+			if (recordMetrics || isDev) {
 				void bypassCoreWebVitalsSampling('commercial');
 			}
 		},
-		[abTestApi, browserId, isDev, pageViewId, shouldBypassSampling],
+		[abTestApi, browserId, isDev, pageViewId, bypassSampling],
 	);
 
 	useEffect(
@@ -139,7 +138,7 @@ export const Metrics = ({ commercialMetricsEnabled, tests }: Props) => {
 			if (isUndefined(isDev)) return;
 			if (isUndefined(pageViewId)) return;
 
-			const bypassSampling = shouldBypassSampling(abTestApi);
+			const recordMetrics = isInSample || bypassSampling(abTestApi);
 
 			// This is a new detection method we are trying, so we want to record it separately to `adBlockerInUse`
 			EventTimer.get().setProperty(
@@ -154,7 +153,7 @@ export const Metrics = ({ commercialMetricsEnabled, tests }: Props) => {
 				adBlockerInUse,
 			})
 				.then(() => {
-					if (bypassSampling || isDev) {
+					if (recordMetrics || isDev) {
 						void bypassCommercialMetricsSampling();
 					}
 				})
@@ -172,7 +171,7 @@ export const Metrics = ({ commercialMetricsEnabled, tests }: Props) => {
 			commercialMetricsEnabled,
 			isDev,
 			pageViewId,
-			shouldBypassSampling,
+			bypassSampling,
 		],
 	);