-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(flags): Add LaunchDarkly integration #14207
Open
aliu39
wants to merge
62
commits into
develop
Choose a base branch
from
aliu/launch-darkly-integration
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+463
−0
Open
Changes from all commits
Commits
Show all changes
62 commits
Select commit
Hold shift + click to select a range
26722d7
Init package files
aliu39 72f9cc0
Merge branch 'develop' into aliu/launch-darkly
aliu39 419cd83
Revert changelog and move types file
aliu39 610da4d
Add ld to dependencies and skeleton code. Get rid of core/
aliu39 5b2e5ec
Fix readme, rename types file, bring back core/
aliu39 66b1253
Fix readme 2
aliu39 023604a
Merge branch 'develop' into aliu/launch-darkly
aliu39 1f99c28
Merge branch 'aliu/launch-darkly' of https://github.com/getsentry/sen…
aliu39 85acc6d
Finish implementing, minus scope.flags
aliu39 3b3a767
Implement flag buffer in sentry scope
aliu39 ab3181d
Revert changelog
aliu39 22684ea
fix types
michellewzhang 79e5b24
docstring
michellewzhang 63649c5
Export FeatureFlag type in index.ts
aliu39 923500f
Merge branch 'aliu/launch-darkly' of https://github.com/getsentry/sen…
aliu39 bd04755
Clean up comments
aliu39 91a4db9
Fix build (uses yalc for scope changes) and use LRUMap util
aliu39 1849673
Merge branch 'develop' into aliu/launch-darkly
aliu39 1ca9d53
Tweak hook name and docs
aliu39 893cd62
FlagBuffer class and interface, add to scope._contexts. Remove old fl…
aliu39 f172683
Call clone in scope.clone()
aliu39 b96c17f
Rewrite as a util fx instead of class. Use in LD integration
aliu39 f9a659b
Delete LD pkg
aliu39 d9c5860
Delete LD from package.json and update lock
aliu39 cf24b75
Readd LD integration
aliu39 5561633
Fix dependencies and rename exports
aliu39 6eab55f
Fix normalize depth of 3 for contexts.flags, improve docstrings
aliu39 d90d346
Rm eslint rules, todos
aliu39 19ccb19
Merge branch 'develop' into aliu/launch-darkly-integration
aliu39 548858d
Update sentry dependency vers
aliu39 c289e63
Update insertToFlagBuffer and unit test it
aliu39 f5c6995
Move ld to devDependencies and rm shim-preact from build:types
aliu39 334aa51
Move to browser integrations, delete package
aliu39 c27f766
Init browser-integration-tests
aliu39 948cea7
Fix test
aliu39 d371316
Merge branch 'develop' into aliu/launch-darkly-integration
aliu39 aef6dba
Finialize test and make handler synchronous
aliu39 7b14926
Move util to browser pkg
aliu39 8ec8680
Review comments, fix buffer size import, clean up todos
aliu39 6e22c17
Restructure src and test modules, rename util and type files
aliu39 dc7e74a
Inline LD types
aliu39 3c686c5
Copy flags when attaching to event
aliu39 88f8251
Add withScope test and delete bad not modified test
aliu39 05036ae
Merge branch 'develop' into aliu/launch-darkly-integration
aliu39 fc79f4a
Del unnecesary devDep
aliu39 7a5f29d
Log insertToFlagBuffer error instead of throwing
aliu39 7e24158
Merge branch 'develop' into aliu/launch-darkly-integration
aliu39 969c5c0
Fix playwright tests
aliu39 ac8faf7
Use Sentry namespace in init
aliu39 b1cb0a5
(Temporarily?) move to exports
aliu39 3b784a6
Skip CDN bundle tests, fix unit test, un-export FLAG_BUFFER_SIZE, upd…
aliu39 b93c062
Merge branch 'develop' into aliu/launch-darkly-integration
aliu39 8ffafd1
Revert lockfile
aliu39 83ee34b
biome formatting
aliu39 a2ba257
Bump size limits for vue (w/tracing) and browser (w/async feedback)
aliu39 611df2c
Merge branch 'develop' into aliu/launch-darkly-integration
aliu39 be9ccc2
Merge branch 'develop' into aliu/launch-darkly-integration
billyvg 7b3382f
make `shouldSkipLaunchDarklyTest` to be generic for feature flags
billyvg d292da2
Merge branch 'develop' into aliu/launch-darkly-integration
billyvg c4ba65b
fix tests
billyvg fb0bf05
Update dev-packages/browser-integration-tests/utils/helpers.ts
billyvg 90577bf
import from core
billyvg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
48 changes: 48 additions & 0 deletions
48
...ges/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/basic/test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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.waitForFunction(bufferSize => { | ||
const ldClient = (window as any).initializeLD(); | ||
for (let i = 1; i <= bufferSize; i++) { | ||
ldClient.variation(`feat${i}`, false); | ||
} | ||
ldClient.variation(`feat${bufferSize + 1}`, true); // eviction | ||
ldClient.variation('feat3', true); // update | ||
return true; | ||
}, 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: true }); | ||
expectedFlags.push({ flag: 'feat3', result: true }); | ||
|
||
expect(event.contexts?.flags?.values).toEqual(expectedFlags); | ||
}); |
35 changes: 35 additions & 0 deletions
35
dev-packages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/init.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import * as Sentry from '@sentry/browser'; | ||
|
||
window.Sentry = Sentry; | ||
window.sentryLDIntegration = Sentry.launchDarklyIntegration(); | ||
|
||
Sentry.init({ | ||
dsn: 'https://[email protected]/1337', | ||
sampleRate: 1.0, | ||
integrations: [window.sentryLDIntegration], | ||
}); | ||
|
||
// Manually mocking this because LD only has mock test utils for the React SDK. | ||
// Also, no SDK has mock utils for FlagUsedHandler's. | ||
const MockLaunchDarkly = { | ||
initialize(_clientId, context, options) { | ||
const flagUsedHandler = options && options.inspectors ? options.inspectors[0].method : undefined; | ||
|
||
return { | ||
variation(key, defaultValue) { | ||
if (flagUsedHandler) { | ||
flagUsedHandler(key, { value: defaultValue }, context); | ||
} | ||
return defaultValue; | ||
}, | ||
}; | ||
}, | ||
}; | ||
|
||
window.initializeLD = () => { | ||
return MockLaunchDarkly.initialize( | ||
'example-client-id', | ||
{ kind: 'user', key: 'example-context-key' }, | ||
{ inspectors: [Sentry.buildLaunchDarklyFlagUsedHandler()] }, | ||
); | ||
}; |
3 changes: 3 additions & 0 deletions
3
...ckages/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/subject.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
document.getElementById('error').addEventListener('click', () => { | ||
throw new Error('Button triggered error'); | ||
}); |
9 changes: 9 additions & 0 deletions
9
...ges/browser-integration-tests/suites/integrations/featureFlags/launchdarkly/template.html
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
<!DOCTYPE html> | ||
<html> | ||
<head> | ||
<meta charset="utf-8" /> | ||
</head> | ||
<body> | ||
<button id="error">Throw Error</button> | ||
</body> | ||
</html> |
65 changes: 65 additions & 0 deletions
65
...browser-integration-tests/suites/integrations/featureFlags/launchdarkly/withScope/test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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.waitForFunction(() => { | ||
const Sentry = (window as any).Sentry; | ||
const errorButton = document.querySelector('#error') as HTMLButtonElement; | ||
const ldClient = (window as any).initializeLD(); | ||
|
||
ldClient.variation('shared', true); | ||
|
||
Sentry.withScope((scope: Scope) => { | ||
ldClient.variation('forked', true); | ||
ldClient.variation('shared', false); | ||
scope.setTag('isForked', true); | ||
if (errorButton) { | ||
errorButton.click(); | ||
} | ||
}); | ||
|
||
ldClient.variation('main', true); | ||
Sentry.getCurrentScope().setTag('isForked', false); | ||
errorButton.click(); | ||
return true; | ||
}); | ||
Comment on lines
+28
to
+48
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same here re: page.evaluate? |
||
|
||
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 }, | ||
]); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1
packages/browser/src/integrations/featureFlags/launchdarkly/index.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { launchDarklyIntegration, buildLaunchDarklyFlagUsedHandler } from './integration'; |
68 changes: 68 additions & 0 deletions
68
packages/browser/src/integrations/featureFlags/launchdarkly/integration.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import type { Client, Event, EventHint, IntegrationFn } from '@sentry/types'; | ||
import type { LDContext, LDEvaluationDetail, LDInspectionFlagUsedHandler } from './types'; | ||
|
||
import { defineIntegration, getCurrentScope } from '@sentry/core'; | ||
import { insertToFlagBuffer } from '../../../utils/featureFlags'; | ||
|
||
/** | ||
* Sentry integration for capturing feature flags from LaunchDarkly. | ||
* | ||
* 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 {launchDarklyIntegration, buildLaunchDarklyFlagUsedInspector} from '@sentry/browser'; | ||
* import * as LaunchDarkly from 'launchdarkly-js-client-sdk'; | ||
* | ||
* Sentry.init(..., integrations: [launchDarklyIntegration()]) | ||
* const ldClient = LaunchDarkly.initialize(..., {inspectors: [buildLaunchDarklyFlagUsedHandler()]}); | ||
* ``` | ||
*/ | ||
export const launchDarklyIntegration = defineIntegration(() => { | ||
return { | ||
name: 'LaunchDarkly', | ||
|
||
processEvent(event: Event, _hint: EventHint, _client: Client): Event { | ||
const scope = getCurrentScope(); | ||
const flagContext = scope.getScopeData().contexts.flags; | ||
const flagBuffer = flagContext ? flagContext.values : []; | ||
|
||
if (event.contexts === undefined) { | ||
event.contexts = {}; | ||
} | ||
event.contexts.flags = { values: [...flagBuffer] }; | ||
return event; | ||
}, | ||
}; | ||
}) satisfies IntegrationFn; | ||
|
||
/** | ||
* LaunchDarkly hook that listens for flag evaluations and updates the `flags` | ||
* context in our Sentry scope. This needs to be registered as an | ||
* 'inspector' in LaunchDarkly initialize() options, separately from | ||
* `launchDarklyIntegration`. Both are needed to collect feature flags on error. | ||
*/ | ||
export function buildLaunchDarklyFlagUsedHandler(): LDInspectionFlagUsedHandler { | ||
return { | ||
name: 'sentry-flag-auditor', | ||
type: 'flag-used', | ||
|
||
synchronous: true, | ||
|
||
/** | ||
* Handle a flag evaluation by storing its name and value on the current scope. | ||
*/ | ||
method: (flagKey: string, flagDetail: LDEvaluationDetail, _context: LDContext) => { | ||
if (typeof flagDetail.value === 'boolean') { | ||
const scopeContexts = getCurrentScope().getScopeData().contexts; | ||
if (!scopeContexts.flags) { | ||
scopeContexts.flags = { values: [] }; | ||
} | ||
const flagBuffer = scopeContexts.flags.values; | ||
insertToFlagBuffer(flagBuffer, flagKey, flagDetail.value); | ||
} | ||
return; | ||
}, | ||
}; | ||
} |
50 changes: 50 additions & 0 deletions
50
packages/browser/src/integrations/featureFlags/launchdarkly/types.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/** | ||
* Inline definitions of LaunchDarkly types so we don't have to include their | ||
* SDK in devDependencies. These are only for type-checking and can be extended | ||
* as needed - for exact definitions, reference `launchdarkly-js-client-sdk`. | ||
*/ | ||
|
||
/** | ||
* Currently, the Sentry integration does not read from values of this type. | ||
*/ | ||
export type LDContext = object; | ||
|
||
/** | ||
* An object that combines the result of a feature flag evaluation with information about | ||
* how it was calculated. | ||
*/ | ||
export interface LDEvaluationDetail { | ||
value: unknown; | ||
// unused optional props: variationIndex and reason | ||
} | ||
|
||
/** | ||
* Callback interface for collecting information about the SDK at runtime. | ||
* | ||
* This interface is used to collect information about flag usage. | ||
* | ||
* This interface should not be used by the application to access flags for the purpose of controlling application | ||
* flow. It is intended for monitoring, analytics, or debugging purposes. | ||
*/ | ||
export interface LDInspectionFlagUsedHandler { | ||
type: 'flag-used'; | ||
|
||
/** | ||
* Name of the inspector. Will be used for logging issues with the inspector. | ||
*/ | ||
name: string; | ||
|
||
/** | ||
* If `true`, then the inspector will be ran synchronously with evaluation. | ||
* Synchronous inspectors execute inline with evaluation and care should be taken to ensure | ||
* they have minimal performance overhead. | ||
*/ | ||
synchronous?: boolean; | ||
|
||
/** | ||
* This method is called when a flag is accessed via a variation method, or it can be called based on actions in | ||
* wrapper SDKs which have different methods of tracking when a flag was accessed. It is not called when a call is made | ||
* to allFlags. | ||
*/ | ||
method: (flagKey: string, flagDetail: LDEvaluationDetail, context: LDContext) => void; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,59 @@ | ||
import { logger } from '@sentry/core'; | ||
import type { FeatureFlag } from '@sentry/types'; | ||
import { DEBUG_BUILD } from '../debug-build'; | ||
|
||
/** | ||
* Ordered LRU cache for storing feature flags in the scope context. The name | ||
* of each flag in the buffer is unique, and the output of getAll() is ordered | ||
* from oldest to newest. | ||
*/ | ||
|
||
/** | ||
* Max size of the LRU flag buffer stored in Sentry scope and event contexts. | ||
*/ | ||
export const FLAG_BUFFER_SIZE = 100; | ||
|
||
/** | ||
* Insert into a FeatureFlag array while maintaining ordered LRU properties. Not | ||
* thread-safe. After inserting: | ||
* - `flags` is sorted in order of recency, with the newest flag at the end. | ||
* - No other flags with the same name exist in `flags`. | ||
* - The length of `flags` does not exceed `maxSize`. The oldest flag is evicted | ||
* as needed. | ||
* | ||
* @param flags The array to insert into. | ||
* @param name Name of the feature flag to insert. | ||
* @param value Value of the feature flag. | ||
* @param maxSize Max number of flags the buffer should store. It's recommended | ||
* to keep this consistent across insertions. Default is DEFAULT_MAX_SIZE | ||
*/ | ||
export function insertToFlagBuffer( | ||
flags: FeatureFlag[], | ||
name: string, | ||
value: boolean, | ||
maxSize: number = FLAG_BUFFER_SIZE, | ||
): void { | ||
if (flags.length > maxSize) { | ||
DEBUG_BUILD && logger.error(`[Feature Flags] insertToFlagBuffer called on a buffer larger than maxSize=${maxSize}`); | ||
return; | ||
} | ||
|
||
// Check if the flag is already in the buffer - O(n) | ||
const index = flags.findIndex(f => f.flag === name); | ||
|
||
if (index !== -1) { | ||
// The flag was found, remove it from its current position - O(n) | ||
flags.splice(index, 1); | ||
} | ||
|
||
if (flags.length === maxSize) { | ||
// If at capacity, pop the earliest flag - O(n) | ||
flags.shift(); | ||
} | ||
|
||
// Push the flag to the end - O(1) | ||
flags.push({ | ||
flag: name, | ||
result: value, | ||
}); | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can just be
page.evaluate
right since we're not waiting for anything async inside