General Improvement

.rubocop.yml

@@ -3,7 +3,7 @@ plugins:
   - rubocop-performance
 
 AllCops:
-  TargetRubyVersion: '3.0'
+  TargetRubyVersion: 3.0
   NewCops: enable
   Exclude:
     - 'sample_start.rb'

lib/solarwinds_apm/api/transaction_name.rb

@@ -21,15 +21,16 @@ module TransactionName
       #
       # === Example:
       #
-      #   class DogfoodsController < ApplicationController
+      #   class OrdersController < ApplicationController
       #
       #     def create
-      #       @dogfood = Dogfood.new(params.permit(:brand, :name))
-      #       @dogfood.save
+      #       @order = Order.new(params.permit(:item, :quantity))
+      #       if @order.save
+      #         custom_name = "orderscontroller.create_for_#{params[:item]}"
+      #         SolarWindsAPM::API.set_transaction_name(custom_name)
+      #       end
       #
-      #       SolarWindsAPM::API.set_transaction_name("dogfoodscontroller.create_for_#{params[:brand]}")
-      #
-      #       redirect_to @dogfood
+      #       redirect_to @order
       #     end
       #
       #   end

lib/solarwinds_apm/config.rb

@@ -87,21 +87,32 @@ def self.set_log_level
       SolarWindsAPM.logger.level = SW_LOG_LEVEL_MAPPING.dig(log_level, :stdlib) || ::Logger::INFO # default log level info
     end
 
+    # e.g. enable_disable_config('STRING', :key, value, false, bool: true)
     def self.enable_disable_config(env_var, key, value, default, bool: false)
-      env_value = ENV[env_var.to_s]&.downcase
+      raw_env_value  = ENV.fetch(env_var, '')
+      env_value      = raw_env_value.downcase
       valid_env_values = bool ? %w[true false] : %w[enabled disabled]
 
-      if env_var && valid_env_values.include?(env_value)
+      if !env_var.empty? && valid_env_values.include?(env_value)
         value = bool ? true?(env_value) : env_value.to_sym
-      elsif env_var && !env_value.to_s.empty?
-        SolarWindsAPM.logger.warn("[#{name}/#{__method__}] #{env_var} must be #{valid_env_values.join('/')} (current setting is #{ENV.fetch(env_var, nil)}). Using default value: #{default}.")
-        return @@config[key.to_sym] = default
+      elsif !env_var.empty? && !raw_env_value.empty?
+        SolarWindsAPM.logger.warn do
+          "[#{name}/#{__method__}] #{env_var} must be #{valid_env_values.join('/')} (current setting is #{raw_env_value}). Using default value: #{default}."
+        end
+        return @@config[key] = default
       end
 
-      return @@config[key.to_sym] = value unless (bool && !boolean?(value)) || (!bool && !symbol?(value))
+      # Validate final value efficiently
+      valid = bool ? boolean?(value) : symbol?(value)
+
+      unless valid
+        SolarWindsAPM.logger.warn do
+          "[#{name}/#{__method__}] :#{key} must be #{valid_env_values.join('/')}. Using default value: #{default}."
+        end
+        return @@config[key] = default
+      end
 
-      SolarWindsAPM.logger.warn("[#{name}/#{__method__}] :#{key} must be a #{valid_env_values.join('/')}. Using default value: #{default}.")
-      @@config[key.to_sym] = default
+      @@config[key] = value
     end
 
     def self.true?(obj)
@@ -211,7 +222,7 @@ def self.[]=(key, value)
         enable_disable_config('SW_APM_TRIGGER_TRACING_MODE', key, value, :enabled)
 
       when :tracing_mode
-        enable_disable_config(nil, key, value, :enabled)
+        enable_disable_config('', key, value, :enabled)
 
       when :tag_sql
         enable_disable_config('SW_APM_TAG_SQL', key, value, false, bool: true)
@@ -242,9 +253,17 @@ def self.compile_settings(settings)
         return
       end
 
-      # `tracing: disabled` is the default
-      disabled = settings.select { |v| !v.key?(:tracing) || v[:tracing] == :disabled }
-      enabled  = settings.select { |v| v[:tracing] == :enabled }
+      # `tracing: disabled` is the default; below only separate the enabled and disabled settings
+      result = settings.each_with_object({ enabled: [], disabled: [] }) do |setting, acc|
+        if setting[:tracing] == :enabled
+          acc[:enabled] << setting
+        elsif !setting.key?(:tracing) || setting[:tracing] == :disabled
+          acc[:disabled] << setting
+        end
+      end
+
+      enabled = result[:enabled]
+      disabled = result[:disabled]
 
       SolarWindsAPM::Config[:enabled_regexps] = compile_regexp(enabled)
       SolarWindsAPM::Config[:disabled_regexps] = compile_regexp(disabled)

