FFM-12192 patch 5 (#52)

* CI-15294 Use string as key for metrics

* CI-15294 delete metrics event

* CI-15294 delete metrics event

* CI-15294 Refactor tests to account for simple string keys

* CI-15294 Bump version and fix comment

Author: erdirowlands

lib/ff/ruby/server/sdk.rb

@@ -17,7 +17,6 @@
 require_relative "sdk/api/evaluation"
 require_relative "sdk/api/inner_client"
 require_relative "sdk/api/auth_service"
-require_relative "sdk/api/metrics_event"
 require_relative "sdk/api/default_cache"
 require_relative "sdk/api/config_builder"
 require_relative "sdk/api/file_map_store"

lib/ff/ruby/server/sdk/api/evaluator.rb

@@ -117,7 +117,9 @@ def evaluate(identifier, target, expected, callback)
 
       if variation != nil
         if callback != nil
-          callback.process_evaluation(flag, target, variation)
+          feature_name = flag.feature
+          variation_identifier = variation.identifier
+          callback.process_evaluation(feature_name, target, variation_identifier)
         end
         return variation
       end

lib/ff/ruby/server/sdk/api/inner_client_flag_evaluate_callback.rb

@@ -21,10 +21,10 @@ def initialize(metrics_processor, logger = nil)
     @metrics_processor = metrics_processor
   end
 
-  def process_evaluation(feature_config, target, variation)
+  def process_evaluation(feature_name, target, variation_identifier)
 
-    @logger.debug "Processing evaluation: #{feature_config&.feature || 'nil feature'}, #{target&.identifier || 'nil target'}"
+    @logger.debug "Processing evaluation: #{feature_name || 'nil feature'}, #{variation_identifier || 'nil variation'},  #{target&.identifier || 'nil target'}"
 
-    @metrics_processor.register_evaluation(target, feature_config, variation)
+    @metrics_processor.register_evaluation(target, feature_name, variation_identifier)
   end
 end

lib/ff/ruby/server/sdk/api/metrics_event.rb

@@ -6,12 +6,9 @@
 require_relative "../../sdk/version"
 require_relative "../common/closeable"
 require_relative "../common/sdk_codes"
-require_relative "../api/metrics_event"
 require_relative "../api/summary_metrics"
 
 class MetricsProcessor < Closeable
-  GLOBAL_TARGET = Target.new(identifier: "__global__cf_target", name: "Global Target").freeze
-
   def init(connector, config, callback)
 
     unless connector.kind_of?(Connector)
@@ -79,38 +76,33 @@ def close
     @config.logger.debug "Closing metrics processor"
   end
 
-  def register_evaluation(target, feature_config, variation)
-    register_evaluation_metric(feature_config, variation)
+  def register_evaluation(target, feature_name, variation_identifier)
+    register_evaluation_metric(feature_name, variation_identifier)
     if target
       register_target_metric(target)
     end
   end
 
   private
 
-  def register_evaluation_metric(feature_config, variation)
+  def register_evaluation_metric(feature_name, variation_identifier)
     # Guard clause to ensure feature_config, @global_target, and variation are valid.
     # While they should be, this adds protection for an edge case we are seeing where the the ConcurrentMap (now replaced with our own thread safe hash)
     # seemed to be accessing invalid areas of memory and seg faulting.
     # Issue being tracked in FFM-12192, and once resolved, can remove these checks in a future release && once the issue is resolved.
-    if feature_config.nil? || !feature_config.respond_to?(:feature) || feature_config.feature.nil?
-      @config.logger.warn("Skipping invalid MetricsEvent: feature_config is missing or incomplete. feature_config=#{feature_config.inspect}")
+    unless feature_name && !feature_name.empty?
+      @config.logger.warn("Skipping invalid MetricsEvent: feature_config is missing or incomplete. feature_config=#{feature_name.inspect}")
       return
     end
 
-    if GLOBAL_TARGET.nil? || !GLOBAL_TARGET.respond_to?(:identifier) || GLOBAL_TARGET.identifier.nil?
-      @config.logger.warn("Skipping invalid MetricsEvent: global_target is missing or incomplete. global_target=#{GLOBAL_TARGET.inspect}")
+    unless variation_identifier && !variation_identifier.empty?
+      @config.logger.warn("Skipping iInvalid MetricsEvent: variation is missing or incomplete. variation=#{variation_identifier.inspect}")
       return
     end
 
-    if variation.nil? || !variation.respond_to?(:identifier) || variation.identifier.nil?
-      @config.logger.warn("Skipping iInvalid MetricsEvent: variation is missing or incomplete. variation=#{variation.inspect}")
-      return
-    end
-
-    event = MetricsEvent.new(feature_config, GLOBAL_TARGET, variation, @config.logger)
     @metric_maps_mutex.synchronize do
-      @evaluation_metrics[event] = (@evaluation_metrics[event] || 0) + 1
+      key = "#{feature_name}\0#{variation_identifier}"
+      @evaluation_metrics[key] = (@evaluation_metrics[key] || 0) + 1
     end
   end
 
@@ -185,31 +177,28 @@ def prepare_summary_metrics_body(evaluation_metrics_clone, target_metrics_clone)
 
     total_count = 0
     evaluation_metrics_clone.each do |key, value|
-      # While Components should not be missing, this adds protection for an edge case we are seeing with very large
-      # project sizes.  Issue being tracked in FFM-12192, and once resolved, can feasibly remove
-      # these checks in a future release.
-      # Initialize an array to collect missing components
-      missing_components = []
-
-      # Check each required component and add to missing_components if absent
-      missing_components << 'feature_config' unless key.respond_to?(:feature_config) && key.feature_config
-      missing_components << 'variation' unless key.respond_to?(:variation) && key.variation
-      missing_components << 'target' unless key.respond_to?(:target) && key.target
-      missing_components << 'count' if value.nil?
-
-      # If any components are missing, log a detailed warning and skip processing
-      unless missing_components.empty?
-        @config.logger.warn "Skipping invalid metrics event: missing #{missing_components.join(', ')} in key: #{key.inspect}, full details: #{key.inspect}"
+      feature_name, variation_identifier = key.split("\0", 2)
+
+      # Although feature_name and variation_identifier should always be present,
+      # this guard provides protection against an edge case where keys reference
+      # other objects in memory. In versions <= 1.4.4, we were keying on the MetricsEvent
+      # class (now deleted). To remediate this, we have transitioned to using strings as keys.
+      # This issue is being tracked in FFM-12192. Once resolved, these checks can be safely
+      # removed in a future release.
+      # If any required data is missing, log a detailed warning and skip processing.
+      unless feature_name && variation_identifier && value.is_a?(Integer) && value > 0
+        @config.logger.warn "Skipping invalid metrics event: missing or invalid feature_name, variation_identifier, or count. Key: #{key.inspect}, Count: #{value.inspect}"
         next
       end
 
       total_count += value
+
       metrics_data = OpenapiClient::MetricsData.new({ :attributes => [] })
       metrics_data.timestamp = (Time.now.to_f * 1000).to_i
       metrics_data.count = value
       metrics_data.metrics_type = "FFMETRICS"
-      metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @feature_name_attribute, :value => key.feature_config.feature }))
-      metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @variation_identifier_attribute, :value => key.variation.identifier }))
+      metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @feature_name_attribute, :value => feature_name }))
+      metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @variation_identifier_attribute, :value => variation_identifier }))
       metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @target_attribute, :value => @global_target_identifier }))
       metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @sdk_type, :value => @server }))
       metrics_data.attributes.push(OpenapiClient::KeyValue.new({ :key => @sdk_language, :value => "ruby" }))

