NH-132896: increase test coverage

.github/instructions/ruby.instructions.md

@@ -236,6 +236,29 @@ end
 - Use Minitest expectations: `assert`, `refute`, `assert_equal`
 - Prefer `assert` and `refute` over `assert_equal true/false`
 
+### Test assertion preferences
+
+- **Always use `assert_equal`** to assert exact expected values. Never use `assert_includes` for partial string matching when the full expected value is deterministic.
+- If `assert_includes` must be used due to non-deterministic output, add a comment explaining why partial matching is necessary (e.g., `# assert_includes used because order is non-deterministic`).
+- Use **`assert_instance_of`** to verify object types instead of `refute_nil` (e.g., `assert_instance_of OpenTelemetry::Context, context` instead of `refute_nil context`).
+- Assert **full object content** (exact string values, complete attribute values) rather than just checking for key existence or non-nil.
+- When code under test logs warnings or errors, **add logger stubbing** to capture and assert the exact log message:
+
+```ruby
+warned = false
+SolarWindsAPM.logger.stub(:warn, ->(_msg = nil, &block) { warned = true if block&.call&.include?('expected message') }) do
+  # code under test
+end
+assert warned, 'Expected warning to be logged'
+```
+
+- Use `assert_equal` consistently instead of mixing `_(x).must_equal y` and `assert_equal y, x` within the same test file. Prefer `assert_equal expected, actual` form.
+- Use `assert_match` with regex when asserting format patterns (e.g., UUID format, hex strings):
+
+```ruby
+assert_match(/^[0-9a-f]{32}$/, result['trace_id'])
+```
+
 Example:
 ```ruby
 describe 'SolarWindsAPM::TokenBucket' do

.github/workflows/run_unit_tests.yml

@@ -70,3 +70,12 @@ jobs:
         export RUN_TESTS=1
         echo "testing with ruby version: $RUBY_VERSION"
         test/test_setup.sh
+
+    - name: Upload coverage to Codecov
+      uses: codecov/codecov-action@v5
+      with:
+        token: ${{ secrets.CODECOV_TOKEN }}
+        files: coverage/coverage.xml
+        flags: ruby-${{ matrix.ruby }}-${{ matrix.os }}
+        fail_ci_if_error: false
+        skip_validation: true

.rubocop.yml

@@ -57,3 +57,5 @@ Style/StringLiterals:
     - 'Gemfile'
 Naming/PredicateMethod:
   Enabled: false
+Style/OneClassPerFile:
+  Enabled: false

Gemfile

@@ -32,6 +32,7 @@ group :development, :test do
   gem 'rubocop-rake', require: false
   gem 'code-scanning-rubocop', '~> 0.6.1'
   gem 'simplecov', require: false, group: :test
+  gem 'simplecov-cobertura', require: false, group: :test
   gem 'simplecov-console', require: false, group: :test
   gem 'webmock' if RUBY_VERSION >= '2.0.0'
   gem 'base64' if RUBY_VERSION >= '3.4.0'

lib/solarwinds_apm/sampling/sampler.rb

@@ -107,8 +107,8 @@ def http_span_metadata(kind, attributes)
 
       method_ = (attributes[ATTR_HTTP_REQUEST_METHOD] || attributes[ATTR_HTTP_METHOD]).to_s
       status = (attributes[ATTR_HTTP_RESPONSE_STATUS_CODE] || attributes[ATTR_HTTP_STATUS_CODE] || 0).to_i
-      scheme = (attributes[ATTR_URL_SCHEME] || attributes[ATTR_HTTP_SCHEME] || 'http').to_s
-      hostname = (attributes[ATTR_SERVER_ADDRESS] || attributes[ATTR_NET_HOST_NAME] || 'localhost').to_s
+      scheme = (attributes[ATTR_URL_SCHEME] || attributes[ATTR_HTTP_SCHEME]).to_s
+      hostname = (attributes[ATTR_SERVER_ADDRESS] || attributes[ATTR_NET_HOST_NAME]).to_s
       path = (attributes[ATTR_URL_PATH] || attributes[ATTR_HTTP_TARGET]).to_s
       url = "#{scheme}://#{hostname}#{path}"
 

lib/solarwinds_apm/support/logger_formatter.rb

@@ -21,7 +21,8 @@ def call(severity, time, progname, msg)
       private
 
       def insert_trace_id(msg)
-        return msg if msg.include?('trace_id=')
+        msg_text = msg.is_a?(::Exception) ? msg.message : msg.to_s
+        return msg if msg_text.include?('trace_id=')
 
         current_trace = SolarWindsAPM::API.current_trace_info
         if current_trace.do_log

test/api/api_test.rb

@@ -0,0 +1,145 @@
+# frozen_string_literal: true
+
+# Copyright (c) 2025 SolarWinds, LLC.
+# All rights reserved.
+
+require 'minitest_helper'
+require_relative '../../lib/solarwinds_apm/config'
+require_relative '../../lib/solarwinds_apm/otel_config'
+require './lib/solarwinds_apm/api'
+
+describe 'API::OpenTelemetry#in_span delegation to OpenTelemetry tracer' do
+  it 'returns nil and warns when block is nil' do
+    warned = false
+    SolarWindsAPM.logger.stub(:warn, ->(_msg = nil, &block) { warned = true if block&.call&.include?('please provide block') }) do
+      result = SolarWindsAPM::API.in_span('test_span')
+      assert_nil result
+    end
+    assert warned, 'Expected a warning to be logged when block is nil'
+  end
+
+  it 'calls in_span with a block and asserts the return value' do
+    OpenTelemetry::SDK.configure
+    result = SolarWindsAPM::API.in_span('test_span') do |span|
+      refute_nil span
+      assert_equal 'test_span', span.name
+      assert span.attributes.empty?
+      assert_equal :internal, span.kind
+      42
+    end
+    assert_equal 42, result
+  end
+
+  it 'passes attributes, kind and other options to in_span' do
+    OpenTelemetry::SDK.configure
+    result = SolarWindsAPM::API.in_span('test_span', attributes: { 'key' => 'value' }, kind: :internal) do |span|
+      refute_nil span
+      assert_equal 'test_span', span.name
+      assert_equal 'value', span.attributes['key']
+      assert_equal :internal, span.kind
+      'done'
+    end
+    assert_equal 'done', result
+  end
+end
+
+describe 'API::CustomMetrics deprecated methods return false' do
+  it 'increment_metric returns false with deprecation' do
+    warned = false
+    SolarWindsAPM.logger.stub(:warn, ->(_msg = nil, &block) { warned = true if block&.call&.include?('increment_metric is deprecated') }) do
+      result = SolarWindsAPM::API.increment_metric('test_metric', 1, false, {})
+      assert_equal false, result
+    end
+    assert warned, 'Expected a deprecation warning to be logged for increment_metric'
+  end
+
+  it 'summary_metric returns false with deprecation' do
+    warned = false
+    SolarWindsAPM.logger.stub(:warn, ->(_msg = nil, &block) { warned = true if block&.call&.include?('summary_metric is deprecated') }) do
+      result = SolarWindsAPM::API.summary_metric('test_metric', 5.0, 1, false, {})
+      assert_equal false, result
+    end
+    assert warned, 'Expected a deprecation warning to be logged for summary_metric'
+  end
+end
+
+describe 'API::Tracer#add_tracer method wrapping with span instrumentation' do
+  let(:sdk) { OpenTelemetry::SDK }
+  let(:exporter) { sdk::Trace::Export::InMemorySpanExporter.new }
+  let(:span_processor) { sdk::Trace::Export::SimpleSpanProcessor.new(exporter) }
+
+  before do
+    ENV['OTEL_SERVICE_NAME'] = __FILE__
+    OpenTelemetry.tracer_provider = sdk::Trace::TracerProvider.new.tap do |provider|
+      provider.add_span_processor(span_processor)
+    end
+  end
+
+  after do
+    ENV.delete('OTEL_SERVICE_NAME')
+  end
+
+  it 'add_tracer wraps an instance method with in_span' do
+    klass = Class.new do
+      include SolarWindsAPM::API::Tracer
+
+      def greeting
+        'hello'
+      end
+      add_tracer :greeting, 'greeting_span'
+    end
+
+    instance = klass.new
+    result = instance.greeting
+    assert_equal 'hello', result
+
+    spans = exporter.finished_spans
+    skip if spans.empty?
+    assert_equal 1, spans.size
+    assert_equal 'greeting_span', spans[0].name
+    assert_equal :internal, spans[0].kind
+  end
+
+  it 'add_tracer uses default span name when nil' do
+    klass = Class.new do
+      include SolarWindsAPM::API::Tracer
+
+      def work
+        'done'
+      end
+      add_tracer :work
+    end
+
+    instance = klass.new
+    result = instance.work
+    assert_equal 'done', result
+
+    spans = exporter.finished_spans
+    skip if spans.empty?
+    assert_equal 1, spans.size
+    assert spans[0].name.end_with?('/add_tracer'), "Expected span name to end with '/add_tracer', got #{spans[0].name}"
+    assert_equal :internal, spans[0].kind
+  end
+
+  it 'add_tracer passes options to in_span' do
+    klass = Class.new do
+      include SolarWindsAPM::API::Tracer
+
+      def compute
+        100
+      end
+      add_tracer :compute, 'compute_span', { attributes: { 'foo' => 'bar' }, kind: :consumer }
+    end
+
+    instance = klass.new
+    result = instance.compute
+    assert_equal 100, result
+
+    spans = exporter.finished_spans
+    skip if spans.empty?
+    assert_equal 1, spans.size
+    assert_equal 'compute_span', spans[0].name
+    assert_equal :consumer, spans[0].kind
+    assert_equal 'bar', spans[0].attributes['foo']
+  end
+end

test/api/current_trace_info_test.rb

@@ -0,0 +1,146 @@
+# frozen_string_literal: true
+
+# Copyright (c) 2025 SolarWinds, LLC.
+# All rights reserved.
+
+require 'minitest_helper'
+require_relative '../../lib/solarwinds_apm/config'
+require_relative '../../lib/solarwinds_apm/otel_config'
+require './lib/solarwinds_apm/api'
+
+describe 'API::CurrentTraceInfo#for_log and #hash_for_log with log_traceId configuration' do
+  describe 'TraceInfo' do
+    it 'returns empty string for_log when log_traceId is :never' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :never
+
+      trace = SolarWindsAPM::API.current_trace_info
+      assert_equal '', trace.for_log
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'returns empty hash for hash_for_log when log_traceId is :never' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :never
+
+      trace = SolarWindsAPM::API.current_trace_info
+      assert_equal({}, trace.hash_for_log)
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'returns trace info for_log when log_traceId is :always' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :always
+
+      trace = SolarWindsAPM::API.current_trace_info
+      result = trace.for_log
+      assert_match(/trace_id=[0-9a-f]{32}/, result)
+      assert_match(/span_id=[0-9a-f]{16}/, result)
+      assert_match(/trace_flags=[0-9a-f]{2}/, result)
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'returns hash for hash_for_log when log_traceId is :always' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :always
+      original_service_name = ENV.fetch('OTEL_SERVICE_NAME', nil)
+      ENV['OTEL_SERVICE_NAME'] = 'test-service-name'
+
+      trace = SolarWindsAPM::API.current_trace_info
+      result = trace.hash_for_log
+      assert_match(/^[0-9a-f]{32}$/, result['trace_id'])
+      assert_match(/^[0-9a-f]{16}$/, result['span_id'])
+      assert_match(/^[0-9a-f]{2}$/, result['trace_flags'])
+      assert_equal 'test-service-name', result['resource.service.name']
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+      ENV['OTEL_SERVICE_NAME'] = original_service_name
+    end
+
+    it 'returns empty for_log when :traced and no active trace' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :traced
+
+      trace = SolarWindsAPM::API.current_trace_info
+      # Without an active trace, trace_id is all zeros, so valid? returns false
+      assert_equal '', trace.for_log
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'returns empty for_log when :sampled and not sampled' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :sampled
+
+      trace = SolarWindsAPM::API.current_trace_info
+      # Without an active sampled trace, should return empty
+      assert_equal '', trace.for_log
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'returns trace info within an active span for :traced' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :traced
+
+      OpenTelemetry::SDK.configure
+      tracer = OpenTelemetry.tracer_provider.tracer('test')
+      tracer.in_span('test_span') do
+        trace = SolarWindsAPM::API.current_trace_info
+        result = trace.for_log
+        assert_match(/trace_id=[0-9a-f]{32}/, result)
+      end
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'returns trace info within a sampled span for :sampled' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :sampled
+
+      OpenTelemetry::SDK.configure
+      tracer = OpenTelemetry.tracer_provider.tracer('test')
+      tracer.in_span('test_span') do
+        trace = SolarWindsAPM::API.current_trace_info
+        result = trace.for_log
+        # The default sampler records & samples, so this should have trace info
+        assert_match(/trace_id=[0-9a-f]{32}/, result)
+      end
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'has boolean do_log attribute' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :always
+
+      trace = SolarWindsAPM::API.current_trace_info
+      assert_equal true, trace.do_log
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+
+    it 'has trace_id, span_id, trace_flags, tracestring attributes' do
+      trace = SolarWindsAPM::API.current_trace_info
+      refute_nil trace.trace_id
+      refute_nil trace.span_id
+      refute_nil trace.trace_flags
+      refute_nil trace.tracestring
+    end
+
+    it 'for_log is memoized' do
+      original = SolarWindsAPM::Config[:log_traceId]
+      SolarWindsAPM::Config[:log_traceId] = :always
+
+      trace = SolarWindsAPM::API.current_trace_info
+      result1 = trace.for_log
+      result2 = trace.for_log
+      assert_same result1, result2
+    ensure
+      SolarWindsAPM::Config[:log_traceId] = original
+    end
+  end
+end

test/api/custom_instrumentation_test.rb

@@ -27,7 +27,7 @@
     ENV.delete('OTEL_SERVICE_NAME')
   end
 
-  it 'test_custom_instrumentation_simple_case' do
+  it 'creates a span with default name and internal kind when add_tracer wraps a method' do
     class MyClass
       include SolarWindsAPM::API::Tracer
 
@@ -52,7 +52,7 @@ def new_method(param1, param2)
     _(finished_spans[0].kind).must_equal(:internal)
   end
 
-  it 'test_custom_instrumentation_simple_case_with_custom_name_and_options' do
+  it 'creates a span with custom name, attributes, and kind when options are provided' do
     class MyClass
       include SolarWindsAPM::API::Tracer
 
@@ -77,7 +77,7 @@ def new_method(param1, param2)
     _(finished_spans[0].kind).must_equal(:consumer)
   end
 
-  it 'test_custom_instrumentation_instance_method' do
+  it 'wraps a class method with add_tracer and applies custom span options' do
     class MyClass
       def self.new_method(param1, param2)
         param1 + param2