lib/solarwinds_apm/opentelemetry/otlp_processor.rb

@@ -25,8 +25,8 @@ class OTLPProcessor
 
       def initialize(txn_manager)
         @txn_manager = txn_manager
-        @meters      = { 'sw.apm.request.metrics' => ::OpenTelemetry.meter_provider.meter('sw.apm.request.metrics') }
         @metrics     = init_response_time_metrics
+        @is_lambda   = SolarWindsAPM::Utils.determine_lambda
         @transaction_name = nil
       end
 
@@ -40,21 +40,24 @@ def on_start(span, parent_context)
         trace_flags = span.context.trace_flags.sampled? ? '01' : '00'
         @txn_manager&.set_root_context_h(span.context.hex_trace_id, "#{span.context.hex_span_id}-#{trace_flags}")
         span.add_attributes({ SW_IS_ENTRY_SPAN => true })
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_start end" }
       rescue StandardError => e
         SolarWindsAPM.logger.info { "[#{self.class}/#{__method__}] processor on_start error: #{e.message}" }
       end
 
       def on_finishing(span)
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_finishing span attributes: #{span.attributes}" }
         return if non_entry_span(span: span)
 
         @transaction_name = calculate_transaction_names(span)
         span.set_attribute(SW_TRANSACTION_NAME, @transaction_name)
         @txn_manager.delete_root_context_h(span.context.hex_trace_id)
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_finishing end" }
       end
 
       # @param [Span] span the (immutable) {Span} that just ended.
       def on_finish(span)
-        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_finish span: #{span.to_span_data.inspect}" }
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_finish span attributes: #{span.attributes}" }
         return if non_entry_span(span: span)
 
         record_request_metrics(span)
@@ -63,10 +66,9 @@ def on_finish(span)
         ::OpenTelemetry.meter_provider.metric_readers.each do |reader|
           reader.pull if reader.respond_to? :pull
         end
-
-        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_finish succeed" }
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] processor on_finish end" }
       rescue StandardError => e
-        SolarWindsAPM.logger.info { "[#{self.class}/#{__method__}] error processing span on_finish: #{e.message}" }
+        SolarWindsAPM.logger.info { "[#{self.class}/#{__method__}] processor on_finish error: #{e.message}" }
       end
 
       # @param [optional Numeric] timeout An optional timeout in seconds.
@@ -94,7 +96,9 @@ def init_response_time_metrics
                                                   unit: 'ms')
         end
 
-        instrument = @meters['sw.apm.request.metrics'].create_histogram('trace.service.response_time', unit: 'ms', description: 'Duration of each entry span for the service, typically meaning the time taken to process an inbound request.')
+        meter = ::OpenTelemetry.meter_provider.meter('sw.apm.request.metrics')
+        instrument = meter.create_histogram('trace.service.response_time', unit: 'ms', description: 'Duration of each entry span for the service, typically meaning the time taken to process an inbound request.')
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Adding ExponentialBucketHistogram for response time metrics: #{instrument.inspect}" }
         { response_time: instrument }
       end
 
@@ -104,37 +108,42 @@ def meter_attributes(span)
           SW_TRANSACTION_NAME => @transaction_name
         }
 
-        if span_http?(span)
+        is_http_span = span_http?(span)
+
+        if is_http_span
           http_status_code = get_http_status_code(span)
           meter_attrs['http.status_code'] = http_status_code if http_status_code != 0
           meter_attrs['http.method'] = span.attributes[HTTP_METHOD] if span.attributes[HTTP_METHOD]
         end
-        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] meter_attrs: #{meter_attrs.inspect}" }
+
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] is_http_span: #{is_http_span}; meter_attrs: #{meter_attrs.inspect}" }
         meter_attrs.compact!
         meter_attrs
       end
 
       def calculate_lambda_transaction_name(span_name)
