diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ff02d986e..bd04c17353 100644 --- 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/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py index fd7ac8b300..2e3c21a813 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-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 index ac8e0b1a8f..13cd7071f5 100644 --- 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