Reduce, refactor, rubocop

Author: kaylareopelle

lib/new_relic/agent/configuration/sampler_config_validator.rb

@@ -13,10 +13,7 @@ module SamplerConfigValidator
         class << self
           def validate_sampling_ratio(ratio)
             return nil if ratio.nil?
-
-            unless valid_ratio?(ratio)
-              return nil
-            end
+            return nil unless valid_ratio?(ratio)
 
             ratio
           end
@@ -27,9 +24,7 @@ def validate_sampler_strategy_with_ratio(strategy_key, ratio_key)
 
               ratio = NewRelic::Agent.config[ratio_key]
 
-              if valid_ratio?(ratio)
-                next strategy
-              end
+              next strategy if valid_ratio?(ratio)
 
               unless @sampler_strategy_warnings[strategy_key]
                 NewRelic::Agent.logger.warn(
@@ -48,11 +43,12 @@ def valid_ratio?(ratio)
             ratio.is_a?(Float) && (0.0..1.0).cover?(ratio)
           end
 
+          # used for testing
           def reset_warnings!
             @sampler_strategy_warnings = {}
           end
         end
       end
     end
   end
-end
+end

lib/new_relic/agent/transaction.rb

@@ -302,8 +302,7 @@ def sampled?
           when 'always_off'
             false
           when 'trace_id_ratio_based'
-            ratio = NewRelic::Agent.config[:'distributed_tracing.sampler.root.trace_id_ratio_based.ratio']
-            trace_ratio_sampled?(ratio)
+            trace_ratio_sampled?(NewRelic::Agent.config[:'distributed_tracing.sampler.root.trace_id_ratio_based.ratio'])
           when 'adaptive'
             NewRelic::Agent.instance.adaptive_sampler.sampled?
           end
@@ -312,8 +311,9 @@ def sampled?
       end
 
       def trace_ratio_sampled?(ratio)
-        upper_bound = (ratio * (2**64 - 1)).ceil
-        ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
+        # if the ratio is 1.0, always sample
+        # otherwise, do math
+        ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < (ratio * (2**64 - 1)).ceil
       end
 
       def trace_id

lib/new_relic/agent/transaction/distributed_tracing.rb

@@ -161,29 +161,21 @@ def assign_payload_and_sampling_params(payload)
           transaction.parent_span_id = payload.id
 
           unless payload.sampled.nil?
-            if payload.sampled == true
+            if payload.sampled
               set_priority_and_sampled(
                 NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_sampled'],
                 NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_sampled.trace_id_ratio_based.ratio'],
                 payload
               )
-            elsif payload.sampled == false
+            else
               set_priority_and_sampled(
                 NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_not_sampled'],
                 NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_not_sampled.trace_id_ratio_based.ratio'],
                 payload
               )
-            else
-              transaction.sampled = payload.sampled
-              transaction.priority = payload.priority if payload.priority
             end
           end
         end
-
-        def default_sampling(payload)
-          transaction.sampled = payload.sampled
-          transaction.priority = payload.priority if payload.priority
-        end
       end
     end
   end

lib/new_relic/agent/transaction/trace_context.rb

@@ -155,16 +155,16 @@ def accept_trace_context(header_data)
         def determine_sampling_decision(payload, trace_flags)
           if trace_flags == '01'
             set_priority_and_sampled(
-                NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_sampled'],
-                NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_sampled.trace_id_ratio_based.ratio'],
-                payload
-              )
+              NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_sampled'],
+              NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_sampled.trace_id_ratio_based.ratio'],
+              payload
+            )
           elsif trace_flags == '00'
             set_priority_and_sampled(
-                NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_not_sampled'],
-                NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_not_sampled.trace_id_ratio_based.ratio'],
-                payload
-              )
+              NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_not_sampled'],
+              NewRelic::Agent.config[:'distributed_tracing.sampler.remote_parent_not_sampled.trace_id_ratio_based.ratio'],
+              payload
+            )
           else
             use_nr_tracestate_sampled(payload)
           end

test/new_relic/agent/configuration/sampler_config_validator_test.rb

@@ -288,4 +288,4 @@ def test_config_system_applies_strategy_fallback_when_accessed
       end
     end
   end
-end
+end

test/new_relic/agent/distributed_tracing/trace_context_cross_agent_test.rb

@@ -57,8 +57,9 @@ def self.focus_tests
               }
 
               if test_case.key?('ratio')
-                key = %w[root remote_parent_sampled remote_parent_not_sampled].find { |f| test_case[f] == 'trace_id_ratio_based'}
-                config.merge!({"distributed_tracing.sampler.#{key}.trace_id_ratio_based.ratio".to_sym => test_case['ratio']}) if key
+                strategies = %w[root remote_parent_sampled remote_parent_not_sampled]
+                key = strategies.find { |f| test_case[f] == 'trace_id_ratio_based' }
+                config["distributed_tracing.sampler.#{key}.trace_id_ratio_based.ratio".to_sym] = test_case['ratio']
               end
 
               with_server_source(config) do
@@ -235,6 +236,7 @@ def verify_attributes(test_case_attributes, actual_attributes, event_type)
             pp(actual_attributes)
             puts '*' * 80
           end
+
           (test_case_attributes['exact'] || []).each do |k, v|
             assert_equal v,
               actual_attributes[k],