Merge pull request #3454 from newrelic/otel_bridge_bugfix_unfinished_segment

Finish OTel span before forcing finish

Author: hannahramadan

lib/new_relic/agent/opentelemetry/abstract_segment_patch.rb

@@ -0,0 +1,28 @@
+# 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 OpenTelemetry
+      module AbstractSegmentPatch
+        def force_finish
+          if instance_variable_defined?(:@otel_span)
+            otel_span = instance_variable_get(:@otel_span)
+            if otel_span.respond_to?(:finish) && !otel_span.instance_variable_get(:@finished)
+              begin
+                otel_span.finish
+
+                return if finished?
+              rescue => e
+                NewRelic::Agent.logger.debug("Error finishing OpenTelemetry span during force_finish: #{e}")
+              end
+            end
+          end
+
+          super
+        end
+      end
+    end
+  end
+end

lib/new_relic/agent/opentelemetry_bridge.rb

@@ -69,11 +69,13 @@ def self.install
         require_relative 'opentelemetry/transaction_patch'
         require_relative 'opentelemetry/context'
         require_relative 'opentelemetry/trace_patch'
+        require_relative 'opentelemetry/abstract_segment_patch'
 
         NewRelic::Agent.logger.warn('OpenTelemetry SDK gem is installed. This may interfere with New Relic instrumentation.') if defined?(OpenTelemetry::SDK)
 
         ::OpenTelemetry::Trace.singleton_class.prepend(NewRelic::Agent::OpenTelemetry::TracePatch)
         Transaction.prepend(OpenTelemetry::TransactionPatch)
+        Transaction::AbstractSegment.prepend(OpenTelemetry::AbstractSegmentPatch)
 
         ::OpenTelemetry.tracer_provider = OpenTelemetry::Trace::TracerProvider.new
         ::OpenTelemetry.propagation = OpenTelemetry::Context::Propagation::TracePropagator.new

test/multiverse/suites/hybrid_agent/abstract_segment_patch_test.rb

@@ -0,0 +1,69 @@
+# 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 OpenTelemetry
+      class AbstractSegmentPatchTest < Minitest::Test
+        def setup
+          harvest_transaction_events!
+          harvest_span_events!
+        end
+
+        def test_force_finish_with_otel_span_that_cannot_finish_segment
+          in_transaction do |txn|
+            txn.stubs(:sampled?).returns(true)
+            segment = Tracer.start_segment(name: 'test_segment')
+            otel_span = segment.instance_variable_get(:@otel_span)
+
+            otel_span.stubs(:instance_variable_get).with(:@finished).returns(false)
+            otel_span.stubs(:finish).returns(nil)
+
+            segment.force_finish
+
+            assert_predicate segment, :finished?
+          end
+        end
+
+        def test_force_finish_with_successful_otel_span_finish
+          in_transaction do |txn|
+            txn.stubs(:sampled?).returns(true)
+            segment = Tracer.start_segment(name: 'test_segment')
+            otel_span = segment.instance_variable_get(:@otel_span)
+
+            otel_span.stubs(:instance_variable_get).with(:@finished).returns(false)
+            otel_span.stubs(:finish) do
+              segment.finish
+            end
+
+            refute_predicate segment, :finished?, 'Segment should start unfinished'
+
+            segment.force_finish
+
+            assert_predicate segment, :finished?, 'Segment should be finished by span.finish'
+          end
+        end
+
+        def test_force_finish_handles_otel_span_exceptions_gracefully
+          in_transaction do |txn|
+            txn.stubs(:sampled?).returns(true)
+            segment = Tracer.start_segment(name: 'test_segment')
+            otel_span = segment.instance_variable_get(:@otel_span)
+
+            otel_span.stubs(:instance_variable_get).with(:@finished).returns(false)
+            otel_span.stubs(:finish).raises(StandardError.new('Test exception'))
+
+            logger_mock = mock()
+            logger_mock.expects(:debug).with(regexp_matches(/Error finishing OpenTelemetry span during force_finish.*Test exception/))
+            NewRelic::Agent.stubs(:logger).returns(logger_mock)
+
+            segment.force_finish
+
+            assert_predicate segment, :finished?, 'Segment should still be finished via fallback after exception'
+          end
+        end
+      end
+    end
+  end
+end