Tornado attributes (#706)

* Remove duplicate HTTP_HOST * Set http.client_ip instead of net.peer.ip Tornado sets remote_ip to the value of X-Forwarded-For or X-Real-IP if the 'xheaders' setting is set. * Add a "handler" attribute, with full class name The spec has a 'http.route' which unfortunately seems difficult to get at. However the full class name is readily available and most helpful. * Add net.peer.ip from _orig_remote_ip * Address review comments - Don't set NET_PEER_IP if '_orig_remote_ip' is not set - Add a comment about the difference between the attributes * Make "handler" attribute into "tornado.handler" * Add CHANGELOG entry * Add test with X-Forwarded-For Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
open-telemetry · Oct 4, 2021 · 3b5071b · 3b5071b
1 parent c143d05
commit 3b5071b
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -18,6 +18,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   ([679](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/679))
 - `opentelemetry-exporter-richconsole` Initial release
   ([#686](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/686))
+- `opentelemetry-instrumentation-tornado` now sets `http.client_ip` and `tornado.handler` attributes
+  ([#706](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/706))
 
 ### Changed
 - `opentelemetry-instrumentation-botocore` Unpatch botocore Endpoint.prepare_request on uninstrument

diff --git a/...entelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/...entelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py
@@ -235,11 +235,17 @@ def _get_attributes_from_request(request):
         SpanAttributes.HTTP_TARGET: request.path,
     }
 
-    if request.host:
-        attrs[SpanAttributes.HTTP_HOST] = request.host
-
     if request.remote_ip:
-        attrs[SpanAttributes.NET_PEER_IP] = request.remote_ip
+        # NET_PEER_IP is the address of the network peer
+        # HTTP_CLIENT_IP is the address of the client, which might be different
+        # if Tornado is set to trust X-Forwarded-For headers (xheaders=True)
+        attrs[SpanAttributes.HTTP_CLIENT_IP] = request.remote_ip
+        if hasattr(request.connection, "context") and getattr(
+            request.connection.context, "_orig_remote_ip", None
+        ):
+            attrs[
+                SpanAttributes.NET_PEER_IP
+            ] = request.connection.context._orig_remote_ip
 
     return extract_attributes_from_object(
         request, _traced_request_attrs, attrs
@@ -252,6 +258,11 @@ def _get_operation_name(handler, request):
     return f"{class_name}.{request.method.lower()}"
 
 
+def _get_full_handler_name(handler):
+    klass = type(handler)
+    return f"{klass.__module__}.{klass.__qualname__}"
+
+
 def _start_span(tracer, handler, start_time) -> _TraceContext:
     token = context.attach(extract(handler.request.headers))
 
@@ -264,6 +275,7 @@ def _start_span(tracer, handler, start_time) -> _TraceContext:
         attributes = _get_attributes_from_request(handler.request)
         for key, value in attributes.items():
             span.set_attribute(key, value)
+        span.set_attribute("tornado.handler", _get_full_handler_name(handler))
 
     activation = trace.use_span(span, end_on_exit=True)
     activation.__enter__()  # pylint: disable=E1101

diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py
@@ -143,8 +143,9 @@ def _test_http_method_call(self, method):
                 SpanAttributes.HTTP_HOST: "127.0.0.1:"
                 + str(self.get_http_port()),
                 SpanAttributes.HTTP_TARGET: "/",
-                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
                 SpanAttributes.HTTP_STATUS_CODE: 201,
+                "tornado.handler": "tests.tornado_test_app.MainHandler",
             },
         )
 
@@ -220,7 +221,7 @@ def _test_async_handler(self, url, handler_name):
                 SpanAttributes.HTTP_HOST: "127.0.0.1:"
                 + str(self.get_http_port()),
                 SpanAttributes.HTTP_TARGET: url,
-                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
                 SpanAttributes.HTTP_STATUS_CODE: 201,
             },
         )
@@ -258,8 +259,9 @@ def test_500(self):
                 SpanAttributes.HTTP_HOST: "127.0.0.1:"
                 + str(self.get_http_port()),
                 SpanAttributes.HTTP_TARGET: "/error",
-                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
                 SpanAttributes.HTTP_STATUS_CODE: 500,
+                "tornado.handler": "tests.tornado_test_app.BadHandler",
             },
         )
 
@@ -292,8 +294,9 @@ def test_404(self):
                 SpanAttributes.HTTP_HOST: "127.0.0.1:"
                 + str(self.get_http_port()),
                 SpanAttributes.HTTP_TARGET: "/missing-url",
-                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
                 SpanAttributes.HTTP_STATUS_CODE: 404,
+                "tornado.handler": "tornado.web.ErrorHandler",
             },
         )
 
@@ -336,8 +339,9 @@ def test_dynamic_handler(self):
                 SpanAttributes.HTTP_HOST: "127.0.0.1:"
                 + str(self.get_http_port()),
                 SpanAttributes.HTTP_TARGET: "/dyna",
-                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
                 SpanAttributes.HTTP_STATUS_CODE: 202,
+                "tornado.handler": "tests.tornado_test_app.DynamicHandler",
             },
         )
 
@@ -377,8 +381,9 @@ def test_handler_on_finish(self):
                 SpanAttributes.HTTP_HOST: "127.0.0.1:"
                 + str(self.get_http_port()),
                 SpanAttributes.HTTP_TARGET: "/on_finish",
-                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1",
                 SpanAttributes.HTTP_STATUS_CODE: 200,
+                "tornado.handler": "tests.tornado_test_app.FinishedHandler",
             },
         )
 
@@ -483,6 +488,30 @@ def test_credential_removal(self):
         self.memory_exporter.clear()
 
 
+class TestTornadoInstrumentationWithXHeaders(TornadoTest):
+    def get_httpserver_options(self):
+        return {"xheaders": True}
+
+    def test_xheaders(self):
+        response = self.fetch("/", headers={"X-Forwarded-For": "12.34.56.78"})
+        self.assertEqual(response.code, 201)
+        spans = self.get_finished_spans()
+        self.assertSpanHasAttributes(
+            spans.by_name("MainHandler.get"),
+            {
+                SpanAttributes.HTTP_METHOD: "GET",
+                SpanAttributes.HTTP_SCHEME: "http",
+                SpanAttributes.HTTP_HOST: "127.0.0.1:"
+                + str(self.get_http_port()),
+                SpanAttributes.HTTP_TARGET: "/",
+                SpanAttributes.HTTP_CLIENT_IP: "12.34.56.78",
+                SpanAttributes.HTTP_STATUS_CODE: 201,
+                SpanAttributes.NET_PEER_IP: "127.0.0.1",
+                "tornado.handler": "tests.tornado_test_app.MainHandler",
+            },
+        )
+
+
 class TornadoHookTest(TornadoTest):
     _client_request_hook = None
     _client_response_hook = None