-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add Flagsmith as feature flag provider #14126
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
Add Flagsmith as feature flag provider #14126
Conversation
@emyller is attempting to deploy a commit to the Sentry Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
Oh this is great! Thanks @emyller Tagging some folks for review here, approved the vercel preview so we can peek at the rendered site |
docs/organization/integrations/feature-flag/flagsmith/index.mdx
Outdated
Show resolved
Hide resolved
docs/organization/integrations/feature-flag/flagsmith/index.mdx
Outdated
Show resolved
Hide resolved
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.
For platform docs, I believe it's not our convention to add pages under integrations/
if there's no integration exported from Sentry. Since we're using openfeature and setup code is the same, I think we could mention the Flagsmith provider and how to import and initialize it, under a section of under "Evaluation Tracking"/integrations/openfeature.mdx
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.
Whipping up a draft of what I think could work.. I'll link it here when done
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.
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.
I'd recommend using this to update the eval tracking section for the organization/integrations/
page, and reverting the changes under docs/platforms/
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.
Sorry if this way of suggesting changes was a little janky 😬 I needed a way to see things on my local dev mode, so I checked out sentry's master branch. Thanks again for adding these docs!
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.
Thanks for the patch! I've applied it with a few edits:
- Kept the screenshot. As per this comment, let me know if it was also intentional.
- Fixed a couple typos.
Also reverted all changes from docs/platforms/
and platform-includes/
— I assume the latter is no longer needed, although this change connects to:
(...) I think we could mention the Flagsmith provider and how to import and initialize it, under a section of
/integrations/openfeature.mdx
.
Ideally, we'd love to see Sentry shipping with Flagsmith integrations and proper documentation. I'm not sure adding a Flagsmith section to the OpenFeature documentation is the right architecture but I'm good with it as a first iteration. We'll coordinate that. 😉
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.
Yup edits look good!
Ideally, we'd love to see Sentry shipping with Flagsmith integrations and proper documentation. I'm not sure adding a Flagsmith section to the OpenFeature documentation is the right architecture but I'm good with it as a first iteration. We'll coordinate that. 😉
Sounds good - if Sentry integrates directly with flagsmith SDKs in the future, we can add to platform docs. Before that I think it's best to link to Openfeature from flagsmith, with the /organizations/
page serving as the integration home page (both eval and change tracking). The other providers follow a similar pattern.
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.
I couldn't find the best place in Sentry docs under /organizations/
to add an instrumentation example for Evaluation Tracking. Instead, I've opened a pull request to our own docs with some important bits regarding OpenFeature. Feel free to help us validate it!
Co-authored-by: Andrew Liu <[email protected]>
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.
Looks good! Thank you!
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.
Thanks @emyller! Really great change. Looking forward to having this in our docs :D
@aliu39 I'm review blocking this temporarily. Before merging please perform hands-on-testing and verify the flagsmith openfeature provider and webhook works prior to merging this PR (if you've already done this @ me and I'll approve!).
@cmanallen @emyller I've verified both the eval and change tracking to work, following the steps from these docs. Pretty smooth setup, nice work! |
@aliu39 Thanks so much for the collaboration! Let me know if there's anything we should do on our side before merging. Enjoy the weekend! |
@emyller feel free to merge! CI was blocked on vercel but I authorized it, sorry we missed it earlier |
@aliu39 I would love to! Though it looks like I don't have the option in the UI. There's also a CI error "Codeowner assignment". Can you help? 🙇 |
@emyller oh I see, I can merge for you |
DESCRIBE YOUR PR
This adds Flagsmith as an option to integrate with feature flags in Sentry.
We made some decisions on how to approach the integrations for now:
Please let us know if that works. I hope the patch looks good!
IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes:
LEGAL BOILERPLATE
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
EXTRA RESOURCES