From f3c8b97820ae8728e03513ebb445948a58ee98dd Mon Sep 17 00:00:00 2001 From: Yoav Alon Date: Mon, 3 Aug 2020 14:04:39 +0300 Subject: [PATCH 1/6] Instrument aioredis Rebuild of #552 with tracking in an aioredis-specific instrument submodule. --- src/scout_apm/async_/instruments/aioredis.py | 35 ++++ src/scout_apm/instruments/aioredis.py | 73 ++++++++ src/scout_apm/instruments/redis.py | 8 + .../instruments/test_aioredis_py36plus.py | 168 ++++++++++++++++++ tox.ini | 1 + 5 files changed, 285 insertions(+) create mode 100644 src/scout_apm/async_/instruments/aioredis.py create mode 100644 src/scout_apm/instruments/aioredis.py create mode 100644 tests/integration/instruments/test_aioredis_py36plus.py diff --git a/src/scout_apm/async_/instruments/aioredis.py b/src/scout_apm/async_/instruments/aioredis.py new file mode 100644 index 00000000..f8c1f7ab --- /dev/null +++ b/src/scout_apm/async_/instruments/aioredis.py @@ -0,0 +1,35 @@ +# coding=utf-8 +from __future__ import absolute_import, division, print_function, unicode_literals + +import wrapt + +from scout_apm.core.tracked_request import TrackedRequest + + +@wrapt.decorator +async def wrapped_redis_execute(wrapped, instance, args, kwargs): + try: + op = args[0] + if isinstance(op, bytes): + op = op.decode() + except (IndexError, TypeError): + op = "Unknown" + + tracked_request = TrackedRequest.instance() + tracked_request.start_span(operation="Redis/{}".format(op)) + + try: + return await wrapped(*args, **kwargs) + finally: + tracked_request.stop_span() + + +@wrapt.decorator +async def wrapped_pipeline_execute(wrapped, instance, args, kwargs): + tracked_request = TrackedRequest.instance() + tracked_request.start_span(operation="Redis/MULTI") + + try: + return await wrapped(*args, **kwargs) + finally: + tracked_request.stop_span() diff --git a/src/scout_apm/instruments/aioredis.py b/src/scout_apm/instruments/aioredis.py new file mode 100644 index 00000000..aef152cc --- /dev/null +++ b/src/scout_apm/instruments/aioredis.py @@ -0,0 +1,73 @@ +# coding=utf-8 +from __future__ import absolute_import, division, print_function, unicode_literals + +import logging + +try: + from aioredis import Redis +except ImportError: # pragma: no cover + Redis = None + +try: + from aioredis.commands import Pipeline +except ImportError: + Pipeline = None + +# The async_ module can only be shipped on Python 3.6+ +try: + from scout_apm.async_.instruments.aioredis import ( + wrapped_redis_execute, + wrapped_pipeline_execute, + ) +except ImportError: + wrapped_redis_execute = None + wrapped_pipeline_execute = None + +logger = logging.getLogger(__name__) + +have_patched_redis_execute = False +have_patched_pipeline_execute = False + + +def ensure_installed(): + global have_patched_redis_execute, have_patched_pipeline_execute + + logger.debug("Instrumenting aioredis.") + + if Redis is None: + logger.debug("Couldn't import aioredis.Redis - probably not installed.") + elif wrapped_redis_execute is None: + logger.debug( + "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6." + ) + elif not have_patched_redis_execute: + try: + Redis.execute = wrapped_redis_execute(Redis.execute) + except Exception as exc: + logger.warning( + "Failed to instrument aioredis.Redis.execute: %r", + exc, + exc_info=exc, + ) + else: + have_patched_redis_execute = True + + if Pipeline is None: + logger.debug( + "Couldn't import aioredis.commands.Pipeline - probably not installed." + ) + elif wrapped_pipeline_execute is None: + logger.debug( + "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6." + ) + elif not have_patched_pipeline_execute: + try: + Pipeline.execute = wrapped_pipeline_execute(Pipeline.execute) + except Exception as exc: + logger.warning( + "Failed to instrument aioredis.commands.Pipeline.execute: %r", + exc, + exc_info=exc, + ) + else: + have_patched_pipeline_execute = True diff --git a/src/scout_apm/instruments/redis.py b/src/scout_apm/instruments/redis.py index 1262ab08..1c2fe985 100644 --- a/src/scout_apm/instruments/redis.py +++ b/src/scout_apm/instruments/redis.py @@ -19,6 +19,11 @@ from redis import StrictRedis as Redis from redis.client import BasePipeline as Pipeline +try: + from scout_apm.async_.instruments.redis import ensure_async_installed +except ImportError: + ensure_async_installed = None + logger = logging.getLogger(__name__) @@ -56,6 +61,9 @@ def ensure_installed(): else: have_patched_pipeline_execute = True + if ensure_async_installed is not None: + ensure_async_installed() + return True diff --git a/tests/integration/instruments/test_aioredis_py36plus.py b/tests/integration/instruments/test_aioredis_py36plus.py new file mode 100644 index 00000000..5e719622 --- /dev/null +++ b/tests/integration/instruments/test_aioredis_py36plus.py @@ -0,0 +1,168 @@ +# coding=utf-8 +from __future__ import absolute_import, division, print_function, unicode_literals + +import logging +import os + +import aioredis +import pytest + +from scout_apm.instruments.aioredis import ensure_installed +from tests.compat import mock +from tests.tools import async_test + + +async def get_redis_conn(): + ensure_installed() + # e.g. export REDIS_URL="redis://localhost:6379/0" + if "REDIS_URL" not in os.environ: + raise pytest.skip("Redis isn't available") + conn = await aioredis.create_connection(os.environ["REDIS_URL"]) + return aioredis.Redis(conn) + + +def test_ensure_installed_twice(caplog): + ensure_installed() + ensure_installed() + + assert caplog.record_tuples == 2 * [ + ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Instrumenting aioredis.", + ) + ] + + +def test_ensure_installed_fail_no_redis_execute(caplog): + mock_not_patched = mock.patch( + "scout_apm.instruments.aioredis.have_patched_redis_execute", new=False + ) + mock_redis = mock.patch("scout_apm.instruments.aioredis.Redis") + with mock_not_patched, mock_redis as mocked_redis: + del mocked_redis.execute + + ensure_installed() + + assert len(caplog.record_tuples) == 2 + assert caplog.record_tuples[0] == ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Instrumenting aioredis.", + ) + logger, level, message = caplog.record_tuples[1] + assert logger == "scout_apm.instruments.aioredis" + assert level == logging.WARNING + assert message.startswith( + "Failed to instrument aioredis.Redis.execute: AttributeError" + ) + + +def test_ensure_installed_fail_no_wrapped_redis_execute(caplog): + mock_not_patched = mock.patch( + "scout_apm.instruments.aioredis.have_patched_redis_execute", new=False + ) + mock_wrapped_redis_execute = mock.patch( + "scout_apm.instruments.aioredis.wrapped_redis_execute", new=None + ) + with mock_not_patched, mock_wrapped_redis_execute: + ensure_installed() + + assert len(caplog.record_tuples) == 2 + assert caplog.record_tuples[0] == ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Instrumenting aioredis.", + ) + assert caplog.record_tuples[1] == ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6.", + ) + + +def test_ensure_installed_fail_no_pipeline_execute(caplog): + mock_not_patched = mock.patch( + "scout_apm.instruments.aioredis.have_patched_pipeline_execute", new=False + ) + mock_pipeline = mock.patch("scout_apm.instruments.aioredis.Pipeline") + with mock_not_patched, mock_pipeline as mocked_pipeline: + del mocked_pipeline.execute + + ensure_installed() + + assert len(caplog.record_tuples) == 2 + assert caplog.record_tuples[0] == ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Instrumenting aioredis.", + ) + logger, level, message = caplog.record_tuples[1] + assert logger == "scout_apm.instruments.aioredis" + assert level == logging.WARNING + assert message.startswith( + "Failed to instrument aioredis.commands.Pipeline.execute: AttributeError" + ) + + +def test_ensure_installed_fail_no_wrapped_pipeline_execute(caplog): + mock_not_patched = mock.patch( + "scout_apm.instruments.aioredis.have_patched_pipeline_execute", new=False + ) + mock_wrapped_pipeline_execute = mock.patch( + "scout_apm.instruments.aioredis.wrapped_pipeline_execute", new=None + ) + with mock_not_patched, mock_wrapped_pipeline_execute: + ensure_installed() + + assert len(caplog.record_tuples) == 2 + assert caplog.record_tuples[0] == ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Instrumenting aioredis.", + ) + assert caplog.record_tuples[1] == ( + "scout_apm.instruments.aioredis", + logging.DEBUG, + "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6.", + ) + + +@async_test +async def test_echo(tracked_request): + redis_conn = await get_redis_conn() + + await redis_conn.echo("Hello World!") + + assert len(tracked_request.complete_spans) == 1 + assert tracked_request.complete_spans[0].operation == "Redis/ECHO" + + +# def test_pipeline_echo(redis_conn, tracked_request): +# with redis_conn.pipeline() as p: +# p.echo("Hello World!") +# p.execute() + +# assert len(tracked_request.complete_spans) == 1 +# assert tracked_request.complete_spans[0].operation == "Redis/MULTI" + + +# def test_execute_command_missing_argument(redis_conn, tracked_request): +# # Redis instrumentation doesn't crash if op is missing. +# # This raises a TypeError (Python 3) or IndexError (Python 2) +# # when calling the original method. +# with pytest.raises(IndexError): +# redis_conn.execute_command() + +# assert len(tracked_request.complete_spans) == 1 +# assert tracked_request.complete_spans[0].operation == "Redis/Unknown" + + +# def test_perform_request_bad_url(redis_conn, tracked_request): +# with pytest.raises(TypeError): +# # Redis instrumentation doesn't crash if op has the wrong type. +# # This raises a TypeError when calling the original method. +# redis_conn.execute_command(None) + +# assert len(tracked_request.complete_spans) == 1 +# assert tracked_request.complete_spans[0].operation == "Redis/None" diff --git a/tox.ini b/tox.ini index 7261c914..b88bab7d 100644 --- a/tox.ini +++ b/tox.ini @@ -16,6 +16,7 @@ passenv = MONGODB_URL REDIS_URL deps = + aioredis ; python_version >= "3.6" bottle cherrypy celery!=4.4.4 # https://github.com/celery/celery/issues/6153 From 72c2e18cb096499284332a83b2a8ea496acec940 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Tue, 1 Sep 2020 12:53:18 +0100 Subject: [PATCH 2/6] Add to instrument names list --- src/scout_apm/instruments/__init__.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/scout_apm/instruments/__init__.py b/src/scout_apm/instruments/__init__.py index 5d0c7c9e..6ae78dc7 100644 --- a/src/scout_apm/instruments/__init__.py +++ b/src/scout_apm/instruments/__init__.py @@ -8,7 +8,16 @@ logger = logging.getLogger(__name__) -instrument_names = ["asyncio", "elasticsearch", "jinja2", "pymongo", "redis", "urllib3"] + +instrument_names = [ + "aioredis", + "asyncio", + "elasticsearch", + "jinja2", + "pymongo", + "redis", + "urllib3", +] def ensure_all_installed(): From 43bc9e90ac5aa69b92d52753da7f21312bf1b3f7 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Thu, 3 Sep 2020 17:15:43 +0100 Subject: [PATCH 3/6] span context decorator --- src/scout_apm/async_/instruments/aioredis.py | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/scout_apm/async_/instruments/aioredis.py b/src/scout_apm/async_/instruments/aioredis.py index f8c1f7ab..61c4b9cc 100644 --- a/src/scout_apm/async_/instruments/aioredis.py +++ b/src/scout_apm/async_/instruments/aioredis.py @@ -16,20 +16,12 @@ async def wrapped_redis_execute(wrapped, instance, args, kwargs): op = "Unknown" tracked_request = TrackedRequest.instance() - tracked_request.start_span(operation="Redis/{}".format(op)) - - try: + with tracked_request.span(operation="Redis/{}".format(op)): return await wrapped(*args, **kwargs) - finally: - tracked_request.stop_span() @wrapt.decorator async def wrapped_pipeline_execute(wrapped, instance, args, kwargs): tracked_request = TrackedRequest.instance() - tracked_request.start_span(operation="Redis/MULTI") - - try: + with tracked_request.span(operation="Redis/MULTI"): return await wrapped(*args, **kwargs) - finally: - tracked_request.stop_span() From 6cf5defe07df3fa28fc039743c2f87f116d69801 Mon Sep 17 00:00:00 2001 From: Adam Johnson Date: Thu, 3 Sep 2020 17:30:10 +0100 Subject: [PATCH 4/6] codestyle --- src/scout_apm/instruments/aioredis.py | 8 +++++--- .../integration/instruments/test_aioredis_py36plus.py | 10 ++++++++-- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/scout_apm/instruments/aioredis.py b/src/scout_apm/instruments/aioredis.py index aef152cc..0ab4f792 100644 --- a/src/scout_apm/instruments/aioredis.py +++ b/src/scout_apm/instruments/aioredis.py @@ -16,8 +16,8 @@ # The async_ module can only be shipped on Python 3.6+ try: from scout_apm.async_.instruments.aioredis import ( - wrapped_redis_execute, wrapped_pipeline_execute, + wrapped_redis_execute, ) except ImportError: wrapped_redis_execute = None @@ -38,7 +38,8 @@ def ensure_installed(): logger.debug("Couldn't import aioredis.Redis - probably not installed.") elif wrapped_redis_execute is None: logger.debug( - "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6." + "Couldn't import scout_apm.async_.instruments.aioredis -" + + " probably using Python < 3.6." ) elif not have_patched_redis_execute: try: @@ -58,7 +59,8 @@ def ensure_installed(): ) elif wrapped_pipeline_execute is None: logger.debug( - "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6." + "Couldn't import scout_apm.async_.instruments.aioredis -" + + " probably using Python < 3.6." ) elif not have_patched_pipeline_execute: try: diff --git a/tests/integration/instruments/test_aioredis_py36plus.py b/tests/integration/instruments/test_aioredis_py36plus.py index 5e719622..991598e9 100644 --- a/tests/integration/instruments/test_aioredis_py36plus.py +++ b/tests/integration/instruments/test_aioredis_py36plus.py @@ -77,7 +77,10 @@ def test_ensure_installed_fail_no_wrapped_redis_execute(caplog): assert caplog.record_tuples[1] == ( "scout_apm.instruments.aioredis", logging.DEBUG, - "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6.", + ( + "Couldn't import scout_apm.async_.instruments.aioredis - probably" + + " using Python < 3.6." + ), ) @@ -124,7 +127,10 @@ def test_ensure_installed_fail_no_wrapped_pipeline_execute(caplog): assert caplog.record_tuples[1] == ( "scout_apm.instruments.aioredis", logging.DEBUG, - "Couldn't import scout_apm.async_.instruments.aioredis - probably using Python < 3.6.", + ( + "Couldn't import scout_apm.async_.instruments.aioredis -" + + " probably using Python < 3.6." + ), ) From 1823bd7af402efd8b6ad50d2a432b30ea142d48d Mon Sep 17 00:00:00 2001 From: Tim Schilling Date: Tue, 4 May 2021 08:33:05 -0500 Subject: [PATCH 5/6] Adding aioredis tests. --- .../instruments/test_aioredis_py36plus.py | 48 +++++++++++-------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/tests/integration/instruments/test_aioredis_py36plus.py b/tests/integration/instruments/test_aioredis_py36plus.py index 991598e9..f78e5392 100644 --- a/tests/integration/instruments/test_aioredis_py36plus.py +++ b/tests/integration/instruments/test_aioredis_py36plus.py @@ -144,31 +144,37 @@ async def test_echo(tracked_request): assert tracked_request.complete_spans[0].operation == "Redis/ECHO" -# def test_pipeline_echo(redis_conn, tracked_request): -# with redis_conn.pipeline() as p: -# p.echo("Hello World!") -# p.execute() +@async_test +async def test_pipeline_echo(tracked_request): + redis_conn = await get_redis_conn() + with redis_conn.pipeline() as p: + p.echo("Hello World!") + p.execute() -# assert len(tracked_request.complete_spans) == 1 -# assert tracked_request.complete_spans[0].operation == "Redis/MULTI" + assert len(tracked_request.complete_spans) == 1 + assert tracked_request.complete_spans[0].operation == "Redis/MULTI" -# def test_execute_command_missing_argument(redis_conn, tracked_request): -# # Redis instrumentation doesn't crash if op is missing. -# # This raises a TypeError (Python 3) or IndexError (Python 2) -# # when calling the original method. -# with pytest.raises(IndexError): -# redis_conn.execute_command() +@async_test +async def test_execute_command_missing_argument(tracked_request): + redis_conn = await get_redis_conn() + # Redis instrumentation doesn't crash if op is missing. + # This raises a TypeError (Python 3) or IndexError (Python 2) + # when calling the original method. + with pytest.raises(IndexError): + redis_conn.execute_command() -# assert len(tracked_request.complete_spans) == 1 -# assert tracked_request.complete_spans[0].operation == "Redis/Unknown" + assert len(tracked_request.complete_spans) == 1 + assert tracked_request.complete_spans[0].operation == "Redis/Unknown" -# def test_perform_request_bad_url(redis_conn, tracked_request): -# with pytest.raises(TypeError): -# # Redis instrumentation doesn't crash if op has the wrong type. -# # This raises a TypeError when calling the original method. -# redis_conn.execute_command(None) +@async_test +async def test_perform_request_bad_url(tracked_request): + redis_conn = await get_redis_conn() + with pytest.raises(TypeError): + # Redis instrumentation doesn't crash if op has the wrong type. + # This raises a TypeError when calling the original method. + redis_conn.execute_command(None) -# assert len(tracked_request.complete_spans) == 1 -# assert tracked_request.complete_spans[0].operation == "Redis/None" + assert len(tracked_request.complete_spans) == 1 + assert tracked_request.complete_spans[0].operation == "Redis/None" From 1d7c3efd415fbeb2e119f358403bc08cf596134d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 4 May 2021 14:49:57 +0000 Subject: [PATCH 6/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/scout_apm/instruments/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scout_apm/instruments/__init__.py b/src/scout_apm/instruments/__init__.py index 6ae78dc7..44aea363 100644 --- a/src/scout_apm/instruments/__init__.py +++ b/src/scout_apm/instruments/__init__.py @@ -11,7 +11,7 @@ instrument_names = [ "aioredis", - "asyncio", + "asyncio", "elasticsearch", "jinja2", "pymongo",