Fix edge-cases mentioned in review

* Improve performance of the hot-paths
* Add error handling in remote configuration
* Complete shutdown process

Author: Strech

lib/datadog/core/configuration/components.rb

@@ -202,6 +202,9 @@ def shutdown!(replacement = nil)
           # Shutdown DI after remote, since remote config triggers DI operations.
           dynamic_instrumentation&.shutdown!
 
+          # Shutdown OpenFeature component
+          open_feature&.shutdown!
+
           # Decommission AppSec
           appsec&.shutdown!
 

lib/datadog/open_feature/component.rb

@@ -11,42 +11,32 @@ module Datadog
   module OpenFeature
     # This class is the entry point for the OpenFeature component
     class Component
-      attr_reader :telemetry, :engine
+      attr_reader :engine
 
       def self.build(settings, agent_settings, logger:, telemetry:)
-        return unless settings.open_feature.enabled
+        return unless settings.respond_to?(:open_feature) && settings.open_feature.enabled
 
-        unless settings.remote.enabled
-          logger.warn('OpenFeature could not be enabled as Remote Configuration is currently disabled. To enable Remote Configuration, see https://docs.datadoghq.com/agent/remote_config')
+        unless settings.respond_to?(:remote) && settings.remote.enabled
+          message = 'OpenFeature could not be enabled as Remote Configuration is currently disabled. ' \
+            'To enable Remote Configuration, see https://docs.datadoghq.com/agent/remote_config'
+          logger.warn(message)
 
           return
         end
 
         new(settings, agent_settings, logger: logger, telemetry: telemetry)
-      rescue
-        logger.warn('OpenFeature is disabled, see logged errors above')
-
-        nil
       end
 
       def initialize(settings, agent_settings, logger:, telemetry:)
-        @settings = settings
-        @agent_settings = agent_settings
-        @logger = logger
-        @telemetry = telemetry
-
         transport = Transport::HTTP.build(agent_settings: agent_settings, logger: logger)
-        @worker = Exposures::Worker.new(settings: settings, transport: transport, logger: logger)
-        @reporter = Exposures::Reporter.new(@worker, telemetry: telemetry, logger: logger)
-        @engine = EvaluationEngine.new(@reporter, telemetry: telemetry, logger: logger)
-      end
+        @worker = Exposures::Worker.new(settings: settings, transport: transport, telemetry: telemetry, logger: logger)
 
-      def flush
-        @worker&.flush
+        reporter = Exposures::Reporter.new(@worker, telemetry: telemetry, logger: logger)
+        @engine = EvaluationEngine.new(reporter, telemetry: telemetry, logger: logger)
       end
 
       def shutdown!
-        @worker&.graceful_shutdown
+        @worker.graceful_shutdown
       end
     end
   end

lib/datadog/open_feature/evaluation_engine.rb

@@ -8,62 +8,51 @@ module Datadog
   module OpenFeature
     # This class performs the evaluation of the feature flag
     class EvaluationEngine
-      attr_accessor :configuration
+      ReconfigurationError = Class.new(StandardError)
 
-      ALLOWED_TYPES = %i[boolean string number float integer object].freeze
+      ALLOWED_TYPES = %w[boolean string number float integer object].freeze
 
       def initialize(reporter, telemetry:, logger:)
         @reporter = reporter
         @telemetry = telemetry
         @logger = logger
 
-        @mutex = Mutex.new
         @evaluator = NoopEvaluator.new(nil)
-        @configuration = nil
       end
 
-      def fetch_value(flag_key:, expected_type:, evaluation_context: nil)
+      def fetch_value(flag_key:, default_value:, expected_type:, evaluation_context: nil)
         unless ALLOWED_TYPES.include?(expected_type)
           message = "unknown type #{expected_type.inspect}, allowed types #{ALLOWED_TYPES.join(", ")}"
