Fix Tornado errors mapping to 500 (#1048)

open-telemetry · Apr 13, 2022 · 9d14265 · 9d14265
1 parent e9f83e1
commit 9d14265
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 4 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,7 +8,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## [Unreleased](https://github.com/open-telemetry/opentelemetry-python/compare/v1.10.0-0.29b0...HEAD)
 
 ### Fixed
-
+- `opentelemetry-instrumentation-tornado` Fix Tornado errors mapping to 500
+  ([#1048])(https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1048)
 - `opentelemetry-instrumentation-urllib` make span attributes available to sampler
   ([1014](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/1014))
 - `opentelemetry-instrumentation-flask` Fix non-recording span bug

diff --git a/...entelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/...entelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
@@ -420,14 +420,15 @@ def _finish_span(tracer, handler, error=None):
             status_code = error.status_code
             if not ctx and status_code == 404:
                 ctx = _start_span(tracer, handler, _time_ns())
-        if status_code != 404:
+        else:
+            status_code = 500
+            reason = None
+        if status_code >= 500:
             finish_args = (
                 type(error),
                 error,
                 getattr(error, "__traceback__", None),
             )
-            status_code = 500
-            reason = None
 
     if not ctx:
         return

diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py
@@ -317,6 +317,41 @@ def test_404(self):
             },
         )
 
+    def test_http_error(self):
+        response = self.fetch("/raise_403")
+        self.assertEqual(response.code, 403)
+
+        spans = self.sorted_spans(self.memory_exporter.get_finished_spans())
+        self.assertEqual(len(spans), 2)
+        server, client = spans
+
+        self.assertEqual(server.name, "RaiseHTTPErrorHandler.get")
+        self.assertEqual(server.kind, SpanKind.SERVER)
+        self.assertSpanHasAttributes(
+            server,
+            {
+                SpanAttributes.HTTP_METHOD: "GET",
+                SpanAttributes.HTTP_SCHEME: "http",
+                SpanAttributes.HTTP_HOST: "127.0.0.1:"
+                + str(self.get_http_port()),
+                SpanAttributes.HTTP_TARGET: "/raise_403",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
+                SpanAttributes.HTTP_STATUS_CODE: 403,
+                "tornado.handler": "tests.tornado_test_app.RaiseHTTPErrorHandler",
+            },
+        )
+
+        self.assertEqual(client.name, "GET")
+        self.assertEqual(client.kind, SpanKind.CLIENT)
+        self.assertSpanHasAttributes(
+            client,
+            {
+                SpanAttributes.HTTP_URL: self.get_url("/raise_403"),
+                SpanAttributes.HTTP_METHOD: "GET",
+                SpanAttributes.HTTP_STATUS_CODE: 403,
+            },
+        )
+
     def test_dynamic_handler(self):
         response = self.fetch("/dyna")
         self.assertEqual(response.code, 404)

diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py
@@ -105,6 +105,11 @@ def get(self):
         self.set_status(200)
 
 
+class RaiseHTTPErrorHandler(tornado.web.RequestHandler):
+    def get(self):
+        raise tornado.web.HTTPError(403)
+
+
 def make_app(tracer):
     app = tornado.web.Application(
         [
@@ -116,6 +121,7 @@ def make_app(tracer):
             (r"/healthz", HealthCheckHandler),
             (r"/ping", HealthCheckHandler),
             (r"/test_custom_response_headers", CustomResponseHeaderHandler),
+            (r"/raise_403", RaiseHTTPErrorHandler),
         ]
     )
     app.tracer = tracer