From 82620b78c3e4158d57237102ad444a7c3ff97c15 Mon Sep 17 00:00:00 2001 From: MikeD <5084545+devmonkey22@users.noreply.github.com> Date: Mon, 12 May 2025 23:17:06 -0400 Subject: [PATCH 1/4] Add tornado WebSocketHandler instrumentation support. (https://github.com/open-telemetry/opentelemetry-python-contrib/issues/2761) --- CHANGELOG.md | 2 + .../instrumentation/tornado/__init__.py | 32 +++++++++---- .../tests/test_instrumentation.py | 48 +++++++++++++++++++ .../tests/tornado_test_app.py | 12 +++++ 4 files changed, 84 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae22221e3f..126f419cb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3464](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3464)) - `opentelemetry-instrumentation-redis` Add support for redis client-specific instrumentation. ([#3143](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3143)) +- `opentelemetry-instrumentation-tornado` Add support for `WebSocketHandler` instrumentation + ([#3448](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2761)) ### Fixed 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 9fbf88a74d..1dccd2ed36 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -162,6 +162,7 @@ def client_response_hook(span, future): from typing import Collection, Dict import tornado.web +import tornado.websocket import wrapt from wrapt import wrap_function_wrapper @@ -351,12 +352,16 @@ def patch_handler_class(tracer, server_histograms, cls, request_hook=None): "prepare", partial(_prepare, tracer, server_histograms, request_hook), ) - _wrap(cls, "on_finish", partial(_on_finish, tracer, server_histograms)) _wrap( cls, "log_exception", partial(_log_exception, tracer, server_histograms), ) + + if issubclass(cls, tornado.websocket.WebSocketHandler): + _wrap(cls, "on_close", partial(_WebSocketHandler_on_close, tracer, server_histograms)) + else: + _wrap(cls, "on_finish", partial(_on_finish, tracer, server_histograms)) return True @@ -365,8 +370,11 @@ def unpatch_handler_class(cls): return unwrap(cls, "prepare") - unwrap(cls, "on_finish") unwrap(cls, "log_exception") + if issubclass(cls, tornado.websocket.WebSocketHandler): + unwrap(cls, "on_close") + else: + unwrap(cls, "on_finish") delattr(cls, _OTEL_PATCHED_KEY) @@ -394,14 +402,18 @@ def _prepare( def _on_finish(tracer, server_histograms, func, handler, args, kwargs): - response = func(*args, **kwargs) - - _record_on_finish_metrics(server_histograms, handler) - - _finish_span(tracer, handler) - - return response - + try: + return func(*args, **kwargs) + finally: + _record_on_finish_metrics(server_histograms, handler) + _finish_span(tracer, handler) + +def _WebSocketHandler_on_close(tracer, server_histograms, func, handler, args, kwargs): + try: + func() + finally: + _record_on_finish_metrics(server_histograms, handler) + _finish_span(tracer, handler) def _log_exception(tracer, server_histograms, func, handler, args, kwargs): error = None diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index daf2ddd846..9a90685ec2 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -13,11 +13,13 @@ # limitations under the License. +import asyncio from unittest.mock import Mock, patch from http_server_mock import HttpServerMock from tornado.httpclient import HTTPClientError from tornado.testing import AsyncHTTPTestCase +import tornado.websocket from opentelemetry import trace from opentelemetry.instrumentation.propagators import ( @@ -450,6 +452,52 @@ def test_handler_on_finish(self): self.assertEqual(auditor.kind, SpanKind.INTERNAL) + @tornado.testing.gen_test() + async def test_websockethandler(self): + ws_client = await tornado.websocket.websocket_connect( + 'ws://127.0.0.1:{}/echo_socket'.format(self.get_http_port()) + ) + + await ws_client.write_message('world') + resp = await ws_client.read_message() + self.assertEqual(resp, 'hello world') + + ws_client.close() + await asyncio.sleep(0.5) + + spans = self.sorted_spans(self.memory_exporter.get_finished_spans()) + self.assertEqual(len(spans), 3) + close_span, msg_span, req_span = spans + + self.assertEqual(req_span.name, "GET /echo_socket") + self.assertEqual(req_span.context.trace_id, msg_span.context.trace_id) + self.assertIsNone(req_span.parent) + self.assertEqual(req_span.kind, SpanKind.SERVER) + self.assertSpanHasAttributes( + req_span, + { + SpanAttributes.HTTP_METHOD: "GET", + SpanAttributes.HTTP_SCHEME: "http", + SpanAttributes.HTTP_HOST: "127.0.0.1:" + + str(self.get_http_port()), + SpanAttributes.HTTP_TARGET: "/echo_socket", + SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1", + SpanAttributes.HTTP_STATUS_CODE: 101, + "tornado.handler": "tests.tornado_test_app.EchoWebSocketHandler", + }, + ) + + self.assertEqual(msg_span.name, "audit_message") + self.assertFalse(msg_span.context.is_remote) + self.assertEqual(msg_span.kind, SpanKind.INTERNAL) + self.assertEqual(msg_span.parent.span_id, req_span.context.span_id) + + self.assertEqual(close_span.name, "audit_on_close") + self.assertFalse(close_span.context.is_remote) + self.assertEqual(close_span.parent.span_id, req_span.context.span_id) + self.assertEqual(close_span.context.trace_id, msg_span.context.trace_id) + self.assertEqual(close_span.kind, SpanKind.INTERNAL) + def test_exclude_lists(self): def test_excluded(path): self.fetch(path) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py index 9e84c74aca..2e8e037506 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py @@ -3,6 +3,7 @@ import tornado.web from tornado import gen +import tornado.websocket class AsyncHandler(tornado.web.RequestHandler): @@ -110,6 +111,16 @@ def get(self): raise tornado.web.HTTPError(403) +class EchoWebSocketHandler(tornado.websocket.WebSocketHandler): + async def on_message(self, message): + with self.application.tracer.start_as_current_span("audit_message"): + self.write_message(f'hello {message}') + + def on_close(self): + with self.application.tracer.start_as_current_span("audit_on_close"): + time.sleep(0.05) + + def make_app(tracer): app = tornado.web.Application( [ @@ -122,6 +133,7 @@ def make_app(tracer): (r"/ping", HealthCheckHandler), (r"/test_custom_response_headers", CustomResponseHeaderHandler), (r"/raise_403", RaiseHTTPErrorHandler), + (r"/echo_socket", EchoWebSocketHandler), ] ) app.tracer = tracer From 3294ca0ec6a05c8b64af917c87dec82791ae91ab Mon Sep 17 00:00:00 2001 From: MikeD <5084545+devmonkey22@users.noreply.github.com> Date: Sun, 18 May 2025 11:15:01 -0400 Subject: [PATCH 2/4] Linting --- .../instrumentation/tornado/__init__.py | 12 ++++++++++-- .../tests/test_instrumentation.py | 12 +++++++----- .../tests/tornado_test_app.py | 4 ++-- 3 files changed, 19 insertions(+), 9 deletions(-) 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 1dccd2ed36..2737e68e67 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/src/opentelemetry/instrumentation/tornado/__init__.py @@ -359,7 +359,11 @@ def patch_handler_class(tracer, server_histograms, cls, request_hook=None): ) if issubclass(cls, tornado.websocket.WebSocketHandler): - _wrap(cls, "on_close", partial(_WebSocketHandler_on_close, tracer, server_histograms)) + _wrap( + cls, + "on_close", + partial(_websockethandler_on_close, tracer, server_histograms), + ) else: _wrap(cls, "on_finish", partial(_on_finish, tracer, server_histograms)) return True @@ -408,13 +412,17 @@ def _on_finish(tracer, server_histograms, func, handler, args, kwargs): _record_on_finish_metrics(server_histograms, handler) _finish_span(tracer, handler) -def _WebSocketHandler_on_close(tracer, server_histograms, func, handler, args, kwargs): + +def _websockethandler_on_close( + tracer, server_histograms, func, handler, args, kwargs +): try: func() finally: _record_on_finish_metrics(server_histograms, handler) _finish_span(tracer, handler) + def _log_exception(tracer, server_histograms, func, handler, args, kwargs): error = None if len(args) == 3: diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index 9a90685ec2..ea09c9b1a7 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -16,10 +16,10 @@ import asyncio from unittest.mock import Mock, patch +import tornado.websocket from http_server_mock import HttpServerMock from tornado.httpclient import HTTPClientError from tornado.testing import AsyncHTTPTestCase -import tornado.websocket from opentelemetry import trace from opentelemetry.instrumentation.propagators import ( @@ -455,12 +455,12 @@ def test_handler_on_finish(self): @tornado.testing.gen_test() async def test_websockethandler(self): ws_client = await tornado.websocket.websocket_connect( - 'ws://127.0.0.1:{}/echo_socket'.format(self.get_http_port()) + f"ws://127.0.0.1:{self.get_http_port()}/echo_socket" ) - await ws_client.write_message('world') + await ws_client.write_message("world") resp = await ws_client.read_message() - self.assertEqual(resp, 'hello world') + self.assertEqual(resp, "hello world") ws_client.close() await asyncio.sleep(0.5) @@ -495,7 +495,9 @@ async def test_websockethandler(self): self.assertEqual(close_span.name, "audit_on_close") self.assertFalse(close_span.context.is_remote) self.assertEqual(close_span.parent.span_id, req_span.context.span_id) - self.assertEqual(close_span.context.trace_id, msg_span.context.trace_id) + self.assertEqual( + close_span.context.trace_id, msg_span.context.trace_id + ) self.assertEqual(close_span.kind, SpanKind.INTERNAL) def test_exclude_lists(self): diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py index 2e8e037506..1523375212 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/tornado_test_app.py @@ -2,8 +2,8 @@ import time import tornado.web -from tornado import gen import tornado.websocket +from tornado import gen class AsyncHandler(tornado.web.RequestHandler): @@ -114,7 +114,7 @@ def get(self): class EchoWebSocketHandler(tornado.websocket.WebSocketHandler): async def on_message(self, message): with self.application.tracer.start_as_current_span("audit_message"): - self.write_message(f'hello {message}') + self.write_message(f"hello {message}") def on_close(self): with self.application.tracer.start_as_current_span("audit_on_close"): From 11460d625768e209c01402fad40a6f072cf358dc Mon Sep 17 00:00:00 2001 From: Riccardo Magliocchetti Date: Mon, 30 Jun 2025 11:22:06 +0200 Subject: [PATCH 3/4] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fa455932b2..d1d2c9641b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `opentelemetry-resource-detector-containerid`: make it more quiet on platforms without cgroups ([#3579](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3579)) +### Added + +- `opentelemetry-instrumentation-tornado` Add support for `WebSocketHandler` instrumentation + ([#3498](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3498)) + ## Version 1.34.0/0.55b0 (2025-06-04) ### Fixed @@ -89,8 +94,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ([#3464](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3464)) - `opentelemetry-instrumentation-redis` Add support for redis client-specific instrumentation. ([#3143](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/3143)) -- `opentelemetry-instrumentation-tornado` Add support for `WebSocketHandler` instrumentation - ([#3448](https://github.com/open-telemetry/opentelemetry-python-contrib/pull/2761)) ### Fixed From abe0c4d195432a0a32ccd981827ce787727ce2d7 Mon Sep 17 00:00:00 2001 From: MikeD <5084545+devmonkey22@users.noreply.github.com> Date: Wed, 9 Jul 2025 12:20:18 -0400 Subject: [PATCH 4/4] Apply refactor changes from #3582 --- .../tests/test_instrumentation.py | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py index 534bf78381..148e520ebc 100644 --- a/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py +++ b/instrumentation/opentelemetry-instrumentation-tornado/tests/test_instrumentation.py @@ -480,13 +480,12 @@ async def test_websockethandler(self): self.assertSpanHasAttributes( req_span, { - SpanAttributes.HTTP_METHOD: "GET", - SpanAttributes.HTTP_SCHEME: "http", - SpanAttributes.HTTP_HOST: "127.0.0.1:" - + str(self.get_http_port()), - SpanAttributes.HTTP_TARGET: "/echo_socket", - SpanAttributes.HTTP_CLIENT_IP: "127.0.0.1", - SpanAttributes.HTTP_STATUS_CODE: 101, + HTTP_METHOD: "GET", + HTTP_SCHEME: "http", + HTTP_HOST: f"127.0.0.1:{self.get_http_port()}", + HTTP_TARGET: "/echo_socket", + HTTP_CLIENT_IP: "127.0.0.1", + HTTP_STATUS_CODE: 101, "tornado.handler": "tests.tornado_test_app.EchoWebSocketHandler", }, )