Fix sample rate implementation (#1188)

segmentio · Nov 19, 2024 · de6f86d · de6f86d
1 parent 46e8819
commit de6f86d
Show file tree

Hide file tree

Showing 4 changed files with 70 additions and 17 deletions.
diff --git a/.changeset/strong-buckets-confess.md b/.changeset/strong-buckets-confess.md
@@ -0,0 +1,5 @@
+---
+'@segment/analytics-signals': patch
+---
+
+Fix sampleRate check
diff --git a/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts b/packages/signals/signals-integration-tests/src/helpers/base-page-object.ts
@@ -41,13 +41,13 @@ export class BasePage {
     page: Page,
     edgeFn: string,
     signalSettings: Partial<SignalsPluginSettingsConfig> = {},
-    options: { updateURL?: (url: string) => string } = {}
+    options: { updateURL?: (url: string) => string; sampleRate?: number } = {}
   ) {
     logConsole(page)
     this.page = page
     this.network = new PageNetworkUtils(page)
     this.edgeFn = edgeFn
-    await this.setupMockedRoutes()
+    await this.setupMockedRoutes(options.sampleRate)
     const url = options.updateURL ? options.updateURL(this.url) : this.url
     await this.page.goto(url, { waitUntil: 'domcontentloaded' })
     void this.invokeAnalyticsLoad({
@@ -92,15 +92,15 @@ export class BasePage {
     return this
   }
 
-  private async setupMockedRoutes() {
+  private async setupMockedRoutes(sampleRate?: number) {
     // clear any existing saved requests
     this.trackingAPI.clear()
     this.signalsAPI.clear()
 
     await Promise.all([
       this.mockSignalsApi(),
       this.mockTrackingApi(),
-      this.mockCDNSettings(),
+      this.mockCDNSettings(sampleRate),
     ])
   }
 
@@ -203,7 +203,7 @@ export class BasePage {
     })
   }
 
-  async mockCDNSettings() {
+  async mockCDNSettings(sampleRate?: number) {
     await this.page.route(
       'https://cdn.segment.com/v1/projects/*/settings',
       (route, request) => {
@@ -218,6 +218,9 @@ export class BasePage {
               downloadURL: this.edgeFnDownloadURL,
               version: 1,
             },
+            autoInstrumentationSettings: {
+              sampleRate: sampleRate ?? 1,
+            },
           },
         }).build()
 

diff --git a/...ges/signals/signals-integration-tests/src/tests/signals-vanilla/signals-ingestion.test.ts b/...ges/signals/signals-integration-tests/src/tests/signals-vanilla/signals-ingestion.test.ts
@@ -5,20 +5,34 @@ const indexPage = new IndexPage()
 
 const basicEdgeFn = `const processSignal = (signal) => {}`
 
-test('ingestion not enabled -> will not send the signal', async ({ page }) => {
-  await indexPage.loadAndWait(page, basicEdgeFn, {
-    enableSignalsIngestion: false,
-    flushAt: 1,
-  })
+test('debug ingestion disabled and sample rate 0 -> will not send the signal', async ({
+  page,
+}) => {
+  await indexPage.loadAndWait(
+    page,
+    basicEdgeFn,
+    {
+      enableSignalsIngestion: false,
+      flushAt: 1,
+    },
+    { sampleRate: 0 }
+  )
   await indexPage.fillNameInput('John Doe')
   await page.waitForTimeout(100)
   expect(indexPage.signalsAPI.getEvents('interaction')).toHaveLength(0)
 })
 
-test('ingestion enabled -> will send the signal', async ({ page }) => {
-  await indexPage.loadAndWait(page, basicEdgeFn, {
-    enableSignalsIngestion: true,
-  })
+test('debug ingestion enabled and sample rate 0 -> will send the signal', async ({
+  page,
+}) => {
+  await indexPage.loadAndWait(
+    page,
+    basicEdgeFn,
+    {
+      enableSignalsIngestion: true,
+    },
+    { sampleRate: 0 }
+  )
 
   await Promise.all([
     indexPage.fillNameInput('John Doe'),
@@ -27,3 +41,21 @@ test('ingestion enabled -> will send the signal', async ({ page }) => {
 
   expect(true).toBe(true)
 })
+
+test('debug ingestion disabled and sample rate 1 -> will send the signal', async ({
+  page,
+}) => {
+  await indexPage.loadAndWait(
+    page,
+    basicEdgeFn,
+    {
+      flushAt: 1,
+      enableSignalsIngestion: false,
+    },
+    {
+      sampleRate: 1,
+    }
+  )
+  await indexPage.fillNameInput('John Doe')
+  expect(indexPage.signalsAPI.getEvents('interaction')).toHaveLength(1)
+})
diff --git a/packages/signals/signals/src/core/signals/settings.ts b/packages/signals/signals/src/core/signals/settings.ts
@@ -66,8 +66,8 @@ export class SignalGlobalSettings {
         if (this.signalsDebug.getEnableSignalsIngestion()) {
           return true
         }
-        if (!this.sampleSuccess) {
-          return false
+        if (this.sampleSuccess) {
+          return true
         }
         return false
       },
@@ -107,9 +107,22 @@ export class SignalGlobalSettings {
         Boolean(val)
       )
     )
+    this.sampleSuccess = this.checkSampleRate(sampleRate ?? 0)
+  }
+  private checkSampleRate(sampleRate: number): boolean {
+    const storage = new WebStorage(window.sessionStorage)
+    const previousSample = storage.getItem<boolean>('segment_sample_success')
+    if (previousSample !== undefined) {
+      return previousSample
+    }
+
     if (sampleRate && Math.random() <= sampleRate) {
-      this.sampleSuccess = true
+      storage.setItem('segment_sample_success', true)
+      return true
     }
+
+    storage.setItem('segment_sample_success', false)
+    return false
   }
 }