-        (ENV['SW_APM_TRANSACTION_NAME'] || ENV['AWS_LAMBDA_FUNCTION_NAME'] || span_name || 'unknown').slice(0, SolarWindsAPM::Constants::MAX_TXN_NAME_LENGTH)
+        txn_name = (ENV['SW_APM_TRANSACTION_NAME'] || ENV['AWS_LAMBDA_FUNCTION_NAME'] || span_name || 'unknown').slice(0, SolarWindsAPM::Constants::MAX_TXN_NAME_LENGTH)
+        SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Lambda transaction name: #{txn_name} (from env_txn=#{ENV.fetch('SW_APM_TRANSACTION_NAME', nil)}, lambda_func=#{ENV.fetch('AWS_LAMBDA_FUNCTION_NAME', nil)}, span_name=#{span_name})" }
+        txn_name
       end
 
       # Get trans_name and url_tran of this span instance.
       # Predecessor order: custom SDK > env var SW_APM_TRANSACTION_NAME > automatic naming
       def calculate_transaction_names(span)
-        return calculate_lambda_transaction_name(span.name) if SolarWindsAPM::Utils.determine_lambda
+        return calculate_lambda_transaction_name(span.name) if @is_lambda
 
         trace_span_id = "#{span.context.hex_trace_id}-#{span.context.hex_span_id}"
         trans_name = @txn_manager.get(trace_span_id)
         if trans_name
-          SolarWindsAPM.logger.debug do
-            "[#{self.class}/#{__method__}] found trans name from txn_manager: #{trans_name} by #{trace_span_id}"
-          end
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Using transaction name from txn_manager: #{trans_name} (#{trace_span_id})" }
           @txn_manager.del(trace_span_id)
         elsif !ENV['SW_APM_TRANSACTION_NAME'].to_s.empty?
           trans_name = ENV.fetch('SW_APM_TRANSACTION_NAME', nil)
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Using transaction name from env var: #{trans_name}" }
         else
-          trans_name = span.attributes[HTTP_ROUTE] || nil
-          trans_name = span.name if trans_name.to_s.empty? && span.name
+          trans_name = span.attributes[HTTP_ROUTE]
+          trans_name = span.name if trans_name.to_s.empty?
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Using transaction name from span.attributes: #{span.attributes[HTTP_ROUTE]} or span.name: #{span.name}" }
         end
         trans_name.to_s.slice(0, SolarWindsAPM::Constants::MAX_TXN_NAME_LENGTH)
       end

lib/solarwinds_apm/opentelemetry/solarwinds_propagator.rb

