From 530a501d1f9384b1f6b7c471a13d9a3eccf3a303 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Sun, 7 Apr 2024 19:04:54 +0200 Subject: [PATCH 1/3] Revisit API to be compatible with current version of OF SDK Signed-off-by: Alexandre Chakroun --- providers/openfeature-flagd-provider/Gemfile | 2 + .../openfeature-flagd-provider/Gemfile.lock | 2 + .../lib/openfeature/flagd/provider.rb | 10 +- .../lib/openfeature/flagd/provider/client.rb | 24 ++-- .../openfeature/flagd/provider/client_spec.rb | 24 ++-- .../spec/openfeature/flagd/provider_spec.rb | 135 ++++++++++++++---- 6 files changed, 150 insertions(+), 47 deletions(-) diff --git a/providers/openfeature-flagd-provider/Gemfile b/providers/openfeature-flagd-provider/Gemfile index c607af6..95ca43f 100644 --- a/providers/openfeature-flagd-provider/Gemfile +++ b/providers/openfeature-flagd-provider/Gemfile @@ -4,3 +4,5 @@ source "https://rubygems.org" # Specify your gem's dependencies in openfeature-flagd-provider.gemspec gemspec + +gem "openfeature-sdk" diff --git a/providers/openfeature-flagd-provider/Gemfile.lock b/providers/openfeature-flagd-provider/Gemfile.lock index d34ba81..5f071e4 100644 --- a/providers/openfeature-flagd-provider/Gemfile.lock +++ b/providers/openfeature-flagd-provider/Gemfile.lock @@ -20,6 +20,7 @@ GEM google-protobuf (~> 3.25) googleapis-common-protos-types (~> 1.0) json (2.7.2) + openfeature-sdk (0.3.0) parallel (1.24.0) parser (3.3.0.5) ast (~> 2.4.1) @@ -63,6 +64,7 @@ PLATFORMS DEPENDENCIES openfeature-flagd-provider! + openfeature-sdk rake (~> 13.0) rspec (~> 3.12.0) rubocop (~> 1.37.1) diff --git a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb index 3d9b633..bb19557 100644 --- a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb +++ b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider.rb @@ -11,8 +11,8 @@ module FlagD # values. The implementation follows the details specified in https://openfeature.dev/docs/specification/sections/providers # # Provider contains functionality to configure the GRPC connection via - # - # OpenFeature::FlagD::Provider.configure do |config| + # flagd_client = OpenFeature::FlagD::Provider.get_client + # flagd_client.configure do |config| # config.host = 'localhost' # config.port = 8379 # config.tls = false @@ -37,6 +37,12 @@ module FlagD # manner; client.resolve_object_value(flag_key: 'object-flag', default_value: { default_value: 'value'}) module Provider class << self + def build_client + ConfiguredClient.new + end + end + + class ConfiguredClient def method_missing(method_name, *args, **kwargs, &) if client.respond_to?(method_name) client.send(method_name, *args, **kwargs, &) diff --git a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb index bd5f2a8..90a67fc 100644 --- a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb +++ b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb @@ -39,28 +39,36 @@ def initialize(configuration: nil) @grpc_client = grpc_client(configuration) end - - def resolve_boolean_value(flag_key:, default_value:, context: nil) + def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) request = Grpc::ResolveBooleanRequest.new(flag_key: flag_key) process_request { @grpc_client.resolve_boolean(request) } end - def resolve_integer_value(flag_key:, default_value:, context: nil) + def fetch_number_value(flag_key:, default_value:, evaluation_context: nil) + case default_value + when Integer + fetch_integer_value(flag_key: flag_key, default_value: default_value, evaluation_context: evaluation_context) + when Float + fetch_float_value(flag_key: flag_key, default_value: default_value, evaluation_context: evaluation_context) + end + end + + def fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) request = Grpc::ResolveIntRequest.new(flag_key: flag_key) process_request { @grpc_client.resolve_int(request) } end - def resolve_float_value(flag_key:, default_value:, context: nil) + def fetch_float_value(flag_key:, default_value:, evaluation_context: nil) request = Grpc::ResolveFloatRequest.new(flag_key: flag_key) process_request { @grpc_client.resolve_float(request) } end - def resolve_string_value(flag_key:, default_value:, context: nil) + def fetch_string_value(flag_key:, default_value:, evaluation_context: nil) request = Grpc::ResolveStringRequest.new(flag_key: flag_key) process_request { @grpc_client.resolve_string(request) } end - def resolve_object_value(flag_key:, default_value:, context: nil) + def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) request = Grpc::ResolveObjectRequest.new(flag_key: flag_key) process_request { @grpc_client.resolve_object(request) } end @@ -72,7 +80,7 @@ def resolve_object_value(flag_key:, default_value:, context: nil) def process_request(&block) response = block.call - ResolutionDetails.new(nil, nil, response.reason, response.value, response.variant).to_h + ResolutionDetails.new(nil, nil, response.reason, response.value, response.variant) rescue GRPC::NotFound => e error_response("FLAG_NOT_FOUND", e.message) rescue GRPC::InvalidArgument => e @@ -86,7 +94,7 @@ def process_request(&block) end def error_response(error_code, error_message) - ResolutionDetails.new(error_code, error_message, "ERROR", nil, nil).to_h + ResolutionDetails.new(error_code, error_message, "ERROR", nil, nil) end def grpc_client(configuration) diff --git a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider/client_spec.rb b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider/client_spec.rb index ad778d9..66535d8 100644 --- a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider/client_spec.rb +++ b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider/client_spec.rb @@ -17,17 +17,17 @@ context "https://openfeature.dev/docs/specification/sections/providers#requirement-221|222" do it do - expect(client).to respond_to(:resolve_boolean_value).with_keywords(:flag_key, :default_value, :context) - expect(client).to respond_to(:resolve_integer_value).with_keywords(:flag_key, :default_value, :context) - expect(client).to respond_to(:resolve_float_value).with_keywords(:flag_key, :default_value, :context) - expect(client).to respond_to(:resolve_string_value).with_keywords(:flag_key, :default_value, :context) - expect(client).to respond_to(:resolve_object_value).with_keywords(:flag_key, :default_value, :context) + expect(client).to respond_to(:fetch_boolean_value).with_keywords(:flag_key, :default_value, :evaluation_context) + expect(client).to respond_to(:fetch_integer_value).with_keywords(:flag_key, :default_value, :evaluation_context) + expect(client).to respond_to(:fetch_float_value).with_keywords(:flag_key, :default_value, :evaluation_context) + expect(client).to respond_to(:fetch_string_value).with_keywords(:flag_key, :default_value, :evaluation_context) + expect(client).to respond_to(:fetch_object_value).with_keywords(:flag_key, :default_value, :evaluation_context) end end context "https://openfeature.dev/docs/specification/sections/providers#requirement-223|224|225|226" do it do - expect(client.resolve_boolean_value(flag_key: "boolean-flag", default_value: false)).to include( + expect(client.fetch_boolean_value(flag_key: "boolean-flag", default_value: false).to_h).to include( error_code: nil, error_message: nil, reason: "STATIC", @@ -37,7 +37,7 @@ end it do - expect(client.resolve_integer_value(flag_key: "integer-flag", default_value: 1)).to include( + expect(client.fetch_integer_value(flag_key: "integer-flag", default_value: 1).to_h).to include( error_code: nil, error_message: nil, reason: "STATIC", @@ -47,7 +47,7 @@ end it do - expect(client.resolve_float_value(flag_key: "float-flag", default_value: 1.1)).to include( + expect(client.fetch_float_value(flag_key: "float-flag", default_value: 1.1).to_h).to include( error_code: nil, error_message: nil, reason: "STATIC", @@ -57,7 +57,7 @@ end it do - expect(client.resolve_string_value(flag_key: "string-flag", default_value: "lololo")).to include( + expect(client.fetch_string_value(flag_key: "string-flag", default_value: "lololo").to_h).to include( error_code: nil, error_message: nil, reason: "STATIC", @@ -67,8 +67,8 @@ end it do - resolution_details = client.resolve_object_value(flag_key: "object-flag", default_value: { "a" => "b" }) - expect(resolution_details).to include( + resolution_details = client.fetch_object_value(flag_key: "object-flag", default_value: { "a" => "b" }) + expect(resolution_details.to_h).to include( error_code: nil, error_message: nil, reason: "STATIC", @@ -80,7 +80,7 @@ context "https://openfeature.dev/docs/specification/sections/providers#requirement-227" do it do - expect(client.resolve_boolean_value(flag_key: "some-non-existant-flag", default_value: false)).to include( + expect(client.fetch_boolean_value(flag_key: "some-non-existant-flag", default_value: false).to_h).to include( value: nil, variant: nil, reason: "ERROR", diff --git a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb index 221f993..d7b1f93 100644 --- a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb +++ b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb @@ -1,23 +1,23 @@ # frozen_string_literal: true require "spec_helper" +require "open_feature/sdk" # https://openfeature.dev/docs/specification/sections/providers RSpec.describe OpenFeature::FlagD::Provider do - context "#configure" do - before do - ENV["FLAGD_HOST"] = nil - ENV["FLAGD_PORT"] = nil - ENV["FLAGD_TLS"] = nil + before do + ENV["FLAGD_HOST"] = nil + ENV["FLAGD_PORT"] = nil + ENV["FLAGD_TLS"] = nil + end - OpenFeature::FlagD::Provider.instance_variable_set(:@configuration, nil) - OpenFeature::FlagD::Provider.instance_variable_set(:@explicit_configuration, nil) - end + subject(:flagd_client) { described_class.build_client } + context "#configure" do context "when defining host, port and tls options of gRPC service it wishes to access with configure method" do subject(:explicit_configuration) do - described_class.configure do |config| + flagd_client.configure do |config| config.host = explicit_host config.port = explicit_port config.tls = explicit_tls @@ -30,9 +30,9 @@ it "expects configuration to be values set from configure method" do explicit_configuration - expect(described_class.configuration.host).to eq(explicit_host) - expect(described_class.configuration.port).to eq(explicit_port) - expect(described_class.configuration.tls).to eq(explicit_tls) + expect(flagd_client.configuration.host).to eq(explicit_host) + expect(flagd_client.configuration.port).to eq(explicit_port) + expect(flagd_client.configuration.tls).to eq(explicit_tls) end context "when defining environment variables" do @@ -44,26 +44,29 @@ it "uses the explicit configuration" do explicit_configuration - expect(described_class.configuration.host).to eq("explicit_host") - expect(described_class.configuration.port).to eq(8013) - expect(described_class.configuration.tls).to be_falsy + expect(flagd_client.configuration.host).to eq(explicit_host) + expect(flagd_client.configuration.port).to eq(explicit_port) + expect(flagd_client.configuration.tls).to eq(explicit_tls) end end end context "when defining environment variables" do + let(:env_host) { "172.16.1.2" } + let(:env_port) { "8014" } + let(:env_tls) { "true" } subject(:env_configuration) do - ENV["FLAGD_HOST"] = "172.16.1.2" - ENV["FLAGD_PORT"] = "8014" - ENV["FLAGD_TLS"] = "true" - described_class.configuration + ENV["FLAGD_HOST"] = env_host + ENV["FLAGD_PORT"] = env_port + ENV["FLAGD_TLS"] = env_tls + flagd_client.configuration end it "uses environment variables when no explicit configuration" do env_configuration - expect(env_configuration.host).to eq("172.16.1.2") - expect(env_configuration.port).to eq(8014) - expect(env_configuration.tls).to be_truthy + expect(env_configuration.host).to eq(env_host) + expect(env_configuration.port).to eq(env_port.to_i) + expect(env_configuration.tls).to eq(env_tls == "true") end end end @@ -71,9 +74,91 @@ # https://openfeature.dev/docs/specification/sections/providers#requirement-211 context "#metadata" do it "metadata name is defined" do - expect(described_class).to respond_to(:metadata) - expect(described_class.metadata).to respond_to(:name) - expect(described_class.metadata.name).to eq("flagd Provider") + expect(flagd_client).to respond_to(:metadata) + expect(flagd_client.metadata).to respond_to(:name) + expect(flagd_client.metadata.name).to eq("flagd Provider") + end + end + + context "OpenFeature SDK integration" do + before do + OpenFeature::SDK.configure do |config| + config.set_provider(OpenFeature::FlagD::Provider.build_client) + end + end + subject(:client) { OpenFeature::SDK.build_client } + + context "get value" do + it do + expect(client.fetch_boolean_value(flag_key: 'boolean-flag', default_value: false)).to be_falsy + end + + it do + expect(client.fetch_number_value(flag_key: "integer-flag", default_value: 1)).to eq(42) + end + + it do + expect(client.fetch_number_value(flag_key: "float-flag", default_value: 1.1)).to eq(4.2) + end + + it do + expect(client.fetch_string_value(flag_key: "string-flag", default_value: "lololo")).to eq("lalala") + end + + it do + expect(client.fetch_object_value(flag_key: "object-flag", default_value: { "a" => "b" })).to be_a(Google::Protobuf::Struct) + end + end + + context "get details" do + it do + expect(client.fetch_boolean_details(flag_key: 'boolean-flag', default_value: false).resolution_details.to_h).to include( + error_code: nil, + error_message: nil, + reason: "STATIC", + value: false, + variant: "off", + ) + end + + it do + expect(client.fetch_number_details(flag_key: "integer-flag", default_value: 1).resolution_details.to_h).to include( + error_code: nil, + error_message: nil, + reason: "STATIC", + value: 42, + variant: "fourty-two", + ) + end + + it do + expect(client.fetch_number_details(flag_key: "float-flag", default_value: 1.1).resolution_details.to_h).to include( + error_code: nil, + error_message: nil, + reason: "STATIC", + value: 4.2, + variant: "four-point-two", + ) + end + + it do + expect(client.fetch_string_details(flag_key: "string-flag", default_value: "lololo").resolution_details.to_h).to include( + error_code: nil, + error_message: nil, + reason: "STATIC", + value: "lalala", + variant: "lilili", + ) + end + + it do + expect(client.fetch_object_details(flag_key: "object-flag", default_value: { "a" => "b" }).resolution_details.to_h).to include( + error_code: nil, + error_message: nil, + reason: "STATIC", + variant: "real-object", + ) + end end end end From ade17ee74b7709e21076758b47b0fd0f5ad25603 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Mon, 8 Apr 2024 23:36:14 +0200 Subject: [PATCH 2/3] Enable flagd targeting by forwarding context Signed-off-by: Alexandre Chakroun --- .../docker/flags.json | 51 +++++++++++++++++++ .../lib/openfeature/flagd/provider/client.rb | 42 +++++++++------ .../spec/openfeature/flagd/provider_spec.rb | 27 ++++++++++ 3 files changed, 104 insertions(+), 16 deletions(-) diff --git a/providers/openfeature-flagd-provider/docker/flags.json b/providers/openfeature-flagd-provider/docker/flags.json index 5c46ccc..e5fd506 100644 --- a/providers/openfeature-flagd-provider/docker/flags.json +++ b/providers/openfeature-flagd-provider/docker/flags.json @@ -36,6 +36,57 @@ "real-object": { "real": "value" } }, "defaultVariant": "real-object" + }, + "boolean-flag-targeting": { + "state": "ENABLED", + "variants": { + "on": true, + "off": false + }, + "defaultVariant": "off", + "targeting": { + "if": [ + { + "==": [ + { + "var": "be_true" + }, + true + ] + }, + "on" + ] + } + }, + "color-palette-experiment": { + "state": "ENABLED", + "defaultVariant": "grey", + "variants": { + "red": "#b91c1c", + "blue": "#0284c7", + "green": "#16a34a", + "grey": "#4b5563" + }, + "targeting": { + "fractional": [ + [ + "red", + 25 + ], + [ + "blue", + 25 + ], + [ + "green", + 25 + ], + [ + "grey", + 25 + ] + ] + } } } } diff --git a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb index 90a67fc..31cfa46 100644 --- a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb +++ b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb @@ -1,9 +1,11 @@ # frozen_string_literal: true require "grpc" +require 'google/protobuf/well_known_types' require_relative "schema/v1/schema_services_pb" require_relative "configuration" + module OpenFeature module FlagD module Provider @@ -15,20 +17,20 @@ module Provider # # * metadata - Returns the associated provider metadata with the name # - # * resolve_boolean_value(flag_key:, default_value:, context: nil) - # manner; client.resolve_boolean(flag_key: 'boolean-flag', default_value: false) + # * fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) + # manner; client.fetch_boolean(flag_key: 'boolean-flag', default_value: false) # - # * resolve_integer_value(flag_key:, default_value:, context: nil) - # manner; client.resolve_integer_value(flag_key: 'integer-flag', default_value: 2) + # * fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) + # manner; client.fetch_integer_value(flag_key: 'integer-flag', default_value: 2) # - # * resolve_float_value(flag_key:, default_value:, context: nil) - # manner; client.resolve_float_value(flag_key: 'float-flag', default_value: 2.0) + # * fetch_float_value(flag_key:, default_value:, evaluation_context: nil) + # manner; client.fetch_float_value(flag_key: 'float-flag', default_value: 2.0) # - # * resolve_string_value(flag_key:, default_value:, context: nil) - # manner; client.resolve_string_value(flag_key: 'string-flag', default_value: 'some-default-value') + # * fetch_string_value(flag_key:, default_value:, evaluation_context: nil) + # manner; client.fetch_string_value(flag_key: 'string-flag', default_value: 'some-default-value') # - # * resolve_object_value(flag_key:, default_value:, context: nil) - # manner; client.resolve_object_value(flag_key: 'flag', default_value: { default_value: 'value'}) + # * fetch_object_value(flag_key:, default_value:, evaluation_context: nil) + # manner; client.fetch_object_value(flag_key: 'flag', default_value: { default_value: 'value'}) class Client PROVIDER_NAME = "flagd Provider" @@ -40,7 +42,7 @@ def initialize(configuration: nil) end def fetch_boolean_value(flag_key:, default_value:, evaluation_context: nil) - request = Grpc::ResolveBooleanRequest.new(flag_key: flag_key) + request = Grpc::ResolveBooleanRequest.new(flag_key: flag_key, context: prepare_evaluation_context(evaluation_context)) process_request { @grpc_client.resolve_boolean(request) } end @@ -54,22 +56,22 @@ def fetch_number_value(flag_key:, default_value:, evaluation_context: nil) end def fetch_integer_value(flag_key:, default_value:, evaluation_context: nil) - request = Grpc::ResolveIntRequest.new(flag_key: flag_key) + request = Grpc::ResolveIntRequest.new(flag_key: flag_key, context: prepare_evaluation_context(evaluation_context)) process_request { @grpc_client.resolve_int(request) } end def fetch_float_value(flag_key:, default_value:, evaluation_context: nil) - request = Grpc::ResolveFloatRequest.new(flag_key: flag_key) - process_request { @grpc_client.resolve_float(request) } + request = Grpc::ResolveFloatRequest.new(flag_key: flag_key, context: prepare_evaluation_context(evaluation_context)) + process_request { @grpc_client.resolve_float(request) } end def fetch_string_value(flag_key:, default_value:, evaluation_context: nil) - request = Grpc::ResolveStringRequest.new(flag_key: flag_key) + request = Grpc::ResolveStringRequest.new(flag_key: flag_key, context: prepare_evaluation_context(evaluation_context)) process_request { @grpc_client.resolve_string(request) } end def fetch_object_value(flag_key:, default_value:, evaluation_context: nil) - request = Grpc::ResolveObjectRequest.new(flag_key: flag_key) + request = Grpc::ResolveObjectRequest.new(flag_key: flag_key, context: prepare_evaluation_context(evaluation_context)) process_request { @grpc_client.resolve_object(request) } end @@ -93,6 +95,14 @@ def process_request(&block) error_response("GENERAL", e.message) end + def prepare_evaluation_context(evaluation_context) + return nil if !evaluation_context + + fields = evaluation_context.fields + fields["targetingKey"] = fields.delete(:targeting_key) + Google::Protobuf::Struct.from_hash(fields) + end + def error_response(error_code, error_message) ResolutionDetails.new(error_code, error_message, "ERROR", nil, nil) end diff --git a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb index d7b1f93..d40ed3f 100644 --- a/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb +++ b/providers/openfeature-flagd-provider/spec/openfeature/flagd/provider_spec.rb @@ -110,6 +110,33 @@ end end + context "get value with evaluated context" do + it do + expect( + client.fetch_boolean_value( + flag_key: 'boolean-flag-targeting', + default_value: false, + evaluation_context: OpenFeature::SDK::EvaluationContext.new(be_true: true) + ) + ).to be_truthy + end + + it do + fetch_value_with_targeting_key = ->(targeting_key) do + client.fetch_boolean_value( + flag_key: 'color-palette-experiment', + default_value: "#b91c1c", + evaluation_context: OpenFeature::SDK::EvaluationContext.new(targeting_key: targeting_key) + ) + end + + initial_value = fetch_value_with_targeting_key.("123") + (0..2).to_a.each do # try with 1000 + expect(fetch_value_with_targeting_key.("123")).to eq(initial_value) + end + end + end + context "get details" do it do expect(client.fetch_boolean_details(flag_key: 'boolean-flag', default_value: false).resolution_details.to_h).to include( From 0a663f12f645817822501d1cb98e6135719ed509 Mon Sep 17 00:00:00 2001 From: Alexandre Chakroun Date: Tue, 9 Apr 2024 13:49:35 +0200 Subject: [PATCH 3/3] review Signed-off-by: Alexandre Chakroun --- .../lib/openfeature/flagd/provider/client.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb index 31cfa46..d2bd177 100644 --- a/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb +++ b/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb @@ -96,7 +96,7 @@ def process_request(&block) end def prepare_evaluation_context(evaluation_context) - return nil if !evaluation_context + return nil unless evaluation_context fields = evaluation_context.fields fields["targetingKey"] = fields.delete(:targeting_key)