feat: Add support for `/decide?v=4` along with start capturing `$feature_flag_called` events by haacked · Pull Request #22 · PostHog/posthog-elixir

haacked · 2025-04-17T02:28:37Z

💡 Motivation and Context

Prior to this PR, this library didn't capture $feature_flag_called events. Now it does it with the additional metadata that /decide?v=4 provides (see PostHog/posthog#29751). Similar to how it's done in PostHog/posthog-js-lite#427.

Updates to the `$feature_flag_called` event

When capturing the $feature_flag_called event, additional information are now captured:

The end result is $feature_flag_called events will have additional properties:

Property Name	Descirption
`$feature_flag_version`	The version of the feature flag. This is visible in the diffs of the feature flag activity log.
`$feature_flag_reason`	The reason the feature flag matched or didn't match.
`$feature_flag_id`	The database id of the feature flag (Just in case a flag was deleted and then recreated with the same key, we can determine the difference.
`$feature_flag_request_id`	The id of the `/decide` request.

Also, we use an LRU to ensure that we don't raise the $feature_flag_called event twice for the same distinct_id and flag key combination, the same way posthog-python and other libraries do it.

Backwards compatibility:

The changes are all backwards compatible with /decide?v=3.

💚 How did you test it?

Unit tests and manual tests.

This ensures backwards comatibility

Less confusing this way.

If the `$feature_flag_called` event is called twice in succession with the same distinct_id and same key, then we only call it once. We do this by implementing an LRU cache of up to 50,000 entries (a number picked from `posthog-python`).

Copilot

Pull Request Overview

This PR adds support for utilizing the new /decide?v=4 endpoint and enhances the feature flag capture mechanism by renaming the value property to payload and including additional metadata on the $feature_flag_called event.

Updated documentation examples to use "payload" instead of "value" for the feature flag's return object
Added instructions for code formatting via bin/fmt in the main README

Reviewed Changes

Copilot reviewed 4 out of 15 changed files in this pull request and generated no comments.

File	Description
examples/feature_flag_demo/README.md	Updated feature flag demo to show "Payload: true" instead of "Value: true"
README.md	Revised code examples to reflect the change from "value" to "payload" and added a code formatting section

Files not reviewed (11)

examples/feature_flag_demo/lib/feature_flag_demo.ex: Language not supported
lib/posthog.ex: Language not supported
lib/posthog/application.ex: Language not supported
lib/posthog/client.ex: Language not supported
lib/posthog/feature_flag.ex: Language not supported
mix.exs: Language not supported
test/posthog/client_test.exs: Language not supported
test/posthog_test.exs: Language not supported
test/support/fixtures/decide-v3.json: Language not supported
test/support/fixtures/decide.json: Language not supported
test/support/hackney_stub.ex: Language not supported

Comments suppressed due to low confidence (2)

README.md:112

[nitpick] Ensure consistency in property naming across the documentation. The example now uses 'payload' instead of 'value', so please confirm that related documentation (such as the event schema table in the PR description) is updated accordingly.

# Returns: %Posthog.FeatureFlag{name: "new-dashboard", payload: true, enabled: true}

README.md:118

[nitpick] Ensure naming consistency for the feature flag return value. The change from 'value' to 'payload' should be reflected across all documentation and examples to avoid confusion.

#   payload: %{"price" => 99, "period" => "monthly"},

haacked · 2025-04-17T02:37:28Z

Hmm, the CI server is reporting:

Run mix format --check-formatted
** (Mix) mix format failed due to --check-formatted.
The following files are not formatted:

  * test/posthog_test.exs

But when I run mix format --check-formatted locally, it doesn't report anything.

Going to try using an older version of elixir on my local machine to see if it fixes the formatting issue.

haacked · 2025-04-17T03:00:14Z

Ok, formatting issues are fixed, but some of the tests for older versions of elixir are broken. How far back do we need to go?

rafaeelaudibert · 2025-04-17T03:40:39Z

Ok, formatting issues are fixed, but some of the tests for older versions of elixir are broken. How far back do we need to go?

1.12 is reasonable, I think we should keep it. Tests are failing because you're using Process. Can you something else rather than that?

rafaeelaudibert · 2025-04-17T03:41:18Z

About formatting, we should make sure the CI is only checking formatting on the most recent version

lib/posthog/application.ex

rafaeelaudibert · 2025-04-17T03:45:25Z

lib/posthog/application.ex

+    children = [
+      # Start Cachex for feature flag event deduplication.
+      # The 50,000 entries limit is the same used for posthog-python, but otherwise arbitrary.
+      {Cachex, name: :posthog_feature_flag_cache, limit: 50_000, policy: Cachex.Policy.LRW}


Does this make sense? Do we not wanna send $feature_flag_called in case we have this in the cache? This lasts across requests - it's a separate process.

Wont this mean that users with less than 50000 clients wont ever have more than 50000 FFs charged to them? (Users can make these supervisors survive servers restarts, Elixir is crazy)

Well, it's the combination of distinct_id and feature_flag. So yeah, if they only have 1 flag and less than 50,000 distinct users, they'd never be charged for more than 50,000.

I took this approach from posthog-python. Many of the other clients do something similar. It seems to me that we should put a time limit on each cache entry. Something like one minute. @dmarticus, do you have context on why we do this in posthog-python?

Ah, the hash includes the feature_flag too, so it's a slightly smaller problem.
The problem is that servers probably restart pretty often in Python; that isn't necessarily true in Elixir: you can keep the Cachex process running indefinitely, so... it might fill the cache pretty quickly.

Will leave that decision up to y'all, just letting you know the quirkyness of working with a distributed language :)

One other thing to consider, we don't charge based on the number of $feature_flag_called events, we charge based on the number of /decide calls made. So this cache doesn't affect the cost. Just reporting.

Ah, I didn't know that. In that case, I'm fine with keeping it this way. I'm working on improving the setup, I don't think the LRU we added actually works how we think, expect changes to this PR through the day.

The one thing I worry about with such a big cache is people being confused why they don't see these events. My gut tells me the reason for this is if they're calling a feature flag in a tight loop, we don't want 10 of the same events all of a sudden.

So I'd suggest we add a default_ttl to the Cachex declaration. Something like 5 minutes would be fine. The 50,000 limit then just ensures we don't ever take up too much memory.

lib/posthog/application.ex

haacked · 2025-04-17T04:36:39Z

Tests are failing because you're using Process

Any suggestions?

haacked · 2025-04-17T04:37:20Z

Any suggestions?

Hmm, I have a GenServer approach that seems to work.

rafaeelaudibert · 2025-04-17T14:35:47Z

@haacked I'm taking a stab at this. I dont think it was the Process thing, but rather Mimic acting poorly on older Elixir versions

Elixir has been improving their typesystem, and if we run these doctests with more recent Elixir (1.18+) then it fails at the type level - which is pretty cool, actually!

Our CI matrix is much more comprehensible of Elixir's actual support matrix, at the cost of being more complex. I think it's a fair tradeoff.

erlef/setup-beam@v1 doesn't support OTP 23

rafaeelaudibert · 2025-04-17T15:39:06Z

@haacked I've changed this to point to a new v1 branch. How soon do you want this to be merged? I can work on getting the rest of the v1 stuff over the line today

haacked · 2025-04-17T15:57:36Z

How soon do you want this to be merged? I can work on getting the rest of the v1 stuff over the line today

I'm not in any hurry. I appreciate the help getting this over the line!

Organized it into a couple functions and also added documentation around it

rafaeelaudibert

@haacked This should be good now! I've pointed it to a v1 branch where I'm working on more stuff. Take a look at my recent changes and let me know! Feel free to merge whenever you're ready

haacked · 2025-04-17T18:35:25Z

@rafaeelaudibert what do you think about giving cache entries a ttl? I think Cachex supports that?

rafaeelaudibert · 2025-04-17T18:38:45Z

@rafaeelaudibert what do you think about giving cache entries a ttl? I think Cachex supports that?

Up to you, really. I particularly think it's to your best interest to keep implementation across SDKs the most similar possible, but if you believe a TTL would be benefitial here, yeah, we can do it

https://hexdocs.pm/cachex/expiring-records.html

haacked · 2025-04-17T18:46:23Z

Up to you, really. I particularly think it's to your best interest to keep implementation across SDKs the most similar possible

Good point. I'll leave as-is. If folks complain, we can revisit.

I also pushed some changes to get the sample app working again. Look good to you?

rafaeelaudibert · 2025-04-17T19:00:38Z

@haacked Did you need that new Application for it to work? I though the existing extra_applications: [:logger, :posthog], would be enough for it to automatically start our Posthog.Application. It worked just fine for me. It shouldn't hurt, though.

haacked · 2025-04-18T00:02:05Z

Here's the error I get when I run the demo app without the new Application:

mix run run.exs --flag "simple-test" --distinct-id "user12345"
==> uniq
Compiling 4 files (.ex)
Generated uniq app
==> posthog
Compiling 8 files (.ex)
Generated posthog app
==> feature_flag_demo
Generated feature_flag_demo app

17:00:12.403 [notice] Application feature_flag_demo exited: exited in: FeatureFlagDemo.Application.start(:normal, [])
    ** (EXIT) an exception was raised:
        ** (UndefinedFunctionError) function FeatureFlagDemo.Application.start/2 is undefined (module FeatureFlagDemo.Application is not available)
            FeatureFlagDemo.Application.start(:normal, [])
            (kernel 10.2.6) application_master.erl:295: :application_master.start_it_old/4

17:00:12.407 [notice] Application posthog exited: :stopped

17:00:12.407 [notice] Application cachex exited: :stopped

17:00:12.407 [notice] Application sleeplocks exited: :stopped

17:00:12.407 [notice] Application jumper exited: :stopped

17:00:12.408 [notice] Application ex_hash_ring exited: :stopped

17:00:12.408 [notice] Application eternal exited: :stopped

17:00:12.408 [notice] Application unsafe exited: :stopped

17:00:12.408 [notice] Application jason exited: :stopped

17:00:12.408 [notice] Application hackney exited: :stopped

17:00:12.408 [notice] Application metrics exited: :stopped

17:00:12.408 [notice] Application ssl_verify_fun exited: :stopped

17:00:12.408 [notice] Application parse_trans exited: :stopped

17:00:12.408 [notice] Application syntax_tools exited: :stopped

17:00:12.408 [notice] Application certifi exited: :stopped

17:00:12.408 [notice] Application mimerl exited: :stopped

17:00:12.408 [notice] Application idna exited: :stopped

17:00:12.408 [notice] Application unicode_util_compat exited: :stopped
** (Mix) Could not start application feature_flag_demo: exited in: FeatureFlagDemo.Application.start(:normal, [])
    ** (EXIT) an exception was raised:
        ** (UndefinedFunctionError) function FeatureFlagDemo.Application.start/2 is undefined (module FeatureFlagDemo.Application is not available)
            FeatureFlagDemo.Application.start(:normal, [])
            (kernel 10.2.6) application_master.erl:295: :application_master.start_it_old/4

haacked added 16 commits April 16, 2025 17:31

Add tests using v4 response

7980c65

Add array payload tests

438a5c4

Normalize v4 response to v3 format

e31282f

This ensures backwards comatibility

Rename value to payload.

0d5d40c

Less confusing this way.

Extract method that provides access to the decide response

95a0e21

Add _decide_request method

9b25c99

Update test names

1298008

Remove api version from non-decide requests

294d126

Add test for $feature_flag_called event

42040e5

Capture $feature_flag_called event

48b66ea

Make sure we don't send event when send_feature_flag_event is false

34742be

Pass id, version, and reason to $feature_flag_called events

0d45209

Add feature_flag_request_id to the $feature_flag_called event.

4a98477

Actually request decide v=4

9184746

De-duplicate $feature_flag_called events

856c44d

If the `$feature_flag_called` event is called twice in succession with the same distinct_id and same key, then we only call it once. We do this by implementing an LRU cache of up to 50,000 entries (a number picked from `posthog-python`).

Add a script to format code

da80228

haacked requested a review from rafaeelaudibert as a code owner April 17, 2025 02:28

haacked requested review from Copilot, dmarticus and rafaeelaudibert and removed request for rafaeelaudibert April 17, 2025 02:28

Copilot AI reviewed Apr 17, 2025

View reviewed changes

Format using older elixir

d358cde

rafaeelaudibert reviewed Apr 17, 2025

View reviewed changes

lib/posthog/application.ex Show resolved Hide resolved

rafaeelaudibert reviewed Apr 17, 2025

View reviewed changes

lib/posthog/application.ex Outdated Show resolved Hide resolved

Stop using Process

f78b873

haacked added 3 commits April 16, 2025 21:38

Replace Process with GenServer

e6fd9a7

Change to LRU

6ad6aa2

Only format code with latest elixir

6b11bf9

rafaeelaudibert added 4 commits April 17, 2025 12:06

chore: Use more modern Elixir/Erlang locally

eec1890

test: Fix doctest complaining after more recent Elixir types

c910c5e

Elixir has been improving their typesystem, and if we run these doctests with more recent Elixir (1.18+) then it fails at the type level - which is pretty cool, actually!

chore: Update CI matrix

800a0d6

Our CI matrix is much more comprehensible of Elixir's actual support matrix, at the cost of being more complex. I think it's a fair tradeoff.

chore: Remove Elixir 1.14.x + OTP 23.x pair

794aeb1

erlef/setup-beam@v1 doesn't support OTP 23

rafaeelaudibert changed the base branch from master to v1 April 17, 2025 15:38

haacked mentioned this pull request Apr 17, 2025

feat(flags): Include version and other feature flag details in /decide?v=4 response PostHog/posthog#29693

Closed

12 tasks

rafaeelaudibert added 4 commits April 17, 2025 14:35

feat: Add docs around initing our Posthog.Application

6d13b6e

refactor: Improve send_feature_flag_called_event handling

2d9d7b1

Organized it into a couple functions and also added documentation around it

chore: Bump to Cachex v4+

ee4d864

chore: Update demo with new deps

820a5f6

rafaeelaudibert approved these changes Apr 17, 2025

View reviewed changes

haacked added 2 commits April 17, 2025 11:40

Update setup instructions for the sample project

f8c0a68

Fix sample project to run

6f69a2d

rafaeelaudibert mentioned this pull request Apr 15, 2025

Mega Issue - posthog-elixir v1.0.0 #16

Closed

3 tasks

haacked merged commit ecfabf1 into v1 Apr 17, 2025
16 checks passed

haacked deleted the haacked/decide-v4 branch April 17, 2025 18:47

Comments

Conversation

haacked commented Apr 17, 2025

💡 Motivation and Context

Updates to the $feature_flag_called event

Backwards compatibility:

💚 How did you test it?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

haacked commented Apr 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

haacked commented Apr 17, 2025

Uh oh!

rafaeelaudibert commented Apr 17, 2025

Uh oh!

rafaeelaudibert commented Apr 17, 2025

Uh oh!

Uh oh!

rafaeelaudibert Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

haacked Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

rafaeelaudibert Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

haacked Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

rafaeelaudibert Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

haacked Apr 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

haacked commented Apr 17, 2025

Uh oh!

haacked commented Apr 17, 2025

Uh oh!

rafaeelaudibert commented Apr 17, 2025

Uh oh!

rafaeelaudibert commented Apr 17, 2025

Uh oh!

haacked commented Apr 17, 2025

Uh oh!

rafaeelaudibert left a comment

Choose a reason for hiding this comment

Uh oh!

haacked commented Apr 17, 2025

Uh oh!

rafaeelaudibert commented Apr 17, 2025

Uh oh!

haacked commented Apr 17, 2025

Uh oh!

Uh oh!

rafaeelaudibert commented Apr 17, 2025

Uh oh!

haacked commented Apr 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Updates to the `$feature_flag_called` event

haacked commented Apr 17, 2025 •

edited

Loading