lib/ff/ruby/server/sdk/api/metrics_processor.rb

@@ -5,7 +5,7 @@ module Ruby
     module Server
       module Sdk
 
-        VERSION = "1.4.4"
+        VERSION = "1.4.5"
       end
     end
   end

lib/ff/ruby/server/sdk/version.rb

@@ -1,4 +1,4 @@
 #!/bin/bash
 
 export ff_ruby_sdk="ff-ruby-server-sdk"
-export ff_ruby_sdk_version="1.4.4"
+export ff_ruby_sdk_version="1.4.5"

scripts/sdk_specs.sh

@@ -118,10 +118,10 @@ def test_metrics
     callback.wait_until_ready
 
     # Here we test register_evaluation then run_one_iteration
-    (1..3).each { metrics_processor.register_evaluation @target, @feature1, @variation1 }
-    (1..3).each { metrics_processor.register_evaluation @target, @feature1, @variation2 }
-    (1..3).each { metrics_processor.register_evaluation @target, @feature2, @variation1 }
-    (1..3).each { metrics_processor.register_evaluation @target, @feature2, @variation2 }
+    (1..3).each { metrics_processor.register_evaluation @target, @feature1.feature, @variation1.identifier }
+    (1..3).each { metrics_processor.register_evaluation @target, @feature1.feature, @variation2.identifier }
+    (1..3).each { metrics_processor.register_evaluation @target, @feature2.feature, @variation1.identifier }
+    (1..3).each { metrics_processor.register_evaluation @target, @feature2.feature, @variation2.identifier }
 
     freq_map = metrics_processor.send :get_frequency_map
     assert_equal 4, freq_map.size, "not enough pending metrics"
