Skip to content
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 featureFlagsIntegration for custom tracking of flag evaluations #14582

Merged
merged 6 commits into from
Dec 6, 2024

Conversation

aliu39
Copy link
Member

@aliu39 aliu39 commented Dec 4, 2024

Follow-up to #14207.

Sentry integration for buffering feature flags manually with an API and automatically capturing them on error events.

@aliu39 aliu39 requested review from billyvg and cmanallen December 4, 2024 22:07
@billyvg billyvg requested review from a team, lforst and chargome and removed request for a team December 4, 2024 22:46
@lforst lforst changed the title feat(flags): add featureFlagsIntegration for custom tracking of flag evaluations feat(flags): Add featureFlagsIntegration for custom tracking of flag evaluations Dec 5, 2024
lforst
lforst previously requested changes Dec 5, 2024
Copy link
Member

@lforst lforst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking because I don't want this to get accidentally merged nvm

@lforst lforst dismissed their stale review December 5, 2024 10:04

unblock

@aliu39
Copy link
Member Author

aliu39 commented Dec 5, 2024

@lforst I discussed with @billyvg and we're ok with making setFlag a top-level API fx (gonna rename it captureFlag). It'll log an error and return if the integration isn't installed. Does this sound good? Lmk and I can make the changes tomorrow

@lforst
Copy link
Member

lforst commented Dec 6, 2024

@lforst I discussed with @billyvg and we're ok with making setFlag a top-level API fx (gonna rename it captureFlag). It'll log an error and return if the integration isn't installed. Does this sound good? Lmk and I can make the changes tomorrow

@aliu39 Sounds great. I would personally call it setFlag or addFlag, since "capture" sounds to me like sending a 1-time thing to Sentry but that's up to you.

@billyvg
Copy link
Member

billyvg commented Dec 6, 2024

addFlag sounds good to me, setFlag is a bit confusing because it sounds like you're setting/changing the flag value.

@aliu39 aliu39 requested a review from billyvg December 6, 2024 19:30
Copy link

codecov bot commented Dec 6, 2024

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
621 1 620 300
View the top 1 failed tests by shortest run time
client-app-routing-instrumentation.test.ts Creates a navigation transaction for app router routes
Stack Traces | 30s run time
client-app-routing-instrumentation.test.ts:19:5 Creates a navigation transaction for app router routes

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

@aliu39 aliu39 enabled auto-merge (squash) December 6, 2024 19:46
@aliu39 aliu39 merged commit a985d64 into develop Dec 6, 2024
123 checks passed
@aliu39 aliu39 deleted the aliu/ff-integration branch December 6, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants