From 6aeb2a148df8ee14d30167e4a4de912295151763 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Thu, 16 Jul 2026 10:35:01 -0700 Subject: [PATCH 1/3] feat(flags): send minimal $feature_flag_called events when server-gated When the /flags?v=2 response carries top-level minimalFlagCalledEvents or the local-evaluation definitions payload carries minimal_flag_called_events, and the evaluated flag has has_experiment=false, serialize the $feature_flag_called event with a strict allowlist of evaluation properties and skip system context, DefaultEventProperties, and the snapshot path's $feature/ and payload extras. Any missing signal (gate absent, has_experiment unknown, experiment-linked flag, legacy response shapes) keeps the full event shape. The local-eval gate is cached in the poller's atomic state alongside flag definitions and survives 304 responses; quota-limit resets clear it. Generated-By: PostHog Code Task-Id: ffe402fd-d75c-4043-8e5d-d2fe513cac6f --- .changeset/minimal-flag-called-events.md | 5 + api/public-api.txt | 3 + capture.go | 71 +++- capture_v1.go | 12 +- feature_flag_errors.go | 3 + feature_flag_evaluations.go | 9 +- feature_flag_evaluations_test.go | 1 + featureflags.go | 50 ++- flags.go | 4 + minimal_flag_called_events_test.go | 503 +++++++++++++++++++++++ posthog.go | 54 ++- 11 files changed, 671 insertions(+), 44 deletions(-) create mode 100644 .changeset/minimal-flag-called-events.md create mode 100644 minimal_flag_called_events_test.go diff --git a/.changeset/minimal-flag-called-events.md b/.changeset/minimal-flag-called-events.md new file mode 100644 index 00000000..aa5562b7 --- /dev/null +++ b/.changeset/minimal-flag-called-events.md @@ -0,0 +1,5 @@ +--- +"posthog-go": minor +--- + +Send minimal `$feature_flag_called` events when the server enables it. When the `/flags?v=2` response carries `minimalFlagCalledEvents: true` (remote evaluation) or the local-evaluation definitions payload carries `minimal_flag_called_events: true`, and the evaluated flag has `has_experiment: false`, the event keeps only a strict allowlist of evaluation properties (`$feature_flag`, `$feature_flag_response`, `$feature_flag_has_experiment`, `$feature_flag_id`, `$feature_flag_version`, `$feature_flag_reason`, `$feature_flag_request_id`, `$feature_flag_evaluated_at`, `$feature_flag_error`, `locally_evaluated`, `$groups`, `$process_person_profile`, `$geoip_disable`, `$is_server`, `$session_id`, `$window_id`, `$device_id`, `$lib`, `$lib_version`); everything else — including `Config.DefaultEventProperties`, system context, and the snapshot path's `$feature/` and `$feature_flag_payload` — is stripped. Any missing signal (gate absent, `has_experiment` unknown, experiment-linked flag, legacy response shapes) keeps today's full event shape. The gate is server-controlled per project; no SDK configuration is added. diff --git a/api/public-api.txt b/api/public-api.txt index b2779954..1ad79cb8 100644 --- a/api/public-api.txt +++ b/api/public-api.txt @@ -195,6 +195,7 @@ type Capture struct { SendFeatureFlags SendFeatureFlagsValue Flags *FeatureFlagEvaluations IsServer bool + } func (msg Capture) APIfy() APIMessage @@ -277,6 +278,7 @@ type CommonResponseFields struct { RequestId string `json:"requestId"` EvaluatedAt *int64 `json:"evaluatedAt"` ErrorsWhileComputingFlags bool `json:"errorsWhileComputingFlags"` + MinimalFlagCalledEvents bool `json:"minimalFlagCalledEvents"` } type CompressionMode uint8 @@ -578,6 +580,7 @@ type FeatureFlagsResponse struct { Flags []FeatureFlag `json:"flags"` GroupTypeMapping *map[string]string `json:"group_type_mapping"` Cohorts map[string]PropertyGroup `json:"cohorts"` + MinimalFlagCalledEvents bool `json:"minimal_flag_called_events"` } type FieldError struct { diff --git a/capture.go b/capture.go index fb314b88..ca75fd3f 100644 --- a/capture.go +++ b/capture.go @@ -117,6 +117,11 @@ type Capture struct { // IsServer controls whether the event includes the $is_server property. // Enqueue overwrites it from Config.GetIsServer. IsServer bool + // minimalFlagCalledEvent marks a $feature_flag_called event for the minimal + // shape: serialization keeps only the allowlisted evaluation properties and + // skips system context. It is set only when the server enabled + // minimal_flag_called_events and the flag has no linked experiment. + minimalFlagCalledEvent bool } func (msg Capture) internal() { @@ -164,15 +169,71 @@ type CaptureInApi struct { SendFeatureFlags SendFeatureFlagsValue `json:"-"` } +// minimalFlagCalledEventAllowlist lists the only event properties kept on a +// minimal $feature_flag_called event, per the cross-SDK contract. Everything +// else — Config.DefaultEventProperties, request-context properties, and +// system context included — is stripped so the minimal shape stays +// predictable. $geoip_disable is kept because, like $process_person_profile, +// it is a processing-control sentinel: stripping it would silently re-enable +// GeoIP enrichment for events from clients that disabled it. $session_id, +// $window_id, and $device_id are linkage identifiers the contract preserves. +// $is_server is kept so server-event classification still works. +var minimalFlagCalledEventAllowlist = []string{ + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "$feature_flag_id", + "$feature_flag_version", + "$feature_flag_reason", + "$feature_flag_request_id", + "$feature_flag_evaluated_at", + "$feature_flag_error", + "locally_evaluated", + "$groups", + propertyProcessPersonProfile, + propertyGeoipDisable, + propertyIsServer, + propertySessionID, + propertyWindowID, + "$device_id", +} + +// minimalFlagCalledEventProperties builds a fresh property set containing only +// the allowlisted minimal $feature_flag_called properties present in props. +func minimalFlagCalledEventProperties(props Properties) Properties { + minimal := NewProperties() + for _, key := range minimalFlagCalledEventAllowlist { + if value, ok := props[key]; ok { + minimal[key] = value + } + } + return minimal +} + +// shouldMinimizeFlagCalledEvent reports whether a $feature_flag_called event +// should use the minimal shape: the server-controlled gate must be on and the +// flag must be known to have no linked experiment. Any missing signal keeps +// the full event shape. +func shouldMinimizeFlagCalledEvent(minimalFlagCalledEvents bool, hasExperiment *bool) bool { + return minimalFlagCalledEvents && hasExperiment != nil && !*hasExperiment +} + // APIfy converts a Capture message into the PostHog batch API representation. func (msg Capture) APIfy() APIMessage { libraryVersion := getVersion() - myProperties := Properties{}. - Merge(msg.Properties). - Set("$lib", SDKName). - Set("$lib_version", libraryVersion). - Merge(getSystemContext().ToProperties()) + var myProperties Properties + if msg.minimalFlagCalledEvent { + myProperties = minimalFlagCalledEventProperties(msg.Properties). + Set("$lib", SDKName). + Set("$lib_version", libraryVersion) + } else { + myProperties = Properties{}. + Merge(msg.Properties). + Set("$lib", SDKName). + Set("$lib_version", libraryVersion). + Merge(getSystemContext().ToProperties()) + } if msg.IsServer { myProperties.Set("$is_server", true) diff --git a/capture_v1.go b/capture_v1.go index 7ceb8227..0667ab9c 100644 --- a/capture_v1.go +++ b/capture_v1.go @@ -268,9 +268,15 @@ func prepareForSendV1(msg Message, logger Logger) (json.RawMessage, APIMessage, // properties APIfy assembles, minus $lib/$lib_version (the PostHog-Sdk-Info // header is the authoritative SDK identity in v1). func (msg Capture) apifyEvent() apiEvent { - myProperties := baseV1Props(msg.IsServer, false). - Merge(msg.Properties). - mergeDefaults(getSystemContext().ToProperties()) + var myProperties Properties + if msg.minimalFlagCalledEvent { + myProperties = baseV1Props(msg.IsServer, false). + Merge(minimalFlagCalledEventProperties(msg.Properties)) + } else { + myProperties = baseV1Props(msg.IsServer, false). + Merge(msg.Properties). + mergeDefaults(getSystemContext().ToProperties()) + } if msg.Groups != nil { myProperties.Set("$groups", msg.Groups) diff --git a/feature_flag_errors.go b/feature_flag_errors.go index af4f32f8..324d1e15 100644 --- a/feature_flag_errors.go +++ b/feature_flag_errors.go @@ -77,6 +77,9 @@ type featureFlagEvaluationResult struct { RequestID *string EvaluatedAt *int64 FlagDetail *FlagDetail + // MinimalFlagCalledEvents carries the minimal $feature_flag_called gate + // from the /flags response that produced this result. + MinimalFlagCalledEvents bool } // classifyError determines the error type string for a given error. diff --git a/feature_flag_evaluations.go b/feature_flag_evaluations.go index cf2c9620..cc36a307 100644 --- a/feature_flag_evaluations.go +++ b/feature_flag_evaluations.go @@ -49,6 +49,10 @@ type evaluatedFlagRecord struct { LocallyEvaluated bool HasExperiment *bool Error *string + // MinimalFlagCalledEvents is the minimal $feature_flag_called gate from + // the source that produced this record (local definitions or the remote + // /flags response). + MinimalFlagCalledEvents bool } // featureFlagEvaluationsHost is the small callback surface a snapshot uses to @@ -57,7 +61,7 @@ type evaluatedFlagRecord struct { // the SDK's Logger; users who want them silenced should pass a Logger that // drops Warnf calls. type featureFlagEvaluationsHost struct { - captureFlagCalledIfNeeded func(distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups) + captureFlagCalledIfNeeded func(distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups, minimal bool) logger Logger } @@ -301,7 +305,8 @@ func (e *FeatureFlagEvaluations) recordAccess(key string) { if alreadyAccessed { return } - e.host.captureFlagCalledIfNeeded(e.distinctId, key, response, e.deviceId, properties, e.groups) + minimal := found && shouldMinimizeFlagCalledEvent(flag.MinimalFlagCalledEvents, flag.HasExperiment) + e.host.captureFlagCalledIfNeeded(e.distinctId, key, response, e.deviceId, properties, e.groups, minimal) } // cloneWith builds a child snapshot with the given flag set. The accessed set diff --git a/feature_flag_evaluations_test.go b/feature_flag_evaluations_test.go index 2787bdef..a2886295 100644 --- a/feature_flag_evaluations_test.go +++ b/feature_flag_evaluations_test.go @@ -907,6 +907,7 @@ func TestCaptureFlagCalled_DedupesByFlagValue(t *testing.T) { nil, NewProperties().Set("$feature_flag", "changing-flag").Set("$feature_flag_response", tc.response), nil, + false, ) } }) diff --git a/featureflags.go b/featureflags.go index 99b5c4a6..c34414e1 100644 --- a/featureflags.go +++ b/featureflags.go @@ -67,6 +67,9 @@ type flagsState struct { cohorts map[string]PropertyGroup groups map[string]string flagsEtag string + // minimalFlagCalledEvents is the server-controlled gate for minimal + // $feature_flag_called events, cached from the local-evaluation payload. + minimalFlagCalledEvents bool } // FeatureFlagsPoller periodically loads feature flag definitions for local evaluation. @@ -219,6 +222,10 @@ type FeatureFlagsResponse struct { GroupTypeMapping *map[string]string `json:"group_type_mapping"` // Cohorts contains cohort definitions referenced by local feature flags. Cohorts map[string]PropertyGroup `json:"cohorts"` + // MinimalFlagCalledEvents reports whether the server enabled minimal + // $feature_flag_called events for this project. The server sends it only + // when the gate is on; absence means full events. + MinimalFlagCalledEvents bool `json:"minimal_flag_called_events"` } // DecideRequestData is the legacy wire-format request body for flag decide calls. @@ -529,11 +536,12 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { if newEtag := res.Header.Get("ETag"); newEtag != "" && currentState != nil { // Atomically swap with updated ETag newState := &flagsState{ - featureFlags: currentState.featureFlags, - flagsByKey: currentState.flagsByKey, - cohorts: currentState.cohorts, - groups: currentState.groups, - flagsEtag: newEtag, + featureFlags: currentState.featureFlags, + flagsByKey: currentState.flagsByKey, + cohorts: currentState.cohorts, + groups: currentState.groups, + flagsEtag: newEtag, + minimalFlagCalledEvents: currentState.minimalFlagCalledEvents, } poller.state.Store(newState) } @@ -590,14 +598,23 @@ func (poller *FeatureFlagsPoller) fetchNewFeatureFlags() { // Atomic swap of entire state poller.state.Store(&flagsState{ - featureFlags: newFlags, - flagsByKey: flagsByKey, - cohorts: parsedCohorts, - groups: groups, - flagsEtag: newEtag, + featureFlags: newFlags, + flagsByKey: flagsByKey, + cohorts: parsedCohorts, + groups: groups, + flagsEtag: newEtag, + minimalFlagCalledEvents: featureFlagsResponse.MinimalFlagCalledEvents, }) } +// getMinimalFlagCalledEvents reports whether the local-evaluation payload +// enabled minimal $feature_flag_called events. False until definitions have +// been loaded, so missing state always yields full events. +func (poller *FeatureFlagsPoller) getMinimalFlagCalledEvents() bool { + state := poller.state.Load() + return state != nil && state.minimalFlagCalledEvents +} + // GetFeatureFlag evaluates one flag using locally loaded definitions when possible. // It returns the flag value, whether that value was locally evaluated, and an error. // If local evaluation is inconclusive and OnlyEvaluateLocally is false, it falls back to /flags. @@ -680,6 +697,10 @@ type flagValueAndPayload struct { err error locallyEvaluated bool hasExperiment *bool + // minimalFlagCalledEvents carries the minimal $feature_flag_called gate + // from whichever source produced the value (local definitions or the + // remote /flags response). + minimalFlagCalledEvents bool } // GetFeatureFlagWithPayload evaluates a feature flag once and returns both its value @@ -717,6 +738,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFl locallyEvaluated := err == nil && result != nil hasExperiment := flag.HasExperiment + minimalFlagCalledEvents := poller.getMinimalFlagCalledEvents() // Fall back to remote evaluation if local didn't produce a result if (err != nil || result == nil) && !flagConfig.OnlyEvaluateLocally { @@ -728,10 +750,12 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFl // Clear local eval error — we successfully made a remote request err = nil // The remote response is now the source of the flag value, so it is - // also the source of has_experiment: reset to unknown and only pick - // it up from the response when the flag is present there. + // also the source of has_experiment and of the minimal-event gate: + // reset both and only pick them up from the response. hasExperiment = nil + minimalFlagCalledEvents = false if flagsResponse != nil { + minimalFlagCalledEvents = flagsResponse.MinimalFlagCalledEvents if flagValue, ok := flagsResponse.FeatureFlags[flagConfig.Key]; ok { result = flagValue } else { @@ -749,7 +773,7 @@ func (poller *FeatureFlagsPoller) GetFeatureFlagWithPayload(flagConfig FeatureFl } } - return flagValueAndPayload{value: result, payload: payload, err: err, locallyEvaluated: locallyEvaluated, hasExperiment: hasExperiment} + return flagValueAndPayload{value: result, payload: payload, err: err, locallyEvaluated: locallyEvaluated, hasExperiment: hasExperiment, minimalFlagCalledEvents: minimalFlagCalledEvents} } func (poller *FeatureFlagsPoller) getFeatureFlag(flagConfig FeatureFlagPayload) (FeatureFlag, error) { diff --git a/flags.go b/flags.go index f6bfa5f2..4aa3eaaa 100644 --- a/flags.go +++ b/flags.go @@ -137,6 +137,10 @@ type CommonResponseFields struct { EvaluatedAt *int64 `json:"evaluatedAt"` // ErrorsWhileComputingFlags reports whether the server had errors computing any flags. ErrorsWhileComputingFlags bool `json:"errorsWhileComputingFlags"` + // MinimalFlagCalledEvents reports whether the server enabled minimal + // $feature_flag_called events for this project. The server sends it only + // when the gate is on; absence means full events. + MinimalFlagCalledEvents bool `json:"minimalFlagCalledEvents"` } // UnmarshalJSON implements custom unmarshaling to handle both v3 and v4 formats diff --git a/minimal_flag_called_events_test.go b/minimal_flag_called_events_test.go new file mode 100644 index 00000000..210449ee --- /dev/null +++ b/minimal_flag_called_events_test.go @@ -0,0 +1,503 @@ +package posthog + +import ( + "fmt" + "net/http" + "net/http/httptest" + "reflect" + "sort" + "strings" + "testing" + "time" +) + +// minimalEventsFlagsResponse builds a v4 /flags response covering the three +// has_experiment shapes: false (plain-flag), true (experiment-flag), and +// absent (unknown-flag). When gated, the response carries the top-level +// minimalFlagCalledEvents field the server only sends when the gate is on. +func minimalEventsFlagsResponse(gated bool) string { + gate := "" + if gated { + gate = `"minimalFlagCalledEvents": true,` + } + return fmt.Sprintf(`{ + %s + "flags": { + "plain-flag": { + "key": "plain-flag", + "enabled": true, + "variant": null, + "reason": {"code": "condition_match", "description": "Matched condition set 1", "condition_index": 0}, + "metadata": {"id": 1, "version": 2, "payload": "{\"foo\": 1}", "has_experiment": false} + }, + "experiment-flag": { + "key": "experiment-flag", + "enabled": true, + "variant": null, + "reason": {"code": "condition_match", "description": "Matched condition set 1", "condition_index": 0}, + "metadata": {"id": 2, "version": 3, "payload": null, "has_experiment": true} + }, + "unknown-flag": { + "key": "unknown-flag", + "enabled": true, + "variant": null, + "reason": {"code": "condition_match", "description": "Matched condition set 1", "condition_index": 0}, + "metadata": {"id": 3, "version": 4, "payload": null} + } + }, + "requestId": "req-42", + "evaluatedAt": 1737312368000 + }`, gate) +} + +// minimalEventsLocalDefinitions builds a local-evaluation definitions payload +// with the same three has_experiment shapes. When gated, the payload carries +// the top-level minimal_flag_called_events field. +func minimalEventsLocalDefinitions(gated bool) string { + gate := "" + if gated { + gate = `"minimal_flag_called_events": true,` + } + return fmt.Sprintf(`{ + %s + "flags": [ + {"id": 1, "key": "plain-flag", "active": true, "has_experiment": false, "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}}, + {"id": 2, "key": "experiment-flag", "active": true, "has_experiment": true, "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}}, + {"id": 3, "key": "unknown-flag", "active": true, "filters": {"groups": [{"properties": [], "rollout_percentage": 100}]}} + ] + }`, gate) +} + +func newMinimalEventsRemoteServer(t *testing.T, flagsResponse string) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case r.URL.Path == "/flags" || r.URL.Path == "/flags/": + w.Write([]byte(flagsResponse)) + case strings.HasPrefix(r.URL.Path, "/batch"): + w.Write([]byte(`{}`)) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + } + })) + t.Cleanup(server.Close) + return server +} + +func newMinimalEventsLocalServer(t *testing.T, definitions string) *httptest.Server { + t.Helper() + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasPrefix(r.URL.Path, "/flags/definitions"): + w.Write([]byte(definitions)) + case strings.HasPrefix(r.URL.Path, "/batch"): + w.Write([]byte(`{}`)) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + } + })) + t.Cleanup(server.Close) + return server +} + +// assertExactPropertyKeys asserts the event carries exactly the expected +// property keys — the strict-allowlist guarantee of the minimal shape. +func assertExactPropertyKeys(t *testing.T, event *CaptureInApi, want []string) { + t.Helper() + got := make([]string, 0, len(event.Properties)) + for k := range event.Properties { + got = append(got, k) + } + sort.Strings(got) + wantSorted := append([]string(nil), want...) + sort.Strings(wantSorted) + if !reflect.DeepEqual(got, wantSorted) { + t.Errorf("expected exactly property keys %v, got %v", wantSorted, got) + } +} + +// assertFullEventShape asserts the event kept the full legacy envelope: +// system context and DefaultEventProperties survive. +func assertFullEventShape(t *testing.T, event *CaptureInApi) { + t.Helper() + if _, ok := event.Properties["$os"]; !ok { + t.Error("expected full event to carry system context ($os)") + } + if event.Properties["app_version"] != "1.2.3" { + t.Errorf("expected full event to carry DefaultEventProperties, got app_version=%v", event.Properties["app_version"]) + } +} + +func withDefaultEventProperties(c *Config) { + c.DefaultEventProperties = NewProperties().Set("app_version", "1.2.3") +} + +func TestGetFeatureFlag_Remote_GatedNoExperiment_SendsMinimalEvent(t *testing.T) { + t.Parallel() + server := newMinimalEventsRemoteServer(t, minimalEventsFlagsResponse(true)) + client, capture, _ := newEvalClient(t, server, withDefaultEventProperties) + + deviceId := "device-1" + if _, err := client.GetFeatureFlag(FeatureFlagPayload{ + Key: "plain-flag", + DistinctId: "user-1", + DeviceId: &deviceId, + Groups: NewGroups().Set("company", "id:5"), + }); err != nil { + t.Fatalf("GetFeatureFlag error: %v", err) + } + + events := waitForEventCount(capture, 1, 5*time.Second) + event := findEvent(events, "$feature_flag_called", "plain-flag") + if event == nil { + t.Fatal("expected $feature_flag_called for plain-flag") + } + assertExactPropertyKeys(t, event, []string{ + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "$feature_flag_id", + "$feature_flag_version", + "$feature_flag_reason", + "$feature_flag_request_id", + "$feature_flag_evaluated_at", + "locally_evaluated", + "$groups", + "$device_id", + "$geoip_disable", + "$is_server", + "$lib", + "$lib_version", + }) + if event.Properties["$device_id"] != "device-1" { + t.Errorf("expected minimal event to keep $device_id, got %v", event.Properties["$device_id"]) + } + if event.Properties["$is_server"] != true { + t.Errorf("expected minimal event to keep $is_server=true, got %v", event.Properties["$is_server"]) + } + if event.Properties["$feature_flag_has_experiment"] != false { + t.Errorf("expected $feature_flag_has_experiment=false, got %v", event.Properties["$feature_flag_has_experiment"]) + } + if event.Properties["locally_evaluated"] != false { + t.Errorf("expected locally_evaluated=false, got %v", event.Properties["locally_evaluated"]) + } +} + +func TestGetFeatureFlag_Remote_FullEventWhenSignalMissing(t *testing.T) { + t.Parallel() + tests := []struct { + name string + gated bool + flagKey string + }{ + {name: "gated flag with experiment", gated: true, flagKey: "experiment-flag"}, + {name: "gated flag with unknown has_experiment", gated: true, flagKey: "unknown-flag"}, + {name: "ungated flag without experiment", gated: false, flagKey: "plain-flag"}, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + server := newMinimalEventsRemoteServer(t, minimalEventsFlagsResponse(test.gated)) + client, capture, _ := newEvalClient(t, server, withDefaultEventProperties) + + if _, err := client.GetFeatureFlag(FeatureFlagPayload{Key: test.flagKey, DistinctId: "user-1"}); err != nil { + t.Fatalf("GetFeatureFlag error: %v", err) + } + + events := waitForEventCount(capture, 1, 5*time.Second) + event := findEvent(events, "$feature_flag_called", test.flagKey) + if event == nil { + t.Fatalf("expected $feature_flag_called for %s", test.flagKey) + } + assertFullEventShape(t, event) + }) + } +} + +func TestGetFeatureFlag_LocalEvaluation_GatedNoExperiment_SendsMinimalEvent(t *testing.T) { + t.Parallel() + server := newMinimalEventsLocalServer(t, minimalEventsLocalDefinitions(true)) + client, capture, _ := newEvalClient(t, server, withDefaultEventProperties, func(c *Config) { + c.PersonalApiKey = "personal-key" + }) + waitForFlagDefinitions(t, client) + + if _, err := client.GetFeatureFlag(FeatureFlagPayload{Key: "plain-flag", DistinctId: "user-1"}); err != nil { + t.Fatalf("GetFeatureFlag error: %v", err) + } + + events := waitForEventCount(capture, 1, 5*time.Second) + event := findEvent(events, "$feature_flag_called", "plain-flag") + if event == nil { + t.Fatal("expected $feature_flag_called for plain-flag") + } + assertExactPropertyKeys(t, event, []string{ + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "locally_evaluated", + "$geoip_disable", + "$is_server", + "$lib", + "$lib_version", + }) + if event.Properties["locally_evaluated"] != true { + t.Errorf("expected locally_evaluated=true, got %v", event.Properties["locally_evaluated"]) + } +} + +func TestGetFeatureFlag_LocalEvaluation_FullEventWhenSignalMissing(t *testing.T) { + t.Parallel() + tests := []struct { + name string + gated bool + flagKey string + }{ + {name: "gated flag with experiment", gated: true, flagKey: "experiment-flag"}, + {name: "gated flag with unknown has_experiment", gated: true, flagKey: "unknown-flag"}, + {name: "ungated flag without experiment", gated: false, flagKey: "plain-flag"}, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + server := newMinimalEventsLocalServer(t, minimalEventsLocalDefinitions(test.gated)) + client, capture, _ := newEvalClient(t, server, withDefaultEventProperties, func(c *Config) { + c.PersonalApiKey = "personal-key" + }) + waitForFlagDefinitions(t, client) + + if _, err := client.GetFeatureFlag(FeatureFlagPayload{Key: test.flagKey, DistinctId: "user-1"}); err != nil { + t.Fatalf("GetFeatureFlag error: %v", err) + } + + events := waitForEventCount(capture, 1, 5*time.Second) + event := findEvent(events, "$feature_flag_called", test.flagKey) + if event == nil { + t.Fatalf("expected $feature_flag_called for %s", test.flagKey) + } + assertFullEventShape(t, event) + }) + } +} + +func TestEvaluateFlags_GatedNoExperiment_SendsMinimalEvent(t *testing.T) { + t.Parallel() + server := newMinimalEventsRemoteServer(t, minimalEventsFlagsResponse(true)) + client, capture, _ := newEvalClient(t, server, withDefaultEventProperties) + + snap, err := client.EvaluateFlags(EvaluateFlagsPayload{DistinctId: "user-1"}) + if err != nil { + t.Fatalf("EvaluateFlags error: %v", err) + } + snap.IsEnabled("plain-flag") + snap.IsEnabled("experiment-flag") + + events := waitForEventCount(capture, 2, 5*time.Second) + + // The gated no-experiment flag sends the minimal shape; the snapshot + // path's extras ($feature/, $feature_flag_payload) are stripped. + minimalEvent := findEvent(events, "$feature_flag_called", "plain-flag") + if minimalEvent == nil { + t.Fatal("expected $feature_flag_called for plain-flag") + } + assertExactPropertyKeys(t, minimalEvent, []string{ + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "$feature_flag_id", + "$feature_flag_version", + "$feature_flag_reason", + "$feature_flag_request_id", + "$feature_flag_evaluated_at", + "locally_evaluated", + // EvaluateFlags normalizes nil Groups to an empty map, so $groups is + // present (and empty) exactly as it is on full snapshot events. + "$groups", + "$geoip_disable", + "$is_server", + "$lib", + "$lib_version", + }) + + // The experiment-linked flag keeps the full envelope. + fullEvent := findEvent(events, "$feature_flag_called", "experiment-flag") + if fullEvent == nil { + t.Fatal("expected $feature_flag_called for experiment-flag") + } + assertFullEventShape(t, fullEvent) + if fullEvent.Properties["$feature/experiment-flag"] != true { + t.Errorf("expected full event to keep $feature/experiment-flag, got %v", fullEvent.Properties["$feature/experiment-flag"]) + } +} + +func TestMinimalFlagCalledEvent_V1WireShape(t *testing.T) { + t.Parallel() + msg := Capture{ + DistinctId: "user-1", + Event: "$feature_flag_called", + Properties: NewProperties(). + Set("$feature_flag", "plain-flag"). + Set("$feature_flag_response", true). + Set("$feature_flag_has_experiment", false). + Set("locally_evaluated", true). + Set("$feature_flag_payload", `{"foo": 1}`). + Set("custom_prop", "junk"), + IsServer: true, + minimalFlagCalledEvent: true, + } + + ev := buildV1Event(msg.apifyEvent(), nil) + + got := make([]string, 0, len(ev.Properties)) + for k := range ev.Properties { + got = append(got, k) + } + sort.Strings(got) + want := []string{"$feature_flag", "$feature_flag_has_experiment", "$feature_flag_response", "$is_server", "locally_evaluated"} + if !reflect.DeepEqual(got, want) { + t.Errorf("expected exactly property keys %v, got %v", want, got) + } + if ev.Properties["$is_server"] != true { + t.Errorf("expected minimal v1 event to keep $is_server=true, got %v", ev.Properties["$is_server"]) + } +} + +func TestMinimalFlagCalledEvent_V1LiftsSessionId(t *testing.T) { + t.Parallel() + msg := Capture{ + DistinctId: "user-1", + Event: "$feature_flag_called", + Properties: NewProperties(). + Set("$feature_flag", "plain-flag"). + Set("$feature_flag_response", true). + Set("$feature_flag_has_experiment", false). + Set(propertySessionID, "sess-1"), + minimalFlagCalledEvent: true, + } + + ev := buildV1Event(msg.apifyEvent(), nil) + + if ev.SessionId != "sess-1" { + t.Errorf("expected $session_id to be lifted to the top-level session_id field, got %q", ev.SessionId) + } + if _, ok := ev.Properties[propertySessionID]; ok { + t.Error("expected $session_id to be removed from properties after the v1 lift") + } +} + +// minimalEventsFallbackDefinitions returns definitions whose only flag is +// gated on a person property the request does not supply, so local evaluation +// is inconclusive and the SDK falls back to a remote /flags request. +func minimalEventsFallbackDefinitions(gated bool) string { + gate := "" + if gated { + gate = `"minimal_flag_called_events": true,` + } + return fmt.Sprintf(`{ + %s + "flags": [ + {"id": 1, "key": "plain-flag", "active": true, "has_experiment": false, "filters": {"groups": [{"properties": [{"key": "region", "operator": "exact", "value": ["USA"], "type": "person"}], "rollout_percentage": 100}]}} + ] + }`, gate) +} + +func TestGetFeatureFlag_RemoteFallback_GateComesFromFlagsResponse(t *testing.T) { + t.Parallel() + tests := []struct { + name string + localGated bool + remoteGated bool + wantMinimal bool + }{ + {name: "local gate on but ungated remote response stays full", localGated: true, remoteGated: false, wantMinimal: false}, + {name: "local gate off but gated remote response goes minimal", localGated: false, remoteGated: true, wantMinimal: true}, + } + for _, test := range tests { + test := test + t.Run(test.name, func(t *testing.T) { + t.Parallel() + definitions := minimalEventsFallbackDefinitions(test.localGated) + flagsResponse := minimalEventsFlagsResponse(test.remoteGated) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch { + case strings.HasPrefix(r.URL.Path, "/flags/definitions"): + w.Write([]byte(definitions)) + case r.URL.Path == "/flags" || r.URL.Path == "/flags/": + w.Write([]byte(flagsResponse)) + case strings.HasPrefix(r.URL.Path, "/batch"): + w.Write([]byte(`{}`)) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + } + })) + t.Cleanup(server.Close) + + client, capture, _ := newEvalClient(t, server, withDefaultEventProperties, func(c *Config) { + c.PersonalApiKey = "personal-key" + }) + waitForFlagDefinitions(t, client) + + if _, err := client.GetFeatureFlag(FeatureFlagPayload{Key: "plain-flag", DistinctId: "user-1"}); err != nil { + t.Fatalf("GetFeatureFlag error: %v", err) + } + + events := waitForEventCount(capture, 1, 5*time.Second) + event := findEvent(events, "$feature_flag_called", "plain-flag") + if event == nil { + t.Fatal("expected $feature_flag_called for plain-flag") + } + if event.Properties["locally_evaluated"] != false { + t.Errorf("expected the flag value to come from the remote fallback (locally_evaluated=false), got %v", event.Properties["locally_evaluated"]) + } + if test.wantMinimal { + assertExactPropertyKeys(t, event, []string{ + "$feature_flag", + "$feature_flag_response", + "$feature_flag_has_experiment", + "locally_evaluated", + "$geoip_disable", + "$is_server", + "$lib", + "$lib_version", + }) + } else { + assertFullEventShape(t, event) + } + }) + } +} + +func TestFetchNewFeatureFlags_304PreservesMinimalGate(t *testing.T) { + t.Parallel() + var requestCount int + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasPrefix(r.URL.Path, "/flags/definitions") { + requestCount++ + w.Header().Set("ETag", `"gate-etag"`) + if requestCount == 1 { + w.Write([]byte(minimalEventsLocalDefinitions(true))) + } else { + w.WriteHeader(http.StatusNotModified) + } + } + })) + t.Cleanup(server.Close) + + poller := newTestPoller(t, server.URL) + poller.fetchNewFeatureFlags() + if !poller.getMinimalFlagCalledEvents() { + t.Fatal("expected the minimal gate to be set from the initial definitions fetch") + } + + // A 304 refresh swaps in a new state carrying the refreshed ETag; the + // gate must survive that swap. + poller.fetchNewFeatureFlags() + if requestCount != 2 { + t.Fatalf("expected 2 definition requests, got %d", requestCount) + } + if !poller.getMinimalFlagCalledEvents() { + t.Error("expected a 304 refresh to preserve the minimal gate") + } +} diff --git a/posthog.go b/posthog.go index 4b65c46e..b127c526 100644 --- a/posthog.go +++ b/posthog.go @@ -868,6 +868,7 @@ func (c *client) getFeatureFlagResultWithContext(ctx context.Context, flagConfig var hasPayload, hasVariant bool var locallyEvaluated bool var hasExperiment *bool + var minimalFlagCalledEvents bool if c.featureFlagsPoller != nil { // Evaluate flag once to get both value and payload (avoids double evaluation) @@ -875,6 +876,7 @@ func (c *client) getFeatureFlagResultWithContext(ctx context.Context, flagConfig flagValue = combined.value locallyEvaluated = combined.locallyEvaluated hasExperiment = combined.hasExperiment + minimalFlagCalledEvents = combined.minimalFlagCalledEvents err = combined.err evalResult.Value = flagValue evalResult.Err = err @@ -895,6 +897,7 @@ func (c *client) getFeatureFlagResultWithContext(ctx context.Context, flagConfig evalResult = *remoteResult flagValue = evalResult.Value err = evalResult.Err + minimalFlagCalledEvents = evalResult.MinimalFlagCalledEvents if f, ok := flagValue.(FlagDetail); ok { flagValue = f.GetValue() evalResult.Value = flagValue @@ -951,7 +954,8 @@ func (c *client) getFeatureFlagResultWithContext(ctx context.Context, flagConfig properties.Set("$feature_flag_error", errorString) } - c.captureFlagCalledIfNeeded(flagConfig.DistinctId, flagConfig.Key, flagValue, flagConfig.DeviceId, properties, flagConfig.Groups) + minimal := shouldMinimizeFlagCalledEvent(minimalFlagCalledEvents, hasExperiment) + c.captureFlagCalledIfNeeded(flagConfig.DistinctId, flagConfig.Key, flagValue, flagConfig.DeviceId, properties, flagConfig.Groups, minimal) } if flagValue == nil { @@ -996,12 +1000,13 @@ func (c *client) getFeatureFlagResultWithContext(ctx context.Context, flagConfig // responsible for building the full properties dict; this helper only handles // dedup and enqueue. It is shared by the legacy per-flag evaluation path and // the FeatureFlagEvaluations snapshot path so both dedupe identically against -// the same per-distinct_id LRU cache. -func (c *client) captureFlagCalledIfNeeded(distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups) { - c.captureFlagCalledIfNeededWithContext(context.Background(), distinctId, key, featureFlagResponse, deviceId, properties, groups) +// the same per-distinct_id LRU cache. When minimal is true the event is +// serialized in the minimal $feature_flag_called shape. +func (c *client) captureFlagCalledIfNeeded(distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups, minimal bool) { + c.captureFlagCalledIfNeededWithContext(context.Background(), distinctId, key, featureFlagResponse, deviceId, properties, groups, minimal) } -func (c *client) captureFlagCalledIfNeededWithContext(ctx context.Context, distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups) { +func (c *client) captureFlagCalledIfNeededWithContext(ctx context.Context, distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups, minimal bool) { deviceIDStr := "" if deviceId != nil { deviceIDStr = *deviceId @@ -1017,10 +1022,11 @@ func (c *client) captureFlagCalledIfNeededWithContext(ctx context.Context, disti return } if err := c.EnqueueWithContext(ctx, Capture{ - DistinctId: distinctId, - Event: "$feature_flag_called", - Properties: properties, - Groups: groups, + DistinctId: distinctId, + Event: "$feature_flag_called", + Properties: properties, + Groups: groups, + minimalFlagCalledEvent: minimal, }); err == nil { c.distinctIdsFeatureFlagsReported.Add(cacheKey, struct{}{}) } @@ -1223,7 +1229,7 @@ func (c *client) evaluateFlagsWithContext(ctx context.Context, payload EvaluateF if _, alreadyLocal := locallyEvaluated[key]; alreadyLocal { continue } - records[key] = recordFromFlagDetail(detail) + records[key] = recordFromFlagDetail(detail, flagsResponse.MinimalFlagCalledEvents) } } } @@ -1265,6 +1271,7 @@ func (c *client) populateLocalEvaluations(records map[string]evaluatedFlagRecord cohorts := poller.getCohorts() fallbackToRemote := false + minimalFlagCalledEvents := poller.getMinimalFlagCalledEvents() const localReason = "Evaluated locally" for _, storedFlag := range featureFlags { @@ -1289,10 +1296,11 @@ func (c *client) populateLocalEvaluations(records map[string]evaluatedFlagRecord } record := evaluatedFlagRecord{ - Key: storedFlag.Key, - LocallyEvaluated: true, - HasExperiment: storedFlag.HasExperiment, - Reason: ptrString(localReason), + Key: storedFlag.Key, + LocallyEvaluated: true, + HasExperiment: storedFlag.HasExperiment, + MinimalFlagCalledEvents: minimalFlagCalledEvents, + Reason: ptrString(localReason), } switch v := value.(type) { case bool: @@ -1326,12 +1334,15 @@ func (c *client) populateLocalEvaluations(records map[string]evaluatedFlagRecord } // recordFromFlagDetail builds an evaluatedFlagRecord from a v4 FlagDetail. -func recordFromFlagDetail(detail FlagDetail) evaluatedFlagRecord { +// minimalFlagCalledEvents is the gate from the /flags response the detail +// came from. +func recordFromFlagDetail(detail FlagDetail, minimalFlagCalledEvents bool) evaluatedFlagRecord { record := evaluatedFlagRecord{ - Key: detail.Key, - Enabled: detail.Enabled, - Variant: detail.Variant, - HasExperiment: detail.Metadata.HasExperiment, + Key: detail.Key, + Enabled: detail.Enabled, + Variant: detail.Variant, + HasExperiment: detail.Metadata.HasExperiment, + MinimalFlagCalledEvents: minimalFlagCalledEvents, } if detail.Failed != nil && *detail.Failed { record.Enabled = false @@ -1358,8 +1369,8 @@ func ptrString(s string) *string { return &s } // featureFlagEvaluationsHostWithContext wires the snapshot's callbacks to this client. func (c *client) featureFlagEvaluationsHostWithContext(ctx context.Context) featureFlagEvaluationsHost { return featureFlagEvaluationsHost{ - captureFlagCalledIfNeeded: func(distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups) { - c.captureFlagCalledIfNeededWithContext(ctx, distinctId, key, featureFlagResponse, deviceId, properties, groups) + captureFlagCalledIfNeeded: func(distinctId, key string, featureFlagResponse interface{}, deviceId *string, properties Properties, groups Groups, minimal bool) { + c.captureFlagCalledIfNeededWithContext(ctx, distinctId, key, featureFlagResponse, deviceId, properties, groups, minimal) }, logger: c.Logger, } @@ -1925,6 +1936,7 @@ func (c *client) getFeatureFlagFromRemote(key string, distinctId string, deviceI result.EvaluatedAt = flagsResponse.EvaluatedAt result.ErrorsWhileComputingFlags = flagsResponse.ErrorsWhileComputingFlags result.QuotaLimited = c.isFeatureFlagsQuotaLimited(flagsResponse) + result.MinimalFlagCalledEvents = flagsResponse.MinimalFlagCalledEvents if result.QuotaLimited { return result From 9d3ea3c29145f5de23d91a8ccf7d76a6736325c6 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Fri, 17 Jul 2026 17:51:15 -0700 Subject: [PATCH 2/3] Keep static platform/runtime identity on minimal flag called events Reviewer feedback on the Python equivalent of this PR pointed out that $os, $os_version, $os_distro, and $go_version were stripped from minimal $feature_flag_called events along with the rest of system context, even though they're cheap, static per process, and useful for debugging flag behavior by platform. Merge system context into the minimal event the same way the full event does, for both the v0 and v1 wire formats. --- .changeset/minimal-flag-called-events.md | 2 +- capture.go | 19 ++++++++++++------- capture_v1.go | 3 ++- minimal_flag_called_events_test.go | 12 +++++++++++- 4 files changed, 26 insertions(+), 10 deletions(-) diff --git a/.changeset/minimal-flag-called-events.md b/.changeset/minimal-flag-called-events.md index aa5562b7..6cabc277 100644 --- a/.changeset/minimal-flag-called-events.md +++ b/.changeset/minimal-flag-called-events.md @@ -2,4 +2,4 @@ "posthog-go": minor --- -Send minimal `$feature_flag_called` events when the server enables it. When the `/flags?v=2` response carries `minimalFlagCalledEvents: true` (remote evaluation) or the local-evaluation definitions payload carries `minimal_flag_called_events: true`, and the evaluated flag has `has_experiment: false`, the event keeps only a strict allowlist of evaluation properties (`$feature_flag`, `$feature_flag_response`, `$feature_flag_has_experiment`, `$feature_flag_id`, `$feature_flag_version`, `$feature_flag_reason`, `$feature_flag_request_id`, `$feature_flag_evaluated_at`, `$feature_flag_error`, `locally_evaluated`, `$groups`, `$process_person_profile`, `$geoip_disable`, `$is_server`, `$session_id`, `$window_id`, `$device_id`, `$lib`, `$lib_version`); everything else — including `Config.DefaultEventProperties`, system context, and the snapshot path's `$feature/` and `$feature_flag_payload` — is stripped. Any missing signal (gate absent, `has_experiment` unknown, experiment-linked flag, legacy response shapes) keeps today's full event shape. The gate is server-controlled per project; no SDK configuration is added. +Send minimal `$feature_flag_called` events when the server enables it. When the `/flags?v=2` response carries `minimalFlagCalledEvents: true` (remote evaluation) or the local-evaluation definitions payload carries `minimal_flag_called_events: true`, and the evaluated flag has `has_experiment: false`, the event keeps only a strict allowlist of evaluation properties (`$feature_flag`, `$feature_flag_response`, `$feature_flag_has_experiment`, `$feature_flag_id`, `$feature_flag_version`, `$feature_flag_reason`, `$feature_flag_request_id`, `$feature_flag_evaluated_at`, `$feature_flag_error`, `locally_evaluated`, `$groups`, `$process_person_profile`, `$geoip_disable`, `$is_server`, `$session_id`, `$window_id`, `$device_id`, `$lib`, `$lib_version`) plus static system context (`$os`, `$os_version`, `$os_distro`, `$go_version`); everything else — including `Config.DefaultEventProperties` and the snapshot path's `$feature/` and `$feature_flag_payload` — is stripped. Any missing signal (gate absent, `has_experiment` unknown, experiment-linked flag, legacy response shapes) keeps today's full event shape. The gate is server-controlled per project; no SDK configuration is added. diff --git a/capture.go b/capture.go index ca75fd3f..4950889e 100644 --- a/capture.go +++ b/capture.go @@ -171,13 +171,17 @@ type CaptureInApi struct { // minimalFlagCalledEventAllowlist lists the only event properties kept on a // minimal $feature_flag_called event, per the cross-SDK contract. Everything -// else — Config.DefaultEventProperties, request-context properties, and -// system context included — is stripped so the minimal shape stays -// predictable. $geoip_disable is kept because, like $process_person_profile, -// it is a processing-control sentinel: stripping it would silently re-enable -// GeoIP enrichment for events from clients that disabled it. $session_id, +// else — Config.DefaultEventProperties and request-context properties +// included — is stripped so the minimal shape stays predictable. +// $geoip_disable is kept because, like $process_person_profile, it is a +// processing-control sentinel: stripping it would silently re-enable GeoIP +// enrichment for events from clients that disabled it. $session_id, // $window_id, and $device_id are linkage identifiers the contract preserves. -// $is_server is kept so server-event classification still works. +// $is_server is kept so server-event classification still works. System +// context ($os, $os_version, $os_distro, $go_version) isn't filtered through +// this allowlist — APIfy merges it into minimal events the same way it does +// for full events, since those are cheap, low-cardinality dimensions kept for +// platform/runtime breakdowns on flag-call debugging. var minimalFlagCalledEventAllowlist = []string{ "$feature_flag", "$feature_flag_response", @@ -226,7 +230,8 @@ func (msg Capture) APIfy() APIMessage { if msg.minimalFlagCalledEvent { myProperties = minimalFlagCalledEventProperties(msg.Properties). Set("$lib", SDKName). - Set("$lib_version", libraryVersion) + Set("$lib_version", libraryVersion). + Merge(getSystemContext().ToProperties()) } else { myProperties = Properties{}. Merge(msg.Properties). diff --git a/capture_v1.go b/capture_v1.go index 0667ab9c..79865028 100644 --- a/capture_v1.go +++ b/capture_v1.go @@ -271,7 +271,8 @@ func (msg Capture) apifyEvent() apiEvent { var myProperties Properties if msg.minimalFlagCalledEvent { myProperties = baseV1Props(msg.IsServer, false). - Merge(minimalFlagCalledEventProperties(msg.Properties)) + Merge(minimalFlagCalledEventProperties(msg.Properties)). + mergeDefaults(getSystemContext().ToProperties()) } else { myProperties = baseV1Props(msg.IsServer, false). Merge(msg.Properties). diff --git a/minimal_flag_called_events_test.go b/minimal_flag_called_events_test.go index 210449ee..309a80c5 100644 --- a/minimal_flag_called_events_test.go +++ b/minimal_flag_called_events_test.go @@ -101,7 +101,10 @@ func newMinimalEventsLocalServer(t *testing.T, definitions string) *httptest.Ser } // assertExactPropertyKeys asserts the event carries exactly the expected -// property keys — the strict-allowlist guarantee of the minimal shape. +// property keys — the strict-allowlist guarantee of the minimal shape. want +// need not include system context ($os, $os_version, $os_distro, +// $go_version): those survive minimization and are appended automatically so +// the assertion stays host-agnostic (e.g. $os_distro is Linux-only). func assertExactPropertyKeys(t *testing.T, event *CaptureInApi, want []string) { t.Helper() got := make([]string, 0, len(event.Properties)) @@ -110,6 +113,9 @@ func assertExactPropertyKeys(t *testing.T, event *CaptureInApi, want []string) { } sort.Strings(got) wantSorted := append([]string(nil), want...) + for k := range getSystemContext().ToProperties() { + wantSorted = append(wantSorted, k) + } sort.Strings(wantSorted) if !reflect.DeepEqual(got, wantSorted) { t.Errorf("expected exactly property keys %v, got %v", wantSorted, got) @@ -356,6 +362,10 @@ func TestMinimalFlagCalledEvent_V1WireShape(t *testing.T) { } sort.Strings(got) want := []string{"$feature_flag", "$feature_flag_has_experiment", "$feature_flag_response", "$is_server", "locally_evaluated"} + for k := range getSystemContext().ToProperties() { + want = append(want, k) + } + sort.Strings(want) if !reflect.DeepEqual(got, want) { t.Errorf("expected exactly property keys %v, got %v", want, got) } From c0b84fa27c5d477edf7a737f68519ac0db2c2592 Mon Sep 17 00:00:00 2001 From: Phil Haack Date: Fri, 17 Jul 2026 21:23:07 -0700 Subject: [PATCH 3/3] Extract shared property selection and tighten allowlist coverage Extract Capture.selectedProperties() to remove the duplicated minimal/full branching between APIfy (v0) and apifyEvent (v1). Add a direct test locking the full 17-entry minimal allowlist contract, and harden the 304 gate- persistence test to fail loudly on unexpected requests. --- capture.go | 35 ++++++++++++-------- capture_v1.go | 13 ++------ minimal_flag_called_events_test.go | 51 +++++++++++++++++++++++++----- 3 files changed, 67 insertions(+), 32 deletions(-) diff --git a/capture.go b/capture.go index 4950889e..f091e300 100644 --- a/capture.go +++ b/capture.go @@ -120,7 +120,9 @@ type Capture struct { // minimalFlagCalledEvent marks a $feature_flag_called event for the minimal // shape: serialization keeps only the allowlisted evaluation properties and // skips system context. It is set only when the server enabled - // minimal_flag_called_events and the flag has no linked experiment. + // minimal_flag_called_events and the flag has no linked experiment. This is + // the resolved per-event decision (shouldMinimizeFlagCalledEvent's output), + // distinct from the plural minimalFlagCalledEvents gate that decision reads. minimalFlagCalledEvent bool } @@ -193,6 +195,9 @@ var minimalFlagCalledEventAllowlist = []string{ "$feature_flag_evaluated_at", "$feature_flag_error", "locally_evaluated", + // $groups is listed for cross-SDK contract parity even though it currently + // has no effect here: APIfy/apifyEvent set it from Capture.Groups after + // this allowlist runs, not from a raw "$groups" key in Properties. "$groups", propertyProcessPersonProfile, propertyGeoipDisable, @@ -222,23 +227,25 @@ func shouldMinimizeFlagCalledEvent(minimalFlagCalledEvents bool, hasExperiment * return minimalFlagCalledEvents && hasExperiment != nil && !*hasExperiment } +// selectedProperties returns the source properties for serialization: the +// allowlisted minimal subset when minimalFlagCalledEvent is set, or the full +// set otherwise. +func (msg Capture) selectedProperties() Properties { + if msg.minimalFlagCalledEvent { + return minimalFlagCalledEventProperties(msg.Properties) + } + return msg.Properties +} + // APIfy converts a Capture message into the PostHog batch API representation. func (msg Capture) APIfy() APIMessage { libraryVersion := getVersion() - var myProperties Properties - if msg.minimalFlagCalledEvent { - myProperties = minimalFlagCalledEventProperties(msg.Properties). - Set("$lib", SDKName). - Set("$lib_version", libraryVersion). - Merge(getSystemContext().ToProperties()) - } else { - myProperties = Properties{}. - Merge(msg.Properties). - Set("$lib", SDKName). - Set("$lib_version", libraryVersion). - Merge(getSystemContext().ToProperties()) - } + myProperties := Properties{}. + Merge(msg.selectedProperties()). + Set("$lib", SDKName). + Set("$lib_version", libraryVersion). + Merge(getSystemContext().ToProperties()) if msg.IsServer { myProperties.Set("$is_server", true) diff --git a/capture_v1.go b/capture_v1.go index 79865028..b5013541 100644 --- a/capture_v1.go +++ b/capture_v1.go @@ -268,16 +268,9 @@ func prepareForSendV1(msg Message, logger Logger) (json.RawMessage, APIMessage, // properties APIfy assembles, minus $lib/$lib_version (the PostHog-Sdk-Info // header is the authoritative SDK identity in v1). func (msg Capture) apifyEvent() apiEvent { - var myProperties Properties - if msg.minimalFlagCalledEvent { - myProperties = baseV1Props(msg.IsServer, false). - Merge(minimalFlagCalledEventProperties(msg.Properties)). - mergeDefaults(getSystemContext().ToProperties()) - } else { - myProperties = baseV1Props(msg.IsServer, false). - Merge(msg.Properties). - mergeDefaults(getSystemContext().ToProperties()) - } + myProperties := baseV1Props(msg.IsServer, false). + Merge(msg.selectedProperties()). + mergeDefaults(getSystemContext().ToProperties()) if msg.Groups != nil { myProperties.Set("$groups", msg.Groups) diff --git a/minimal_flag_called_events_test.go b/minimal_flag_called_events_test.go index 309a80c5..b2d3d0c4 100644 --- a/minimal_flag_called_events_test.go +++ b/minimal_flag_called_events_test.go @@ -397,6 +397,39 @@ func TestMinimalFlagCalledEvent_V1LiftsSessionId(t *testing.T) { } } +// TestMinimalFlagCalledEventProperties_KeepsExactlyAllowlist locks the full +// minimalFlagCalledEventAllowlist contract in one place. The expected list is +// hardcoded rather than derived from minimalFlagCalledEventAllowlist itself, +// so shrinking the allowlist shrinks only the input here and this test still +// catches the regression. +func TestMinimalFlagCalledEventProperties_KeepsExactlyAllowlist(t *testing.T) { + t.Parallel() + want := []string{ + "$feature_flag", "$feature_flag_response", "$feature_flag_has_experiment", + "$feature_flag_id", "$feature_flag_version", "$feature_flag_reason", + "$feature_flag_request_id", "$feature_flag_evaluated_at", "$feature_flag_error", + "locally_evaluated", "$groups", propertyProcessPersonProfile, propertyGeoipDisable, + propertyIsServer, propertySessionID, propertyWindowID, "$device_id", + } + props := NewProperties() + for _, key := range want { + props.Set(key, "kept") + } + props.Set("custom_prop", "junk").Set("$feature/plain-flag", true).Set("app_version", "1.2.3") + + minimal := minimalFlagCalledEventProperties(props) + got := make([]string, 0, len(minimal)) + for k := range minimal { + got = append(got, k) + } + sort.Strings(got) + wantSorted := append([]string(nil), want...) + sort.Strings(wantSorted) + if !reflect.DeepEqual(got, wantSorted) { + t.Errorf("expected exactly allowlist keys %v, got %v", wantSorted, got) + } +} + // minimalEventsFallbackDefinitions returns definitions whose only flag is // gated on a person property the request does not supply, so local evaluation // is inconclusive and the SDK falls back to a remote /flags request. @@ -483,14 +516,16 @@ func TestFetchNewFeatureFlags_304PreservesMinimalGate(t *testing.T) { t.Parallel() var requestCount int server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if strings.HasPrefix(r.URL.Path, "/flags/definitions") { - requestCount++ - w.Header().Set("ETag", `"gate-etag"`) - if requestCount == 1 { - w.Write([]byte(minimalEventsLocalDefinitions(true))) - } else { - w.WriteHeader(http.StatusNotModified) - } + if !strings.HasPrefix(r.URL.Path, "/flags/definitions") { + t.Errorf("unexpected request to %s", r.URL.Path) + return + } + requestCount++ + w.Header().Set("ETag", `"gate-etag"`) + if requestCount == 1 { + w.Write([]byte(minimalEventsLocalDefinitions(true))) + } else { + w.WriteHeader(http.StatusNotModified) } })) t.Cleanup(server.Close)