From 90714e146fe00976ae16d271a0739326d0e04351 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Sun, 29 Dec 2024 20:53:11 -0800 Subject: [PATCH 01/19] Init[WIP] --- packages/browser/src/index.ts | 1 + .../featureFlags/unleash/index.ts | 1 + .../featureFlags/unleash/integration.ts | 36 +++++++++++++++++++ .../featureFlags/unleash/types.ts | 11 ++++++ 4 files changed, 49 insertions(+) create mode 100644 packages/browser/src/integrations/featureFlags/unleash/index.ts create mode 100644 packages/browser/src/integrations/featureFlags/unleash/integration.ts create mode 100644 packages/browser/src/integrations/featureFlags/unleash/types.ts diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index e6f57c13fe6b..e6e089cbe04c 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -75,3 +75,4 @@ export { } from './integrations/featureFlags'; export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './integrations/featureFlags/launchdarkly'; export { openFeatureIntegration, OpenFeatureIntegrationHook } from './integrations/featureFlags/openfeature'; +export { unleashIntegration } from './integrations/featureFlags/unleash'; diff --git a/packages/browser/src/integrations/featureFlags/unleash/index.ts b/packages/browser/src/integrations/featureFlags/unleash/index.ts new file mode 100644 index 000000000000..934ff196ee95 --- /dev/null +++ b/packages/browser/src/integrations/featureFlags/unleash/index.ts @@ -0,0 +1 @@ +export { unleashIntegration } from './integration'; diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts new file mode 100644 index 000000000000..c4e3bec09f20 --- /dev/null +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -0,0 +1,36 @@ +import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; + +import { defineIntegration } from '@sentry/core'; +import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags'; +import type { UnleashClient } from './types'; + +/** + * Sentry integration for capturing feature flags from the Unleash SDK. + * + * See the [feature flag documentation](https://develop.sentry.dev/sdk/expected-features/#feature-flags) for more information. + * + * @example + * ``` + * import * as Sentry from '@sentry/browser'; + * TODO: + * + * Sentry.init({ + * dsn: '___PUBLIC_DSN___', + * integrations: [TODO:] + * }); + * ``` + */ +export const unleashIntegration = defineIntegration((openFeatureClient: UnleashClient ) => { + return { + name: 'Unleash', + + processEvent(event: Event, _hint: EventHint, _client: Client): Event { + return copyFlagsFromScopeToEvent(event); + }, + + setupOnce() { + openFeatureClient.isEnabled = Proxy(); + openFeatureClient.getVariant = Proxy(); + }, + }; +}) satisfies IntegrationFn; diff --git a/packages/browser/src/integrations/featureFlags/unleash/types.ts b/packages/browser/src/integrations/featureFlags/unleash/types.ts new file mode 100644 index 000000000000..520bfcbdc182 --- /dev/null +++ b/packages/browser/src/integrations/featureFlags/unleash/types.ts @@ -0,0 +1,11 @@ +export type Variant = { + variantName: string; + enabled: boolean; + payload?: object; // TODO: + // TODO: +} + +export interface UnleashClient { + isEnabled(featureName: string): boolean; + getVariant(featureName: string): Variant; +} From e714b9eeaeeca523710dea3a2f30b9ab33aa3bca Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 8 Jan 2025 16:19:53 -0800 Subject: [PATCH 02/19] Working patch and basic test for isEnabled --- .../featureFlags/unleash/basic/test.ts | 47 +++++++++++++++++++ .../integrations/featureFlags/unleash/init.js | 25 ++++++++++ .../featureFlags/unleash/subject.js | 3 ++ .../featureFlags/unleash/template.html | 9 ++++ .../featureFlags/unleash/integration.ts | 24 ++++++++-- .../featureFlags/unleash/types.ts | 17 ++++--- 6 files changed, 115 insertions(+), 10 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/template.html diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts new file mode 100644 index 000000000000..79361c137128 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts @@ -0,0 +1,47 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers'; + +const FLAG_BUFFER_SIZE = 100; // Corresponds to constant in featureFlags.ts, in browser utils. + +sentryTest('Basic test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { + if (shouldSkipFeatureFlagsTest()) { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + await page.goto(url); + + await page.evaluate(bufferSize => { + const client = new (window as any).UnleashClient(); + for (let i = 1; i <= bufferSize; i++) { + client.isEnabled(`feat${i}`); + } + client.isEnabled(`feat${bufferSize + 1}`); // eviction + client.isEnabled('feat3'); // update + }, FLAG_BUFFER_SIZE); + + const reqPromise = waitForErrorRequest(page); + await page.locator('#error').click(); + const req = await reqPromise; + const event = envelopeRequestParser(req); + + const expectedFlags = [{ flag: 'feat2', result: false }]; + for (let i = 4; i <= FLAG_BUFFER_SIZE; i++) { + expectedFlags.push({ flag: `feat${i}`, result: false }); + } + expectedFlags.push({ flag: `feat${FLAG_BUFFER_SIZE + 1}`, result: false }); + expectedFlags.push({ flag: 'feat3', result: false }); + + expect(event.contexts?.flags?.values).toEqual(expectedFlags); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js new file mode 100644 index 000000000000..f40861483bea --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -0,0 +1,25 @@ +import * as Sentry from '@sentry/browser'; + +window.UnleashClient = class { + isEnabled(_toggleName) { + return false; + } + + getVariant(_toggleName) { + return { + name: 'disabled', + enabled: false, + }; + } +}; + +window.Sentry = Sentry; +window.sentryUnleashIntegration = Sentry.unleashIntegration(window.UnleashClient); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1.0, + integrations: [window.sentryUnleashIntegration], +}); + + diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/subject.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/subject.js new file mode 100644 index 000000000000..e6697408128c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/subject.js @@ -0,0 +1,3 @@ +document.getElementById('error').addEventListener('click', () => { + throw new Error('Button triggered error'); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/template.html b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/template.html new file mode 100644 index 000000000000..9330c6c679f4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index c4e3bec09f20..470f70cb9924 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -2,7 +2,7 @@ import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags'; -import type { UnleashClient } from './types'; +import type { UnleashClient, UnleashClientClass } from './types'; /** * Sentry integration for capturing feature flags from the Unleash SDK. @@ -20,7 +20,7 @@ import type { UnleashClient } from './types'; * }); * ``` */ -export const unleashIntegration = defineIntegration((openFeatureClient: UnleashClient ) => { +export const unleashIntegration = defineIntegration((unleashClientClass: UnleashClientClass) => { return { name: 'Unleash', @@ -29,8 +29,24 @@ export const unleashIntegration = defineIntegration((openFeatureClient: UnleashC }, setupOnce() { - openFeatureClient.isEnabled = Proxy(); - openFeatureClient.getVariant = Proxy(); + const sentryIsEnabled = { + apply: ( + target: (this: UnleashClient, toggleName: string) => boolean, + thisArg: UnleashClient, + args: [toggleName: string] + ) => { + const result = Reflect.apply(target, thisArg, args); + insertFlagToScope(args[0], result); + return result; + } + }; + const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; + const originalIsEnabled = unleashClientPrototype.isEnabled.bind(unleashClientPrototype); + unleashClientPrototype.isEnabled = new Proxy( + originalIsEnabled, + sentryIsEnabled + ); }, }; }) satisfies IntegrationFn; + diff --git a/packages/browser/src/integrations/featureFlags/unleash/types.ts b/packages/browser/src/integrations/featureFlags/unleash/types.ts index 520bfcbdc182..8ba56246e7ac 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/types.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/types.ts @@ -1,11 +1,16 @@ -export type Variant = { - variantName: string; +export interface IVariant { + name: string; enabled: boolean; - payload?: object; // TODO: - // TODO: + feature_enabled?: boolean; + payload?: { + type: string; + value: string; + }; } export interface UnleashClient { - isEnabled(featureName: string): boolean; - getVariant(featureName: string): Variant; + isEnabled(this: UnleashClient, featureName: string): boolean; + getVariant(this: UnleashClient, featureName: string): IVariant; } + +export type UnleashClientClass = new (...args: unknown[]) => UnleashClient; From e02b91b2b8b24184dac7ea8536cf262049803067 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 8 Jan 2025 16:22:09 -0800 Subject: [PATCH 03/19] Biome check --- .../integrations/featureFlags/unleash/integration.ts | 10 +++------- .../src/integrations/featureFlags/unleash/types.ts | 4 ++-- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 470f70cb9924..6362d95e39cc 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -33,20 +33,16 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash apply: ( target: (this: UnleashClient, toggleName: string) => boolean, thisArg: UnleashClient, - args: [toggleName: string] + args: [toggleName: string], ) => { const result = Reflect.apply(target, thisArg, args); insertFlagToScope(args[0], result); return result; - } + }, }; const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; const originalIsEnabled = unleashClientPrototype.isEnabled.bind(unleashClientPrototype); - unleashClientPrototype.isEnabled = new Proxy( - originalIsEnabled, - sentryIsEnabled - ); + unleashClientPrototype.isEnabled = new Proxy(originalIsEnabled, sentryIsEnabled); }, }; }) satisfies IntegrationFn; - diff --git a/packages/browser/src/integrations/featureFlags/unleash/types.ts b/packages/browser/src/integrations/featureFlags/unleash/types.ts index 8ba56246e7ac..8fc3c57843dd 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/types.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/types.ts @@ -3,8 +3,8 @@ export interface IVariant { enabled: boolean; feature_enabled?: boolean; payload?: { - type: string; - value: string; + type: string; + value: string; }; } From 7ec8d02ef72dc0a9d2023913d46a8a4e0f154448 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 8 Jan 2025 16:29:33 -0800 Subject: [PATCH 04/19] Working patch and test for getVariant --- .../featureFlags/unleash/basic/test.ts | 42 ++++++++++++++++++- .../featureFlags/unleash/integration.ts | 32 +++++++++++--- 2 files changed, 68 insertions(+), 6 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts index 79361c137128..283b934a4e96 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts @@ -6,7 +6,7 @@ import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest const FLAG_BUFFER_SIZE = 100; // Corresponds to constant in featureFlags.ts, in browser utils. -sentryTest('Basic test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { +sentryTest('Basic isEnabled test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { if (shouldSkipFeatureFlagsTest()) { sentryTest.skip(); } @@ -45,3 +45,43 @@ sentryTest('Basic test with eviction, update, and no async tasks', async ({ getL expect(event.contexts?.flags?.values).toEqual(expectedFlags); }); + +sentryTest('Basic getVariant test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { + if (shouldSkipFeatureFlagsTest()) { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + await page.goto(url); + + await page.evaluate(bufferSize => { + const client = new (window as any).UnleashClient(); + for (let i = 1; i <= bufferSize; i++) { + client.getVariant(`feat${i}`); + } + client.getVariant(`feat${bufferSize + 1}`); // eviction + client.getVariant('feat3'); // update + }, FLAG_BUFFER_SIZE); + + const reqPromise = waitForErrorRequest(page); + await page.locator('#error').click(); + const req = await reqPromise; + const event = envelopeRequestParser(req); + + const expectedFlags = [{ flag: 'feat2', result: false }]; + for (let i = 4; i <= FLAG_BUFFER_SIZE; i++) { + expectedFlags.push({ flag: `feat${i}`, result: false }); + } + expectedFlags.push({ flag: `feat${FLAG_BUFFER_SIZE + 1}`, result: false }); + expectedFlags.push({ flag: 'feat3', result: false }); + + expect(event.contexts?.flags?.values).toEqual(expectedFlags); +}); diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 6362d95e39cc..75be61d19e11 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -2,22 +2,28 @@ import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags'; -import type { UnleashClient, UnleashClientClass } from './types'; +import type { IVariant, UnleashClient, UnleashClientClass } from './types'; /** - * Sentry integration for capturing feature flags from the Unleash SDK. + * Sentry integration for capturing feature flag evaluations from the Unleash SDK. * * See the [feature flag documentation](https://develop.sentry.dev/sdk/expected-features/#feature-flags) for more information. * * @example * ``` + * import { UnleashClient } from 'unleash-proxy-client'; * import * as Sentry from '@sentry/browser'; - * TODO: + * + * const unleashIntegration = Sentry.unleashIntegration(UnleashClient); * * Sentry.init({ * dsn: '___PUBLIC_DSN___', - * integrations: [TODO:] + * integrations: [unleashIntegration], * }); + * + * const unleashClient = new UnleashClient(...); + * unleashClient.isEnabled('my-feature'); + * Sentry.captureException(new Error('something went wrong')); * ``` */ export const unleashIntegration = defineIntegration((unleashClientClass: UnleashClientClass) => { @@ -29,6 +35,8 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash }, setupOnce() { + const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; + const sentryIsEnabled = { apply: ( target: (this: UnleashClient, toggleName: string) => boolean, @@ -40,9 +48,23 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash return result; }, }; - const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; const originalIsEnabled = unleashClientPrototype.isEnabled.bind(unleashClientPrototype); unleashClientPrototype.isEnabled = new Proxy(originalIsEnabled, sentryIsEnabled); + + const sentryGetVariant = { + apply: ( + target: (this: UnleashClient, toggleName: string) => IVariant, + thisArg: UnleashClient, + args: [toggleName: string], + ) => { + const variant = Reflect.apply(target, thisArg, args); + const result = variant.enabled; + insertFlagToScope(args[0], result); + return variant; + }, + }; + const originalGetVariant = unleashClientPrototype.getVariant.bind(unleashClientPrototype); + unleashClientPrototype.getVariant = new Proxy(originalGetVariant, sentryGetVariant); }, }; }) satisfies IntegrationFn; From f1a5574a82ac2aeaff037bca4dedf7f9169abe77 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:02:01 -0800 Subject: [PATCH 05/19] Fix docstr sample code --- .../suites/integrations/featureFlags/unleash/init.js | 2 -- .../src/integrations/featureFlags/unleash/integration.ts | 7 +++++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index f40861483bea..149fb9bd4ae5 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -21,5 +21,3 @@ Sentry.init({ sampleRate: 1.0, integrations: [window.sentryUnleashIntegration], }); - - diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 75be61d19e11..344e110ddc4d 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -21,8 +21,11 @@ import type { IVariant, UnleashClient, UnleashClientClass } from './types'; * integrations: [unleashIntegration], * }); * - * const unleashClient = new UnleashClient(...); - * unleashClient.isEnabled('my-feature'); + * const unleash = new UnleashClient(...); + * unleash.start(); + * + * unleash.isEnabled('my-feature'); + * unleash.getVariant('other-feature'); * Sentry.captureException(new Error('something went wrong')); * ``` */ From 34af880ed0769e04e01d8ce06ae822d49ad4e144 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:56:37 -0800 Subject: [PATCH 06/19] Fix patching for correct access to --- .../featureFlags/unleash/basic/test.ts | 16 +++++--- .../integrations/featureFlags/unleash/init.js | 37 ++++++++++++++++--- .../featureFlags/unleash/integration.ts | 6 +-- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts index 283b934a4e96..d58c346e4c18 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts @@ -36,12 +36,14 @@ sentryTest('Basic isEnabled test with eviction, update, and no async tasks', asy const req = await reqPromise; const event = envelopeRequestParser(req); - const expectedFlags = [{ flag: 'feat2', result: false }]; - for (let i = 4; i <= FLAG_BUFFER_SIZE; i++) { + const expectedFlags = [{ flag: 'feat2', result: true }]; + expectedFlags.push({ flag: 'feat4', result: true }); + expectedFlags.push({ flag: 'feat5', result: true }); + for (let i = 6; i <= FLAG_BUFFER_SIZE; i++) { expectedFlags.push({ flag: `feat${i}`, result: false }); } expectedFlags.push({ flag: `feat${FLAG_BUFFER_SIZE + 1}`, result: false }); - expectedFlags.push({ flag: 'feat3', result: false }); + expectedFlags.push({ flag: 'feat3', result: true }); expect(event.contexts?.flags?.values).toEqual(expectedFlags); }); @@ -76,12 +78,14 @@ sentryTest('Basic getVariant test with eviction, update, and no async tasks', as const req = await reqPromise; const event = envelopeRequestParser(req); - const expectedFlags = [{ flag: 'feat2', result: false }]; - for (let i = 4; i <= FLAG_BUFFER_SIZE; i++) { + const expectedFlags = [{ flag: 'feat2', result: true }]; + expectedFlags.push({ flag: 'feat4', result: true }); + expectedFlags.push({ flag: 'feat5', result: true }); + for (let i = 6; i <= FLAG_BUFFER_SIZE; i++) { expectedFlags.push({ flag: `feat${i}`, result: false }); } expectedFlags.push({ flag: `feat${FLAG_BUFFER_SIZE + 1}`, result: false }); - expectedFlags.push({ flag: 'feat3', result: false }); + expectedFlags.push({ flag: 'feat3', result: true }); expect(event.contexts?.flags?.values).toEqual(expectedFlags); }); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index 149fb9bd4ae5..74f919346f90 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -1,16 +1,43 @@ import * as Sentry from '@sentry/browser'; window.UnleashClient = class { - isEnabled(_toggleName) { - return false; - } + constructor() { + this._featureToVariant = { + 'feat2': {name: 'variant1', enabled: true, feature_enabled: true, payload: {type: 'string', value: 'test'}}, + 'feat3': {name: 'eu-west', enabled: true, feature_enabled: true}, + 'feat4': { + name: 'paid-orgs', + enabled: true, + feature_enabled: true, + payload: { + type: 'json', + value: '{"foo": {"bar": "baz"}, "hello": [1, 2, 3]}', + }, + }, + + // Enabled feature with no configured variants. + 'feat5': {name: 'disabled', enabled: false, feature_enabled: true}, + + // Disabled feature. + 'feat6': {name: 'disabled', enabled: false, feature_enabled: false}, + }; - getVariant(_toggleName) { - return { + // Variant returned for features that don't exist. + // `feature_enabled` may be defined in prod, but we want to test the undefined case. + this._fallbackVariant = { name: 'disabled', enabled: false, }; } + + isEnabled(toggleName) { + const variant = this._featureToVariant[toggleName] || this._fallbackVariant; + return variant.feature_enabled || false; + } + + getVariant(toggleName) { + return this._featureToVariant[toggleName] || this._fallbackVariant; + } }; window.Sentry = Sentry; diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 344e110ddc4d..9b8268426af0 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -51,7 +51,7 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash return result; }, }; - const originalIsEnabled = unleashClientPrototype.isEnabled.bind(unleashClientPrototype); + const originalIsEnabled = unleashClientPrototype.isEnabled; unleashClientPrototype.isEnabled = new Proxy(originalIsEnabled, sentryIsEnabled); const sentryGetVariant = { @@ -61,12 +61,12 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash args: [toggleName: string], ) => { const variant = Reflect.apply(target, thisArg, args); - const result = variant.enabled; + const result = variant.feature_enabled || false; // undefined means the feature does not exist. insertFlagToScope(args[0], result); return variant; }, }; - const originalGetVariant = unleashClientPrototype.getVariant.bind(unleashClientPrototype); + const originalGetVariant = unleashClientPrototype.getVariant; unleashClientPrototype.getVariant = new Proxy(originalGetVariant, sentryGetVariant); }, }; From c8b0078305379606f0e03b0b7a359d09c319751f Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 9 Jan 2025 15:59:31 -0800 Subject: [PATCH 07/19] ini.js format --- .../suites/integrations/featureFlags/unleash/init.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index 74f919346f90..b387706c6ee6 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -3,9 +3,9 @@ import * as Sentry from '@sentry/browser'; window.UnleashClient = class { constructor() { this._featureToVariant = { - 'feat2': {name: 'variant1', enabled: true, feature_enabled: true, payload: {type: 'string', value: 'test'}}, - 'feat3': {name: 'eu-west', enabled: true, feature_enabled: true}, - 'feat4': { + feat2: {name: 'variant1', enabled: true, feature_enabled: true, payload: {type: 'string', value: 'test'}}, + feat3: {name: 'eu-west', enabled: true, feature_enabled: true}, + feat4: { name: 'paid-orgs', enabled: true, feature_enabled: true, @@ -16,10 +16,10 @@ window.UnleashClient = class { }, // Enabled feature with no configured variants. - 'feat5': {name: 'disabled', enabled: false, feature_enabled: true}, + feat5: {name: 'disabled', enabled: false, feature_enabled: true}, // Disabled feature. - 'feat6': {name: 'disabled', enabled: false, feature_enabled: false}, + feat6: {name: 'disabled', enabled: false, feature_enabled: false}, }; // Variant returned for features that don't exist. From 2543733713e52d2099d27561711c99f0e1b5d169 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 9 Jan 2025 16:04:13 -0800 Subject: [PATCH 08/19] init.js format2 --- .../suites/integrations/featureFlags/unleash/init.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index b387706c6ee6..c73f544d0724 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -3,8 +3,8 @@ import * as Sentry from '@sentry/browser'; window.UnleashClient = class { constructor() { this._featureToVariant = { - feat2: {name: 'variant1', enabled: true, feature_enabled: true, payload: {type: 'string', value: 'test'}}, - feat3: {name: 'eu-west', enabled: true, feature_enabled: true}, + feat2: { name: 'variant1', enabled: true, feature_enabled: true, payload: { type: 'string', value: 'test' } }, + feat3: { name: 'eu-west', enabled: true, feature_enabled: true }, feat4: { name: 'paid-orgs', enabled: true, @@ -16,10 +16,10 @@ window.UnleashClient = class { }, // Enabled feature with no configured variants. - feat5: {name: 'disabled', enabled: false, feature_enabled: true}, + feat5: { name: 'disabled', enabled: false, feature_enabled: true }, // Disabled feature. - feat6: {name: 'disabled', enabled: false, feature_enabled: false}, + feat6: { name: 'disabled', enabled: false, feature_enabled: false }, }; // Variant returned for features that don't exist. From 211a1ab2935ca8a2afd13006bb49b42e5100339a Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 10 Jan 2025 00:48:19 -0800 Subject: [PATCH 09/19] Disable eslint/unbound-method --- .../src/integrations/featureFlags/unleash/integration.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 9b8268426af0..607fa6c05a1a 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -51,6 +51,7 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash return result; }, }; + // eslint-disable-next-line @typescript-eslint/unbound-method const originalIsEnabled = unleashClientPrototype.isEnabled; unleashClientPrototype.isEnabled = new Proxy(originalIsEnabled, sentryIsEnabled); @@ -66,6 +67,7 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash return variant; }, }; + // eslint-disable-next-line @typescript-eslint/unbound-method const originalGetVariant = unleashClientPrototype.getVariant; unleashClientPrototype.getVariant = new Proxy(originalGetVariant, sentryGetVariant); }, From 2898386b6f0d1d9f35a1ee61eaf6796a5e0ab651 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 10 Jan 2025 12:21:19 -0800 Subject: [PATCH 10/19] Rm getVariant code --- .../featureFlags/unleash/basic/test.ts | 44 +------------------ .../featureFlags/unleash/integration.ts | 18 +------- 2 files changed, 2 insertions(+), 60 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts index d58c346e4c18..dc2dee988fef 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts @@ -6,7 +6,7 @@ import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest const FLAG_BUFFER_SIZE = 100; // Corresponds to constant in featureFlags.ts, in browser utils. -sentryTest('Basic isEnabled test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { +sentryTest('Basic test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { if (shouldSkipFeatureFlagsTest()) { sentryTest.skip(); } @@ -47,45 +47,3 @@ sentryTest('Basic isEnabled test with eviction, update, and no async tasks', asy expect(event.contexts?.flags?.values).toEqual(expectedFlags); }); - -sentryTest('Basic getVariant test with eviction, update, and no async tasks', async ({ getLocalTestUrl, page }) => { - if (shouldSkipFeatureFlagsTest()) { - sentryTest.skip(); - } - - await page.route('https://dsn.ingest.sentry.io/**/*', route => { - return route.fulfill({ - status: 200, - contentType: 'application/json', - body: JSON.stringify({ id: 'test-id' }), - }); - }); - - const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); - await page.goto(url); - - await page.evaluate(bufferSize => { - const client = new (window as any).UnleashClient(); - for (let i = 1; i <= bufferSize; i++) { - client.getVariant(`feat${i}`); - } - client.getVariant(`feat${bufferSize + 1}`); // eviction - client.getVariant('feat3'); // update - }, FLAG_BUFFER_SIZE); - - const reqPromise = waitForErrorRequest(page); - await page.locator('#error').click(); - const req = await reqPromise; - const event = envelopeRequestParser(req); - - const expectedFlags = [{ flag: 'feat2', result: true }]; - expectedFlags.push({ flag: 'feat4', result: true }); - expectedFlags.push({ flag: 'feat5', result: true }); - for (let i = 6; i <= FLAG_BUFFER_SIZE; i++) { - expectedFlags.push({ flag: `feat${i}`, result: false }); - } - expectedFlags.push({ flag: `feat${FLAG_BUFFER_SIZE + 1}`, result: false }); - expectedFlags.push({ flag: 'feat3', result: true }); - - expect(event.contexts?.flags?.values).toEqual(expectedFlags); -}); diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 607fa6c05a1a..886afab01884 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -2,7 +2,7 @@ import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; import { defineIntegration } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags'; -import type { IVariant, UnleashClient, UnleashClientClass } from './types'; +import type { UnleashClient, UnleashClientClass } from './types'; /** * Sentry integration for capturing feature flag evaluations from the Unleash SDK. @@ -54,22 +54,6 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash // eslint-disable-next-line @typescript-eslint/unbound-method const originalIsEnabled = unleashClientPrototype.isEnabled; unleashClientPrototype.isEnabled = new Proxy(originalIsEnabled, sentryIsEnabled); - - const sentryGetVariant = { - apply: ( - target: (this: UnleashClient, toggleName: string) => IVariant, - thisArg: UnleashClient, - args: [toggleName: string], - ) => { - const variant = Reflect.apply(target, thisArg, args); - const result = variant.feature_enabled || false; // undefined means the feature does not exist. - insertFlagToScope(args[0], result); - return variant; - }, - }; - // eslint-disable-next-line @typescript-eslint/unbound-method - const originalGetVariant = unleashClientPrototype.getVariant; - unleashClientPrototype.getVariant = new Proxy(originalGetVariant, sentryGetVariant); }, }; }) satisfies IntegrationFn; From b972f6e7e6275519161c418e75377ff8b49b4947 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 10 Jan 2025 12:37:07 -0800 Subject: [PATCH 11/19] Add withScope test --- .../featureFlags/unleash/basic/test.ts | 23 +++++-- .../integrations/featureFlags/unleash/init.js | 10 +-- .../featureFlags/unleash/withScope/test.ts | 65 +++++++++++++++++++ 3 files changed, 86 insertions(+), 12 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/withScope/test.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts index dc2dee988fef..5bb72caddd24 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/basic/test.ts @@ -24,11 +24,19 @@ sentryTest('Basic test with eviction, update, and no async tasks', async ({ getL await page.evaluate(bufferSize => { const client = new (window as any).UnleashClient(); - for (let i = 1; i <= bufferSize; i++) { + + client.isEnabled('feat1'); + client.isEnabled('strFeat'); + client.isEnabled('noPayloadFeat'); + client.isEnabled('jsonFeat'); + client.isEnabled('noVariantFeat'); + client.isEnabled('disabledFeat'); + + for (let i = 7; i <= bufferSize; i++) { client.isEnabled(`feat${i}`); } client.isEnabled(`feat${bufferSize + 1}`); // eviction - client.isEnabled('feat3'); // update + client.isEnabled('noPayloadFeat'); // update (move to tail) }, FLAG_BUFFER_SIZE); const reqPromise = waitForErrorRequest(page); @@ -36,14 +44,15 @@ sentryTest('Basic test with eviction, update, and no async tasks', async ({ getL const req = await reqPromise; const event = envelopeRequestParser(req); - const expectedFlags = [{ flag: 'feat2', result: true }]; - expectedFlags.push({ flag: 'feat4', result: true }); - expectedFlags.push({ flag: 'feat5', result: true }); - for (let i = 6; i <= FLAG_BUFFER_SIZE; i++) { + const expectedFlags = [{ flag: 'strFeat', result: true }]; + expectedFlags.push({ flag: 'jsonFeat', result: true }); + expectedFlags.push({ flag: 'noVariantFeat', result: true }); + expectedFlags.push({ flag: 'disabledFeat', result: false }); + for (let i = 7; i <= FLAG_BUFFER_SIZE; i++) { expectedFlags.push({ flag: `feat${i}`, result: false }); } expectedFlags.push({ flag: `feat${FLAG_BUFFER_SIZE + 1}`, result: false }); - expectedFlags.push({ flag: 'feat3', result: true }); + expectedFlags.push({ flag: 'noPayloadFeat', result: true }); expect(event.contexts?.flags?.values).toEqual(expectedFlags); }); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index c73f544d0724..301f8da7ad50 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -3,9 +3,9 @@ import * as Sentry from '@sentry/browser'; window.UnleashClient = class { constructor() { this._featureToVariant = { - feat2: { name: 'variant1', enabled: true, feature_enabled: true, payload: { type: 'string', value: 'test' } }, - feat3: { name: 'eu-west', enabled: true, feature_enabled: true }, - feat4: { + strFeat: { name: 'variant1', enabled: true, feature_enabled: true, payload: { type: 'string', value: 'test' } }, + noPayloadFeat: { name: 'eu-west', enabled: true, feature_enabled: true }, + jsonFeat: { name: 'paid-orgs', enabled: true, feature_enabled: true, @@ -16,10 +16,10 @@ window.UnleashClient = class { }, // Enabled feature with no configured variants. - feat5: { name: 'disabled', enabled: false, feature_enabled: true }, + noVariantFeat: { name: 'disabled', enabled: false, feature_enabled: true }, // Disabled feature. - feat6: { name: 'disabled', enabled: false, feature_enabled: false }, + disabledFeat: { name: 'disabled', enabled: false, feature_enabled: false }, }; // Variant returned for features that don't exist. diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/withScope/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/withScope/test.ts new file mode 100644 index 000000000000..2d821bf6c81d --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/withScope/test.ts @@ -0,0 +1,65 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +import { envelopeRequestParser, shouldSkipFeatureFlagsTest, waitForErrorRequest } from '../../../../../utils/helpers'; + +import type { Scope } from '@sentry/browser'; + +sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ getLocalTestUrl, page }) => { + if (shouldSkipFeatureFlagsTest()) { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + await page.goto(url); + + const forkedReqPromise = waitForErrorRequest(page, event => !!event.tags && event.tags.isForked === true); + const mainReqPromise = waitForErrorRequest(page, event => !!event.tags && event.tags.isForked === false); + + await page.evaluate(() => { + const Sentry = (window as any).Sentry; + const errorButton = document.querySelector('#error') as HTMLButtonElement; + const unleash = new (window as any).UnleashClient(); + + unleash.isEnabled('strFeat'); + + Sentry.withScope((scope: Scope) => { + unleash.isEnabled('disabledFeat'); + unleash.isEnabled('strFeat'); + scope.setTag('isForked', true); + if (errorButton) { + errorButton.click(); + } + }); + + unleash.isEnabled('noPayloadFeat'); + Sentry.getCurrentScope().setTag('isForked', false); + errorButton.click(); + return true; + }); + + const forkedReq = await forkedReqPromise; + const forkedEvent = envelopeRequestParser(forkedReq); + + const mainReq = await mainReqPromise; + const mainEvent = envelopeRequestParser(mainReq); + + expect(forkedEvent.contexts?.flags?.values).toEqual([ + { flag: 'disabledFeat', result: false }, + { flag: 'strFeat', result: true }, + ]); + + expect(mainEvent.contexts?.flags?.values).toEqual([ + { flag: 'strFeat', result: true }, + { flag: 'noPayloadFeat', result: true }, + ]); +}); From dbf0f64bc7ce72aa8775ee3f1b1478cf4c3d216f Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 15 Jan 2025 10:29:19 -0800 Subject: [PATCH 12/19] Rewrite patching with fill() --- .../featureFlags/unleash/integration.ts | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 886afab01884..eb7df04b8a2e 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -1,6 +1,6 @@ import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; -import { defineIntegration } from '@sentry/core'; +import { defineIntegration, fill, logger } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags'; import type { UnleashClient, UnleashClientClass } from './types'; @@ -39,21 +39,28 @@ export const unleashIntegration = defineIntegration((unleashClientClass: Unleash setupOnce() { const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; - - const sentryIsEnabled = { - apply: ( - target: (this: UnleashClient, toggleName: string) => boolean, - thisArg: UnleashClient, - args: [toggleName: string], - ) => { - const result = Reflect.apply(target, thisArg, args); - insertFlagToScope(args[0], result); - return result; - }, - }; - // eslint-disable-next-line @typescript-eslint/unbound-method - const originalIsEnabled = unleashClientPrototype.isEnabled; - unleashClientPrototype.isEnabled = new Proxy(originalIsEnabled, sentryIsEnabled); + fill(unleashClientPrototype, 'isEnabled', _wrappedIsEnabled); }, }; }) satisfies IntegrationFn; + +/** + * Wraps the UnleashClient.isEnabled method to capture feature flag evaluations. Its only side effect is writing to Sentry scope. + * + * @param original - The original method. + * @returns Wrapped method. Results should match the original. + */ +function _wrappedIsEnabled(original: (this: UnleashClient, ...args: unknown[]) => unknown): (this: UnleashClient, ...args: unknown[]) => unknown { + return function (this: UnleashClient, ...args: unknown[]): unknown { + const toggleName = args[0]; + const result = original.apply(this, args); + + if (typeof toggleName === 'string' && typeof result === 'boolean') { + insertFlagToScope(toggleName, result); + } else { + // TODO: test this branch + logger.error(`[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: ${toggleName} (${typeof toggleName}), result: ${result} (${typeof result})`); + } + return result; + }; +} From ba090a28efc69768ae5cf908548ef4a931b3b4da Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 15 Jan 2025 10:46:59 -0800 Subject: [PATCH 13/19] Change integration input to an options dict --- .../suites/integrations/featureFlags/unleash/init.js | 2 +- .../src/integrations/featureFlags/unleash/integration.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index 301f8da7ad50..2a991bfde3f0 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -41,7 +41,7 @@ window.UnleashClient = class { }; window.Sentry = Sentry; -window.sentryUnleashIntegration = Sentry.unleashIntegration(window.UnleashClient); +window.sentryUnleashIntegration = Sentry.unleashIntegration({unleashClientClass: window.UnleashClient}); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index eb7df04b8a2e..608ca4200eb3 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -14,7 +14,7 @@ import type { UnleashClient, UnleashClientClass } from './types'; * import { UnleashClient } from 'unleash-proxy-client'; * import * as Sentry from '@sentry/browser'; * - * const unleashIntegration = Sentry.unleashIntegration(UnleashClient); + * const unleashIntegration = Sentry.unleashIntegration({unleashClientClass: UnleashClient}); * * Sentry.init({ * dsn: '___PUBLIC_DSN___', @@ -29,7 +29,7 @@ import type { UnleashClient, UnleashClientClass } from './types'; * Sentry.captureException(new Error('something went wrong')); * ``` */ -export const unleashIntegration = defineIntegration((unleashClientClass: UnleashClientClass) => { +export const unleashIntegration = defineIntegration(({unleashClientClass}: {unleashClientClass: UnleashClientClass}) => { return { name: 'Unleash', From bba93f7b00c52f85f75a26e45a75ab237729a502 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 15 Jan 2025 10:54:33 -0800 Subject: [PATCH 14/19] Update wrapper docstr --- .../src/integrations/featureFlags/unleash/integration.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 608ca4200eb3..fe75e53bdcb5 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -47,6 +47,9 @@ export const unleashIntegration = defineIntegration(({unleashClientClass}: {unle /** * Wraps the UnleashClient.isEnabled method to capture feature flag evaluations. Its only side effect is writing to Sentry scope. * + * This wrapper is safe for all isEnabled signatures. If the signature does not match (this: UnleashClient, toggleName: string, ...args: unknown[]) => boolean, + * we log an error and return the original result. + * * @param original - The original method. * @returns Wrapped method. Results should match the original. */ From 776bb58242a4cc4f64fb41e9bb158284f6ef3416 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 15 Jan 2025 13:11:38 -0800 Subject: [PATCH 15/19] Add badSignature test --- .../featureFlags/unleash/badSignature/init.js | 17 +++++++ .../featureFlags/unleash/badSignature/test.ts | 49 +++++++++++++++++++ .../featureFlags/unleash/integration.ts | 1 - 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js new file mode 100644 index 000000000000..3e9a79894532 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js @@ -0,0 +1,17 @@ +import * as Sentry from '@sentry/browser'; + +window.UnleashClient = class { + isEnabled(x) { + return x; + } +}; + +window.Sentry = Sentry; +window.sentryUnleashIntegration = Sentry.unleashIntegration({unleashClientClass: window.UnleashClient}); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1.0, + integrations: [window.sentryUnleashIntegration], + debug: true, // Required to test logging. +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts new file mode 100644 index 000000000000..70c25fa8f560 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts @@ -0,0 +1,49 @@ +import { expect } from '@playwright/test'; + +import { sentryTest } from '../../../../../utils/fixtures'; + +import { shouldSkipFeatureFlagsTest } from '../../../../../utils/helpers'; + +sentryTest('Logs and returns if isEnabled does not match expected signature', async ({ getLocalTestUrl, page }) => { + if (shouldSkipFeatureFlagsTest()) { + sentryTest.skip(); + } + + await page.route('https://dsn.ingest.sentry.io/**/*', route => { + return route.fulfill({ + status: 200, + contentType: 'application/json', + body: JSON.stringify({ id: 'test-id' }), + }); + }); + + const url = await getLocalTestUrl({ testDir: __dirname, skipDsnRouteHandler: true }); + await page.goto(url); + + const errorLogs: string[] = []; + page.on('console', msg => { + if (msg.type() == 'error') { + errorLogs.push(msg.text()); + } + }); + + const results = await page.evaluate(() => { + const unleash = new (window as any).UnleashClient(); + const res1 = unleash.isEnabled('my-feature'); + const res2 = unleash.isEnabled(999); + const res3 = unleash.isEnabled({}); + return [res1, res2, res3]; + }); + + // Test that the expected results are still returned. Note isEnabled is identity function for this test. + expect(results).toEqual(['my-feature', 999, {}]); + + // Expected error logs. + expect(errorLogs).toEqual( + expect.arrayContaining([ + expect.stringContaining('[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: my-feature (string), result: my-feature (string)'), + expect.stringContaining('[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: 999 (number), result: 999 (number)'), + expect.stringContaining('[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: [object Object] (object), result: [object Object] (object)'), + ]), + ); +}); diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index fe75e53bdcb5..29652a1273d8 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -61,7 +61,6 @@ function _wrappedIsEnabled(original: (this: UnleashClient, ...args: unknown[]) = if (typeof toggleName === 'string' && typeof result === 'boolean') { insertFlagToScope(toggleName, result); } else { - // TODO: test this branch logger.error(`[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: ${toggleName} (${typeof toggleName}), result: ${result} (${typeof result})`); } return result; From fae120bd138696da3bbc781b5420a0d557fb5d59 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 15 Jan 2025 13:13:23 -0800 Subject: [PATCH 16/19] Fix formatting (yarn run biome check --apply) --- .../featureFlags/unleash/badSignature/init.js | 2 +- .../featureFlags/unleash/badSignature/test.ts | 12 +++++-- .../integrations/featureFlags/unleash/init.js | 2 +- .../featureFlags/unleash/integration.ts | 34 +++++++++++-------- 4 files changed, 31 insertions(+), 19 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js index 3e9a79894532..dc92fbc296a4 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/init.js @@ -7,7 +7,7 @@ window.UnleashClient = class { }; window.Sentry = Sentry; -window.sentryUnleashIntegration = Sentry.unleashIntegration({unleashClientClass: window.UnleashClient}); +window.sentryUnleashIntegration = Sentry.unleashIntegration({ unleashClientClass: window.UnleashClient }); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts index 70c25fa8f560..dacb1c0154a3 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts @@ -41,9 +41,15 @@ sentryTest('Logs and returns if isEnabled does not match expected signature', as // Expected error logs. expect(errorLogs).toEqual( expect.arrayContaining([ - expect.stringContaining('[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: my-feature (string), result: my-feature (string)'), - expect.stringContaining('[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: 999 (number), result: 999 (number)'), - expect.stringContaining('[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: [object Object] (object), result: [object Object] (object)'), + expect.stringContaining( + '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: my-feature (string), result: my-feature (string)', + ), + expect.stringContaining( + '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: 999 (number), result: 999 (number)', + ), + expect.stringContaining( + '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: [object Object] (object), result: [object Object] (object)', + ), ]), ); }); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js index 2a991bfde3f0..9f1f28730cf7 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/init.js @@ -41,7 +41,7 @@ window.UnleashClient = class { }; window.Sentry = Sentry; -window.sentryUnleashIntegration = Sentry.unleashIntegration({unleashClientClass: window.UnleashClient}); +window.sentryUnleashIntegration = Sentry.unleashIntegration({ unleashClientClass: window.UnleashClient }); Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 29652a1273d8..e910b6946573 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -29,20 +29,22 @@ import type { UnleashClient, UnleashClientClass } from './types'; * Sentry.captureException(new Error('something went wrong')); * ``` */ -export const unleashIntegration = defineIntegration(({unleashClientClass}: {unleashClientClass: UnleashClientClass}) => { - return { - name: 'Unleash', +export const unleashIntegration = defineIntegration( + ({ unleashClientClass }: { unleashClientClass: UnleashClientClass }) => { + return { + name: 'Unleash', - processEvent(event: Event, _hint: EventHint, _client: Client): Event { - return copyFlagsFromScopeToEvent(event); - }, + processEvent(event: Event, _hint: EventHint, _client: Client): Event { + return copyFlagsFromScopeToEvent(event); + }, - setupOnce() { - const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; - fill(unleashClientPrototype, 'isEnabled', _wrappedIsEnabled); - }, - }; -}) satisfies IntegrationFn; + setupOnce() { + const unleashClientPrototype = unleashClientClass.prototype as UnleashClient; + fill(unleashClientPrototype, 'isEnabled', _wrappedIsEnabled); + }, + }; + }, +) satisfies IntegrationFn; /** * Wraps the UnleashClient.isEnabled method to capture feature flag evaluations. Its only side effect is writing to Sentry scope. @@ -53,7 +55,9 @@ export const unleashIntegration = defineIntegration(({unleashClientClass}: {unle * @param original - The original method. * @returns Wrapped method. Results should match the original. */ -function _wrappedIsEnabled(original: (this: UnleashClient, ...args: unknown[]) => unknown): (this: UnleashClient, ...args: unknown[]) => unknown { +function _wrappedIsEnabled( + original: (this: UnleashClient, ...args: unknown[]) => unknown, +): (this: UnleashClient, ...args: unknown[]) => unknown { return function (this: UnleashClient, ...args: unknown[]): unknown { const toggleName = args[0]; const result = original.apply(this, args); @@ -61,7 +65,9 @@ function _wrappedIsEnabled(original: (this: UnleashClient, ...args: unknown[]) = if (typeof toggleName === 'string' && typeof result === 'boolean') { insertFlagToScope(toggleName, result); } else { - logger.error(`[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: ${toggleName} (${typeof toggleName}), result: ${result} (${typeof result})`); + logger.error( + `[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: ${toggleName} (${typeof toggleName}), result: ${result} (${typeof result})`, + ); } return result; }; From bcc0b864f52c7adc76ff3f251e3cb692575ad6f3 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 16 Jan 2025 08:56:20 -0800 Subject: [PATCH 17/19] Only log/test if debug build --- .../featureFlags/unleash/badSignature/test.ts | 30 +++++++++++-------- .../featureFlags/unleash/integration.ts | 3 +- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts index dacb1c0154a3..9b95d4d51c81 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/unleash/badSignature/test.ts @@ -8,6 +8,8 @@ sentryTest('Logs and returns if isEnabled does not match expected signature', as if (shouldSkipFeatureFlagsTest()) { sentryTest.skip(); } + const bundleKey = process.env.PW_BUNDLE || ''; + const hasDebug = !bundleKey.includes('_min'); await page.route('https://dsn.ingest.sentry.io/**/*', route => { return route.fulfill({ @@ -39,17 +41,19 @@ sentryTest('Logs and returns if isEnabled does not match expected signature', as expect(results).toEqual(['my-feature', 999, {}]); // Expected error logs. - expect(errorLogs).toEqual( - expect.arrayContaining([ - expect.stringContaining( - '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: my-feature (string), result: my-feature (string)', - ), - expect.stringContaining( - '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: 999 (number), result: 999 (number)', - ), - expect.stringContaining( - '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: [object Object] (object), result: [object Object] (object)', - ), - ]), - ); + if (hasDebug) { + expect(errorLogs).toEqual( + expect.arrayContaining([ + expect.stringContaining( + '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: my-feature (string), result: my-feature (string)', + ), + expect.stringContaining( + '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: 999 (number), result: 999 (number)', + ), + expect.stringContaining( + '[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: [object Object] (object), result: [object Object] (object)', + ), + ]), + ); + } }); diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index e910b6946573..9b97f11f9228 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -1,6 +1,7 @@ import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; import { defineIntegration, fill, logger } from '@sentry/core'; +import { DEBUG_BUILD } from '../../../debug-build'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../../utils/featureFlags'; import type { UnleashClient, UnleashClientClass } from './types'; @@ -64,7 +65,7 @@ function _wrappedIsEnabled( if (typeof toggleName === 'string' && typeof result === 'boolean') { insertFlagToScope(toggleName, result); - } else { + } else if (DEBUG_BUILD) { logger.error( `[Feature Flags] UnleashClient.isEnabled does not match expected signature. arg0: ${toggleName} (${typeof toggleName}), result: ${result} (${typeof result})`, ); From c142b4f347f2497c00a04347b906b649ab459961 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Thu, 16 Jan 2025 10:36:27 -0800 Subject: [PATCH 18/19] Make UnleashClientClass type more specific --- .../src/integrations/featureFlags/unleash/types.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/browser/src/integrations/featureFlags/unleash/types.ts b/packages/browser/src/integrations/featureFlags/unleash/types.ts index 8fc3c57843dd..c87798859911 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/types.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/types.ts @@ -13,4 +13,11 @@ export interface UnleashClient { getVariant(this: UnleashClient, featureName: string): IVariant; } -export type UnleashClientClass = new (...args: unknown[]) => UnleashClient; +export interface IConfig { + [key: string]: unknown; + appName: string; + clientKey: string; + url: URL | string; +} + +export type UnleashClientClass = new (config: IConfig) => UnleashClient; From dcf2595d259a45e00ab39e1a9b0ed14b960a4a2f Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Tue, 21 Jan 2025 10:08:22 -0800 Subject: [PATCH 19/19] Simplify docstr --- .../src/integrations/featureFlags/unleash/integration.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/browser/src/integrations/featureFlags/unleash/integration.ts b/packages/browser/src/integrations/featureFlags/unleash/integration.ts index 9b97f11f9228..c451afb831ba 100644 --- a/packages/browser/src/integrations/featureFlags/unleash/integration.ts +++ b/packages/browser/src/integrations/featureFlags/unleash/integration.ts @@ -15,11 +15,9 @@ import type { UnleashClient, UnleashClientClass } from './types'; * import { UnleashClient } from 'unleash-proxy-client'; * import * as Sentry from '@sentry/browser'; * - * const unleashIntegration = Sentry.unleashIntegration({unleashClientClass: UnleashClient}); - * * Sentry.init({ * dsn: '___PUBLIC_DSN___', - * integrations: [unleashIntegration], + * integrations: [Sentry.unleashIntegration({unleashClientClass: UnleashClient})], * }); * * const unleash = new UnleashClient(...);