Support groups in feature flags

macobo commented 3 years ago

Is your feature request related to a problem?

Usecases:

I want to do roll-outs via groups rather than users (e.g. roll out to 20% of organizations)
I want to toggle feature flag for groups having properties (e.g. roll out feature to Posthog, enable "simple mode" for small users, etc)

Describe the solution you'd like

In the UI, users can choose to "aggregate" by a certain group type. In that case, we roll out by that group type instead of users.

You should know what groups user is part of when calling isFeatureEnabled, which should forward groups info to /decide endpoint

In JS, we can rely on posthog.groups calls and persisted group information when querying decide + reload flags when groups change.

In other client libraries, we should add possibility passing groups parameter when querying a flag.

Let's not allow mixing person and group properties as that can cause integration issues. Instead you can only choose properties you're aggregating on. Reasoning: Users might change the flag and be confused why flag is disabled for all if group is not specified.

UI should have clear indicators that groups require changing feature flag calls.

Describe alternatives you've considered

Not supporting groups and feature flags

Additional context

Considerations:

Timing issues. In posthog-js .loaded should be called before /decide
We should call /decide once when .groups gets called - debounce for say 20ms?
We need to query clickhouse for person properties - yuck. Need to calibrate that as needed.

Thank you for your feature request – we love each and every one!

weyert commented 3 years ago

Thank you for creating this issue, my main use case, is operations flags. @macobo Would this the solution for my earlier issue? https://github.com/PostHog/posthog/issues/5969

Based on a discussion with @samwinslow and the above mentioned ticket I think my wish would be achievable with this change. Love to hear your thoughts about it @macobo

In my service were I want to control logic based on whether the feature flag is enabled/disabled would have some initialisation code at startup, like:

// initialise the groups so we can check feature flags against
posthognode.group('store:1',  { properties: { killswitch: true} })
posthognode.group('store:2',  { properties: { killswitch: true} })
posthognode.group('store:3,  { properties: { killswitch: true} })
// create catch-all group for other kind of flags
posthognode.group('operations')

// in some request handler or job queuer

// check if the feature flag is enabled for the given store
const shouldQueueJob = posthogNode.isFeatureEnabled('allow-pricing-syncing', { group: 'store:1' })
if (shouldQueueJob) {
   jobScheduler.enqueueJob(`pricing-sync`, TimeSpan.fromMinutes(5))
}

macobo commented 3 years ago

Yes I believe so!

Note the example would not work quite like that, but instead something like that:

// initialise the groups so we can check feature flags against
posthognode.group('store', 'id:1', { properties: { killswitch: true} })
posthognode.group('store', 'id:2',   { properties: { killswitch: true} })
posthognode.group('store', 'id:3',   { properties: { killswitch: true} })

// in some request handler or job queuer

// check if the feature flag is enabled for the given store
const shouldQueueJob = posthogNode.isFeatureEnabled('allow-pricing-syncing', { store: 'id:1' })
if (shouldQueueJob) {
   jobScheduler.enqueueJob(`pricing-sync`, TimeSpan.fromMinutes(5))
}

However the feature is still under development for now - We'll update this ticket + make a proper release announcement with docs as we're readying completion! :)

weyert commented 3 years ago

@macobo Awesome, sounds really promising! Yeah my code snipper was mostly made up based on my conversation with @samwinslow and what I understood from the groups PR from posthog-js. Thank you for your revised code snippet looks good to me

weyert commented 3 years ago

@macobo If you think I can help with the development work of this issue; don't hesitate to reach out to me on Posthog Users Slack. Happy to help to realise this

macobo commented 3 years ago

Thank you!

cc @clarkus we should think through the needed design changes on feature flag pages.

macobo commented 3 years ago

This is currently soft-blocked by work @tiina303 and @yakkomajuri are doing regarding person data consistency.

We don't store groups with properties in postgres right now, but for this we might need to due to latency constraints within /decide endpoint. Will try and reach out and understand what the latest state of their work is.

clarkus commented 3 years ago

I have an initial design for supporting groups in feature flags at https://www.figma.com/file/gQBj9YnNgD8YW4nBwCVLZf/PostHog-App?node-id=5400%3A35869. There are inline comments there with open questions as well as some context setting.

weyert commented 3 years ago

@macobo Looking at the changes in posthog-node it appears that API has changed? If I understand it all correctly instead of calling .group it's now groupIdentify?

macobo commented 3 years ago

We now have support for this (behind a feature flag). We'll be releasing this next release. :)

weyert commented 3 years ago

Yes, after playing with it yesterday it works great! Only the small issue with the zero rollout percentage.

Thank all of you at Posthog, really happy with this :D

PostHog / posthog