From bc111cdba75a7779f5c7145237376241551440e8 Mon Sep 17 00:00:00 2001 From: Kim Gustyr Date: Fri, 7 Aug 2026 12:32:30 +0100 Subject: [PATCH] feat: Surface engine evaluation reason and variant on flags Flag now carries reason and variant, populated from the engine's evaluation result in local evaluation and from the reason and variant fields of /flags and /identities responses when evaluating remotely. Remote evaluation reads both fields optionally, so they stay empty against an API that predates them. The engine did not compute a variant at all: FlagResult now reports "control" when an identity falls in a multivariate feature's leftover allocation, the selected variant's key when one is bucketed, and nothing for standard features, unkeyed variants, or evaluation without an identity. Variant keys come from the multivariate feature options in the environment document. Bump engine-test-data to v3.10.0, which asserts variant across the corpus; v3.7.0's expected results have no variant field, so they contradict any implementation of it. beep boop --- .gitmodules | 2 +- client_test.go | 91 ++++++++++++++++++++++++ fixtures/fixture.go | 58 +++++++++++++++ flagengine/engine-test-data | 2 +- flagengine/engine.go | 8 +++ flagengine/engine_eval/context.go | 3 + flagengine/engine_eval/mappers.go | 1 + flagengine/engine_eval/result.go | 4 ++ flagengine/features/models.go | 1 + models.go | 8 +++ models_test.go | 114 +++++++++++++++++++++++++++++- 11 files changed, 288 insertions(+), 4 deletions(-) diff --git a/.gitmodules b/.gitmodules index f46350d8..5e406b4e 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,4 +1,4 @@ [submodule "flagengine/engine-test-data"] path = flagengine/engine-test-data url = git@github.com:Flagsmith/engine-test-data.git - branch = v3.7.0 + branch = v3.10.0 diff --git a/client_test.go b/client_test.go index fae8d670..c4cc8847 100644 --- a/client_test.go +++ b/client_test.go @@ -235,6 +235,8 @@ func TestGetFlags(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value) + assert.Equal(t, fixtures.Feature1Reason, allFlags[0].Reason) + assert.Empty(t, allFlags[0].Variant) } func TestGetFlagsTransientIdentity(t *testing.T) { @@ -261,6 +263,8 @@ func TestGetFlagsTransientIdentity(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value) + assert.Equal(t, fixtures.Feature1IdentityReason, allFlags[0].Reason) + assert.Equal(t, fixtures.Feature1IdentityVariant, allFlags[0].Variant) } func TestGetFlagsTransientTraits(t *testing.T) { @@ -372,6 +376,7 @@ func TestGetEnvironmentFlagsUseslocalEnvironmentWhenAvailable(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1Value, allFlags[0].Value) + assert.Equal(t, "DEFAULT", allFlags[0].Reason) } func TestGetEnvironmentFlagsCallsAPIWhenLocalEnvironmentNotAvailable(t *testing.T) { @@ -447,6 +452,91 @@ func TestGetEnvironmentFlagsIgnoresSegmentOverrides(t *testing.T) { assert.NoError(t, err) assert.Equal(t, fixtures.Feature1Value, flag.Value) assert.Equal(t, "some_value", flag.Value) + assert.Equal(t, "DEFAULT", flag.Reason) +} + +func TestGetIdentityFlagsAppliesSegmentOverridesWithReason(t *testing.T) { + // Given + ctx := context.Background() + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, fixtures.EnvironmentJsonWithSegmentOverride) + })) + defer server.Close() + + // When + client := flagsmith.NewClient(fixtures.EnvironmentAPIKey, + flagsmith.WithLocalEvaluation(ctx), + flagsmith.WithBaseURL(server.URL+"/api/v1/")) + err := client.UpdateEnvironment(ctx) + assert.NoError(t, err) + + flags, err := client.GetIdentityFlags(ctx, "test_identity", nil) + + // Then + assert.NoError(t, err) + flag, err := flags.GetFlag(fixtures.Feature1Name) + assert.NoError(t, err) + assert.Equal(t, "segment_override", flag.Value) + assert.Equal(t, "TARGETING_MATCH; segment=Test Segment", flag.Reason) +} + +func TestGetIdentityFlagsSetsVariantForMultivariateFeature(t *testing.T) { + // Given + ctx := context.Background() + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, fixtures.EnvironmentJsonWithMultivariateFeature) + })) + defer server.Close() + + // When + client := flagsmith.NewClient(fixtures.EnvironmentAPIKey, + flagsmith.WithLocalEvaluation(ctx), + flagsmith.WithBaseURL(server.URL+"/api/v1/")) + err := client.UpdateEnvironment(ctx) + assert.NoError(t, err) + + flags, err := client.GetIdentityFlags(ctx, "test_identity", nil) + + // Then + assert.NoError(t, err) + flag, err := flags.GetFlag(fixtures.MVFeatureName) + assert.NoError(t, err) + assert.Equal(t, fixtures.MVFeatureVariantValue, flag.Value) + assert.Equal(t, fixtures.MVFeatureVariantKey, flag.Variant) + assert.Equal(t, "SPLIT; weight=100", flag.Reason) +} + +func TestGetEnvironmentFlagsHasNoVariantForMultivariateFeature(t *testing.T) { + // Given + ctx := context.Background() + server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) { + rw.Header().Set("Content-Type", "application/json") + rw.WriteHeader(http.StatusOK) + _, _ = io.WriteString(rw, fixtures.EnvironmentJsonWithMultivariateFeature) + })) + defer server.Close() + + // When + client := flagsmith.NewClient(fixtures.EnvironmentAPIKey, + flagsmith.WithLocalEvaluation(ctx), + flagsmith.WithBaseURL(server.URL+"/api/v1/")) + err := client.UpdateEnvironment(ctx) + assert.NoError(t, err) + + flags, err := client.GetEnvironmentFlags(ctx) + + // Then: without an identity there is nothing to bucket, so the control value is + // served without a variant + assert.NoError(t, err) + flag, err := flags.GetFlag(fixtures.MVFeatureName) + assert.NoError(t, err) + assert.Equal(t, "control_value", flag.Value) + assert.Empty(t, flag.Variant) + assert.Equal(t, "DEFAULT", flag.Reason) } func TestGetIdentityFlagsUseslocalEnvironmentWhenAvailable(t *testing.T) { @@ -499,6 +589,7 @@ func TestGetIdentityFlagsUseslocalOverridesWhenAvailable(t *testing.T) { assert.Equal(t, fixtures.Feature1Name, allFlags[0].FeatureName) assert.Equal(t, fixtures.Feature1ID, allFlags[0].FeatureID) assert.Equal(t, fixtures.Feature1OverriddenValue, allFlags[0].Value) + assert.Equal(t, "TARGETING_MATCH; segment=identity_overrides", allFlags[0].Reason) } func TestGetIdentityFlagsCallsAPIWhenLocalEnvironmentNotAvailableWithTraits(t *testing.T) { diff --git a/fixtures/fixture.go b/fixtures/fixture.go index b899f6ad..ef83a056 100644 --- a/fixtures/fixture.go +++ b/fixtures/fixture.go @@ -10,6 +10,13 @@ const EnvironmentAPIKey = "ser.test_key" const Feature1Value = "some_value" const Feature1Name = "feature_1" const Feature1ID = 1 +const Feature1Reason = "DEFAULT" +const Feature1IdentityReason = "SPLIT; weight=50.0" +const Feature1IdentityVariant = "treatment" + +const MVFeatureName = "mv_feature" +const MVFeatureVariantKey = "treatment" +const MVFeatureVariantValue = "variant_value" const Feature1OverriddenValue = "some-overridden-value" const ClientAPIKey = "B62qaMZNwfiqT76p38ggrQ" @@ -163,6 +170,54 @@ const EnvironmentJsonWithSegmentOverride = ` } ` +// EnvironmentJsonWithMultivariateFeature contains a single multivariate feature whose +// only variant is keyed and allocated 100%, so every identity is bucketed into it. +const EnvironmentJsonWithMultivariateFeature = ` +{ + "api_key": "B62qaMZNwfiqT76p38ggrQ", + "name": "Test Environment", + "updated_at": "2023-12-06T10:21:54.079725Z", + "project": { + "name": "Test project", + "organisation": { + "feature_analytics": false, + "name": "Test Org", + "id": 1, + "persist_trait_data": true, + "stop_serving_flags": false + }, + "id": 1, + "hide_disabled_flags": false, + "segments": [] + }, + "segment_overrides": [], + "id": 1, + "feature_states": [{ + "multivariate_feature_state_values": [{ + "id": 1, + "multivariate_feature_option": { + "id": 1, + "value": "variant_value", + "key": "treatment" + }, + "percentage_allocation": 100, + "mv_fs_value_uuid": "1e1e1e1e-1e1e-1e1e-1e1e-1e1e1e1e1e1e" + }], + "feature_state_value": "control_value", + "id": 2, + "featurestate_uuid": "f0c8f0c8-f0c8-f0c8-f0c8-f0c8f0c8f0c8", + "feature": { + "name": "mv_feature", + "type": "MULTIVARIATE", + "id": 2 + }, + "segment_id": null, + "enabled": true + }], + "identity_overrides": [] +} +` + const FlagsJson = ` [{ "id": 1, @@ -177,6 +232,7 @@ const FlagsJson = ` "project": 1 }, "feature_state_value": "some_value", + "reason": "DEFAULT", "enabled": true, "environment": 1, "identity": null, @@ -198,6 +254,8 @@ const IdentityResponseJson = ` "project": 1 }, "feature_state_value": "some_value", + "reason": "SPLIT; weight=50.0", + "variant": "treatment", "enabled": true, "environment": 1, "identity": null, diff --git a/flagengine/engine-test-data b/flagengine/engine-test-data index 4b29dc77..2e710ea7 160000 --- a/flagengine/engine-test-data +++ b/flagengine/engine-test-data @@ -1 +1 @@ -Subproject commit 4b29dc772a764364af2dd504ecefbdf74cf5473f +Subproject commit 2e710ea7c6994110d17d1adb40b2ec2e90e7cf44 diff --git a/flagengine/engine.go b/flagengine/engine.go index 170804fd..645b9d78 100644 --- a/flagengine/engine.go +++ b/flagengine/engine.go @@ -9,6 +9,8 @@ import ( "github.com/Flagsmith/flagsmith-go-client/v5/flagengine/utils" ) +const controlVariantKey = "control" + type featureContextWithSegmentName struct { featureContext *engine_eval.FeatureContext segmentName string @@ -130,9 +132,13 @@ func GetEvaluationResult(ec *engine_eval.EngineEvaluationContext) engine_eval.Ev // getFlagResultFromFeatureContext creates a FlagResult from a FeatureContext. func getFlagResultFromFeatureContext(featureName string, featureContext *engine_eval.FeatureContext, identityKey *string, reason string) engine_eval.FlagResult { value := featureContext.Value + variantKey := "" // Handle multivariate features if len(featureContext.Variants) > 0 && identityKey != nil && featureContext.Key != "" { + // Default to the control bucket; a selected variant overrides this + variantKey = controlVariantKey + // Sort variants by priority (lower priority value = higher priority) sortedVariants := getSortedVariantsByPriority(featureContext.Variants) @@ -146,6 +152,7 @@ func getFlagResultFromFeatureContext(featureName string, featureContext *engine_ cumulativeWeight += variant.Weight if hashPercentage <= cumulativeWeight { value = variant.Value + variantKey = variant.Key reason = fmt.Sprintf("SPLIT; weight=%g", variant.Weight) break } @@ -156,6 +163,7 @@ func getFlagResultFromFeatureContext(featureName string, featureContext *engine_ Enabled: featureContext.Enabled, Name: featureName, Value: value, + Variant: variantKey, Reason: reason, Metadata: featureContext.Metadata, } diff --git a/flagengine/engine_eval/context.go b/flagengine/engine_eval/context.go index 286f3245..d8bc8fc7 100644 --- a/flagengine/engine_eval/context.go +++ b/flagengine/engine_eval/context.go @@ -52,6 +52,9 @@ type FeatureContext struct { // Represents a multivariate value for a feature flag. type FeatureValue struct { + // A stable identifier for the variant, reported as the flag result's variant + // when this value is selected. Empty if the variant is not keyed. + Key string `json:"key,omitempty"` // The value of the feature. Value any `json:"value"` // The weight of the feature value variant, as a percentage number (i.e. 100.0). diff --git a/flagengine/engine_eval/mappers.go b/flagengine/engine_eval/mappers.go index 133f6093..e4d85a95 100644 --- a/flagengine/engine_eval/mappers.go +++ b/flagengine/engine_eval/mappers.go @@ -72,6 +72,7 @@ func mapMultivariateFeatureStateValuesToVariants(multivariateValues []*features. Value: mv.MultivariateFeatureOption.Value, Weight: mv.PercentageAllocation, Priority: mv.Priority(), + Key: mv.MultivariateFeatureOption.Key, }) } return variants diff --git a/flagengine/engine_eval/result.go b/flagengine/engine_eval/result.go index d1c56e69..3e215140 100644 --- a/flagengine/engine_eval/result.go +++ b/flagengine/engine_eval/result.go @@ -18,6 +18,10 @@ type FlagResult struct { Reason string `json:"reason,omitempty"` // Feature flag value. Value any `json:"value,omitempty"` + // Key of the multivariate variant the value was selected from: "control" when + // the identity falls in the control bucket, or the selected variant's key. + // Empty for standard features, unkeyed variants, and evaluation without an identity. + Variant string `json:"variant,omitempty"` // Metadata about the feature. Metadata FeatureMetadata `json:"metadata,omitempty"` } diff --git a/flagengine/features/models.go b/flagengine/features/models.go index 1392ad30..1e0ec13b 100644 --- a/flagengine/features/models.go +++ b/flagengine/features/models.go @@ -58,6 +58,7 @@ func (fs *FeatureStateModel) UnmarshalJSON(bytes []byte) error { type MultivariateFeatureOptionModel struct { ID int `json:"id"` Value interface{} `json:"value"` + Key string `json:"key"` } type MultivariateFeatureStateValueModel struct { diff --git a/models.go b/models.go index 36aef4f6..6ea0dee6 100644 --- a/models.go +++ b/models.go @@ -14,6 +14,8 @@ type Flag struct { IsDefault bool FeatureID int FeatureName string + Reason string + Variant string } type Trait = trait.Trait @@ -36,6 +38,8 @@ func makeFlagFromEngineEvaluationFlagResult(flagResult *engine_eval.FlagResult) IsDefault: false, FeatureID: featureID, FeatureName: flagResult.Name, + Reason: flagResult.Reason, + Variant: flagResult.Variant, } } @@ -67,6 +71,8 @@ type jsonFlag struct { Enabled bool `json:"enabled"` Value interface{} `json:"feature_state_value"` Feature jsonFeature `json:"feature"` + Reason string `json:"reason"` + Variant string `json:"variant"` } func (jf *jsonFlag) toFlag() Flag { @@ -76,6 +82,8 @@ func (jf *jsonFlag) toFlag() Flag { IsDefault: false, FeatureID: jf.Feature.ID, FeatureName: jf.Feature.Name, + Reason: jf.Reason, + Variant: jf.Variant, } } func makeFlagsFromAPIFlags(flagsJson []byte, analyticsProcessor *AnalyticsProcessor, defaultFlagHandler func(string) (Flag, error)) (Flags, error) { diff --git a/models_test.go b/models_test.go index 005b8177..d25c5b47 100644 --- a/models_test.go +++ b/models_test.go @@ -3,9 +3,69 @@ package flagsmith import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/Flagsmith/flagsmith-go-client/v5/flagengine/engine_eval" ) +func TestMakeFlagsFromAPIFlagsSetsReasonAndVariant(t *testing.T) { + // Given + flagsJson := []byte(`[{ + "enabled": true, + "feature_state_value": "test-value", + "feature": {"id": 123, "name": "test_feature"}, + "reason": "TARGETING_MATCH; segment=premium", + "variant": "treatment" + }]`) + + // When + flags, err := makeFlagsFromAPIFlags(flagsJson, nil, nil) + + // Then + require.NoError(t, err) + require.Len(t, flags.flags, 1) + assert.Equal(t, "TARGETING_MATCH; segment=premium", flags.flags[0].Reason) + assert.Equal(t, "treatment", flags.flags[0].Variant) +} + +func TestMakeFlagsFromAPIFlagsNoReasonOrVariantIsEmpty(t *testing.T) { + // Given: a response from an API that predates the reason and variant fields + flagsJson := []byte(`[{ + "enabled": true, + "feature_state_value": "test-value", + "feature": {"id": 123, "name": "test_feature"} + }]`) + + // When + flags, err := makeFlagsFromAPIFlags(flagsJson, nil, nil) + + // Then + require.NoError(t, err) + require.Len(t, flags.flags, 1) + assert.Empty(t, flags.flags[0].Reason) + assert.Empty(t, flags.flags[0].Variant) +} + +func TestMakeFlagsFromAPIFlagsNullVariantIsEmpty(t *testing.T) { + // Given: a standard feature, for which the API reports a null variant + flagsJson := []byte(`[{ + "enabled": true, + "feature_state_value": "test-value", + "feature": {"id": 123, "name": "test_feature"}, + "reason": "DEFAULT", + "variant": null + }]`) + + // When + flags, err := makeFlagsFromAPIFlags(flagsJson, nil, nil) + + // Then + require.NoError(t, err) + require.Len(t, flags.flags, 1) + assert.Empty(t, flags.flags[0].Variant) +} + func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { tests := []struct { name string @@ -88,11 +148,11 @@ func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { }, }, { - name: "flag with reason field (should be ignored in conversion)", + name: "flag with reason field", input: &engine_eval.FlagResult{ Enabled: true, Name: "reason_feature", - Reason: "TARGETING_MATCH", + Reason: "TARGETING_MATCH; segment=premium_segment", Value: "reason_value", }, expected: Flag{ @@ -101,6 +161,45 @@ func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { IsDefault: false, FeatureID: 0, FeatureName: "reason_feature", + Reason: "TARGETING_MATCH; segment=premium_segment", + }, + }, + { + name: "multivariate flag with selected variant", + input: &engine_eval.FlagResult{ + Enabled: true, + Name: "mv_feature", + Reason: "SPLIT; weight=30", + Value: "variant_value", + Variant: "treatment", + }, + expected: Flag{ + Enabled: true, + Value: "variant_value", + IsDefault: false, + FeatureID: 0, + FeatureName: "mv_feature", + Reason: "SPLIT; weight=30", + Variant: "treatment", + }, + }, + { + name: "multivariate flag in the control bucket", + input: &engine_eval.FlagResult{ + Enabled: true, + Name: "mv_feature", + Reason: "DEFAULT", + Value: "control_value", + Variant: "control", + }, + expected: Flag{ + Enabled: true, + Value: "control_value", + IsDefault: false, + FeatureID: 0, + FeatureName: "mv_feature", + Reason: "DEFAULT", + Variant: "control", }, }, } @@ -124,6 +223,12 @@ func TestMakeFlagFromEngineEvaluationFlagResult(t *testing.T) { if result.FeatureName != tt.expected.FeatureName { t.Errorf("Expected FeatureName %v, got %v", tt.expected.FeatureName, result.FeatureName) } + if result.Reason != tt.expected.Reason { + t.Errorf("Expected Reason %v, got %v", tt.expected.Reason, result.Reason) + } + if result.Variant != tt.expected.Variant { + t.Errorf("Expected Variant %v, got %v", tt.expected.Variant, result.Variant) + } }) } } @@ -142,6 +247,7 @@ func TestMakeFlagsFromEngineEvaluationResult(t *testing.T) { Enabled: true, Name: "feature1", Value: "value1", + Reason: "DEFAULT", }, "feature2": { Enabled: false, @@ -163,6 +269,7 @@ func TestMakeFlagsFromEngineEvaluationResult(t *testing.T) { IsDefault: false, FeatureID: 0, FeatureName: "feature1", + Reason: "DEFAULT", }, { Enabled: false, @@ -250,6 +357,9 @@ func TestMakeFlagsFromEngineEvaluationResult(t *testing.T) { if actualFlag.FeatureName != expectedFlag.FeatureName { t.Errorf("Flag %s: Expected FeatureName %v, got %v", expectedFlag.FeatureName, expectedFlag.FeatureName, actualFlag.FeatureName) } + if actualFlag.Reason != expectedFlag.Reason { + t.Errorf("Flag %s: Expected Reason %v, got %v", expectedFlag.FeatureName, expectedFlag.Reason, actualFlag.Reason) + } } // Test that analytics processor and default flag handler are set correctly