@@ -162,18 +162,8 @@ def config.logger
 
     # Register evaluations
     (1..10).each do |i|
-      feature = OpenapiClient::FeatureConfig.new
-      feature.feature = "feature-#{i}"
-      variation = OpenapiClient::Variation.new
-      variation.identifier = "variation-#{i}"
-      variation.value = "value-#{i}"
-      variation.name = "Test-#{i}"
-
-      variation2 = OpenapiClient::Variation.new
-      variation2.identifier = "variation2-#{i}"
-      variation2.value = "value2-#{i}"
-      variation2.name = "Test2-#{i}"
-      feature.variations = [variation, @variation2]
+      feature = "feature-#{i}"
+      variation = "variation-#{i}"
       metrics_processor.register_evaluation(@target, feature, variation)
     end
 
@@ -258,19 +248,9 @@ def config.logger
 
     # Define a method to register evaluations
     register_evals = Proc.new do |feature_id|
-      feature = OpenapiClient::FeatureConfig.new
-      feature.feature = "feature-#{feature_id}"
-      variation = OpenapiClient::Variation.new
-      variation.identifier = "variation-#{feature_id}"
-      variation.value = "value-#{feature_id}"
-      variation.name = "Test-#{feature_id}"
-
-      variation2 = OpenapiClient::Variation.new
-      variation2.identifier = "variation2-#{feature_id}"
-      variation2.value = "value2-#{feature_id}"
-      variation2.name = "Test2-#{feature_id}"
-
-      feature.variations = [variation, variation2]
+      feature = "feature-#{feature_id}"
+      variation = "variation-#{feature_id}"
+
       metrics_processor.register_evaluation(@target, feature, variation)
     end
 
@@ -316,13 +296,11 @@ def config.logger
     metric_found = {}
     expected_metrics_set.each { |metric| metric_found[metric] = false }
 
-    freq_map.each_key do |metrics_event|
-      # Extract feature name and variation identifier from the metrics_event
-      actual_feature_name = metrics_event.feature_config.feature
-      actual_variation_id = metrics_event.variation.identifier
+    freq_map.each_key do |key|
+      feature_name, variation_identifier = key.split("\0", 2)
 
       # Create the key for the current metric
-      metric_key = [actual_feature_name, actual_variation_id]
+      metric_key = [feature_name, variation_identifier]
 
       if metric_found.key?(metric_key)
         metric_found[metric_key] = true
@@ -353,19 +331,9 @@ def config.logger
       callback.wait_until_ready
 
       # Register some evaluations
