ref(flags): instrument sdk featureFlagsIntegration to track FE flag evals #81954
Conversation
This file contains hidden or 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
github-actions
bot
added
the
Scope: Frontend
billyvg
reviewed
Dec 11, 2024
billyvg
reviewed
Dec 11, 2024
static/app/utils/featureFlags.ts
Outdated
| * the Sentry SDK, in the event context. If the FeatureFlagsIntegration is not | ||
| * installed, the callback is a no-op. | ||
| */ | ||
| export function getSentryFeaturesHook(): (name: string, value: unknown) => void { |
There was a problem hiding this comment.
This doesn't really hook into anything
There was a problem hiding this comment.
wdym? "hook" is the term I'm using to describe the callback taking in a flag name/val. This term is used sometimes in SDK setup code
There was a problem hiding this comment.
sorry this is just semantics but you're not really returning a hook, but rather a handler for your hook. the "hook" is when features.includes() gets called.
There was a problem hiding this comment.
ok got it, what if I rename all instances of "hook" to "handler"?
…sentry into aliu/use-js-flags-integration
Bundle ReportChanges will increase total bundle size by 89.16kB (0.28%) ⬆️. This is within the configured threshold ✅ Detailed changes
|
billyvg
approved these changes
Dec 16, 2024
aliu39
added a commit
to getsentry/sentry-javascript
that referenced
this pull request
Dec 16, 2024
…#14741) This isn't exported in types-hoist/index, but we found users can still import it from a `build/` file. Discussed with @billyvg we don't want users to accidentally import it this way. Note we currently use this type in sentry, until getsentry/sentry#81954 is rolled out.
evanh
pushed a commit
that referenced
this pull request
Dec 17, 2024
…vals (#81954) Re-opening https://github.com/getsentry/sentry/pull/81159/files after releasing a custom flag tracking integration in JS SDK 8.43. WIth this integration, we don't need to manage our own buffer in featureObserver anymore. Sentry employees shouldn't notice any changes, this is a proof-of-concept for the new SDK. The SDK has its own tests already, so I'm only unit testing the add(__)FeaturesHook fxs. Manually tested with dev-ui sending to a test org: https://andrew-onboarding.sentry.io/share/issue/664703eb3a98441b8a203dc44e803cee/
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Re-opening https://github.com/getsentry/sentry/pull/81159/files after releasing a custom flag tracking integration in JS SDK 8.43. WIth this integration, we don't need to manage our own buffer in featureObserver anymore. Sentry employees shouldn't notice any changes, this is a proof-of-concept for the new SDK.
The SDK has its own tests already, so I'm only unit testing the add(__)FeaturesHook fxs.
Manually tested with dev-ui sending to a test org: https://andrew-onboarding.sentry.io/share/issue/664703eb3a98441b8a203dc44e803cee/