From 537aeac62bdb8436e5654ea3e129672116c9f7c4 Mon Sep 17 00:00:00 2001
From: Shaun McCormick <splittingred@gmail.com>
Date: Thu, 29 Oct 2020 15:39:31 -0500
Subject: [PATCH] Ensure error span tag is always set when an exception is
 raised

---
 CHANGELOG.md                                   | 1 +
 lib/gruf/lightstep/server_interceptor.rb       | 6 ++++--
 spec/gruf/lightstep/server_interceptor_spec.rb | 4 ++--
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1ce909f..c1378ba 100644
--- a/CHANGELOG.md
+++ b/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
diff --git a/lib/gruf/lightstep/server_interceptor.rb b/lib/gruf/lightstep/server_interceptor.rb
index 76f363c..87232d2 100644
--- a/lib/gruf/lightstep/server_interceptor.rb
+++ b/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
diff --git a/spec/gruf/lightstep/server_interceptor_spec.rb b/spec/gruf/lightstep/server_interceptor_spec.rb
index 77b9124..a9701dd 100644
--- a/spec/gruf/lightstep/server_interceptor_spec.rb
+++ b/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