Refactor config validation

Move validation to a separate module and use a transform to call those
methods.

Remove ratio validation checks and fallbacks in Transaction,
TraceContext, and DistributedTracing.

Author: kaylareopelle

lib/new_relic/agent/configuration/default_source.rb

@@ -4,6 +4,7 @@
 
 require 'forwardable'
 require_relative '../../constants'
+require_relative 'sampler_config_validator'
 
 module NewRelic
   module Agent
@@ -1457,24 +1458,36 @@ def self.convert_to_constant_list(raw_value)
           :default => 'default',
           :public => true,
           :type => String,
-          :allowed_from_server => true,
+          :allowed_from_server => false,
           :allowlist => %w[default adaptive always_on always_off trace_id_ratio_based],
+          :transform => SamplerConfigValidator.validate_sampler_strategy_with_ratio(
+            :'distributed_tracing.sampler.root',
+            :'distributed_tracing.sampler.root.trace_id_ratio_based.ratio'
+          ),
           :description => 'This setting controls the behavior of transaction sampling for transactions without a remote parent, traces that originate within this instance of the New Relic agent. Available values are `default`, `adaptive`, `always_on`, `always_off`, and `trace_id_ratio_based`. At this time `default` and `adaptive` are the same.'
         },
         :'distributed_tracing.sampler.remote_parent_sampled' => {
           :default => 'default',
           :public => true,
           :type => String,
-          :allowed_from_server => true,
+          :allowed_from_server => false,
           :allowlist => %w[default adaptive always_on always_off trace_id_ratio_based],
+          :transform => SamplerConfigValidator.validate_sampler_strategy_with_ratio(
+            :'distributed_tracing.sampler.remote_parent_sampled',
+            :'distributed_tracing.sampler.remote_parent_sampled.trace_id_ratio_based.ratio'
+          ),
           :description => 'This setting controls the behavior of transaction sampling when a remote parent is sampled. Available values are `default`, `adaptive`, `always_on`, `always_off`, and `trace_id_ratio_based`. At this time `default` and `adaptive` are the same.'
         },
         :'distributed_tracing.sampler.remote_parent_not_sampled' => {
           :default => 'default',
           :public => true,
           :type => String,
-          :allowed_from_server => true,
+          :allowed_from_server => false,
           :allowlist => %w[default adaptive always_on always_off trace_id_ratio_based],
+          :transform => SamplerConfigValidator.validate_sampler_strategy_with_ratio(
+            :'distributed_tracing.sampler.remote_parent_not_sampled',
+            :'distributed_tracing.sampler.remote_parent_not_sampled.trace_id_ratio_based.ratio'
+          ),
           :description => 'This setting controls the behavior of transaction sampling when a remote parent is not sampled. Available values are `default`, `adaptive`, `always_on`, `always_off`, and `trace_id_ratio_based`. At this time `default` and `adaptive` are the same.'
         },
         :'distributed_tracing.sampler.root.trace_id_ratio_based.ratio' => {
@@ -1483,6 +1496,7 @@ def self.convert_to_constant_list(raw_value)
           :type => Float,
           :allow_nil => true,
           :allowed_from_server => false,
+          :transform => SamplerConfigValidator.method(:validate_sampling_ratio),
           :description => 'The ratio used for the trace_id_ratio_based sampling decision for the root sampler. This must be a float between 0.0 and 1.0. If you provide an invalid value, the sampler will not use the trace_id_ratio_based sampler and will return to the default behavior. If you do not provide a value, the sampler will not use the trace_id_ratio_based_sampler and fall back to the default sampler.'
         },
         :'distributed_tracing.sampler.remote_parent_sampled.trace_id_ratio_based.ratio' => {
@@ -1491,6 +1505,7 @@ def self.convert_to_constant_list(raw_value)
           :type => Float,
           :allow_nil => true,
           :allowed_from_server => false,
+          :transform => SamplerConfigValidator.method(:validate_sampling_ratio),
           :description => 'The ratio used for the trace_id_ratio_based sampling decision for the remote parent sampled sampler. This must be a float between 0.0 and 1.0. If you provide an invalid value, the sampler will not use the trace_id_ratio_based sampler and will return to the default behavior. If you do not provide a value, the sampler will not use the trace_id_ratio_based_sampler and fall back to the default sampler.'
         },
         :'distributed_tracing.sampler.remote_parent_not_sampled.trace_id_ratio_based.ratio' => {
@@ -1499,6 +1514,7 @@ def self.convert_to_constant_list(raw_value)
           :type => Float,
           :allow_nil => true,
           :allowed_from_server => false,
+          :transform => SamplerConfigValidator.method(:validate_sampling_ratio),
           :description => 'The ratio used for the trace_id_ratio_based sampling decision for the remote parent not sampled sampler. This must be a float between 0.0 and 1.0. If you provide an invalid value or do not provide a value, the sampler will not use the trace_id_ratio_based_sampler and fall back to the default sampler.'
         },
         # Elasticsearch