-
-          return ResolutionDetails.new(
-            error_code: Ext::UNKNOWN_TYPE,
-            error_message: message,
-            reason: Ext::ERROR,
-            log?: false,
-            error?: true
+          return ResolutionDetails.build_error(
+            value: default_value, error_code: Ext::UNKNOWN_TYPE, error_message: message
           )
         end
 
-        result = @evaluator.get_assignment(flag_key, evaluation_context, expected_type)
+        context = evaluation_context&.fields || {}
+        result = @evaluator.get_assignment(flag_key, default_value, context, expected_type)
+
         @reporter.report(result, flag_key: flag_key, context: evaluation_context)
 
         result
       rescue => e
         @telemetry.report(e, description: 'OpenFeature: Failed to fetch flag value')
 
-        ResolutionDetails.new(
-          error_code: Ext::PROVIDER_FATAL,
-          error_message: e.message,
-          reason: Ext::ERROR,
-          error?: true,
-          log?: false
+        ResolutionDetails.build_error(
+          value: default_value, error_code: Ext::PROVIDER_FATAL, error_message: e.message
         )
       end
 
-      def reconfigure!
-        @logger.debug('OpenFeature: Removing configuration') if @configuration.nil?
+      def reconfigure!(configuration)
+        @logger.debug('OpenFeature: Removing configuration') if configuration.nil?
 
-        @mutex.synchronize do
-          @evaluator = NoopEvaluator.new(@configuration)
-        end
+        @evaluator = NoopEvaluator.new(configuration)
       rescue => e
-        error_message = 'OpenFeature: Failed to reconfigure, reverting to the previous configuration'
+        message = 'OpenFeature: Failed to reconfigure, reverting to the previous configuration'
 
-        @logger.error("#{error_message}, error #{e.inspect}")
-        @telemetry.report(e, description: error_message)
+        @logger.error("#{message}, error #{e.inspect}")
+        @telemetry.report(e, description: message)
 
-        raise e
+        raise ReconfigurationError, e.message
       end
     end
   end

lib/datadog/open_feature/exposures/batch_builder.rb

@@ -12,7 +12,7 @@ def initialize(settings)
         def payload_for(events)
           {
             context: @context,
-            exposures: events.map(&:to_h)
+            exposures: events
           }
         end
 

lib/datadog/open_feature/exposures/buffer.rb

@@ -7,11 +7,16 @@ module OpenFeature
     module Exposures
       # This class is a buffer for exposure events that evicts at random and
       # keeps track of the number of dropped events
+      #
+      # WARNING: This class does not work as intended on JRuby
       class Buffer < Core::Buffer::CRuby
         DEFAULT_LIMIT = 1_000
 
+        attr_reader :dropped_count
+
         def initialize(limit = DEFAULT_LIMIT)
           @dropped = 0
+          @dropped_count = 0
 
           super
         end
@@ -21,10 +26,10 @@ def initialize(limit = DEFAULT_LIMIT)
         def drain!
           drained = super
 
-          dropped = @dropped
+          @dropped_count = @dropped
           @dropped = 0
 
-          [drained, dropped]
+          drained
         end
 
         def replace!(item)

lib/datadog/open_feature/exposures/deduplicator.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-require 'zlib'
 require_relative '../../core/utils/lru_cache'
 
 module Datadog
@@ -15,25 +14,16 @@ def initialize(limit: DEFAULT_CACHE_LIMIT)
           @mutex = Mutex.new
         end
 
-        def duplicate?(event)
-          cache_key = digest(event.flag_key, event.targeting_key)
-          cache_digest = digest(event.allocation_key, event.variation_key)
-
+        def duplicate?(key, value)
           @mutex.synchronize do
-            stored = @cache[cache_key]
-            return true if stored == cache_digest
+            stored = @cache[key]
+            return true if stored == value
 
-            @cache[cache_key] = cache_digest
+            @cache[key] = value
           end
 
           false
         end
-
-        private
-
-        def digest(left, right)
-          Zlib.crc32("#{left}:#{right}")
-        end
       end
     end
   end

lib/datadog/open_feature/exposures/event.rb

@@ -6,19 +6,21 @@ module Datadog
   module OpenFeature
     module Exposures
       # A data model for an exposure event
-      class Event
+      module Event
         TARGETING_KEY_FIELD = 'targeting_key'
-        ALLOWED_FIELD_TYPES = [
-          String,
-          Integer,
-          Float,
-          TrueClass,
-          FalseClass
-        ].freeze
+        ALLOWED_FIELD_TYPES = [String, Integer, Float, TrueClass, FalseClass].freeze
 
         class << self
+          def cache_key(result, flag_key:, context:)
+            "#{flag_key}:#{context.targeting_key}"
+          end
+
+          def cache_value(result, flag_key:, context:)
+            "#{result.allocation_key}:#{result.variant}"
+          end
+
           def build(result, flag_key:, context:)
-            payload = {
+            {
               timestamp: current_timestamp_ms,
               allocation: {
                 key: result.allocation_key
@@ -33,13 +35,13 @@ def build(result, flag_key:, context:)
                 id: context.targeting_key,
                 attributes: extract_attributes(context)
               }
-            }
-
-            new(payload)
+            }.freeze
           end
 
           private
 
+          # NOTE: We take all filds of the context that does not support nesting
+          #       and will ignore targeting key as it will be set as `subject.id`
           def extract_attributes(context)
             context.fields.select do |key, value|
               next false if key == TARGETING_KEY_FIELD
@@ -52,30 +54,6 @@ def current_timestamp_ms
             (Datadog::Core::Utils::Time.now.to_f * 1000).to_i
           end
         end
-
-        def initialize(payload)
-          @payload = payload.freeze
-        end
-
-        def flag_key
-          @payload.dig(:flag, :key)
-        end
-
-        def targeting_key
-          @payload.dig(:subject, :id)
-        end
-
-        def allocation_key
-          @payload.dig(:allocation, :key)
-        end
-
-        def variation_key
-          @payload.dig(:variant, :key)
-        end
-
-        def to_h
-          @payload
-        end
       end
     end
   end

lib/datadog/open_feature/exposures/reporter.rb

@@ -15,16 +15,22 @@ def initialize(worker, telemetry:, logger:)
           @deduplicator = Deduplicator.new
         end
 
+        # NOTE: Reporting expects evaluation context to be always present, but it
+        #       might be missing depending on the customer way of using flags evaluation API.
+        #       In addition to that the evaluation result must be marked for reporting.
         def report(result, flag_key:, context:)
-          return false unless result.log?
           return false if context.nil?
+          return false unless result.log?
 
-          event = Event.build(result, flag_key: flag_key, context: context)
-          return false if @deduplicator.duplicate?(event)
+          key = Event.cache_key(result, flag_key: flag_key, context: context)
+          value = Event.cache_value(result, flag_key: flag_key, context: context)
+          return false if @deduplicator.duplicate?(key, value)
 
+          event = Event.build(result, flag_key: flag_key, context: context)
           @worker.enqueue(event)
         rescue => e
           @logger.debug { "OpenFeature: Failed to report resolution details: #{e.class}: #{e.message}" }
+          @telemetry.report(e, description: 'OpenFeature: Failed to report resolution details')
 
           false
         end