Merge pull request #15 from bigcommerce/PEN-2031

Ensure error span tag is always set when an exception is raised

CHANGELOG.md

@@ -2,6 +2,7 @@ Changelog for the gruf-lightstep gem.
 
 ### Pending Release
 
+- Ensure `error` span tag is always set when an exception is raised 
 - Move from whitelist -> allowlist in server interceptor
 
 ### 1.3.0

lib/gruf/lightstep/server_interceptor.rb

@@ -21,6 +21,8 @@ module Lightstep
     # Intercepts inbound calls to provide LightStep tracing
     #
     class ServerInterceptor < Gruf::Interceptors::ServerInterceptor
+      DEFAULT_ERROR_CLASSES = %w[GRPC::Unknown GRPC::Internal GRPC::DataLoss GRPC::FailedPrecondition GRPC::Unavailable GRPC::DeadlineExceeded GRPC::Cancelled].freeze
+
       ##
       # Handle the gruf around hook and trace sampled requests
       #
@@ -44,7 +46,7 @@ def call(&_block)
           begin
             result = yield
           rescue StandardError => e
-            span.set_tag('error', true) if error?(e)
+            span.set_tag('error', error?(e))
             span.set_tag('grpc.error', true)
             span.set_tag('grpc.error_code', code_for(e))
             span.set_tag('grpc.error_class', e.class)
@@ -104,7 +106,7 @@ def error?(exception)
       # @return [Array]
       #
       def error_classes
-        options.fetch(:error_classes, %w[GRPC::Unknown GRPC::Internal GRPC::DataLoss GRPC::FailedPrecondition GRPC::Unavailable GRPC::DeadlineExceeded GRPC::Cancelled])
+        @error_classes ||= (options.fetch(:error_classes, nil) || DEFAULT_ERROR_CLASSES)
       end
     end
   end

spec/gruf/lightstep/server_interceptor_spec.rb

@@ -86,7 +86,7 @@
         it 'should trace the request as a grpc error but not a span error' do
           expect {
             expect(tracer).to receive(:start_span).once.and_yield(span)
-            expect(span).to_not receive(:set_tag).with('error', true)
+            expect(span).to receive(:set_tag).with('error', false)
             expect(span).to receive(:set_tag).with('grpc.error', true).ordered
             expect(span).to receive(:set_tag).with('grpc.error_code', exception.code).ordered
             expect(span).to receive(:set_tag).with('grpc.error_class', exception.class).ordered
@@ -101,7 +101,7 @@
         it 'should trace the request as a non grpc error' do
           expect {
             expect(tracer).to receive(:start_span).once.and_yield(span)
-            expect(span).to_not receive(:set_tag).with('error', true)
+            expect(span).to receive(:set_tag).with('error', false)
             expect(span).to receive(:set_tag).with('grpc.error', true).ordered
             expect(span).to receive(:set_tag).with('grpc.error_code', ::Gruf::Lightstep.default_error_code).ordered
             expect(span).to receive(:set_tag).with('grpc.error_class', exception.class).ordered