lib/new_relic/agent/configuration/sampler_config_validator.rb

@@ -0,0 +1,58 @@
+# This file is distributed under New Relic's license terms.
+# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
+# frozen_string_literal: true
+
+module NewRelic
+  module Agent
+    module Configuration
+      # Handles validation for the distributed tracing sampler configs
+      # Focuses on validating the trace id ratio based ratios
+      module SamplerConfigValidator
+        @sampler_strategy_warnings = {}
+
+        class << self
+          def validate_sampling_ratio(ratio)
+            return nil if ratio.nil?
+
+            unless valid_ratio?(ratio)
+              return nil
+            end
+
+            ratio
+          end
+
+          def validate_sampler_strategy_with_ratio(strategy_key, ratio_key)
+            proc do |strategy|
+              next strategy unless strategy == 'trace_id_ratio_based'
+
+              ratio = NewRelic::Agent.config[ratio_key]
+
+              if valid_ratio?(ratio)
+                next strategy
+              end
+
+              unless @sampler_strategy_warnings[strategy_key]
+                NewRelic::Agent.logger.warn(
+                  "Invalid or missing ratio for #{ratio_key} (value: #{ratio.inspect}). " \
+                  "Falling back to 'default' for #{strategy_key}."
+                )
+
+                @sampler_strategy_warnings[strategy_key] = true
+              end
+
+              'default'
+            end
+          end
+
+          def valid_ratio?(ratio)
+            ratio.is_a?(Float) && (0.0..1.0).cover?(ratio)
+          end
+
+          def reset_warnings!
+            @sampler_strategy_warnings = {}
+          end
+        end
+      end
+    end
+  end
+end

lib/new_relic/agent/transaction.rb

@@ -303,24 +303,16 @@ def sampled?
             false
           when 'trace_id_ratio_based'
             ratio = NewRelic::Agent.config[:'distributed_tracing.sampler.root.trace_id_ratio_based.ratio']
-            if valid_ratio?(ratio)
-              upper_bound = (ratio * (2**64 - 1)).ceil
-              ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
-            else
-              NewRelic::Agent.instance.adaptive_sampler.sampled?
-            end
+            upper_bound = (ratio * (2**64 - 1)).ceil
+
+            ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
           when 'adaptive'
             NewRelic::Agent.instance.adaptive_sampler.sampled?
           end
         end
         @sampled
       end
 
-      def valid_ratio?(ratio)
-        # refactor, this should be validated earlier and default already selected
-        ratio.is_a?(Float) && (0.0..1.0).cover?(ratio)
-      end
-
       def trace_id
         @trace_id ||= NewRelic::Agent::GuidGenerator.generate_guid(32)
       end

lib/new_relic/agent/transaction/distributed_tracing.rb

@@ -196,19 +196,15 @@ def set_priority_and_sampled_newrelic_header(sampler, ratio, payload)
             transaction.sampled = false
             transaction.priority = 0
           when 'trace_id_ratio_based'
-            if ratio.is_a?(Float) && (0.0..1.0).cover?(ratio)
-              upper_bound = (ratio * (2**64 - 1)).ceil
-              sampled = ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
-
-              if sampled
-                transaction.sampled = true
-                transaction.priority = 2.0
-              else
-                transaction.sampled = false
-                transaction.priority = 0
-              end
+            upper_bound = (ratio * (2**64 - 1)).ceil
+            sampled = ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
+
+            if sampled
+              transaction.sampled = true
+              transaction.priority = 2.0
             else
-              use_nr_tracestate_sampled(payload)
+              transaction.sampled = false
+              transaction.priority = 0
             end
           when 'adaptive'
             use_nr_tracestate_sampled(payload)

lib/new_relic/agent/transaction/trace_context.rb

@@ -190,19 +190,15 @@ def set_priority_and_sampled(sampler, ratio, payload)
             transaction.sampled = false
             transaction.priority = 0
           when 'trace_id_ratio_based'
-            if ratio.is_a?(Float) && (0.0..1.0).cover?(ratio)
-              upper_bound = (ratio * (2**64 - 1)).ceil
-              sampled = ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
-
-              if sampled
-                transaction.sampled = true
-                transaction.priority = 2.0
-              else
-                transaction.sampled = false
-                transaction.priority = 0
-              end
+            upper_bound = (ratio * (2**64 - 1)).ceil
+            sampled = ratio == 1.0 || trace_id[8, 8].unpack1('Q>') < upper_bound
+
+            if sampled
+              transaction.sampled = true
+              transaction.priority = 2.0
             else
-              use_nr_tracestate_sampled(payload)
+              transaction.sampled = false
+              transaction.priority = 0
             end
           when 'adaptive'
             use_nr_tracestate_sampled(payload)