From 63743be53a863b3a51a430f9486f1d47fd5ca8c0 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Wed, 18 Mar 2026 14:33:58 -0400 Subject: [PATCH 1/2] minor improvement --- lib/solarwinds_apm.rb | 4 +- lib/solarwinds_apm/api/transaction_name.rb | 8 ++- lib/solarwinds_apm/config.rb | 55 ++++++++++++++----- lib/solarwinds_apm/constants.rb | 3 + .../opentelemetry/otlp_processor.rb | 19 +++---- .../opentelemetry/solarwinds_propagator.rb | 19 ++----- .../solarwinds_response_propagator.rb | 17 ++++-- lib/solarwinds_apm/otel_config.rb | 9 ++- .../patch/tag_sql/sw_dbo_utils.rb | 6 +- lib/solarwinds_apm/sampling/http_sampler.rb | 5 +- lib/solarwinds_apm/sampling/oboe_sampler.rb | 2 +- lib/solarwinds_apm/sampling/sampler.rb | 13 +++-- lib/solarwinds_apm/sampling/settings.rb | 28 ++++++++-- lib/solarwinds_apm/sampling/token_bucket.rb | 5 +- lib/solarwinds_apm/support/otlp_endpoint.rb | 11 ++++ .../support/resource_detector.rb | 5 +- test/api/set_transaction_name_test.rb | 1 + test/opentelemetry/otlp_processor_test.rb | 2 +- .../otel_config_log_bridge_test.rb | 4 ++ .../otel_config_propagator_test.rb | 1 + 20 files changed, 147 insertions(+), 70 deletions(-) diff --git a/lib/solarwinds_apm.rb b/lib/solarwinds_apm.rb index 602c1736..a993a5d4 100644 --- a/lib/solarwinds_apm.rb +++ b/lib/solarwinds_apm.rb @@ -53,6 +53,6 @@ end end rescue StandardError => e - warn "[solarwinds_apm/error] Problem loading: #{e.inspect}" - warn e.backtrace + SolarWindsAPM.logger.error { "[solarwinds_apm/error] Problem loading: #{e.inspect}" } + SolarWindsAPM.logger.error { e.backtrace&.join("\n") } end diff --git a/lib/solarwinds_apm/api/transaction_name.rb b/lib/solarwinds_apm/api/transaction_name.rb index 8860a231..c569d9a4 100644 --- a/lib/solarwinds_apm/api/transaction_name.rb +++ b/lib/solarwinds_apm/api/transaction_name.rb @@ -57,13 +57,15 @@ def set_transaction_name(custom_name = nil) if current_span.context.valid? current_trace_id = current_span.context.hex_trace_id - entry_span_id, trace_flags = solarwinds_processor.txn_manager.get_root_context_h(current_trace_id)&.split('-') - if entry_span_id.to_s.empty? || trace_flags.to_s.empty? + root_context = solarwinds_processor.txn_manager.get_root_context_h(current_trace_id) + parts = root_context&.split('-') + if parts.nil? || parts.length != 2 SolarWindsAPM.logger.warn do - "[#{name}/#{__method__}] Set transaction name failed: record not found in the transaction manager." + "[#{name}/#{__method__}] Set transaction name failed: record not found or malformed in the transaction manager." end status = false else + entry_span_id, _trace_flags = parts solarwinds_processor.txn_manager.set("#{current_trace_id}-#{entry_span_id}", custom_name) SolarWindsAPM.logger.debug do "[#{name}/#{__method__}] Cached custom transaction name for #{current_trace_id}-#{entry_span_id} as #{custom_name}" diff --git a/lib/solarwinds_apm/config.rb b/lib/solarwinds_apm/config.rb index 0738f19f..09b03d5e 100644 --- a/lib/solarwinds_apm/config.rb +++ b/lib/solarwinds_apm/config.rb @@ -26,6 +26,16 @@ module Config 6 => { stdlib: ::Logger::DEBUG, otel: 'debug' } }.freeze @@config = {} + @@config_mutex = ::Mutex.new + + # Type expectations for known configuration keys + CONFIG_TYPE_VALIDATORS = { + transaction_settings: [Array, NilClass], + debug_level: [Integer, NilClass], + tracing_mode: [Symbol, NilClass], + trigger_tracing_mode: [Symbol, NilClass], + tag_sql: [TrueClass, FalseClass, NilClass] + }.freeze ## # load_config_file @@ -49,7 +59,8 @@ def self.load_config_file config_files << config_file if File.exist?(config_file) # Check for file set by env variable - config_files << config_file_from_env if ENV.key?('SW_APM_CONFIG_RUBY') + config_file_env = config_file_from_env if ENV.key?('SW_APM_CONFIG_RUBY') + config_files << config_file_env if config_file_env # Check for default config file config_file = File.join(Dir.pwd, 'solarwinds_apm_config.rb') @@ -68,15 +79,15 @@ def self.load_config_file def self.config_file_from_env if File.exist?(ENV.fetch('SW_APM_CONFIG_RUBY', nil)) && !File.directory?(ENV.fetch('SW_APM_CONFIG_RUBY', nil)) - config_file = ENV.fetch('SW_APM_CONFIG_RUBY', nil) + ENV.fetch('SW_APM_CONFIG_RUBY', nil) elsif File.exist?(File.join(ENV.fetch('SW_APM_CONFIG_RUBY', nil), 'solarwinds_apm_config.rb')) - config_file = File.join(ENV.fetch('SW_APM_CONFIG_RUBY', nil), 'solarwinds_apm_config.rb') + File.join(ENV.fetch('SW_APM_CONFIG_RUBY', nil), 'solarwinds_apm_config.rb') else SolarWindsAPM.logger.warn do "[#{name}/#{__method__}] Could not find the configuration file set by the SW_APM_CONFIG_RUBY environment variable: #{ENV.fetch('SW_APM_CONFIG_RUBY', nil)}" end + nil end - config_file end def self.set_log_level @@ -99,7 +110,7 @@ def self.enable_disable_config(env_var, key, value, default, bool: false) 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 + return @@config_mutex.synchronize { @@config[key] = default } end # Validate final value efficiently @@ -109,10 +120,10 @@ def self.enable_disable_config(env_var, key, value, default, bool: false) SolarWindsAPM.logger.warn do "[#{name}/#{__method__}] :#{key} must be #{valid_env_values.join('/')}. Using default value: #{default}." end - return @@config[key] = default + return @@config_mutex.synchronize { @@config[key] = default } end - @@config[key] = value + @@config_mutex.synchronize { @@config[key] = value } end def self.true?(obj) @@ -134,10 +145,12 @@ def self.symbol?(obj) # to create an output similar to the content of the config file # def self.print_config - SolarWindsAPM.logger.debug { "[#{name}/#{__method__}] General configurations list blow:" } - @@config.each do |k, v| - SolarWindsAPM.logger.debug do - "[#{name}/#{__method__}] Config Key/Value: #{k}, #{v.inspect}" + SolarWindsAPM.logger.debug { "[#{name}/#{__method__}] General configurations listed below:" } + @@config_mutex.synchronize do + @@config.each do |k, v| + SolarWindsAPM.logger.debug do + "[#{name}/#{__method__}] Config Key/Value: #{k}, #{v.inspect}" + end end end nil @@ -151,7 +164,7 @@ def self.print_config # This will be called when require 'solarwinds_apm/config' happen # def self.initialize - @@config[:transaction_name] = {} + @@config_mutex.synchronize { @@config[:transaction_name] = {} } # Always load the template, it has all the keys and defaults defined, # no guarantee of completeness in the user's config file @@ -175,7 +188,7 @@ def self.merge!(data) end def self.[](key) - @@config[key.to_sym] + @@config_mutex.synchronize { @@config[key.to_sym] } end ## @@ -187,7 +200,19 @@ def self.[](key) # def self.[]=(key, value) key = key.to_sym - @@config[key] = value + + # Validate type for known configuration keys + if CONFIG_TYPE_VALIDATORS.key?(key) + allowed = CONFIG_TYPE_VALIDATORS[key] + unless allowed.any? { |type| value.is_a?(type) } + SolarWindsAPM.logger.warn do + "[#{name}/#{__method__}] Invalid type for :#{key}: expected #{allowed.map(&:name).join('/')}, got #{value.class}. Ignoring." + end + return + end + end + + @@config_mutex.synchronize { @@config[key] = value } case key when :sampling_rate @@ -225,7 +250,7 @@ def self.[]=(key, value) SolarWindsAPM.logger.warn { ':log_args is deprecated' } else - @@config[key.to_sym] = value + @@config_mutex.synchronize { @@config[key.to_sym] = value } end end diff --git a/lib/solarwinds_apm/constants.rb b/lib/solarwinds_apm/constants.rb index e3982640..4eb4548c 100644 --- a/lib/solarwinds_apm/constants.rb +++ b/lib/solarwinds_apm/constants.rb @@ -13,6 +13,9 @@ module Constants HTTP_ROUTE = 'http.route' HTTP_STATUS_CODE = 'http.status_code' HTTP_URL = 'http.url' + HTTP_REQUEST_METHOD = 'http.request.method' + HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code' + INVALID_HTTP_STATUS_CODE = 0 INTL_SWO_AO_COLLECTOR = 'collector.appoptics.com' INTL_SWO_AO_STG_COLLECTOR = 'collector-stg.appoptics.com' INTL_SWO_COMMA = ',' diff --git a/lib/solarwinds_apm/opentelemetry/otlp_processor.rb b/lib/solarwinds_apm/opentelemetry/otlp_processor.rb index dded1de0..871f7a66 100644 --- a/lib/solarwinds_apm/opentelemetry/otlp_processor.rb +++ b/lib/solarwinds_apm/opentelemetry/otlp_processor.rb @@ -16,15 +16,15 @@ class OTLPProcessor SW_IS_ENTRY_SPAN = 'sw.is_entry_span' SW_IS_ERROR = 'sw.is_error' - HTTP_METHOD = 'http.method' - HTTP_ROUTE = 'http.route' - HTTP_STATUS_CODE = 'http.status_code' - HTTP_URL = 'http.url' + HTTP_METHOD = SolarWindsAPM::Constants::HTTP_METHOD + HTTP_ROUTE = SolarWindsAPM::Constants::HTTP_ROUTE + HTTP_STATUS_CODE = SolarWindsAPM::Constants::HTTP_STATUS_CODE + HTTP_URL = SolarWindsAPM::Constants::HTTP_URL - HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code' - HTTP_REQUEST_METHOD = 'http.request.method' + HTTP_RESPONSE_STATUS_CODE = SolarWindsAPM::Constants::HTTP_RESPONSE_STATUS_CODE + HTTP_REQUEST_METHOD = SolarWindsAPM::Constants::HTTP_REQUEST_METHOD - INVALID_HTTP_STATUS_CODE = 0 + INVALID_HTTP_STATUS_CODE = SolarWindsAPM::Constants::INVALID_HTTP_STATUS_CODE def initialize(txn_manager) @txn_manager = txn_manager @@ -155,17 +155,16 @@ def calculate_transaction_names(span) def record_request_metrics(span) meter_attrs = meter_attributes(span) span_time = calculate_span_time(start_time: span.start_timestamp, end_time: span.end_timestamp) - span_time = (span_time / 1e3).round SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] entry span, response_time: #{span_time}." } @metrics[:response_time].record(span_time, attributes: meter_attrs) end - # Calculate span time in microseconds (us) using start and end time + # Calculate span time in milliseconds (ms) using start and end time # in nanoseconds (ns). OTel span start/end_time are optional. def calculate_span_time(start_time: nil, end_time: nil) return 0 if start_time.nil? || end_time.nil? - ((end_time.to_i - start_time.to_i) / 1e3).round + ((end_time.to_i - start_time.to_i) / 1_000_000.0).round end # Calculate if this span instance has_error diff --git a/lib/solarwinds_apm/opentelemetry/solarwinds_propagator.rb b/lib/solarwinds_apm/opentelemetry/solarwinds_propagator.rb index 4dd4ab42..6d084d02 100644 --- a/lib/solarwinds_apm/opentelemetry/solarwinds_propagator.rb +++ b/lib/solarwinds_apm/opentelemetry/solarwinds_propagator.rb @@ -35,7 +35,7 @@ class TextMapPropagator # if extraction fails def extract(carrier, context: ::OpenTelemetry::Context.current, getter: ::OpenTelemetry::Context::Propagation.text_map_getter) - SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] extract context: #{context.inspect}" } + SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] extract context: #{context.inspect}" } if SolarWindsAPM.logger.debug? context = ::OpenTelemetry::Context.new({}) if context.nil? context = inject_extracted_header(carrier, context, getter, XTRACEOPTIONS_HEADER_NAME, INTL_SWO_X_OPTIONS_KEY) @@ -55,32 +55,24 @@ def extract(carrier, context: ::OpenTelemetry::Context.current, def inject(carrier, context: ::OpenTelemetry::Context.current, setter: ::OpenTelemetry::Context::Propagation.text_map_setter) span_context = ::OpenTelemetry::Trace.current_span(context)&.context - SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] span_context #{span_context.inspect}" } return unless span_context&.valid? trace_flag = span_context.trace_flags.sampled? ? 1 : 0 sw_value = "#{span_context.hex_span_id}-0#{trace_flag}" - trace_state_header = carrier[TRACESTATE_HEADER_NAME].nil? ? nil : carrier[TRACESTATE_HEADER_NAME] - SolarWindsAPM.logger.debug do - "[#{self.class}/#{__method__}] sw_value: #{sw_value}; trace_state_header: #{trace_state_header}" - end + trace_state_header = carrier[TRACESTATE_HEADER_NAME] + + SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] sw_value: #{sw_value}; trace_state_header: #{trace_state_header}" } if SolarWindsAPM.logger.debug? # prepare carrier with carrier's or new tracestate if trace_state_header.nil? # only create new trace state if valid span_id unless span_context.span_id == ::OpenTelemetry::Trace::INVALID_SPAN_ID trace_state = ::OpenTelemetry::Trace::Tracestate.create({ SolarWindsAPM::Constants::INTL_SWO_TRACESTATE_KEY => sw_value }) - SolarWindsAPM.logger.debug do - "[#{self.class}/#{__method__}] creating new trace state: #{trace_state.inspect}" - end setter.set(carrier, TRACESTATE_HEADER_NAME, Utils.trace_state_header(trace_state)) end else trace_state_from_string = ::OpenTelemetry::Trace::Tracestate.from_string(trace_state_header) trace_state = trace_state_from_string.set_value(SolarWindsAPM::Constants::INTL_SWO_TRACESTATE_KEY, sw_value) - SolarWindsAPM.logger.debug do - "[#{self.class}/#{__method__}] updating/adding trace state for injection #{trace_state.inspect}" - end setter.set(carrier, TRACESTATE_HEADER_NAME, Utils.trace_state_header(trace_state)) end rescue StandardError => e @@ -93,7 +85,7 @@ def inject(carrier, context: ::OpenTelemetry::Context.current, # # @return [Array] a list of fields that will be used by this propagator. def fields - TRACESTATE_HEADER_NAME + [TRACESTATE_HEADER_NAME] end private @@ -101,7 +93,6 @@ def fields def inject_extracted_header(carrier, context, getter, header, inject_key) extracted_header = getter.get(carrier, header) context = context.set_value(inject_key, extracted_header) if extracted_header - SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] #{header}: #{inject_key} = #{extracted_header}" } context end end diff --git a/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb b/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb index 4eeff61f..6c103269 100644 --- a/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb +++ b/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb @@ -62,15 +62,20 @@ def inject(carrier, context: ::OpenTelemetry::Context.current, private + W3C_UNSANITIZE_MAP = { + SolarWindsAPM::Constants::INTL_SWO_EQUALS_W3C_SANITIZED => SolarWindsAPM::Constants::INTL_SWO_EQUALS, + ':' => SolarWindsAPM::Constants::INTL_SWO_EQUALS, + SolarWindsAPM::Constants::INTL_SWO_COMMA_W3C_SANITIZED => SolarWindsAPM::Constants::INTL_SWO_COMMA + }.freeze + + W3C_UNSANITIZE_PATTERN = Regexp.union(W3C_UNSANITIZE_MAP.keys).freeze + # 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.gsub(SolarWindsAPM::Constants::INTL_SWO_COMMA_W3C_SANITIZED, - SolarWindsAPM::Constants::INTL_SWO_COMMA) + return '' if sanitized.nil? + + sanitized.gsub(W3C_UNSANITIZE_PATTERN, W3C_UNSANITIZE_MAP) end end end diff --git a/lib/solarwinds_apm/otel_config.rb b/lib/solarwinds_apm/otel_config.rb index 388897a1..160c5f4a 100644 --- a/lib/solarwinds_apm/otel_config.rb +++ b/lib/solarwinds_apm/otel_config.rb @@ -17,12 +17,18 @@ module OTelConfig @@config = {} @@config_map = {} @@agent_enabled = false + @@initialized = false RESOURCE_ATTRIBUTES = 'RESOURCE_ATTRIBUTES' def self.initialize return unless defined?(::OpenTelemetry::SDK::Configurator) + if @@initialized + SolarWindsAPM.logger.warn { "[#{name}/#{__method__}] OTelConfig already initialized. Skipping re-initialization." } + return + end + is_lambda = SolarWindsAPM::Utils.determine_lambda # add response propagator to rack instrumentation @@ -39,7 +45,7 @@ def self.initialize return if otlp_endpoint.token.nil? end - ENV['OTEL_RESOURCE_ATTRIBUTES'] = "sw.apm.version=#{SolarWindsAPM::Version::STRING},sw.data.module=apm,service.name=#{ENV.fetch('OTEL_SERVICE_NAME', nil)}," + ENV['OTEL_RESOURCE_ATTRIBUTES'].to_s + ENV['OTEL_RESOURCE_ATTRIBUTES'] = "sw.apm.version=#{SolarWindsAPM::Version::STRING},sw.data.module=apm,service.name=#{ENV.fetch('OTEL_SERVICE_NAME', nil)}," + ENV['OTEL_RESOURCE_ATTRIBUTES'].to_s unless ENV['OTEL_RESOURCE_ATTRIBUTES'].to_s.include?('sw.apm.version=') # resource attributes mandatory_resource = SolarWindsAPM::ResourceDetector.detect @@ -114,6 +120,7 @@ def self.initialize ) @@agent_enabled = true + @@initialized = true nil end diff --git a/lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils.rb b/lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils.rb index 7ac8332d..730a530b 100644 --- a/lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils.rb +++ b/lib/solarwinds_apm/patch/tag_sql/sw_dbo_utils.rb @@ -1,8 +1,10 @@ # frozen_string_literal: true -# Copyright The OpenTelemetry Authors +# © 2023 SolarWinds Worldwide, LLC. All rights reserved. # -# SPDX-License-Identifier: Apache-2.0 +# Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at:http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. module SolarWindsAPM module Patch diff --git a/lib/solarwinds_apm/sampling/http_sampler.rb b/lib/solarwinds_apm/sampling/http_sampler.rb index fe0488d6..5d6568b1 100644 --- a/lib/solarwinds_apm/sampling/http_sampler.rb +++ b/lib/solarwinds_apm/sampling/http_sampler.rb @@ -99,9 +99,10 @@ def settings_request loop do response = fetch_with_timeout(@setting_url) - # Check for nil response from timeout + # Check for nil response from timeout or non-success response unless response.is_a?(Net::HTTPSuccess) - @logger.warn { "[#{self.class}/#{__method__}] Failed to retrieve settings due to timeout." } + @logger.warn { "[#{self.class}/#{__method__}] Failed to retrieve settings: #{response.nil? ? 'timeout' : "HTTP #{response.code}"}" } + sleep(sleep_duration) next end diff --git a/lib/solarwinds_apm/sampling/oboe_sampler.rb b/lib/solarwinds_apm/sampling/oboe_sampler.rb index b4b30311..cfe1e3ba 100644 --- a/lib/solarwinds_apm/sampling/oboe_sampler.rb +++ b/lib/solarwinds_apm/sampling/oboe_sampler.rb @@ -29,7 +29,7 @@ def initialize(logger) @counters = SolarWindsAPM::Metrics::Counter.new @buckets = { SolarWindsAPM::BucketType::DEFAULT => - SolarWindsAPM::TokenBucket.new(SolarWindsAPM::TokenBucketSettings.new(nil, nil, 'DEFUALT')), + SolarWindsAPM::TokenBucket.new(SolarWindsAPM::TokenBucketSettings.new(nil, nil, 'DEFAULT')), SolarWindsAPM::BucketType::TRIGGER_RELAXED => SolarWindsAPM::TokenBucket.new(SolarWindsAPM::TokenBucketSettings.new(nil, nil, 'TRIGGER_RELAXED')), SolarWindsAPM::BucketType::TRIGGER_STRICT => diff --git a/lib/solarwinds_apm/sampling/sampler.rb b/lib/solarwinds_apm/sampling/sampler.rb index 95ddb069..6a42b05c 100644 --- a/lib/solarwinds_apm/sampling/sampler.rb +++ b/lib/solarwinds_apm/sampling/sampler.rb @@ -10,10 +10,10 @@ module SolarWindsAPM class Sampler < OboeSampler RUBY_SEM_CON = ::OpenTelemetry::SemanticConventions::Trace - ATTR_HTTP_REQUEST_METHOD = 'http.request.method' - ATTR_HTTP_METHOD = RUBY_SEM_CON::HTTP_METHOD - ATTR_HTTP_RESPONSE_STATUS_CODE = 'http.response.status_code' - ATTR_HTTP_STATUS_CODE = RUBY_SEM_CON::HTTP_STATUS_CODE + ATTR_HTTP_REQUEST_METHOD = SolarWindsAPM::Constants::HTTP_REQUEST_METHOD + ATTR_HTTP_METHOD = SolarWindsAPM::Constants::HTTP_METHOD + ATTR_HTTP_RESPONSE_STATUS_CODE = SolarWindsAPM::Constants::HTTP_RESPONSE_STATUS_CODE + ATTR_HTTP_STATUS_CODE = SolarWindsAPM::Constants::HTTP_STATUS_CODE ATTR_URL_SCHEME = 'url.scheme' ATTR_HTTP_SCHEME = RUBY_SEM_CON::HTTP_SCHEME ATTR_SERVER_ADDRESS = 'server.address' @@ -38,8 +38,9 @@ def wait_until_ready(timeout = 10) deadline = Time.now + timeout while Time.now < deadline # The @settings hash is populated by another thread (e.g., HttpSampler) - unless @settings.empty? - @ready = !@settings[:signature_key].nil? + settings_snapshot = @settings_mutex.synchronize { @settings.dup } + unless settings_snapshot.empty? + @ready = !settings_snapshot[:signature_key].nil? return @ready end sleep 0.1 diff --git a/lib/solarwinds_apm/sampling/settings.rb b/lib/solarwinds_apm/sampling/settings.rb index 5b54bcc6..789f2517 100644 --- a/lib/solarwinds_apm/sampling/settings.rb +++ b/lib/solarwinds_apm/sampling/settings.rb @@ -8,19 +8,39 @@ module SolarWindsAPM module SamplingSettings + # Merges remote (server-side) and local (config-based) sampling settings. + # + # Precedence algorithm: + # 1. Start with local tracing_mode flags if set, otherwise use remote flags as baseline. + # 2. Apply local trigger_mode: set or clear the TRIGGERED_TRACE bit accordingly. + # 3. If the remote OVERRIDE bit is set, the remote flags take precedence: + # AND remote flags into the result (masking out locally-enabled bits the server disallows), + # then re-set the OVERRIDE bit to preserve it in the final result. + # + # Bit layout (see SolarWindsAPM::Flags): + # SAMPLE_START - enables dice-roll sampling + # SAMPLE_THROUGH_ALWAYS - enables parent-based pass-through + # TRIGGERED_TRACE - enables trigger tracing + # OVERRIDE - server override; remote flags mask local flags def self.merge(remote, local) SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] SamplingSettings merge with remote: #{remote.inspect}; local: #{local.inspect}" } + + # Step 1: Use local tracing_mode as baseline flags, fall back to remote flags flags = local[:tracing_mode] || remote[:flags] + # Step 2: Apply local trigger_mode by setting or clearing the TRIGGERED_TRACE bit if local[:trigger_mode] == :enabled - flags |= SolarWindsAPM::Flags::TRIGGERED_TRACE + flags |= SolarWindsAPM::Flags::TRIGGERED_TRACE # set the trigger trace bit elsif local[:trigger_mode] == :disabled - flags &= ~ SolarWindsAPM::Flags::TRIGGERED_TRACE + flags &= ~SolarWindsAPM::Flags::TRIGGERED_TRACE # clear the trigger trace bit end + # Step 3: If remote has OVERRIDE set, remote flags take precedence. + # AND with remote flags to mask out any locally-enabled bits the server disallows, + # then re-set OVERRIDE so downstream code knows the override was applied. if remote[:flags].anybits?(SolarWindsAPM::Flags::OVERRIDE) - flags &= remote[:flags] - flags |= SolarWindsAPM::Flags::OVERRIDE + flags &= remote[:flags] # mask: only keep bits allowed by remote + flags |= SolarWindsAPM::Flags::OVERRIDE # preserve the override indicator end SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] final flags: #{flags}" } diff --git a/lib/solarwinds_apm/sampling/token_bucket.rb b/lib/solarwinds_apm/sampling/token_bucket.rb index 9a87027f..214cfd47 100644 --- a/lib/solarwinds_apm/sampling/token_bucket.rb +++ b/lib/solarwinds_apm/sampling/token_bucket.rb @@ -64,8 +64,9 @@ def calculate_tokens now = Time.now.to_f elapsed = now - @last_update_time @last_update_time = now - @tokens += elapsed * @rate - @tokens = [@tokens, @capacity].min + # Recompute tokens from elapsed time rather than accumulating + # to avoid floating-point drift over many calls + @tokens = [@tokens + (elapsed * @rate), @capacity].min end # settings is from json sampler diff --git a/lib/solarwinds_apm/support/otlp_endpoint.rb b/lib/solarwinds_apm/support/otlp_endpoint.rb index 124d4e84..f0c1a3ae 100644 --- a/lib/solarwinds_apm/support/otlp_endpoint.rb +++ b/lib/solarwinds_apm/support/otlp_endpoint.rb @@ -27,6 +27,13 @@ def initialize @service_name = nil end + # Configures OTLP token and endpoint ENV variables for all signal types. + # This method sets the following ENV variables when not already set by the user: + # - OTEL_EXPORTER_OTLP_{TRACES,METRICS,LOGS}_HEADERS (bearer token) + # - OTEL_EXPORTER_OTLP_HEADERS (fallback bearer token) + # - OTEL_EXPORTER_OTLP_{TRACES,METRICS,LOGS}_ENDPOINT (SWO OTLP endpoint) + # - SW_APM_COLLECTOR (APM collector endpoint) + # User-set ENV values are never overwritten. def config_otlp_token_and_endpoint matches = ENV['SW_APM_COLLECTOR'].to_s.match(SWO_APM_ENDPOINT_REGEX) @@ -45,6 +52,8 @@ def config_otlp_token_and_endpoint # APM Libraries should only set the bearer token header as a convenience if: # The OTEL config for exporter OTLP headers is not already set, i.e. explicitly configured by the end user, AND # The OTLP export endpoint is SWO, i.e. host is otel.collector.*.*.solarwinds.com + # Sets bearer token headers for the given signal type. + # Only sets ENV if the user has not already configured headers and the endpoint is SWO. def config_token(data_type) data_type_upper = data_type.upcase @@ -59,6 +68,8 @@ def config_token(data_type) ENV['OTEL_EXPORTER_OTLP_HEADERS'] = "authorization=Bearer #{@token}" if ENV['OTEL_EXPORTER_OTLP_HEADERS'].to_s.empty? && ENV["OTEL_EXPORTER_OTLP_#{data_type_upper}_HEADERS"].to_s.empty? end + # Sets the OTLP endpoint for the given signal type. + # Only sets ENV if neither signal-specific nor general endpoint is configured by the user. def configure_otlp_endpoint(data_type, matches) data_type_upper = data_type.upcase data_type_lower = data_type.downcase diff --git a/lib/solarwinds_apm/support/resource_detector.rb b/lib/solarwinds_apm/support/resource_detector.rb index dc1f30a4..adccbe8b 100644 --- a/lib/solarwinds_apm/support/resource_detector.rb +++ b/lib/solarwinds_apm/support/resource_detector.rb @@ -64,7 +64,10 @@ def self.detect_uams_client_id response = nil ::OpenTelemetry::Common::Utilities.untraced do - response = Net::HTTP.get_response(url) + http = Net::HTTP.new(url.host, url.port) + http.open_timeout = 2 + http.read_timeout = 2 + response = http.get(url.request_uri) end raise 'Response returned non-200 status code' unless response&.code.to_i == 200 diff --git a/test/api/set_transaction_name_test.rb b/test/api/set_transaction_name_test.rb index f45e7648..30861537 100644 --- a/test/api/set_transaction_name_test.rb +++ b/test/api/set_transaction_name_test.rb @@ -14,6 +14,7 @@ @span = create_span @dummy_span = create_span @dummy_span.context.instance_variable_set(:@span_id, 'fake_span_id') # with fake span_id, should still find the right root span + SolarWindsAPM::OTelConfig.class_variable_set(:@@initialized, false) SolarWindsAPM::OTelConfig.initialize @solarwinds_processor = SolarWindsAPM::OTelConfig[:metrics_processor] end diff --git a/test/opentelemetry/otlp_processor_test.rb b/test/opentelemetry/otlp_processor_test.rb index 078d4212..0c580a4d 100644 --- a/test/opentelemetry/otlp_processor_test.rb +++ b/test/opentelemetry/otlp_processor_test.rb @@ -32,7 +32,7 @@ result = @processor.send(:calculate_span_time, start_time: span_data.start_timestamp, end_time: span_data.end_timestamp) - _(result).must_equal 44_853 + _(result).must_equal 45 result = @processor.send(:calculate_span_time, start_time: span_data.start_timestamp, end_time: nil) _(result).must_equal 0 diff --git a/test/solarwinds_apm/otel_config_log_bridge_test.rb b/test/solarwinds_apm/otel_config_log_bridge_test.rb index a1abaf26..49b75804 100644 --- a/test/solarwinds_apm/otel_config_log_bridge_test.rb +++ b/test/solarwinds_apm/otel_config_log_bridge_test.rb @@ -11,6 +11,10 @@ describe 'Log Bridge Initialization Test' do describe 'check if log bridge is enabled' do + before do + SolarWindsAPM::OTelConfig.class_variable_set(:@@initialized, false) + end + after do ENV.delete('OTEL_RUBY_INSTRUMENTATION_LOGGER_ENABLED') end diff --git a/test/solarwinds_apm/otel_config_propagator_test.rb b/test/solarwinds_apm/otel_config_propagator_test.rb index b035ec06..530b6e51 100644 --- a/test/solarwinds_apm/otel_config_propagator_test.rb +++ b/test/solarwinds_apm/otel_config_propagator_test.rb @@ -15,6 +15,7 @@ SolarWindsAPM::OTelConfig.class_variable_set(:@@agent_enabled, true) SolarWindsAPM::OTelConfig.class_variable_set(:@@config, {}) SolarWindsAPM::OTelConfig.class_variable_set(:@@config_map, {}) + SolarWindsAPM::OTelConfig.class_variable_set(:@@initialized, false) end # propagation in_code testing From e89f30605b78e0bef4a25f57ec75959d6554c1d2 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Wed, 18 Mar 2026 15:02:20 -0400 Subject: [PATCH 2/2] lint --- .rubocop.yml | 2 ++ .../opentelemetry/solarwinds_response_propagator.rb | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index e6e525d4..54651a73 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -57,3 +57,5 @@ Style/StringLiterals: - 'Gemfile' Naming/PredicateMethod: Enabled: false +Style/OneClassPerFile: + Enabled: false diff --git a/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb b/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb index 6c103269..87a2ec9d 100644 --- a/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb +++ b/lib/solarwinds_apm/opentelemetry/solarwinds_response_propagator.rb @@ -60,8 +60,6 @@ def inject(carrier, context: ::OpenTelemetry::Context.current, SolarWindsAPM.logger.debug { "[#{self.class}/#{__method__}] Injection failed: #{e.message}" } end - private - W3C_UNSANITIZE_MAP = { SolarWindsAPM::Constants::INTL_SWO_EQUALS_W3C_SANITIZED => SolarWindsAPM::Constants::INTL_SWO_EQUALS, ':' => SolarWindsAPM::Constants::INTL_SWO_EQUALS,