fix: update server instrumentation to not reflect 400 status as error (…

…#1076) * chore: update server instrumentation to not reflect 400 status as error * chore: fix typo in tests
open-telemetry · Jan 6, 2022 · a8a596f · a8a596f
1 parent 0a1d79c
commit a8a596f
Show file tree

Hide file tree

Showing 4 changed files with 15 additions and 3 deletions.
diff --git a/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb b/instrumentation/rack/lib/opentelemetry/instrumentation/rack/middlewares/tracer_middleware.rb
@@ -151,7 +151,7 @@ def create_request_span_name(request_uri_or_path_info, env)
           end
 
           def set_attributes_after_request(span, status, headers, _response)
-            span.status = OpenTelemetry::Trace::Status.error unless (100..399).include?(status.to_i)
+            span.status = OpenTelemetry::Trace::Status.error unless (100..499).include?(status.to_i)
             span.set_attribute('http.status_code', status)
 
             # NOTE: if data is available, it would be good to do this:

diff --git a/...tation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_test.rb b/...tation/rack/test/opentelemetry/instrumentation/rack/middlewares/tracer_middleware_test.rb
@@ -259,6 +259,18 @@
         end
       end
     end
+
+    describe '#called with 400 level http status code' do
+      let(:app) do
+        ->(_env) { [404, { 'Foo-Bar' => 'foo bar response header' }, ['Not Found']] }
+      end
+
+      it 'leaves status code unset' do
+        _(first_span.attributes['http.status_code']).must_equal 404
+        _(first_span.kind).must_equal :server
+        _(first_span.status.code).must_equal OpenTelemetry::Trace::Status::UNSET
+      end
+    end
   end
 
   describe 'config[:quantization]' do

diff --git a/...tation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb b/...tation/sinatra/lib/opentelemetry/instrumentation/sinatra/middlewares/tracer_middleware.rb
@@ -43,7 +43,7 @@ def trace_response(span, env, resp)
             span.set_attribute('http.status_code', status)
             span.set_attribute('http.route', env['sinatra.route'].split.last) if env['sinatra.route']
             span.name = env['sinatra.route'] if env['sinatra.route']
-            span.status = OpenTelemetry::Trace::Status.error unless (100..399).include?(status.to_i)
+            span.status = OpenTelemetry::Trace::Status.error unless (100..499).include?(status.to_i)
           end
         end
       end

diff --git a/instrumentation/sinatra/test/opentelemetry/instrumentation/sinatra_test.rb b/instrumentation/sinatra/test/opentelemetry/instrumentation/sinatra_test.rb
@@ -128,7 +128,7 @@
     it 'does not create unhandled exceptions for missing routes' do
       get '/one/missing_example/not_present'
 
-      _(exporter.finished_spans.first.status.code).must_equal OpenTelemetry::Trace::Status::ERROR
+      _(exporter.finished_spans.first.status.code).must_equal OpenTelemetry::Trace::Status::UNSET
       _(exporter.finished_spans.first.attributes).must_equal(
         'http.method' => 'GET',
         'http.url' => '/missing_example/not_present',