Signals: add support for allow/disallow list, fix network signals bug (#1147)

Author: silesky

.changeset/strong-rats-lay.md

@@ -0,0 +1,5 @@
+---
+'@segment/analytics-signals': minor
+---
+
+Update network signals to add support for allow/disallow

packages/signals/signals-integration-tests/src/helpers/base-page-object.ts

@@ -1,7 +1,10 @@
 import { CDNSettingsBuilder } from '@internal/test-helpers'
-import { Page, Request } from '@playwright/test'
+import { Page, Request, Route } from '@playwright/test'
 import { logConsole } from './log-console'
 import { SegmentEvent } from '@segment/analytics-next'
+import { Signal, SignalsPluginSettingsConfig } from '@segment/analytics-signals'
+
+type FulfillOptions = Parameters<Route['fulfill']>['0']
 
 export class BasePage {
   protected page!: Page
@@ -18,30 +21,67 @@ export class BasePage {
     this.url = 'http://localhost:5432/src/tests' + path
   }
 
+  /**
+   * Load and setup routes
+   * and wait for analytics and signals to be initialized
+   */
+  async loadAndWait(...args: Parameters<BasePage['load']>) {
+    await this.load(...args)
+    await this.waitForSignalsAssets()
+    return this
+  }
+
   /**
    * load and setup routes
-   * @param page
-   * @param edgeFn - edge function to be loaded
    */
-  async load(page: Page, edgeFn: string) {
+  async load(
+    page: Page,
+    edgeFn: string,
+    signalSettings: Partial<SignalsPluginSettingsConfig> = {}
+  ) {
     logConsole(page)
     this.page = page
     this.edgeFn = edgeFn
     await this.setupMockedRoutes()
     await this.page.goto(this.url)
+    await this.invokeAnalyticsLoad(signalSettings)
   }
 
   /**
-   *  Wait for analytics and signals to be initialized
-   * Signals can be captured before this, so it's useful to have this method
+   * Wait for analytics and signals to be initialized
    */
-  async waitForAnalyticsInit() {
+  async waitForSignalsAssets() {
+    // this is kind of an approximation of full initialization
     return Promise.all([
       this.waitForCDNSettingsResponse(),
       this.waitForEdgeFunctionResponse(),
     ])
   }
 
+  /**
+   * Invoke the analytics load sequence, but do not wait for analytics to full initialize
+   * Full initialization means that the CDN settings and edge function have been loaded
+   */
+  private async invokeAnalyticsLoad(
+    signalSettings: Partial<SignalsPluginSettingsConfig> = {}
+  ) {
+    await this.page.evaluate(
+      ({ signalSettings }) => {
+        window.signalsPlugin = new window.SignalsPlugin({
+          disableSignalsRedaction: true,
+          flushInterval: 500,
+          ...signalSettings,
+        })
+        window.analytics.load({
+          writeKey: '<SOME_WRITE_KEY>',
+          plugins: [window.signalsPlugin],
+        })
+      },
+      { signalSettings }
+    )
+    return this
+  }
+
   private async setupMockedRoutes() {
     // clear any existing saved requests
     this.signalsApiReqs = []
@@ -97,6 +137,92 @@ export class BasePage {
     )
   }
 
+  async waitForSignalsEmit(
+    filter: (signal: Signal) => boolean,
+    {
+      expectedSignalCount,
+      maxTimeoutMs = 10000,
+      failOnEmit = false,
+    }: {
+      expectedSignalCount?: number
+      maxTimeoutMs?: number
+      failOnEmit?: boolean
+    } = {}
+  ) {
+    return this.page.evaluate(
+      ([filter, expectedSignalCount, maxTimeoutMs, failOnEmit]) => {
+        return new Promise((resolve, reject) => {
+          let signalCount = 0
+          const to = setTimeout(() => {
+            if (failOnEmit) {
+              resolve('No signal emitted')
+            } else {
+              reject('Timed out waiting for signals')
+            }
+          }, maxTimeoutMs)
+          window.signalsPlugin.onSignal((signal) => {
+            signalCount++
+            if (
+              eval(filter)(signal) &&
+              signalCount === (expectedSignalCount ?? 1)
+            ) {
+              if (failOnEmit) {
+                reject(
+                  `Signal should not have been emitted: ${JSON.stringify(
+                    signal,
+                    null,
+                    2
+                  )}`
+                )
+              } else {
+                resolve(signal)
+              }
+              clearTimeout(to)
+            }
+          })
+        })
+      },
+      [
+        filter.toString(),
+        expectedSignalCount,
+        maxTimeoutMs,
+        failOnEmit,
+      ] as const
+    )
+  }
+
+  async mockTestRoute(url?: string, response?: Partial<FulfillOptions>) {
+    await this.page.route(url || 'http://localhost:5432/api/foo', (route) => {
+      return route.fulfill({
+        contentType: 'application/json',
+        status: 200,
+        body: JSON.stringify({ someResponse: 'yep' }),
+        ...response,
+      })
+    })
+  }
+
+  async makeFetchCall(
+    url?: string,
+    request?: Partial<RequestInit>
+  ): Promise<void> {
+    return this.page.evaluate(
+      ({ url, request }) => {
+        return fetch(url || 'http://localhost:5432/api/foo', {
+          method: 'POST',
+          headers: {
+            'Content-Type': 'application/json',
+          },
+          body: JSON.stringify({ foo: 'bar' }),
+          ...request,
+        })
+          .then(console.log)
+          .catch(console.error)
+      },
+      { url, request }
+    )
+  }
+
   waitForSignalsApiFlush(timeout = 5000) {
     return this.page.waitForResponse('https://signals.segment.io/v1/*', {
       timeout,

packages/signals/signals-integration-tests/src/tests/signals-vanilla/all-segment-events.test.ts

@@ -76,7 +76,7 @@ test('Should dispatch events from signals that occurred before analytics was ins
   // add a user defined signal before analytics is instantiated
   void indexPage.addUserDefinedSignal()
 
-  await indexPage.waitForAnalyticsInit()
+  await indexPage.waitForSignalsAssets()
 
   await Promise.all([
     indexPage.waitForSignalsApiFlush(),

packages/signals/signals-integration-tests/src/tests/signals-vanilla/basic.test.ts

@@ -14,16 +14,15 @@ const basicEdgeFn = `
   }`
 
 test.beforeEach(async ({ page }) => {
-  await indexPage.load(page, basicEdgeFn)
-  await indexPage.waitForAnalyticsInit()
+  await indexPage.loadAndWait(page, basicEdgeFn)
 })
 
 test('network signals', async () => {
   /**
    * Make a fetch call, see if it gets sent to the signals endpoint
    */
-  await indexPage.mockRandomJSONApi()
-  await indexPage.makeFetchCallToRandomJSONApi()
+  await indexPage.mockTestRoute()
+  await indexPage.makeFetchCall()
   await indexPage.waitForSignalsApiFlush()
   const batch = indexPage.lastSignalsApiReq.postDataJSON()
     .batch as SegmentEvent[]

packages/signals/signals-integration-tests/src/tests/signals-vanilla/index-page.ts

@@ -33,32 +33,6 @@ export class IndexPage extends BasePage {
     })
   }
 
-  async mockRandomJSONApi() {
-    await this.page.route('http://localhost:5432/api/foo', (route) => {
-      return route.fulfill({
-        contentType: 'application/json',
-        status: 200,
-        body: JSON.stringify({
-          someResponse: 'yep',
-        }),
-      })
-    })
-  }
-
-  async makeFetchCallToRandomJSONApi(): Promise<void> {
-    return this.page.evaluate(() => {
-      return fetch('http://localhost:5432/api/foo', {
-        method: 'POST',
-        headers: {
-          'Content-Type': 'application/json',
-        },
-        body: JSON.stringify({ foo: 'bar' }),
-      })
-        .then(console.log)
-        .catch(console.error)
-    })
-  }
-
   async clickButton() {
     return this.page.click('#some-button')
   }

packages/signals/signals-integration-tests/src/tests/signals-vanilla/network-signals-filter.test.ts

@@ -0,0 +1,60 @@
+import { test, expect } from '@playwright/test'
+import { IndexPage } from './index-page'
+import type { SegmentEvent } from '@segment/analytics-next'
+
+const indexPage = new IndexPage()
+
+const basicEdgeFn = `
+    // this is a process signal function
+    const processSignal = (signal) => {
+      if (signal.type === 'interaction') {
+        const eventName = signal.data.eventType + ' ' + '[' + signal.type + ']'
+        analytics.track(eventName, signal.data)
+      }
+  }`
+
+test('network signals allow and disallow list', async ({ page }) => {
+  await indexPage.loadAndWait(page, basicEdgeFn, {
+    networkSignalsAllowList: ['allowed-api.com'],
+    networkSignalsDisallowList: ['https://disallowed-api.com/api/foo'],
+  })
+
+  // test that the allowed signals were emitted + sent
+  const ALLOWED_URL = 'https://allowed-api.com/api/bar'
+  const emittedNetworkSignalsAllowed = indexPage.waitForSignalsEmit(
+    (el) => el.type === 'network'
+  )
+  await indexPage.mockTestRoute(ALLOWED_URL)
+  await indexPage.makeFetchCall(ALLOWED_URL)
+  await emittedNetworkSignalsAllowed
+
+  await indexPage.waitForSignalsApiFlush()
+  const batch = indexPage.lastSignalsApiReq.postDataJSON()
+    .batch as SegmentEvent[]
+  const networkEvents = batch.filter(
+    (el: SegmentEvent) => el.properties!.type === 'network'
+  )
+  const allowedRequestsAndResponses = networkEvents.filter(
+    (el) => el.properties!.data.url === ALLOWED_URL
+  )
+  expect(allowedRequestsAndResponses).toHaveLength(2)
+  const [request, response] = allowedRequestsAndResponses
+  expect(request.properties!.data.data).toEqual({
+    foo: 'bar',
+  })
+  expect(response.properties!.data.data).toEqual({
+    someResponse: 'yep',
+  })
+
+  // test the disallowed signals were not emitted (using the emitter to test this)
+  const DISALLOWED_URL = 'https://disallowed-api.com/api/foo'
+  const emittedNetworkSignalsDisallowed = indexPage.waitForSignalsEmit(
+    (el) => el.type === 'network',
+    {
+      failOnEmit: true,
+    }
+  )
+  await indexPage.mockTestRoute(DISALLOWED_URL)
+  await indexPage.makeFetchCall(DISALLOWED_URL)
+  await emittedNetworkSignalsDisallowed
+})

packages/signals/signals-integration-tests/src/tests/signals-vanilla/signals-bundle.ts

@@ -1,16 +1,16 @@
 import { AnalyticsBrowser } from '@segment/analytics-next'
 import { SignalsPlugin } from '@segment/analytics-signals'
 
-const analytics = new AnalyticsBrowser()
-;(window as any).analytics = analytics
+declare global {
+  interface Window {
+    analytics: AnalyticsBrowser
+    SignalsPlugin: typeof SignalsPlugin
+    signalsPlugin: SignalsPlugin
+  }
+}
 
-const signalsPlugin = new SignalsPlugin({
-  disableSignalsRedaction: true,
-})
-
-;(window as any).signalsPlugin = signalsPlugin
-
-analytics.load({
-  writeKey: '<SOME_WRITE_KEY>',
-  plugins: [signalsPlugin],
-})
+/**
+ * Not instantiating the analytics object here, as it will be instantiated in the test
+ */
+;(window as any).SignalsPlugin = SignalsPlugin
+;(window as any).analytics = new AnalyticsBrowser()

packages/signals/signals/package.json

@@ -59,6 +59,7 @@
   "devDependencies": {
     "@internal/config-webpack": "workspace:^",
     "@internal/test-helpers": "workspace:^",
-    "fake-indexeddb": "^6.0.0"
+    "fake-indexeddb": "^6.0.0",
+    "node-fetch": "^2.6.7"
   }
 }

packages/signals/signals/src/core/analytics-service/__tests__/analytics-service.test.ts

@@ -17,7 +17,7 @@ describe(AnalyticsService, () => {
   })
 
   it('should return the correct write key', () => {
-    expect(service.writeKey).toBe('foo')
+    expect(service.instance.settings.writeKey).toBe('foo')
   })
 
   describe('createSegmentInstrumentationEventGenerator', () => {

packages/signals/signals/src/core/analytics-service/index.ts

@@ -8,12 +8,10 @@ type EdgeFunctionSettings = { downloadURL: string; version?: number }
  * Helper / facade that wraps the analytics, and abstracts away the details of the analytics instance.
  */
 export class AnalyticsService {
-  writeKey: string
   instance: AnyAnalytics
   edgeFnSettings?: EdgeFunctionSettings
   constructor(analyticsInstance: AnyAnalytics) {
     this.instance = analyticsInstance
-    this.writeKey = analyticsInstance.settings.writeKey
     this.edgeFnSettings = this.parseEdgeFnSettings(
       analyticsInstance.settings.cdnSettings
     )