-      feature = OpenapiClient::FeatureConfig.new
-      feature.feature = "feature-error-test"
-      variation = OpenapiClient::Variation.new
-      variation.identifier = "variation-error-test"
-      variation.value = "value-error-test"
-      variation.name = "Test-Error"
-
-      variation2 = OpenapiClient::Variation.new
-      variation2.identifier = "variation2-error-test"
-      variation2.value = "value2-error-test"
-      variation2.name = "Test2-Error"
-
-      feature.variations = [variation, variation2]
+      feature = "feature-error-test"
+      variation = "variation-error-test"
+
       metrics_processor.register_evaluation(@target, feature, variation)
 
       # Attempt to send data, which should raise an exception
@@ -379,18 +347,10 @@ def config.logger
       assert_empty target_metrics_map, "Target metrics map should be cleared even if an error occurs"
 
       # Verify that the MetricsProcessor remains operational by registering another evaluation
-      feature_new = OpenapiClient::FeatureConfig.new
-      feature_new.feature = "feature-new"
-      variation_new = OpenapiClient::Variation.new
-      variation_new.identifier = "variation-new"
-      variation_new.value = "value-new"
-      variation_new.name = "Test-New"
-
-      variation2_new = OpenapiClient::Variation.new
-      variation2_new.identifier = "variation2-new"
-      variation2_new.value = "value2-new"
-      variation2_new.name = "Test2-New"
-      metrics_processor.register_evaluation(@target, feature, variation)
+      feature_new = "feature-new"
+      variation_new = "variation-new"
+
+      metrics_processor.register_evaluation(@target, feature_new, variation_new)
 
       # Ensure that the new evaluation is registered correctly
       freq_map = metrics_processor.send(:get_frequency_map)
@@ -465,18 +425,6 @@ def assert_target_data(target_data)
     assert targets.key?("target-id")
   end
 
-  def test_comparable_metrics_event_equals_and_hash
-
-    event1 = MetricsEvent.new(@feature1, @target, @variation1, Logger.new(STDOUT))
-    event2 = MetricsEvent.new(@feature1, @target, @variation1, Logger.new(STDOUT))
-
-    assert(event1 == event2)
-
-    event1 = MetricsEvent.new(@feature1, @target, @variation1, Logger.new(STDOUT))
-    event2 = MetricsEvent.new(@feature2, @target, @variation2, Logger.new(STDOUT))
-
-    assert(event1 != event2)
-  end
 
   def test_metrics_processor_prevents_invalid_metrics_event
     logger = Logger.new(STDOUT)
@@ -495,15 +443,15 @@ def test_metrics_processor_prevents_invalid_metrics_event
     callback.wait_until_ready
 
     # Attempt to register invalid evaluations
-    metrics_processor.register_evaluation(@target, nil, @variation1)
+    metrics_processor.register_evaluation(@target, nil, @variation1.identifier)
     metrics_processor.register_evaluation(@target, nil, nil)
-    metrics_processor.register_evaluation(nil, @feature1, nil)
+    metrics_processor.register_evaluation(nil, @feature1.feature, nil)
 
     # Register some valid evaluations
-    metrics_processor.register_evaluation(@target, @feature1, @variation1)
-    metrics_processor.register_evaluation(@target, @feature2, @variation2)
+    metrics_processor.register_evaluation(@target, @feature1.feature, @variation1.identifier)
+    metrics_processor.register_evaluation(@target, @feature2.feature, @variation2.identifier)
     # Nil target, which is a valid input to variation methods
-    metrics_processor.register_evaluation(nil, @feature2, @variation2)
+    metrics_processor.register_evaluation(nil, @feature2.feature, @variation2.identifier)
 
     # Run iteration
     metrics_processor.send(:run_one_iteration)
@@ -535,11 +483,4 @@ def test_metrics_processor_prevents_invalid_metrics_event
     assert_equal 1, captured_metrics[0].target_data.size, "There should only be a single target"
   end
 
-  def test_metrics_event_eql_with_invalid_object
-    event = MetricsEvent.new(@feature1, @target, @variation1, Logger.new(STDOUT))
-    non_event = "Not a MetricsEvent"
-
-    refute_equal(event, non_event, "MetricsEvent should not be equal to a non-MetricsEvent object")
-  end
-
 end