From 9d142659115f589e343bd472ef622f0799af6c6f Mon Sep 17 00:00:00 2001
From: Nikolay Gribanov <nicholasg.ix35@gmail.com>
Date: Wed, 13 Apr 2022 20:08:42 +0300
Subject: [PATCH] Fix Tornado errors mapping to 500 (#1048)

---
 CHANGELOG.md                                  |  3 +-
 .../instrumentation/tornado/__init__.py       |  7 ++--
 .../tests/test_instrumentation.py             | 35 +++++++++++++++++++
 .../tests/tornado_test_app.py                 |  6 ++++
 4 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 7092f512e2..c65af97b98 100644
--- 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/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
index 128fa56431..a59cdd59f8 100644
--- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
+++ b/instrumentation/opentelemetry-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
index 3948d29d58..8dcc94b683 100644
--- 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
index 1f280e295f..9e84c74aca 100644
--- 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