PostHog / posthog-go

Official PostHog Go library
MIT License
21 stars 18 forks source link

Bug in local evaluation of feature flags without multivariates when calling GetFeatureFlag #77

Open dpovey opened 1 month ago

dpovey commented 1 month ago

I have a boolean flag that is defined with filters with seperate rollout percentages.

Here is a rough structure of the flag

flag posthog.FeatureFlag = {
  Key = ...
  IsSimpleFlag = false
  RolloutPercentage = nil
  Active = true
  Filters = {
    AggregationGroupTypeIndex = nil
    Groups = [
        { 
            Properties = ...
            RolloutPercentage = 50
            Variant = nil
        },
        ...
    ],
    Multivariate = nil,
   Payloads = {}
   }
   EnsureExperienceContinuity = false
  }

I can see that my condition matches in matchFeatureFlagProperties, but in the processing of this code:

  if isMatch {
            variantOverride := condition.Variant
            multivariates := flag.Filters.Multivariate

            if variantOverride != nil && multivariates != nil && multivariates.Variants != nil && containsVariant(multivariates.Variants, *variantOverride) {
                return *variantOverride, nil
            } else {
                return getMatchingVariant(flag, distinctId)
            }
        }

It doesn't correctly handle the case where there are no variants. It calls getMatchingVariant when it could just cal checkIfSimpleFlagEnabled like so:

                if isMatch {
            variantOverride := condition.Variant
            multivariates := flag.Filters.Multivariate

            if variantOverride != nil && multivariates != nil && multivariates.Variants != nil && containsVariant(multivariates.Variants, *variantOverride) {
                return *variantOverride, nil
            } else if multivariates == nil || multivariates.Variants == nil || len(multivariates.Variants) == 0 {
                // Handle boolean flag with condition-specific rollout percentage
                if condition.RolloutPercentage != nil {
                    return checkIfSimpleFlagEnabled(flag.Key, distinctId, *condition.RolloutPercentage)
                }
                return true, nil
            } else {
                return getMatchingVariant(flag, distinctId)
            }
        }

Or perhaps some variation that has a different salt. This is preventing me being able to use Posthog as I need to be able to locally evaluate these flags.

counterleft commented 4 days ago

@dpovey I'm curious, what did your getFeatureFlag() call look like?

I also had a boolean flag that evaluated against filters (against users) and it worked when I used PersonProperties:

flagValue, _ := client.GetFeatureFlag(
    FeatureFlagPayload{
        Key:              "flagName",
        DistinctId:       userDistinctId,
        PersonProperties: NewProperties().Set("distinct_id", userDistinctId),
    },
)

It didn't work without the PersonProperties. I'm guessing that Key + DistinctId only evaluates for simple flags (and using filters makes a flag not simple).

My test flag looked like:

"flags": [
    {
      "id": 1,
      "name": "",
      "key": "test-boolean-flag-with-rollout-conditions",
      "filters": {
        "groups": [
          {
            "properties": [
              {
                "key": "distinct_id",
                "type": "person",
                "value": ["1"],
                "operator": "exact"
              }
            ],
            "rollout_percentage": 100
          }
        ]
      },
      "deleted": false,
      "active": true,
      "is_simple_flag": false,
      "rollout_percentage": null
    }
  ]

I realized my getFeatureFlag() call mimics what the create-a-flag documentation on PostHog's Admin recommends as well:

Screenshot 2024-11-29 at 19 38 12