Change RC and reconfiguration flow

Author: Strech

lib/datadog/open_feature/evaluation_engine.rb

@@ -60,6 +60,8 @@ def reconfigure!
 
         @logger.error("#{error_message}, error #{e.inspect}")
         @telemetry.report(e, description: error_message)
+
+        raise e
       end
     end
   end

lib/datadog/open_feature/remote.rb

@@ -4,6 +4,7 @@
 
 module Datadog
   module OpenFeature
+    # This module contains the remote configuration functionality for OpenFeature
     module Remote
       ReadError = Class.new(StandardError)
 
@@ -33,24 +34,22 @@ def receivers(telemetry)
                 next telemetry.error("OpenFeature: RemoteConfig change is not present on #{change.type}")
               end
 
-              # NOTE: We don't validate config and immediately mark content as
-              #       applied, even tho it might be a failure.
-              #
-              #       The re-configuration process is fail-safe by itself and
-              #       will be changed once RC updated to deliver patches.
+              # NOTE: In the current RC implementation we immediately apply the configuration,
+              #       but that might change if we need to apply patches instead.
               case change.type
               when :insert, :update
                 # @type var content: Core::Remote::Configuration::Content
                 engine.configuration = read_content(content)
+                engine.reconfigure!
+
                 content.applied
               when :delete
                 # NOTE: For now, we treat deletion as clearing the configuration
                 #       In a multi-config scenario, we might track configs per path
                 engine.configuration = nil
+                engine.reconfigure!
               end
             end
-
-            engine.reconfigure!
           end
 
           [receiver]

spec/datadog/open_feature/evaluation_engine_spec.rb

@@ -166,7 +166,7 @@
           .with(error, description: match(/OpenFeature: Failed to reconfigure/))
 
         engine.configuration = '{}'
-        expect { engine.reconfigure! }.not_to raise_error
+        expect { engine.reconfigure! }.to raise_error(error)
       end
 
       it 'persists previouly configured evaluator' do
@@ -175,9 +175,9 @@
         allow(reporter).to receive(:report)
 
         engine.configuration = '{}'
-        expect { engine.reconfigure! }.not_to change {
-          engine.fetch_value(flag_key: 'test', expected_type: :string).value
-        }.from('hello')
+
+        expect { engine.reconfigure! }.to raise_error(error)
+        expect(engine.fetch_value(flag_key: 'test', expected_type: :string).value).to eq('hello')
       end
     end
 

spec/datadog/open_feature/remote_spec.rb

@@ -105,6 +105,21 @@
       end
     end
 
+    context 'when change type is insert and reconfigure fails' do
+      before { allow(engine).to receive(:reconfigure!).and_raise(error) }
+
+      let(:error) { StandardError.new('Ooops') }
+      let(:transaction) do
+        repository.transaction { |_, t| t.insert(content.path, target, content) }
+      end
+
+      it 'does not acknowledge applied change and logs error' do
+        expect { receiver.call(repository, transaction) }.to raise_error(error)
+
+        expect(content.apply_state).to eq(Datadog::Core::Remote::Configuration::Content::ApplyState::UNACKNOWLEDGED)
+      end
+    end
+
     context 'when change type is update' do
       before do
         txn = repository.transaction { |_, t| t.insert(content.path, target, content) }
@@ -175,9 +190,9 @@
         Datadog::Core::Remote::Configuration::Path.parse('datadog/1/FFE_FLAGS/other/config')
       end
 
-      it 'logs error when content missing and still reconfigures' do
+      it 'logs error when content is missing and does not reconfigure the engine' do
         expect(telemetry).to receive(:error).with(/OpenFeature: RemoteConfig change is not present/)
-        expect(engine).to receive(:reconfigure!)
+        expect(engine).not_to receive(:reconfigure!)
 
         receiver.call(repository, changes)
       end