Skip to content

Commit 6a00101

Browse files
haackedhaacked
authored
chore(flags): Decide v4 support - adds version, id, reason, and requestId to $feature_flag_called events (#91)
* Add unit tests for decide v4 Handle `/decide` v3 and v4 responses Normalize them into `DecideResponse` which is a union of v3 and v4 responses. Separate local eval tests with decide tests * Change getFeatureFlagFromDecide to return more info It now returns the requestId and the `FeatureFlagDetail` * Update posthog.go * Add version, id, reason, and requestId to `$feature_flag_called` event This is in support of PostHog/posthog#29693 * Log error while computing flags
1 parent b3335b9 commit 6a00101

File tree

6 files changed

+573
-15
lines changed

6 files changed

+573
-15
lines changed

‎decide.go

Lines changed: 134 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,137 @@ type DecideRequestData struct {
1919
GroupProperties map[string]Properties `json:"group_properties"`
2020
}
2121

22+
// FlagDetail represents a feature flag in v4 format
23+
type FlagDetail struct {
24+
Key string `json:"key"`
25+
Enabled bool `json:"enabled"`
26+
Variant *string `json:"variant"`
27+
Reason *FlagReason `json:"reason"`
28+
Metadata FlagMetadata `json:"metadata"`
29+
}
30+
31+
// FlagReason represents why a flag was enabled/disabled
32+
type FlagReason struct {
33+
Code string `json:"code"`
34+
Description string `json:"description"`
35+
ConditionIndex int `json:"condition_index"`
36+
}
37+
38+
// FlagMetadata contains additional information about a flag
39+
type FlagMetadata struct {
40+
ID int `json:"id"`
41+
Version int `json:"version"`
42+
Payload *string `json:"payload"`
43+
Description *string `json:"description,omitempty"`
44+
}
45+
46+
// GetValue returns the variant if it exists, otherwise returns the enabled status
47+
func (f FlagDetail) GetValue() interface{} {
48+
if f.Variant != nil {
49+
return *f.Variant
50+
}
51+
return f.Enabled
52+
}
53+
54+
// NewFlagDetail creates a new FlagDetail from a key, value, and optional payload
55+
func NewFlagDetail(key string, value interface{}, payload *string) FlagDetail {
56+
var variant *string
57+
var enabled bool
58+
59+
switch v := value.(type) {
60+
case string:
61+
variant = &v
62+
enabled = true
63+
case bool:
64+
enabled = v
65+
default:
66+
enabled = false
67+
}
68+
69+
return FlagDetail{
70+
Key: key,
71+
Enabled: enabled,
72+
Variant: variant,
73+
Reason: nil,
74+
Metadata: FlagMetadata{
75+
Payload: payload,
76+
},
77+
}
78+
}
79+
80+
// DecideResponse represents the response from the decide endpoint v3 or v4.
81+
// It is a normalized super set of the v3 and v4 formats.
2282
type DecideResponse struct {
83+
CommonResponseFields
84+
85+
// v4 flags format
86+
Flags map[string]FlagDetail `json:"flags,omitempty"`
87+
88+
// v3 legacy fields
2389
FeatureFlags map[string]interface{} `json:"featureFlags"`
2490
FeatureFlagPayloads map[string]string `json:"featureFlagPayloads"`
25-
QuotaLimited *[]string `json:"quota_limited"`
91+
}
92+
93+
// CommonResponseFields contains fields common to all decide response versions
94+
type CommonResponseFields struct {
95+
QuotaLimited *[]string `json:"quota_limited"`
96+
RequestId string `json:"requestId"`
97+
ErrorsWhileComputingFlags bool `json:"errorsWhileComputingFlags"`
98+
}
99+
100+
// UnmarshalJSON implements custom unmarshaling to handle both v3 and v4 formats
101+
func (r *DecideResponse) UnmarshalJSON(data []byte) error {
102+
// First try v4 format
103+
type V4Response struct {
104+
CommonResponseFields
105+
Flags map[string]FlagDetail `json:"flags"`
106+
}
107+
108+
var v4 V4Response
109+
if err := json.Unmarshal(data, &v4); err == nil && v4.Flags != nil {
110+
// It's a v4 response
111+
r.Flags = v4.Flags
112+
r.CommonResponseFields = v4.CommonResponseFields
113+
114+
// Calculate v3 format fields from Flags
115+
r.FeatureFlags = make(map[string]interface{})
116+
r.FeatureFlagPayloads = make(map[string]string)
117+
for key, flag := range r.Flags {
118+
r.FeatureFlags[key] = flag.GetValue()
119+
if flag.Metadata.Payload != nil {
120+
r.FeatureFlagPayloads[key] = *flag.Metadata.Payload
121+
}
122+
}
123+
return nil
124+
}
125+
126+
// If not v4, try v3 format
127+
type V3Response struct {
128+
CommonResponseFields
129+
FeatureFlags map[string]interface{} `json:"featureFlags"`
130+
FeatureFlagPayloads map[string]string `json:"featureFlagPayloads"`
131+
}
132+
133+
var v3 V3Response
134+
if err := json.Unmarshal(data, &v3); err != nil {
135+
return err
136+
}
137+
138+
// Store v3 format data
139+
r.FeatureFlags = v3.FeatureFlags
140+
r.FeatureFlagPayloads = v3.FeatureFlagPayloads
141+
r.CommonResponseFields = v3.CommonResponseFields
142+
143+
// Construct v4 format from v3 data
144+
r.Flags = make(map[string]FlagDetail)
145+
for key, value := range v3.FeatureFlags {
146+
var payloadPtr *string
147+
if payload, ok := v3.FeatureFlagPayloads[key]; ok {
148+
payloadPtr = &payload
149+
}
150+
r.Flags[key] = NewFlagDetail(key, value, payloadPtr)
151+
}
152+
return nil
26153
}
27154

28155
// decider defines the interface for making decide requests
@@ -66,7 +193,8 @@ func (d *decideClient) makeDecideRequest(distinctId string, groups Groups, perso
66193
return nil, fmt.Errorf("unable to marshal decide endpoint request data: %v", err)
67194
}
68195

69-
decideEndpoint := "decide/?v=3"
196+
// Try v4 endpoint first
197+
decideEndpoint := "decide/?v=4"
70198
url, err := url.Parse(d.endpoint + "/" + decideEndpoint)
71199
if err != nil {
72200
return nil, fmt.Errorf("creating url: %v", err)
@@ -106,5 +234,9 @@ func (d *decideClient) makeDecideRequest(distinctId string, groups Groups, perso
106234
return nil, fmt.Errorf("error parsing response from /decide/: %v", err)
107235
}
108236

237+
if decideResponse.ErrorsWhileComputingFlags {
238+
d.errorf("error while computing feature flags, some flags may be missing or incorrect. Learn more at https://posthog.com/docs/feature-flags/best-practices")
239+
}
240+
109241
return &decideResponse, nil
110242
}

‎feature_flags_decide_test.go

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
package posthog
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"strings"
7+
8+
"testing"
9+
)
10+
11+
/*
12+
Tests of the feature flag api against the decide endpoint, no local evaluation.
13+
This is primarily here to ensure we handle the different versions of the decide
14+
endpoint correctly.
15+
*/
16+
func TestDecide(t *testing.T) {
17+
validateCapturedEvent := func(t *testing.T, client Client) {
18+
lastEvent := client.GetLastCapturedEvent()
19+
20+
if lastEvent == nil {
21+
return
22+
}
23+
if lastEvent.Event != "$feature_flag_called" {
24+
t.Errorf("Expected a $feature_flag_called event, got: %v", lastEvent)
25+
}
26+
27+
if lastEvent.Properties["$feature_flag_request_id"] != "42853c54-1431-4861-996e-3a548989fa2c" {
28+
t.Errorf("Expected $feature_flag_request_id property to be 42853c54-1431-4861-996e-3a548989fa2c, got: %v", lastEvent.Properties["$feature_flag_request_id"])
29+
}
30+
}
31+
32+
tests := []struct {
33+
name string
34+
fixture string
35+
}{
36+
{name: "v3", fixture: "test-decide-v3.json"},
37+
{name: "v4", fixture: "test-decide-v4.json"},
38+
}
39+
40+
for _, test := range tests {
41+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
42+
if strings.HasPrefix(r.URL.Path, "/decide") {
43+
w.Write([]byte(fixture(test.fixture)))
44+
}
45+
}))
46+
defer server.Close()
47+
48+
t.Run(test.name, func(t *testing.T) {
49+
subTests := []struct {
50+
name string
51+
flagKey string
52+
expected interface{}
53+
}{
54+
{name: "IsFeatureEnabled", flagKey: "enabled-flag", expected: true},
55+
{name: "IsFeatureEnabled", flagKey: "disabled-flag", expected: false},
56+
{name: "IsFeatureEnabled", flagKey: "non-existent-flag", expected: false}, // Note: This differs from posthog-node which returns undefined for non-existent flags
57+
}
58+
59+
for _, subTest := range subTests {
60+
t.Run(subTest.name+" "+subTest.flagKey, func(t *testing.T) {
61+
client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
62+
Endpoint: server.URL,
63+
})
64+
defer client.Close()
65+
66+
isMatch, _ := client.IsFeatureEnabled(
67+
FeatureFlagPayload{
68+
Key: subTest.flagKey,
69+
DistinctId: "some-distinct-id",
70+
PersonProperties: NewProperties().Set("region", "USA"),
71+
},
72+
)
73+
74+
if isMatch != subTest.expected {
75+
t.Errorf("Expected IsFeatureEnabled to return %v but was %v", subTest.expected, isMatch)
76+
}
77+
78+
validateCapturedEvent(t, client)
79+
})
80+
}
81+
})
82+
83+
t.Run(test.name, func(t *testing.T) {
84+
subTests := []struct {
85+
name string
86+
flagKey string
87+
expected interface{}
88+
}{
89+
{name: "GetFeatureFlag", flagKey: "enabled-flag", expected: true},
90+
{name: "GetFeatureFlag", flagKey: "disabled-flag", expected: false},
91+
{name: "GetFeatureFlag", flagKey: "multi-variate-flag", expected: "hello"},
92+
{name: "GetFeatureFlag", flagKey: "non-existent-flag", expected: false}, // Note: This differs from posthog-node which returns undefined for non-existent flags
93+
}
94+
95+
for _, subTest := range subTests {
96+
t.Run(subTest.name+" "+subTest.flagKey, func(t *testing.T) {
97+
client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
98+
Endpoint: server.URL,
99+
})
100+
defer client.Close()
101+
102+
flag, _ := client.GetFeatureFlag(
103+
FeatureFlagPayload{
104+
Key: subTest.flagKey,
105+
DistinctId: "some-distinct-id",
106+
PersonProperties: NewProperties().Set("region", "USA"),
107+
},
108+
)
109+
110+
if flag != subTest.expected {
111+
t.Errorf("Expected GetFeatureFlag to return %v but was %v", subTest.expected, flag)
112+
}
113+
114+
validateCapturedEvent(t, client)
115+
})
116+
}
117+
})
118+
119+
t.Run(test.name, func(t *testing.T) {
120+
subTests := []struct {
121+
name string
122+
flagKey string
123+
expected interface{}
124+
}{
125+
{name: "GetFeatureFlagPayload", flagKey: "enabled-flag", expected: "{\"foo\": 1}"},
126+
{name: "GetFeatureFlagPayload", flagKey: "disabled-flag", expected: ""}, // Incorrectly returns "" for disabled flags
127+
{name: "GetFeatureFlagPayload", flagKey: "multi-variate-flag", expected: "this is the payload"},
128+
{name: "GetFeatureFlagPayload", flagKey: "non-existent-flag", expected: ""}, // Incorrectly returns "" for non-existent flags
129+
}
130+
131+
for _, subTest := range subTests {
132+
t.Run(subTest.name+" "+subTest.flagKey, func(t *testing.T) {
133+
client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
134+
Endpoint: server.URL,
135+
})
136+
defer client.Close()
137+
138+
payload, _ := client.GetFeatureFlagPayload(
139+
FeatureFlagPayload{
140+
Key: subTest.flagKey,
141+
DistinctId: "some-distinct-id",
142+
PersonProperties: NewProperties().Set("region", "USA"),
143+
},
144+
)
145+
146+
if payload != subTest.expected {
147+
t.Errorf("Expected GetFeatureFlagPayload to return %v but was %v", subTest.expected, payload)
148+
}
149+
150+
validateCapturedEvent(t, client)
151+
})
152+
}
153+
})
154+
}
155+
}
156+
157+
func TestDecideV4(t *testing.T) {
158+
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
159+
if strings.HasPrefix(r.URL.Path, "/decide") {
160+
w.Write([]byte(fixture("test-decide-v4.json")))
161+
}
162+
}))
163+
defer server.Close()
164+
165+
client, _ := NewWithConfig("Csyjlnlun3OzyNJAafdlv", Config{
166+
Endpoint: server.URL,
167+
})
168+
defer client.Close()
169+
170+
tests := []struct {
171+
name string
172+
flagKey string
173+
expected interface{}
174+
expectedVersion int
175+
expectedReason string
176+
expectedId int
177+
}{
178+
{name: "GetFeatureFlag", flagKey: "enabled-flag", expected: true, expectedVersion: 23, expectedReason: "Matched conditions set 3", expectedId: 1},
179+
{name: "GetFeatureFlag", flagKey: "disabled-flag", expected: false, expectedVersion: 12, expectedReason: "No matching condition set", expectedId: 3},
180+
{name: "GetFeatureFlag", flagKey: "multi-variate-flag", expected: "hello", expectedVersion: 42, expectedReason: "Matched conditions set 2", expectedId: 4},
181+
}
182+
183+
for _, test := range tests {
184+
t.Run(test.name, func(t *testing.T) {
185+
flag, _ := client.GetFeatureFlag(
186+
FeatureFlagPayload{
187+
Key: test.flagKey,
188+
DistinctId: "some-distinct-id",
189+
},
190+
)
191+
192+
if flag != test.expected {
193+
t.Errorf("Expected GetFeatureFlag(%s) to return %v but was %v", test.flagKey, test.expected, flag)
194+
}
195+
196+
capturedEvent := client.GetLastCapturedEvent()
197+
if capturedEvent == nil {
198+
t.Errorf("Expected a $feature_flag_called for %s event, got: %v", test.flagKey, capturedEvent)
199+
}
200+
201+
if capturedEvent.Properties["$feature_flag"] != test.flagKey {
202+
t.Errorf("Expected $feature_flag property to be %v, got: %v", test.flagKey, capturedEvent.Properties["$feature_flag"])
203+
}
204+
205+
if capturedEvent.Properties["$feature_flag_request_id"] != "42853c54-1431-4861-996e-3a548989fa2c" {
206+
t.Errorf("Expected $feature_flag_request_id property to be 42853c54-1431-4861-996e-3a548989fa2c, got: %v", capturedEvent.Properties["$feature_flag_request_id"])
207+
}
208+
209+
if capturedEvent.Properties["$feature_flag_response"] != test.expected {
210+
t.Errorf("Expected $feature_flag_response property for %s to be %v, got: %v", test.flagKey, test.expected, capturedEvent.Properties["$feature_flag_response"])
211+
}
212+
213+
if version, ok := capturedEvent.Properties["$feature_flag_version"].(int); !ok || version != test.expectedVersion {
214+
t.Errorf("Expected $feature_flag_version property for %s to be %v, got: %v", test.flagKey, test.expectedVersion, capturedEvent.Properties["$feature_flag_version"])
215+
}
216+
217+
if reason, ok := capturedEvent.Properties["$feature_flag_reason"].(string); !ok || reason != test.expectedReason {
218+
t.Errorf("Expected $feature_flag_reason property for %s to be %v, got: %v", test.flagKey, test.expectedReason, capturedEvent.Properties["$feature_flag_reason"])
219+
}
220+
221+
if id, ok := capturedEvent.Properties["$feature_flag_id"].(int); !ok || int(id) != test.expectedId {
222+
t.Errorf("Expected $feature_flag_id for %s to be %v, got: %v", test.flagKey, test.expectedId, capturedEvent.Properties["$feature_flag_id"])
223+
}
224+
})
225+
}
226+
}

‎feature_flags_test.go renamed to ‎feature_flags_local_test.go

File renamed without changes.

‎fixtures/test-decide-v3.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,6 @@
3636
"elementsChainAsString": true,
3737
"surveys": false,
3838
"heatmaps": false,
39-
"siteApps": []
39+
"siteApps": [],
40+
"requestId": "42853c54-1431-4861-996e-3a548989fa2c"
4041
}

0 commit comments

Comments
 (0)