squash: added more tests... woof

open-telemetry · Oct 9, 2023 · e988093 · e988093
1 parent 3a615ee
commit e988093
Show file tree

Hide file tree

Showing 3 changed files with 21 additions and 19 deletions.
diff --git a/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/subscriber.rb b/instrumentation/active_job/lib/opentelemetry/instrumentation/active_job/subscriber.rb
@@ -56,11 +56,7 @@ def finish(_name, _id, payload)
           tokens = otel&.fetch(:ctx_tokens)
 
           exception = payload[:error] || payload[:exception_object]
-          if exception
-            on_exception(exception, span)
-          else
-            span&.status = OpenTelemetry::Trace::Status.ok
-          end
+          on_exception(exception, span) if exception
         rescue StandardError => e
           OpenTelemetry.handle_error(exception: e)
         ensure
@@ -70,7 +66,7 @@ def finish(_name, _id, payload)
         def finish_span(span, tokens)
           # closes the span after all attributes have been finalized
           begin
-            # Relies on span status being immutable
+            span&.status = OpenTelemetry::Trace::Status.ok if span&.status&.code == OpenTelemetry::Trace::Status::UNSET
             span&.finish
           rescue StandardError => e
             OpenTelemetry.handle_error(exception: e)
@@ -144,9 +140,9 @@ def on_start(name, _id, payload)
         # @return [Array] Context tokens that must be detached when finished
         def attach_consumer_context(span)
           consumer_context = OpenTelemetry::Trace.context_with_span(span)
-          aj_context = OpenTelemetry::Instrumentation::ActiveJob.context_with_span(span, parent_context: consumer_context)
+          internal_context = OpenTelemetry::Instrumentation::ActiveJob.context_with_span(span, parent_context: consumer_context)
 
-          [consumer_context, aj_context].map { |context| OpenTelemetry::Context.attach(context) }
+          [consumer_context, internal_context].map { |context| OpenTelemetry::Context.attach(context) }
         end
 
         # TODO: refactor into a strategy

diff --git a/instrumentation/active_job/test/opentelemetry/instrumentation/active_job/subscriber_test.rb b/instrumentation/active_job/test/opentelemetry/instrumentation/active_job/subscriber_test.rb
@@ -19,6 +19,7 @@
   let(:publish_span) { spans.find { |s| s.name == 'default publish' } }
   let(:process_span) { spans.find { |s| s.name == 'default process' } }
   let(:discard_span) { spans.find { |s| s.name == 'discard.active_job' } }
+  let(:retry_span) { spans.find { |s| s.name == 'retry_stopped.active_job' } }
 
   before do
     OpenTelemetry::Instrumentation::ActiveJob::Subscriber.uninstall
@@ -231,27 +232,32 @@
 
       it 'tracks correctly for jobs that do retry in Rails 6 or earlier' do
         skip "ActiveJob #{ActiveJob.version} starts at 0 in newer versions" if ActiveJob.version >= Gem::Version.new('7')
-        begin
-          RetryJob.perform_later
-        rescue StandardError
-          nil
-        end
+        _ { RetryJob.perform_later }.must_raise StandardError
 
         executions = spans.filter { |s| s.kind == :consumer }.map { |s| s.attributes['rails.active_job.execution.counter'] }.compact.max
         _(executions).must_equal(2) # total of 3 runs. The initial and 2 retries.
       end
 
       it 'tracks correctly for jobs that do retry in Rails 7 or later' do
         skip "ActiveJob #{ActiveJob.version} starts at 1 in older versions" if ActiveJob.version < Gem::Version.new('7')
-        begin
-          RetryJob.perform_later
-        rescue StandardError
-          nil
-        end
+        _ { RetryJob.perform_later }.must_raise StandardError
 
         executions = spans.filter { |s| s.kind == :consumer }.map { |s| s.attributes['rails.active_job.execution.counter'] }.compact.max
         _(executions).must_equal(1)
       end
+
+      it 'records retry errors' do
+        _ { RetryJob.perform_later }.must_raise StandardError
+
+        _(process_span.status.code).must_equal OpenTelemetry::Trace::Status::ERROR
+        _(process_span.status.description).must_equal 'from retry job'
+
+        _(retry_span.status.code).must_equal OpenTelemetry::Trace::Status::ERROR
+        _(retry_span.status.description).must_equal 'from retry job'
+        _(retry_span.events.first.name).must_equal 'exception'
+        _(retry_span.events.first.attributes['exception.type']).must_equal 'StandardError'
+        _(retry_span.events.first.attributes['exception.message']).must_equal 'from retry job'
+      end
     end
 
     describe 'active_job.provider_job_id' do

diff --git a/instrumentation/active_job/test/test_helper.rb b/instrumentation/active_job/test/test_helper.rb
@@ -20,7 +20,7 @@ class RetryJob < ActiveJob::Base
   retry_on StandardError, wait: 0, attempts: 2
 
   def perform
-    raise StandardError
+    raise StandardError, 'from retry job'
   end
 end