From a4d653a0f230413941d3278ae3658647dee6d653 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 4 Dec 2024 14:05:33 -0800 Subject: [PATCH 1/5] feat(flags): add featureFlagsIntegration for custom tracking of flag evaluations --- .../featureFlags/featureFlags/basic/test.ts | 48 ++++++++++++++ .../featureFlags/featureFlags/init.js | 12 ++++ .../featureFlags/featureFlags/subject.js | 3 + .../featureFlags/featureFlags/template.html | 9 +++ .../featureFlags/withScope/test.ts | 65 +++++++++++++++++++ packages/browser/src/index.ts | 10 ++- .../featureFlags/featureFlagsIntegration.ts | 47 ++++++++++++++ .../src/integrations/featureFlags/index.ts | 4 ++ 8 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/subject.js create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/template.html create mode 100644 dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts create mode 100644 packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts create mode 100644 packages/browser/src/integrations/featureFlags/index.ts diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts new file mode 100644 index 000000000000..9cb4bf4b3f58 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts @@ -0,0 +1,48 @@ +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 flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); + for (let i = 1; i <= bufferSize; i++) { + flagsIntegration.setFlag(`feat${i}`, false); + } + flagsIntegration.setFlag(`feat${bufferSize + 1}`, true); // eviction + flagsIntegration.setFlag('feat3', true); // update + return true; + }, FLAG_BUFFER_SIZE); + + const reqPromise = waitForErrorRequest(page); + await page.locator('#error').click(); // trigger error + 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: true }); + expectedFlags.push({ flag: 'feat3', result: true }); + + expect(event.contexts?.flags?.values).toEqual(expectedFlags); +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js new file mode 100644 index 000000000000..894f46aaa102 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js @@ -0,0 +1,12 @@ +import * as Sentry from '@sentry/browser'; + +window.Sentry = Sentry; + +// Not using this as we want to test the getIntegrationByName() approach +// window.sentryFeatureFlagsIntegration = Sentry.featureFlagsIntegration(); + +Sentry.init({ + dsn: 'https://public@dsn.ingest.sentry.io/1337', + sampleRate: 1.0, + integrations: [Sentry.featureFlagsIntegration()], +}); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/subject.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/subject.js new file mode 100644 index 000000000000..e6697408128c --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/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/featureFlags/template.html b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/template.html new file mode 100644 index 000000000000..9330c6c679f4 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/template.html @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts new file mode 100644 index 000000000000..e4f12411d1e5 --- /dev/null +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/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 flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); + + flagsIntegration.setFlag('shared', true); + + Sentry.withScope((scope: Scope) => { + flagsIntegration.setFlag('forked', true); + flagsIntegration.setFlag('shared', false); + scope.setTag('isForked', true); + if (errorButton) { + errorButton.click(); + } + }); + + flagsIntegration.setFlag('main', true); + 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: 'forked', result: true }, + { flag: 'shared', result: false }, + ]); + + expect(mainEvent.contexts?.flags?.values).toEqual([ + { flag: 'shared', result: true }, + { flag: 'main', result: true }, + ]); +}); diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index acb9b119d1a9..9ba819071dd2 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -69,5 +69,11 @@ export { makeBrowserOfflineTransport } from './transports/offline'; export { browserProfilingIntegration } from './profiling/integration'; export { spotlightBrowserIntegration } from './integrations/spotlight'; export { browserSessionIntegration } from './integrations/browsersession'; -export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './integrations/featureFlags/launchdarkly'; -export { openFeatureIntegration, OpenFeatureIntegrationHook } from './integrations/featureFlags/openfeature'; +export { + featureFlagsIntegration, + type FeatureFlagsIntegration, + launchDarklyIntegration, + buildLaunchDarklyFlagUsedHandler, + openFeatureIntegration, + OpenFeatureIntegrationHook, +} from './integrations/featureFlags'; diff --git a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts new file mode 100644 index 000000000000..6c05a28c5526 --- /dev/null +++ b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts @@ -0,0 +1,47 @@ +import type { Client, Event, EventHint, Integration, IntegrationFn } from '@sentry/core'; + +import { defineIntegration } from '@sentry/core'; +import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../utils/featureFlags'; + +export interface FeatureFlagsIntegration extends Integration { + setFlag: (name: string, value: unknown) => void; +} + +/** + * Sentry integration for buffering feature flags manually with an API, and + * capturing them on error events. We recommend you do this on each flag + * evaluation. Flags are buffered per Sentry scope and limited to 100 per event. + * + * See the [feature flag documentation](https://develop.sentry.dev/sdk/expected-features/#feature-flags) for more information. + * + * @example + * ``` + * import * as Sentry from '@sentry/browser'; + * import { featureFlagsIntegration, type FeatureFlagsIntegration } from '@sentry/browser'; + * + * // Setup + * Sentry.init(..., integrations: [featureFlagsIntegration()]) + * + * // Verify + * const flagsIntegration = Sentry.getClient()?.getIntegrationByName('FeatureFlags'); + * if (flagsIntegration) { + * flagsIntegration.setFlag('my-flag', true); + * } else { + * // check your setup + * } + * Sentry.captureException(Exception('broke')); // 'my-flag' should be captured to this Sentry event. + * ``` + */ +export const featureFlagsIntegration = defineIntegration(() => { + return { + name: 'FeatureFlags', + + processEvent(event: Event, _hint: EventHint, _client: Client): Event { + return copyFlagsFromScopeToEvent(event); + }, + + setFlag(name: string, value: unknown): void { + insertFlagToScope(name, value); + }, + }; +}) as IntegrationFn; diff --git a/packages/browser/src/integrations/featureFlags/index.ts b/packages/browser/src/integrations/featureFlags/index.ts new file mode 100644 index 000000000000..008a7e9016cc --- /dev/null +++ b/packages/browser/src/integrations/featureFlags/index.ts @@ -0,0 +1,4 @@ +export * from './launchdarkly'; +export * from './openfeature'; + +export { featureFlagsIntegration, type FeatureFlagsIntegration } from './featureFlagsIntegration'; From 675b0470d55af88de64c3bb8b4210e43f32010af Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Wed, 4 Dec 2024 19:15:09 -0800 Subject: [PATCH 2/5] review comments --- packages/browser/src/index.ts | 6 ++---- .../integrations/featureFlags/featureFlagsIntegration.ts | 4 ++-- packages/browser/src/integrations/featureFlags/index.ts | 3 --- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index 9ba819071dd2..e6f57c13fe6b 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -72,8 +72,6 @@ export { browserSessionIntegration } from './integrations/browsersession'; export { featureFlagsIntegration, type FeatureFlagsIntegration, - launchDarklyIntegration, - buildLaunchDarklyFlagUsedHandler, - openFeatureIntegration, - OpenFeatureIntegrationHook, } from './integrations/featureFlags'; +export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './integrations/featureFlags/launchdarkly'; +export { openFeatureIntegration, OpenFeatureIntegrationHook } from './integrations/featureFlags/openfeature'; diff --git a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts index 6c05a28c5526..23f06ea607de 100644 --- a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts +++ b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts @@ -17,10 +17,10 @@ export interface FeatureFlagsIntegration extends Integration { * @example * ``` * import * as Sentry from '@sentry/browser'; - * import { featureFlagsIntegration, type FeatureFlagsIntegration } from '@sentry/browser'; + * import { type FeatureFlagsIntegration } from '@sentry/browser'; * * // Setup - * Sentry.init(..., integrations: [featureFlagsIntegration()]) + * Sentry.init(..., integrations: [Sentry.featureFlagsIntegration()]) * * // Verify * const flagsIntegration = Sentry.getClient()?.getIntegrationByName('FeatureFlags'); diff --git a/packages/browser/src/integrations/featureFlags/index.ts b/packages/browser/src/integrations/featureFlags/index.ts index 008a7e9016cc..2106ee7accf0 100644 --- a/packages/browser/src/integrations/featureFlags/index.ts +++ b/packages/browser/src/integrations/featureFlags/index.ts @@ -1,4 +1 @@ -export * from './launchdarkly'; -export * from './openfeature'; - export { featureFlagsIntegration, type FeatureFlagsIntegration } from './featureFlagsIntegration'; From 94c120e251dd0b7b435b31286d1094c378a258b6 Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:07:43 -0800 Subject: [PATCH 3/5] Move api to top-level, rename addFlag --- .../featureFlags/featureFlags/basic/test.ts | 8 ++-- .../featureFlags/featureFlags/init.js | 3 -- .../featureFlags/withScope/test.ts | 9 ++-- packages/browser/src/index.ts | 2 +- .../featureFlags/featureFlagsIntegration.ts | 46 +++++++++---------- .../src/integrations/featureFlags/index.ts | 2 +- test-results/.last-run.json | 4 ++ 7 files changed, 37 insertions(+), 37 deletions(-) create mode 100644 test-results/.last-run.json diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts index 9cb4bf4b3f58..1d4da54d6676 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts @@ -23,12 +23,12 @@ sentryTest('Basic test with eviction, update, and no async tasks', async ({ getL await page.goto(url); await page.evaluate(bufferSize => { - const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); + const Sentry = (window as any).Sentry; for (let i = 1; i <= bufferSize; i++) { - flagsIntegration.setFlag(`feat${i}`, false); + Sentry.addFlag(`feat${i}`, false); } - flagsIntegration.setFlag(`feat${bufferSize + 1}`, true); // eviction - flagsIntegration.setFlag('feat3', true); // update + Sentry.addFlag(`feat${bufferSize + 1}`, true); // eviction + Sentry.addFlag('feat3', true); // update return true; }, FLAG_BUFFER_SIZE); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js index 894f46aaa102..51e214e157d8 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js @@ -2,9 +2,6 @@ import * as Sentry from '@sentry/browser'; window.Sentry = Sentry; -// Not using this as we want to test the getIntegrationByName() approach -// window.sentryFeatureFlagsIntegration = Sentry.featureFlagsIntegration(); - Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', sampleRate: 1.0, diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts index e4f12411d1e5..0f0f03d5dece 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts @@ -28,20 +28,19 @@ sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ await page.evaluate(() => { const Sentry = (window as any).Sentry; const errorButton = document.querySelector('#error') as HTMLButtonElement; - const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); - flagsIntegration.setFlag('shared', true); + Sentry.addFlag('shared', true); Sentry.withScope((scope: Scope) => { - flagsIntegration.setFlag('forked', true); - flagsIntegration.setFlag('shared', false); + Sentry.addFlag('forked', true); + Sentry.addFlag('shared', false); scope.setTag('isForked', true); if (errorButton) { errorButton.click(); } }); - flagsIntegration.setFlag('main', true); + Sentry.addFlag('main', true); Sentry.getCurrentScope().setTag('isForked', false); errorButton.click(); return true; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index e6f57c13fe6b..cb836ccb819e 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -71,7 +71,7 @@ export { spotlightBrowserIntegration } from './integrations/spotlight'; export { browserSessionIntegration } from './integrations/browsersession'; export { featureFlagsIntegration, - type FeatureFlagsIntegration, + addFlag, } from './integrations/featureFlags'; export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './integrations/featureFlags/launchdarkly'; export { openFeatureIntegration, OpenFeatureIntegrationHook } from './integrations/featureFlags/openfeature'; diff --git a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts index 23f06ea607de..a2419e415255 100644 --- a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts +++ b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts @@ -1,11 +1,8 @@ -import type { Client, Event, EventHint, Integration, IntegrationFn } from '@sentry/core'; +import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; -import { defineIntegration } from '@sentry/core'; +import { defineIntegration, getClient, logger } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../utils/featureFlags'; - -export interface FeatureFlagsIntegration extends Integration { - setFlag: (name: string, value: unknown) => void; -} +import { DEBUG_BUILD } from '../../debug-build'; /** * Sentry integration for buffering feature flags manually with an API, and @@ -17,19 +14,11 @@ export interface FeatureFlagsIntegration extends Integration { * @example * ``` * import * as Sentry from '@sentry/browser'; - * import { type FeatureFlagsIntegration } from '@sentry/browser'; * - * // Setup - * Sentry.init(..., integrations: [Sentry.featureFlagsIntegration()]) + * Sentry.init(..., integrations: [Sentry.featureFlagsIntegration()]); * - * // Verify - * const flagsIntegration = Sentry.getClient()?.getIntegrationByName('FeatureFlags'); - * if (flagsIntegration) { - * flagsIntegration.setFlag('my-flag', true); - * } else { - * // check your setup - * } - * Sentry.captureException(Exception('broke')); // 'my-flag' should be captured to this Sentry event. + * Sentry.addFlag('my-flag', true); + * Sentry.captureException(Exception('broke')); // 'my-flag' should be captured on this Sentry event. * ``` */ export const featureFlagsIntegration = defineIntegration(() => { @@ -38,10 +27,21 @@ export const featureFlagsIntegration = defineIntegration(() => { processEvent(event: Event, _hint: EventHint, _client: Client): Event { return copyFlagsFromScopeToEvent(event); - }, - - setFlag(name: string, value: unknown): void { - insertFlagToScope(name, value); - }, + } }; -}) as IntegrationFn; +}) as IntegrationFn; + + +/** + * Records a flag and its value to be sent on subsequent error events. We + * recommend you do this on flag evaluations. Flags are buffered per Sentry + * scope and limited to 100 per event. + */ +export function addFlag(name: string, value: unknown): void { + const client = getClient(); + if (!client || !client.getIntegrationByName('FeatureFlags')) { + DEBUG_BUILD && logger.error('Must enable the Feature Flags Integration to use the addFlag function.'); + return; + } + insertFlagToScope(name, value); +} diff --git a/packages/browser/src/integrations/featureFlags/index.ts b/packages/browser/src/integrations/featureFlags/index.ts index 2106ee7accf0..3b9a0d45bbd9 100644 --- a/packages/browser/src/integrations/featureFlags/index.ts +++ b/packages/browser/src/integrations/featureFlags/index.ts @@ -1 +1 @@ -export { featureFlagsIntegration, type FeatureFlagsIntegration } from './featureFlagsIntegration'; +export { featureFlagsIntegration, addFlag } from './featureFlagsIntegration'; diff --git a/test-results/.last-run.json b/test-results/.last-run.json new file mode 100644 index 000000000000..5fca3f84bc7b --- /dev/null +++ b/test-results/.last-run.json @@ -0,0 +1,4 @@ +{ + "status": "failed", + "failedTests": [] +} \ No newline at end of file From b28bc635195f6917fa7e907676473c95fea2d33b Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:21:37 -0800 Subject: [PATCH 4/5] Revert "Move api to top-level, rename addFlag" This reverts commit 94c120e251dd0b7b435b31286d1094c378a258b6. --- .../featureFlags/featureFlags/basic/test.ts | 8 ++-- .../featureFlags/featureFlags/init.js | 3 ++ .../featureFlags/withScope/test.ts | 9 ++-- packages/browser/src/index.ts | 2 +- .../featureFlags/featureFlagsIntegration.ts | 46 +++++++++---------- .../src/integrations/featureFlags/index.ts | 2 +- test-results/.last-run.json | 4 -- 7 files changed, 37 insertions(+), 37 deletions(-) delete mode 100644 test-results/.last-run.json diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts index 1d4da54d6676..9cb4bf4b3f58 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts @@ -23,12 +23,12 @@ sentryTest('Basic test with eviction, update, and no async tasks', async ({ getL await page.goto(url); await page.evaluate(bufferSize => { - const Sentry = (window as any).Sentry; + const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); for (let i = 1; i <= bufferSize; i++) { - Sentry.addFlag(`feat${i}`, false); + flagsIntegration.setFlag(`feat${i}`, false); } - Sentry.addFlag(`feat${bufferSize + 1}`, true); // eviction - Sentry.addFlag('feat3', true); // update + flagsIntegration.setFlag(`feat${bufferSize + 1}`, true); // eviction + flagsIntegration.setFlag('feat3', true); // update return true; }, FLAG_BUFFER_SIZE); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js index 51e214e157d8..894f46aaa102 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/init.js @@ -2,6 +2,9 @@ import * as Sentry from '@sentry/browser'; window.Sentry = Sentry; +// Not using this as we want to test the getIntegrationByName() approach +// window.sentryFeatureFlagsIntegration = Sentry.featureFlagsIntegration(); + Sentry.init({ dsn: 'https://public@dsn.ingest.sentry.io/1337', sampleRate: 1.0, diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts index 0f0f03d5dece..e4f12411d1e5 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts @@ -28,19 +28,20 @@ sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ await page.evaluate(() => { const Sentry = (window as any).Sentry; const errorButton = document.querySelector('#error') as HTMLButtonElement; + const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); - Sentry.addFlag('shared', true); + flagsIntegration.setFlag('shared', true); Sentry.withScope((scope: Scope) => { - Sentry.addFlag('forked', true); - Sentry.addFlag('shared', false); + flagsIntegration.setFlag('forked', true); + flagsIntegration.setFlag('shared', false); scope.setTag('isForked', true); if (errorButton) { errorButton.click(); } }); - Sentry.addFlag('main', true); + flagsIntegration.setFlag('main', true); Sentry.getCurrentScope().setTag('isForked', false); errorButton.click(); return true; diff --git a/packages/browser/src/index.ts b/packages/browser/src/index.ts index cb836ccb819e..e6f57c13fe6b 100644 --- a/packages/browser/src/index.ts +++ b/packages/browser/src/index.ts @@ -71,7 +71,7 @@ export { spotlightBrowserIntegration } from './integrations/spotlight'; export { browserSessionIntegration } from './integrations/browsersession'; export { featureFlagsIntegration, - addFlag, + type FeatureFlagsIntegration, } from './integrations/featureFlags'; export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './integrations/featureFlags/launchdarkly'; export { openFeatureIntegration, OpenFeatureIntegrationHook } from './integrations/featureFlags/openfeature'; diff --git a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts index a2419e415255..23f06ea607de 100644 --- a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts +++ b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts @@ -1,8 +1,11 @@ -import type { Client, Event, EventHint, IntegrationFn } from '@sentry/core'; +import type { Client, Event, EventHint, Integration, IntegrationFn } from '@sentry/core'; -import { defineIntegration, getClient, logger } from '@sentry/core'; +import { defineIntegration } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../utils/featureFlags'; -import { DEBUG_BUILD } from '../../debug-build'; + +export interface FeatureFlagsIntegration extends Integration { + setFlag: (name: string, value: unknown) => void; +} /** * Sentry integration for buffering feature flags manually with an API, and @@ -14,11 +17,19 @@ import { DEBUG_BUILD } from '../../debug-build'; * @example * ``` * import * as Sentry from '@sentry/browser'; + * import { type FeatureFlagsIntegration } from '@sentry/browser'; * - * Sentry.init(..., integrations: [Sentry.featureFlagsIntegration()]); + * // Setup + * Sentry.init(..., integrations: [Sentry.featureFlagsIntegration()]) * - * Sentry.addFlag('my-flag', true); - * Sentry.captureException(Exception('broke')); // 'my-flag' should be captured on this Sentry event. + * // Verify + * const flagsIntegration = Sentry.getClient()?.getIntegrationByName('FeatureFlags'); + * if (flagsIntegration) { + * flagsIntegration.setFlag('my-flag', true); + * } else { + * // check your setup + * } + * Sentry.captureException(Exception('broke')); // 'my-flag' should be captured to this Sentry event. * ``` */ export const featureFlagsIntegration = defineIntegration(() => { @@ -27,21 +38,10 @@ export const featureFlagsIntegration = defineIntegration(() => { processEvent(event: Event, _hint: EventHint, _client: Client): Event { return copyFlagsFromScopeToEvent(event); - } - }; -}) as IntegrationFn; - + }, -/** - * Records a flag and its value to be sent on subsequent error events. We - * recommend you do this on flag evaluations. Flags are buffered per Sentry - * scope and limited to 100 per event. - */ -export function addFlag(name: string, value: unknown): void { - const client = getClient(); - if (!client || !client.getIntegrationByName('FeatureFlags')) { - DEBUG_BUILD && logger.error('Must enable the Feature Flags Integration to use the addFlag function.'); - return; - } - insertFlagToScope(name, value); -} + setFlag(name: string, value: unknown): void { + insertFlagToScope(name, value); + }, + }; +}) as IntegrationFn; diff --git a/packages/browser/src/integrations/featureFlags/index.ts b/packages/browser/src/integrations/featureFlags/index.ts index 3b9a0d45bbd9..2106ee7accf0 100644 --- a/packages/browser/src/integrations/featureFlags/index.ts +++ b/packages/browser/src/integrations/featureFlags/index.ts @@ -1 +1 @@ -export { featureFlagsIntegration, addFlag } from './featureFlagsIntegration'; +export { featureFlagsIntegration, type FeatureFlagsIntegration } from './featureFlagsIntegration'; diff --git a/test-results/.last-run.json b/test-results/.last-run.json deleted file mode 100644 index 5fca3f84bc7b..000000000000 --- a/test-results/.last-run.json +++ /dev/null @@ -1,4 +0,0 @@ -{ - "status": "failed", - "failedTests": [] -} \ No newline at end of file From debbf09c95fb07b72f867c31666346ccc8f7b03a Mon Sep 17 00:00:00 2001 From: Andrew Liu <159852527+aliu39@users.noreply.github.com> Date: Fri, 6 Dec 2024 11:26:37 -0800 Subject: [PATCH 5/5] Rename to addFeatureFlag --- .../integrations/featureFlags/featureFlags/basic/test.ts | 6 +++--- .../featureFlags/featureFlags/withScope/test.ts | 8 ++++---- .../integrations/featureFlags/featureFlagsIntegration.ts | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts index 9cb4bf4b3f58..ed909b19d1fa 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/basic/test.ts @@ -25,10 +25,10 @@ sentryTest('Basic test with eviction, update, and no async tasks', async ({ getL await page.evaluate(bufferSize => { const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); for (let i = 1; i <= bufferSize; i++) { - flagsIntegration.setFlag(`feat${i}`, false); + flagsIntegration.addFeatureFlag(`feat${i}`, false); } - flagsIntegration.setFlag(`feat${bufferSize + 1}`, true); // eviction - flagsIntegration.setFlag('feat3', true); // update + flagsIntegration.addFeatureFlag(`feat${bufferSize + 1}`, true); // eviction + flagsIntegration.addFeatureFlag('feat3', true); // update return true; }, FLAG_BUFFER_SIZE); diff --git a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts index e4f12411d1e5..97ecf2d961a7 100644 --- a/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts +++ b/dev-packages/browser-integration-tests/suites/integrations/featureFlags/featureFlags/withScope/test.ts @@ -30,18 +30,18 @@ sentryTest('Flag evaluations in forked scopes are stored separately.', async ({ const errorButton = document.querySelector('#error') as HTMLButtonElement; const flagsIntegration = (window as any).Sentry.getClient().getIntegrationByName('FeatureFlags'); - flagsIntegration.setFlag('shared', true); + flagsIntegration.addFeatureFlag('shared', true); Sentry.withScope((scope: Scope) => { - flagsIntegration.setFlag('forked', true); - flagsIntegration.setFlag('shared', false); + flagsIntegration.addFeatureFlag('forked', true); + flagsIntegration.addFeatureFlag('shared', false); scope.setTag('isForked', true); if (errorButton) { errorButton.click(); } }); - flagsIntegration.setFlag('main', true); + flagsIntegration.addFeatureFlag('main', true); Sentry.getCurrentScope().setTag('isForked', false); errorButton.click(); return true; diff --git a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts index 23f06ea607de..01bc73190202 100644 --- a/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts +++ b/packages/browser/src/integrations/featureFlags/featureFlagsIntegration.ts @@ -4,7 +4,7 @@ import { defineIntegration } from '@sentry/core'; import { copyFlagsFromScopeToEvent, insertFlagToScope } from '../../utils/featureFlags'; export interface FeatureFlagsIntegration extends Integration { - setFlag: (name: string, value: unknown) => void; + addFeatureFlag: (name: string, value: unknown) => void; } /** @@ -25,7 +25,7 @@ export interface FeatureFlagsIntegration extends Integration { * // Verify * const flagsIntegration = Sentry.getClient()?.getIntegrationByName('FeatureFlags'); * if (flagsIntegration) { - * flagsIntegration.setFlag('my-flag', true); + * flagsIntegration.addFeatureFlag('my-flag', true); * } else { * // check your setup * } @@ -40,7 +40,7 @@ export const featureFlagsIntegration = defineIntegration(() => { return copyFlagsFromScopeToEvent(event); }, - setFlag(name: string, value: unknown): void { + addFeatureFlag(name: string, value: unknown): void { insertFlagToScope(name, value); }, };