@@ -40,6 +40,9 @@ def extract(carrier, context: ::OpenTelemetry::Context.current,
           context = ::OpenTelemetry::Context.new({}) if context.nil?
           context = inject_extracted_header(carrier, context, getter, XTRACEOPTIONS_HEADER_NAME, INTL_SWO_X_OPTIONS_KEY)
           inject_extracted_header(carrier, context, getter, XTRACEOPTIONS_SIGNATURE_HEADER_NAME, INTL_SWO_SIGNATURE_KEY)
+        rescue StandardError => e
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Extraction failed: #{e.message}" }
+          context || ::OpenTelemetry::Context.current
         end
 
         # Inject trace context into the supplied carrier.
@@ -80,10 +83,13 @@ def inject(carrier, context: ::OpenTelemetry::Context.current,
             end
             setter.set(carrier, TRACESTATE_HEADER_NAME, Utils.trace_state_header(trace_state))
           end
+        rescue StandardError => e
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Injection failed: #{e.message}" }
         end
 
-        # Returns the predefined propagation fields. If your carrier is reused, you
-        # should delete the fields returned by this method before calling +inject+.
+        # Returns the predefined propagation fields, required by upstream.
+        # If your carrier is reused, you should delete the fields returned by
+        # this method before calling +inject+.
         #
         # @return [Array<String>] a list of fields that will be used by this propagator.
         def fields

lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb

@@ -56,31 +56,20 @@ def inject(carrier, context: ::OpenTelemetry::Context.current,
                        xtraceoptions_response)
           end
           setter.set(carrier, HTTP_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS, exposed_headers.join(','))
-        end
-
-        # Returns the predefined propagation fields. If your carrier is reused, you
-        # should delete the fields returned by this method before calling +inject+.
-        #
-        # @return [Array<String>] a list of fields that will be used by this propagator.
-        def fields
-          TRACESTATE_HEADER_NAME
+        rescue StandardError => e
+          SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Injection failed: #{e.message}" }
         end
 
         private
-
-        # sw_xtraceoptions_response_key -> xtrace_options_response
+        # SW_XTRACEOPTIONS_RESPONSE_KEY -> xtrace_options_response
         def recover_response_from_tracestate(span_context)
           sanitized = span_context.tracestate.value(SW_XTRACEOPTIONS_RESPONSE_KEY)
           sanitized = '' if sanitized.nil?
           sanitized = sanitized.gsub(SolarWindsAPM::Constants::INTL_SWO_EQUALS_W3C_SANITIZED,
                                      SolarWindsAPM::Constants::INTL_SWO_EQUALS)
           sanitized = sanitized.gsub(':', SolarWindsAPM::Constants::INTL_SWO_EQUALS)
-          sanitized = sanitized.gsub(SolarWindsAPM::Constants::INTL_SWO_COMMA_W3C_SANITIZED,
-                                     SolarWindsAPM::Constants::INTL_SWO_COMMA)
-          SolarWindsAPM.logger.debug do
-            "[#{self.class}/#{__method__}] recover_response_from_tracestate sanitized: #{sanitized.inspect}"
-          end
-          sanitized
+          sanitized.gsub(SolarWindsAPM::Constants::INTL_SWO_COMMA_W3C_SANITIZED,
+                         SolarWindsAPM::Constants::INTL_SWO_COMMA)
         end
       end
     end

lib/solarwinds_apm/sampling.rb

@@ -10,7 +10,6 @@
 require 'fileutils'
 require 'tempfile'
 require 'uri'
-require 'opentelemetry-sdk'
 
 require_relative 'sampling/sampling_constants'
 require_relative 'sampling/dice'

lib/solarwinds_apm/sampling/dice.rb

@@ -12,7 +12,7 @@ class Dice
     attr_reader :rate, :scale
 
     def initialize(settings)
-      @scale = settings[:scale]
+      @scale = settings[:scale] || 1_000_000
       @rate = settings[:rate] || 0
       @random_generator = Random.new
     end

lib/solarwinds_apm/sampling/http_sampler.rb

@@ -25,6 +25,7 @@ def initialize(config)
       @pid = nil
       @thread = nil
 
+      @logger.debug { "[#{self.class}/#{__method__}] HttpSampler initialized: url=#{@url}, service=#{@service}, hostname=#{@hostname}, setting_url=#{@setting_url}" }
       reset_on_fork
     end
 
@@ -57,7 +58,6 @@ def hostname
     def fetch_with_timeout(url, timeout_seconds = nil)
       uri = url
       timeout = timeout_seconds || REQUEST_TIMEOUT
-
       deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + timeout
 
       remaining = lambda {
@@ -94,26 +94,33 @@ def fetch_with_timeout(url, timeout_seconds = nil)
 
     # a endless loop within a thread (non-blocking)
     def settings_request
+      @logger.debug { "[#{self.class}/#{__method__}] Starting settings request loop" }
+      sleep_duration = GET_SETTING_DURATION
       loop do
-        @logger.debug { "Retrieving sampling settings from #{@setting_url}" }
-
         response = fetch_with_timeout(@setting_url)
-        parsed = response.nil? ? nil : JSON.parse(response.body)
 
-        @logger.debug { "parsed settings in json: #{parsed.inspect}" }
+        # Check for nil response from timeout
+        unless response.is_a?(Net::HTTPSuccess)
+          @logger.warn { "[#{self.class}/#{__method__}] Failed to retrieve settings due to timeout." }
+          next
+        end
+
+        parsed = JSON.parse(response.body)
 
         if update_settings(parsed)
           # update the settings before the previous ones expire with some time to spare
           expiry = (parsed['timestamp'].to_i + parsed['ttl'].to_i)
           expiry_timeout = expiry - REQUEST_TIMEOUT - Time.now.to_i
-          sleep([0, expiry_timeout].max)
+          sleep_duration = [0, expiry_timeout].max
         else
-          @logger.warn { 'Retrieved sampling settings are invalid. Ensure proper configuration.' }
-          sleep(GET_SETTING_DURATION)
+          @logger.warn { "[#{self.class}/#{__method__}] Retrieved sampling settings are invalid. Ensure proper configuration." }
         end
+      rescue JSON::ParserError => e
+        @logger.warn { "[#{self.class}/#{__method__}] JSON parsing error: #{e.message}" }
       rescue StandardError => e
-        @logger.warn { "Failed to retrieve sampling settings (#{e.message}), tracing will be disabled until valid ones are available." }
-        sleep(GET_SETTING_DURATION)
+        @logger.warn { "[#{self.class}/#{__method__}] Failed to retrieve sampling settings (#{e.message}), tracing will be disabled until valid ones are available." }
+      ensure
+        sleep(sleep_duration)
       end
     end
   end