fix(flags): correctly emit feature flag events with the FF response on get_feature_flag_payload calls

[dmarticus]

Issue

The Python SDK incorrectly sends feature flag events with null values when calling get_feature_flag_payload(), even though the actual payload is successfully retrieved and returned to the user.

Behavior

When calling get_feature_flag_payload(), the following sequence occurs:

1. get_feature_flag_payload() internally calls get_feature_flag() to get the match value
2. During this call, get_feature_flag() is forced to use only_evaluate_locally=True (different from the default passed to get_feature_flag_payload)
3. An event is sent immediately with null values:

{
    "properties": {
        "$feature_flag": "config-right-brain",
        "$feature_flag_response": null,
        "locally_evaluated": false,
        "$feature/config-right-brain": null
    },
    "event": "$feature_flag_called"
}

The code then proceeds to call /decide to get the actual payload
The correct payload is returned to the user, but no updated event is sent

Root Cause

The issue stems from forcing only_evaluate_locally=True when getting the match value in get_feature_flag_payload(). This causes the SDK to:

• Try local evaluation first
• Send an event immediately with null values
• Then fall back to remote evaluation to get the payload
• Never update the originally sent event with the actual values

Fix

Removed the forced only_evaluate_locally=True setting in get_feature_flag_payload() and instead use the value passed to the method. This ensures the event is sent with the correct feature flag response after evaluation is complete.

This fixes the issue described here: https://posthoghelp.zendesk.com/agent/tickets/20416

[dmarticus]

testing that we call capture when calling decide with match_value as None.

[dmarticus]

full suite of tests confirming that get_feature_flag_payload sends events with actual data in the responses.

[dmarticus]

the bug in this case was that we were not evaluating the feature flag payload if we couldn't compute it locally, which shouldn't happen in the event where we still want to use remote evaluation at some point – ideally we won't fall back, but we should sometimes, and making this payload explicitly true was preventing us from falling back always.

[dmarticus]

@EDsCODE you wrote this originally; given that this pattern feels to intentionally deviate from the other function arguments, is there any reason why you did this originally? I want to make sure I'm not regressing some other behavior.

[neilkakkar]

the problem now is you're making two requests to /decide in some cases to fetch the payload (once to get the match value, 2nd time to get the payload).

I can see both cases being not ideal though! I think I'd recommend setting send_feature_flag_events to False here, and add a comment about this, and then manually handle sending the event in the right case in the end.

Also, I expect all SDKs for payloads to follow this same format, so might have the same problem, worth a quick check in posthog-node atleast (the other popular SDK)

[dmarticus]

I like that idea! I feel like I want to be a bit more clever than that, though, because I don't want the event to contain the feature flag payload as the feature_flag_response, I still want to track whatever response the feature flag returned. A few things I could do on that front:

Add a new method to return both the response and payload, and use that method to get both subsets of data

        def get_feature_flags_and_payloads(
            self,
            distinct_id,
            groups=None,
            person_properties=None,
            group_properties=None,
            disable_geoip=None
        ):
            resp_data = self.get_decide(
                distinct_id,
                groups,
                person_properties,
                group_properties,
                disable_geoip
            )
            return {
                "featureFlags": resp_data.get("featureFlags", []),
                "featureFlagPayloads": resp_data.get("featureFlagPayloads", {})
            }

And then do something like this in get_feature_flag_payloads

    def get_feature_flag_payload(
        self,
        key,
        distinct_id,
        *,
        match_value=None,
        groups={},
        person_properties={},
        group_properties={},
        only_evaluate_locally=False,
        send_feature_flag_events=True,
        disable_geoip=None,
    ):
        if self.disabled:
            return None

        if match_value is None:
            match_value = self.get_feature_flag(
                key,
                distinct_id,
                groups=groups,
                person_properties=person_properties,
                group_properties=group_properties,
                send_feature_flag_events=False,
                # Disable automatic sending of feature flag events because we're manually handling event dispatch.
                # This prevents sending events with empty data when `get_feature_flag` cannot be evaluated locally.
                only_evaluate_locally=True,  # Enable local evaluation of feature flags to avoid making multiple requests to `/decide`.
                disable_geoip=disable_geoip,
            )

        response = None

        if match_value is not None:
            response = self._compute_payload_locally(key, match_value)

        if response is None and not only_evaluate_locally:
            decide_responses, decide_payloads = self.get_feature_flags_and_payloads(
                distinct_id, groups, person_properties, group_properties, disable_geoip
            )
            response = decide_payloads.get(str(key).lower(), None)
            payload = decide_payloads.get(str(key).lower(), None)

        feature_flag_reported_key = f"{key}_{str(response)}"

        if (
            feature_flag_reported_key not in self.distinct_ids_feature_flags_reported[distinct_id]
            and send_feature_flag_events  # noqa: W503
        ):
            self.capture(
                distinct_id,
                "$feature_flag_called",
                {
                    "$feature_flag": key,
                    "$feature_flag_response": response,
                    "locally_evaluated": flag_was_locally_evaluated,
                    f"$feature/{key}": response,
                },
                groups=groups,
                disable_geoip=disable_geoip,
            )
            self.distinct_ids_feature_flags_reported[distinct_id].add(feature_flag_reported_key)

        return payload

Another thought is to include both the feature_flag_response and feature_flag_payload as properties on the event (basically do what I did above, but add the payload to the capture call (the event would like this)

self.capture(
                distinct_id,
                "$feature_flag_called",
                {
                    "$feature_flag": key,
                    "$feature_flag_response": response,
                    "$feature_flag_payload": payload,
                    "locally_evaluated": flag_was_locally_evaluated,
                    f"$feature/{key}": response,
                },
                groups=groups,
                disable_geoip=disable_geoip,
            )

I guess it's up to me to decide what to do here, since it seems like we haven't been doing this with our SDKs, so just brainstorming a bit. Seems easier to not include the payload in the event (since we don't have a nice way of showing it in the flags UI), but I like the idea of tracking both things (more changes, but it gives the users a more holistic view of their events overall, because then they'll be able to differentiate between feature_flag_called events with and without payloads).

[dmarticus]

Slack thread on this: https://posthog.slack.com/archives/C07Q2U4BH4L/p1732202609363039?thread_ts=1732144656.105669&cid=C07Q2U4BH4L

[dmarticus]

FYI, closing the loop: I made the decision to include the payload response on the event; couldn't hurt to have more things to break out by!

[havenbarnes]

Nice 🚢

[daibhin]

Don't forget to update the version and changelog as I did in this PR: #142