From 2d17828beb5d99ef755b6587fd0ea958e50668c3 Mon Sep 17 00:00:00 2001 From: Thomas Poignant Date: Mon, 27 Jan 2025 09:42:10 +0100 Subject: [PATCH] fix: Use exporter metadata in remote evaluations (#2983) * fix: Use exporter metadata in remote evaluations Signed-off-by: Thomas Poignant * fix(python): Add exporter metadata to the evaluation context Signed-off-by: Thomas Poignant --------- Signed-off-by: Thomas Poignant --- .pre-commit-config.yaml | 2 +- exporter/data_exporter_test.go | 66 +++++++++++++++++-- exporter/feature_event.go | 3 +- exporter/feature_event_test.go | 17 ++--- ffcontext/context.go | 3 +- ffcontext/context_test.go | 19 ++++++ ffcontext/goff_context_specifics.go | 8 +++ .../gofeatureflag_python_provider/provider.py | 43 +++++++----- .../request_flag_evaluation.py | 7 +- .../test_gofeatureflag_python_provider.py | 44 +++++++++++++ variation.go | 2 +- website/docs/concepts/evaluation-context.md | 16 +++-- 12 files changed, 186 insertions(+), 44 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4a86ec7f10c..079c593cb28 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -11,7 +11,7 @@ repos: entry: golangci-lint run --enable-only=gci --fix - repo: https://github.com/psf/black-pre-commit-mirror - rev: 23.11.0 + rev: 24.10.0 hooks: - id: black language_version: python3.12 diff --git a/exporter/data_exporter_test.go b/exporter/data_exporter_test.go index 5a1c997c6fb..d0011f20dac 100644 --- a/exporter/data_exporter_test.go +++ b/exporter/data_exporter_test.go @@ -10,8 +10,10 @@ import ( "github.com/stretchr/testify/assert" "github.com/thejerf/slogassert" + ffclient "github.com/thomaspoignant/go-feature-flag" "github.com/thomaspoignant/go-feature-flag/exporter" "github.com/thomaspoignant/go-feature-flag/ffcontext" + "github.com/thomaspoignant/go-feature-flag/retriever/fileretriever" "github.com/thomaspoignant/go-feature-flag/testutils/mock" "github.com/thomaspoignant/go-feature-flag/utils/fflog" ) @@ -27,7 +29,7 @@ func TestDataExporterScheduler_flushWithTime(t *testing.T) { inputEvents := []exporter.FeatureEvent{ exporter.NewFeatureEvent( ffcontext.NewEvaluationContextBuilder("ABCD").AddCustom("anonymous", true).Build(), "random-key", - "YO", "defaultVar", false, "", "SERVER"), + "YO", "defaultVar", false, "", "SERVER", nil), } for _, event := range inputEvents { @@ -50,7 +52,7 @@ func TestDataExporterScheduler_flushWithNumberOfEvents(t *testing.T) { for i := 0; i <= 100; i++ { inputEvents = append(inputEvents, exporter.NewFeatureEvent( ffcontext.NewEvaluationContextBuilder("ABCD").AddCustom("anonymous", true).Build(), - "random-key", "YO", "defaultVar", false, "", "SERVER")) + "random-key", "YO", "defaultVar", false, "", "SERVER", nil)) } for _, event := range inputEvents { dc.AddEvent(event) @@ -70,7 +72,7 @@ func TestDataExporterScheduler_defaultFlush(t *testing.T) { for i := 0; i <= 100000; i++ { inputEvents = append(inputEvents, exporter.NewFeatureEvent( ffcontext.NewEvaluationContextBuilder("ABCD").AddCustom("anonymous", true).Build(), - "random-key", "YO", "defaultVar", false, "", "SERVER")) + "random-key", "YO", "defaultVar", false, "", "SERVER", nil)) } for _, event := range inputEvents { dc.AddEvent(event) @@ -97,7 +99,7 @@ func TestDataExporterScheduler_exporterReturnError(t *testing.T) { for i := 0; i <= 200; i++ { inputEvents = append(inputEvents, exporter.NewFeatureEvent( ffcontext.NewEvaluationContextBuilder("ABCD").AddCustom("anonymous", true).Build(), - "random-key", "YO", "defaultVar", false, "", "SERVER")) + "random-key", "YO", "defaultVar", false, "", "SERVER", nil)) } for _, event := range inputEvents { dc.AddEvent(event) @@ -117,7 +119,7 @@ func TestDataExporterScheduler_nonBulkExporter(t *testing.T) { for i := 0; i < 100; i++ { inputEvents = append(inputEvents, exporter.NewFeatureEvent( ffcontext.NewEvaluationContextBuilder("ABCD").AddCustom("anonymous", true).Build(), - "random-key", "YO", "defaultVar", false, "", "SERVER")) + "random-key", "YO", "defaultVar", false, "", "SERVER", nil)) } for _, event := range inputEvents { dc.AddEvent(event) @@ -127,3 +129,57 @@ func TestDataExporterScheduler_nonBulkExporter(t *testing.T) { assert.Equal(t, inputEvents[:100], mockExporter.GetExportedEvents()) } + +func TestAddExporterMetadataFromContextToExporter(t *testing.T) { + tests := []struct { + name string + ctx ffcontext.EvaluationContext + want map[string]interface{} + }{ + { + name: "extract exporter metadata from context", + ctx: ffcontext.NewEvaluationContextBuilder("targeting-key").AddCustom("gofeatureflag", map[string]interface{}{ + "exporterMetadata": map[string]interface{}{ + "key1": "value1", + "key2": 123, + "key3": true, + "key4": 123.45, + }, + }).Build(), + want: map[string]interface{}{ + "key1": "value1", + "key2": 123, + "key3": true, + "key4": 123.45, + }, + }, + { + name: "no exporter metadata in the context", + ctx: ffcontext.NewEvaluationContextBuilder("targeting-key").Build(), + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + mockExporter := &mock.Exporter{} + config := ffclient.Config{ + Retriever: &fileretriever.Retriever{Path: "../testdata/flag-config.yaml"}, + DataExporter: ffclient.DataExporter{ + Exporter: mockExporter, + FlushInterval: 100 * time.Millisecond, + }, + } + goff, err := ffclient.New(config) + assert.NoError(t, err) + + _, err = goff.BoolVariation("test-flag", tt.ctx, false) + assert.NoError(t, err) + + time.Sleep(120 * time.Millisecond) + assert.Equal(t, 1, len(mockExporter.GetExportedEvents())) + got := mockExporter.GetExportedEvents()[0].Metadata + assert.Equal(t, tt.want, got) + }) + } +} diff --git a/exporter/feature_event.go b/exporter/feature_event.go index b2e0a41a80a..9a42e5cc432 100644 --- a/exporter/feature_event.go +++ b/exporter/feature_event.go @@ -17,12 +17,12 @@ func NewFeatureEvent( failed bool, version string, source string, + metadata FeatureEventMetadata, ) FeatureEvent { contextKind := "user" if ctx.IsAnonymous() { contextKind = "anonymousUser" } - return FeatureEvent{ Kind: "feature", ContextKind: contextKind, @@ -34,6 +34,7 @@ func NewFeatureEvent( Default: failed, Version: version, Source: source, + Metadata: metadata, } } diff --git a/exporter/feature_event_test.go b/exporter/feature_event_test.go index 28b34cea101..2f80df96a21 100644 --- a/exporter/feature_event_test.go +++ b/exporter/feature_event_test.go @@ -12,13 +12,14 @@ import ( func TestNewFeatureEvent(t *testing.T) { type args struct { - user ffcontext.Context - flagKey string - value interface{} - variation string - failed bool - version string - source string + user ffcontext.Context + flagKey string + value interface{} + variation string + failed bool + version string + source string + exporterMetadata exporter.FeatureEventMetadata } tests := []struct { name string @@ -44,7 +45,7 @@ func TestNewFeatureEvent(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - assert.Equalf(t, tt.want, exporter.NewFeatureEvent(tt.args.user, tt.args.flagKey, tt.args.value, tt.args.variation, tt.args.failed, tt.args.version, tt.args.source), "NewFeatureEvent(%v, %v, %v, %v, %v, %v, %V)", tt.args.user, tt.args.flagKey, tt.args.value, tt.args.variation, tt.args.failed, tt.args.version, tt.args.source) + assert.Equalf(t, tt.want, exporter.NewFeatureEvent(tt.args.user, tt.args.flagKey, tt.args.value, tt.args.variation, tt.args.failed, tt.args.version, tt.args.source, tt.args.exporterMetadata), "NewFeatureEvent(%v, %v, %v, %v, %v, %v, %V)", tt.args.user, tt.args.flagKey, tt.args.value, tt.args.variation, tt.args.failed, tt.args.version, tt.args.source) }) } } diff --git a/ffcontext/context.go b/ffcontext/context.go index 65891579b82..58683e8e33b 100644 --- a/ffcontext/context.go +++ b/ffcontext/context.go @@ -81,12 +81,13 @@ func (u EvaluationContext) ExtractGOFFProtectedFields() GoffContextSpecifics { case map[string]string: goff.addCurrentDateTime(v["currentDateTime"]) goff.addListFlags(v["flagList"]) + goff.addExporterMetadata(v["exporterMetadata"]) case map[string]interface{}: goff.addCurrentDateTime(v["currentDateTime"]) goff.addListFlags(v["flagList"]) + goff.addExporterMetadata(v["exporterMetadata"]) case GoffContextSpecifics: return v } - return goff } diff --git a/ffcontext/context_test.go b/ffcontext/context_test.go index 308e6cda3e8..98a8fb585b1 100644 --- a/ffcontext/context_test.go +++ b/ffcontext/context_test.go @@ -142,6 +142,25 @@ func Test_ExtractGOFFProtectedFields(t *testing.T) { FlagList: []string{"flag1", "flag2"}, }, }, + { + name: "context goff specifics with exporter metadata", + ctx: ffcontext.NewEvaluationContextBuilder("my-key").AddCustom("gofeatureflag", map[string]interface{}{ + "exporterMetadata": map[string]interface{}{ + "toto": 123, + "titi": 123.45, + "tutu": true, + "tata": "bonjour", + }, + }).Build(), + want: ffcontext.GoffContextSpecifics{ + ExporterMetadata: map[string]interface{}{ + "toto": 123, + "titi": 123.45, + "tutu": true, + "tata": "bonjour", + }, + }, + }, } for _, tt := range tests { diff --git a/ffcontext/goff_context_specifics.go b/ffcontext/goff_context_specifics.go index 78178f0f4eb..34d1b2bc14c 100644 --- a/ffcontext/goff_context_specifics.go +++ b/ffcontext/goff_context_specifics.go @@ -7,6 +7,8 @@ type GoffContextSpecifics struct { CurrentDateTime *time.Time `json:"currentDateTime"` // FlagList is the list of flags to evaluate in a bulk evaluation. FlagList []string `json:"flagList"` + // ExporterMetadata is the metadata to be used by the exporter. + ExporterMetadata map[string]interface{} `json:"exporterMetadata"` } // addCurrentDateTime adds the current date time to the context. @@ -40,3 +42,9 @@ func (g *GoffContextSpecifics) addListFlags(flagList any) { } } } + +func (g *GoffContextSpecifics) addExporterMetadata(exporterMetadata any) { + if value, ok := exporterMetadata.(map[string]interface{}); ok { + g.ExporterMetadata = value + } +} diff --git a/openfeature/providers/python-provider/gofeatureflag_python_provider/provider.py b/openfeature/providers/python-provider/gofeatureflag_python_provider/provider.py index 083fa2b9ee2..6847f62c907 100644 --- a/openfeature/providers/python-provider/gofeatureflag_python_provider/provider.py +++ b/openfeature/providers/python-provider/gofeatureflag_python_provider/provider.py @@ -1,12 +1,19 @@ import json -from http import HTTPStatus -from threading import Thread -from typing import List, Optional, Type, Union -from urllib.parse import urljoin - import pylru import urllib3 import websocket +from gofeatureflag_python_provider.data_collector_hook import DataCollectorHook +from gofeatureflag_python_provider.metadata import GoFeatureFlagMetadata +from gofeatureflag_python_provider.options import BaseModel, GoFeatureFlagOptions +from gofeatureflag_python_provider.request_flag_evaluation import ( + RequestFlagEvaluation, + convert_evaluation_context, +) +from gofeatureflag_python_provider.response_flag_evaluation import ( + JsonType, + ResponseFlagEvaluation, +) +from http import HTTPStatus from openfeature.evaluation_context import EvaluationContext from openfeature.exception import ( ErrorCode, @@ -17,21 +24,11 @@ ) from openfeature.flag_evaluation import FlagResolutionDetails, Reason from openfeature.hook import Hook -from openfeature.provider.metadata import Metadata from openfeature.provider import AbstractProvider +from openfeature.provider.metadata import Metadata from pydantic import PrivateAttr, ValidationError - -from gofeatureflag_python_provider.data_collector_hook import DataCollectorHook -from gofeatureflag_python_provider.metadata import GoFeatureFlagMetadata -from gofeatureflag_python_provider.options import BaseModel, GoFeatureFlagOptions -from gofeatureflag_python_provider.request_flag_evaluation import ( - RequestFlagEvaluation, - convert_evaluation_context, -) -from gofeatureflag_python_provider.response_flag_evaluation import ( - JsonType, - ResponseFlagEvaluation, -) +from threading import Thread +from typing import List, Optional, Type, Union AbstractProviderMetaclass = type(AbstractProvider) BaseModelMetaclass = type(BaseModel) @@ -196,6 +193,16 @@ def generic_go_feature_flag_resolver( "/v1/feature/{}/eval".format(flag_key), ) + # add exporter metadata to the context if it exists + if self.options.exporter_metadata: + goff_request.gofeatureflag["exporterMetadata"] = ( + self.options.exporter_metadata + ) + goff_request.gofeatureflag["exporterMetadata"]["openfeature"] = True + goff_request.gofeatureflag["exporterMetadata"][ + "provider" + ] = "python" + response = self._http_client.request( method="POST", url=url, diff --git a/openfeature/providers/python-provider/gofeatureflag_python_provider/request_flag_evaluation.py b/openfeature/providers/python-provider/gofeatureflag_python_provider/request_flag_evaluation.py index 1eb40c76c7f..fcc7d57230a 100644 --- a/openfeature/providers/python-provider/gofeatureflag_python_provider/request_flag_evaluation.py +++ b/openfeature/providers/python-provider/gofeatureflag_python_provider/request_flag_evaluation.py @@ -1,15 +1,13 @@ import hashlib import json -from typing import Optional, Any - +from gofeatureflag_python_provider.options import BaseModel from openfeature.evaluation_context import EvaluationContext from openfeature.exception import ( TargetingKeyMissingError, InvalidContextError, ) from pydantic import SkipValidation - -from gofeatureflag_python_provider.options import BaseModel +from typing import Optional, Any, Dict class GoFeatureFlagEvaluationContext(BaseModel): @@ -56,3 +54,4 @@ def convert_evaluation_context( class RequestFlagEvaluation(BaseModel): user: GoFeatureFlagEvaluationContext defaultValue: SkipValidation[Any] = None + gofeatureflag: Optional[Dict] = {} diff --git a/openfeature/providers/python-provider/tests/test_gofeatureflag_python_provider.py b/openfeature/providers/python-provider/tests/test_gofeatureflag_python_provider.py index cfd46b775ae..a66334b549d 100644 --- a/openfeature/providers/python-provider/tests/test_gofeatureflag_python_provider.py +++ b/openfeature/providers/python-provider/tests/test_gofeatureflag_python_provider.py @@ -689,6 +689,50 @@ def test_url_parsing(mock_request): assert got == want +@patch("urllib3.poolmanager.PoolManager.request") +def test_should_call_evaluation_api_with_exporter_metadata( + mock_request: Mock, +): + flag_key = "bool_targeting_match" + default_value = False + mock_request.side_effect = [ + Mock( + status="200", data=_read_mock_file(flag_key) + ), # first call to get the flag + Mock(status="200", data={}), # second call to send the data + Mock(status="200", data={}), + ] + goff_provider = GoFeatureFlagProvider( + options=GoFeatureFlagOptions( + endpoint="https://gofeatureflag.org/", + data_flush_interval=100, + disable_cache_invalidation=True, + exporter_metadata={"version": "1.0.0", "name": "myapp", "id": 123}, + ) + ) + api.set_provider(goff_provider) + client = api.get_client(domain="test-client") + + client.get_boolean_details( + flag_key=flag_key, + default_value=default_value, + evaluation_context=_default_evaluation_ctx, + ) + + api.shutdown() + got = json.loads(mock_request.call_args.kwargs["body"])["gofeatureflag"] + want = { + "exporterMetadata": { + "version": "1.0.0", + "name": "myapp", + "id": 123, + "provider": "python", + "openfeature": True, + }, + } + assert got == want + + def _read_mock_file(flag_key: str) -> str: # This hacky if is here to make test run inside pycharm and from the root of the project if os.getcwd().endswith("/tests"): diff --git a/variation.go b/variation.go index f9d32cd9545..e154d3ac067 100644 --- a/variation.go +++ b/variation.go @@ -270,7 +270,7 @@ func notifyVariation[T model.JSONType]( ) { if result.TrackEvents { event := exporter.NewFeatureEvent(ctx, flagKey, result.Value, result.VariationType, result.Failed, result.Version, - "SERVER") + "SERVER", ctx.ExtractGOFFProtectedFields().ExporterMetadata) g.CollectEventData(event) } } diff --git a/website/docs/concepts/evaluation-context.md b/website/docs/concepts/evaluation-context.md index c3143468f14..77da9cdf1c6 100644 --- a/website/docs/concepts/evaluation-context.md +++ b/website/docs/concepts/evaluation-context.md @@ -67,10 +67,16 @@ The targeting key is used to ensure that a user consistently receives the same v For instance, **GO Feature Flag** ensures that in cases where a feature is being rolled out to a percentage of users, based on the targeting key, they will see the same variation each time they encounter the feature flag. ## Reserved properties in the evaluation context +:::danger +If you put a key named `gofeatureflag` in your evaluation context, it may break internal features of GO Feature Flag. +This property name is reserved for internal use. +::: + When you create an evaluation context some fields are reserved for GO Feature Flag. -Those fields are used by GO Feature Flag directly, you can use them as will in your targeting queries but you should be aware that they are used internally for GO Feature Flag. +Those fields are used by GO Feature Flag directly, you can use them as will in your targeting queries, but you should be aware that they are used internally for GO Feature Flag. -| Field | Description | -|---------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `gofeatureflag.currentDateTime` | If this property is set, we will use this date as base for all the rollout strategies which implies dates _(experimentation, progressive and scheduled)_.
**Format:** Date following the RF3339 format. | -| `gofeatureflag.flagList` | If this property is set, in the bulk evaluation mode (for the client SDK) we will only evaluate the flags in this list.
If empty or not set the default behavior is too evaluate all the flags.
**Format:** []string | +| Field | Description | +|----------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `gofeatureflag.currentDateTime` | If this property is set, we will use this date as base for all the rollout strategies which implies dates _(experimentation, progressive and scheduled)_.
**Format:** Date following the RF3339 format. | +| `gofeatureflag.flagList` | If this property is set, in the bulk evaluation mode (for the client SDK) we will only evaluate the flags in this list.
If empty or not set the default behavior is too evaluate all the flags.
**Format:** []string | +| `gofeatureflag.exporterMetadata` | If this property is set, we will add all the fields in the feature event send to the provider.
**Format:** map[string]string\|number\|bool |