From 200ea027206e85074320976cc9d98a3d48eac607 Mon Sep 17 00:00:00 2001
From: Francis Bogsanyi <francis.bogsanyi@shopify.com>
Date: Thu, 25 Jan 2024 11:59:34 -0500
Subject: [PATCH] feat: add spans to Trace::ExportError (#1582)

* feat: add spans to Trace::ExportError

* add class comment

* document initialization argument

* Apply suggestions from code review

---------

Co-authored-by: Sam Handler <sam.handler@shopify.com>
---
 sdk/lib/opentelemetry/sdk/trace/export.rb     | 14 ++++++++++-
 .../sdk/trace/export/batch_span_processor.rb  | 23 ++++++++-----------
 2 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/sdk/lib/opentelemetry/sdk/trace/export.rb b/sdk/lib/opentelemetry/sdk/trace/export.rb
index 16700c64b8..3089a1ec0c 100644
--- a/sdk/lib/opentelemetry/sdk/trace/export.rb
+++ b/sdk/lib/opentelemetry/sdk/trace/export.rb
@@ -10,7 +10,19 @@ module Trace
       # The Export module contains the built-in exporters and span processors for the OpenTelemetry
       # reference implementation.
       module Export
-        ExportError = Class.new(OpenTelemetry::Error)
+        # Raised when an export fails; spans are available via :spans accessor
+        class ExportError < OpenTelemetry::Error
+          # Returns the {Span} array for this exception
+          #
+          # @return [Array<OpenTelemetry::SDK::Trace::Span>]
+          attr_reader :spans
+
+          # @param [Array<OpenTelemetry::SDK::Trace::Span>] spans the array of spans that failed to export
+          def initialize(spans)
+            super("Unable to export #{spans.size} spans")
+            @spans = spans
+          end
+        end
 
         # Result codes for the SpanExporter#export method and the SpanProcessor#force_flush and SpanProcessor#shutdown methods.
 
diff --git a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb
index 694c1e4d14..5e11b51356 100644
--- a/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb
+++ b/sdk/lib/opentelemetry/sdk/trace/export/batch_span_processor.rb
@@ -110,7 +110,7 @@ def force_flush(timeout: nil) # rubocop:disable Metrics/MethodLength
               remaining_timeout = OpenTelemetry::Common::Utilities.maybe_timeout(timeout, start_time)
               return TIMEOUT if remaining_timeout&.zero?
 
-              batch = snapshot.shift(@batch_size).map!(&:to_span_data)
+              batch = snapshot.shift(@batch_size)
               result_code = export_batch(batch, timeout: remaining_timeout)
               return result_code unless result_code == SUCCESS
             end
@@ -162,7 +162,7 @@ def work
                 @condition.wait(@mutex, @delay_seconds) while spans.empty? && @keep_running
                 return unless @keep_running
 
-                fetch_batch
+                spans.shift(@batch_size)
               end
 
               @metrics_reporter.observe_value('otel.bsp.buffer_utilization', value: spans.size / max_queue_size.to_f)
@@ -183,24 +183,25 @@ def reset_on_fork(restart_thread: true)
             OpenTelemetry.handle_error(exception: e, message: 'unexpected error in BatchSpanProcessor#reset_on_fork')
           end
 
-          def export_batch(batch, timeout: @exporter_timeout_seconds)
+          def export_batch(span_array, timeout: @exporter_timeout_seconds)
+            batch = span_array.map(&:to_span_data)
             result_code = @export_mutex.synchronize { @exporter.export(batch, timeout: timeout) }
-            report_result(result_code, batch)
+            report_result(result_code, span_array)
             result_code
           rescue StandardError => e
-            report_result(FAILURE, batch)
+            report_result(FAILURE, span_array)
             @metrics_reporter.add_to_counter('otel.bsp.error', labels: { 'reason' => e.class.to_s })
             FAILURE
           end
 
-          def report_result(result_code, batch)
+          def report_result(result_code, span_array)
             if result_code == SUCCESS
               @metrics_reporter.add_to_counter('otel.bsp.export.success')
-              @metrics_reporter.add_to_counter('otel.bsp.exported_spans', increment: batch.size)
+              @metrics_reporter.add_to_counter('otel.bsp.exported_spans', increment: span_array.size)
             else
-              OpenTelemetry.handle_error(exception: ExportError.new("Unable to export #{batch.size} spans"))
+              OpenTelemetry.handle_error(exception: ExportError.new(span_array))
               @metrics_reporter.add_to_counter('otel.bsp.export.failure')
-              report_dropped_spans(batch, reason: 'export-failure')
+              report_dropped_spans(span_array, reason: 'export-failure')
             end
           end
 
@@ -208,10 +209,6 @@ def report_dropped_spans(dropped_spans, reason:, function: nil)
             @metrics_reporter.add_to_counter('otel.bsp.dropped_spans', increment: dropped_spans.size, labels: { 'reason' => reason, OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => function }.compact)
           end
 
-          def fetch_batch
-            spans.shift(@batch_size).map!(&:to_span_data)
-          end
-
           def lock
             @mutex.synchronize do
               yield