diff --git a/codecov.yml b/codecov.yml index c2441c970..6ca30f640 100644 --- a/codecov.yml +++ b/codecov.yml @@ -1,23 +1,25 @@ ignore: - - "newrelic/packages/**/*" - - "newrelic/packages/*" - "newreilc/hooks/component_sentry.py" - - "newrelic/hooks/adapter_meinheld.py" + - "newrelic/admin/*" + - "newrelic/console.py" - "newrelic/hooks/adapter_flup.py" + - "newrelic/hooks/adapter_meinheld.py" - "newrelic/hooks/adapter_paste.py" - "newrelic/hooks/component_piston.py" + - "newrelic/hooks/database_oursql.py" + - "newrelic/hooks/database_psycopg2ct.py" + - "newrelic/hooks/datastore_aioredis.py" + - "newrelic/hooks/datastore_aredis.py" + - "newrelic/hooks/datastore_motor.py" - "newrelic/hooks/datastore_pyelasticsearch.py" - - "newrelic/hooks/external_pywapi.py" + - "newrelic/hooks/datastore_umemcache.py" - "newrelic/hooks/external_dropbox.py" - "newrelic/hooks/external_facepy.py" + - "newrelic/hooks/external_pywapi.py" - "newrelic/hooks/external_xmlrpclib.py" - "newrelic/hooks/framework_pylons.py" - "newrelic/hooks/framework_web2py.py" - - "newrelic/hooks/middleware_weberror.py" - "newrelic/hooks/framework_webpy.py" - - "newrelic/hooks/datastore_motor.py" - - "newrelic/hooks/database_oursql.py" - - "newrelic/hooks/database_psycopg2ct.py" - - "newrelic/hooks/datastore_umemcache.py" - - "newrelic/admin/*" - - "newrelic/console.py" + - "newrelic/hooks/middleware_weberror.py" + - "newrelic/packages/*" + - "newrelic/packages/**/*" diff --git a/newrelic/config.py b/newrelic/config.py index 797b22350..8a041ad34 100644 --- a/newrelic/config.py +++ b/newrelic/config.py @@ -2674,12 +2674,14 @@ def _process_module_builtin_defaults(): "aioredis.connection", "newrelic.hooks.datastore_aioredis", "instrument_aioredis_connection" ) + # Redis v4.2+ _process_module_definition( - "redis.asyncio.client", "newrelic.hooks.datastore_aioredis", "instrument_aioredis_client" + "redis.asyncio.client", "newrelic.hooks.datastore_redis", "instrument_asyncio_redis_client" ) + # Redis v4.2+ _process_module_definition( - "redis.asyncio.commands", "newrelic.hooks.datastore_aioredis", "instrument_aioredis_client" + "redis.asyncio.commands", "newrelic.hooks.datastore_redis", "instrument_asyncio_redis_client" ) _process_module_definition( diff --git a/newrelic/hooks/datastore_aioredis.py b/newrelic/hooks/datastore_aioredis.py index 47c987971..daced369a 100644 --- a/newrelic/hooks/datastore_aioredis.py +++ b/newrelic/hooks/datastore_aioredis.py @@ -60,7 +60,12 @@ def _nr_wrapper_AioRedis_method_(wrapped, instance, args, kwargs): # Method will return synchronously without executing, # it will be added to the command stack and run later. aioredis_version = get_package_version_tuple("aioredis") - if aioredis_version and aioredis_version < (2,): + # This conditional is for versions of aioredis that are outside + # New Relic's supportability window but will still work. New + # Relic does not provide testing/support for this. In order to + # keep functionality without affecting coverage metrics, this + # segment is excluded from coverage analysis. + if aioredis_version and aioredis_version < (2,): # pragma: no cover # AioRedis v1 uses a RedisBuffer instead of a real connection for queueing up pipeline commands from aioredis.commands.transaction import _RedisBuffer @@ -72,8 +77,6 @@ def _nr_wrapper_AioRedis_method_(wrapped, instance, args, kwargs): # AioRedis v2 uses a Pipeline object for a client and internally queues up pipeline commands if aioredis_version: from aioredis.client import Pipeline - else: - from redis.asyncio.client import Pipeline if isinstance(instance, Pipeline): return wrapped(*args, **kwargs) @@ -137,7 +140,12 @@ async def wrap_Connection_send_command(wrapped, instance, args, kwargs): return await wrapped(*args, **kwargs) -def wrap_RedisConnection_execute(wrapped, instance, args, kwargs): +# This wrapper is for versions of aioredis that are outside +# New Relic's supportability window but will still work. New +# Relic does not provide testing/support for this. In order to +# keep functionality without affecting coverage metrics, this +# segment is excluded from coverage analysis. +def wrap_RedisConnection_execute(wrapped, instance, args, kwargs): # pragma: no cover # RedisConnection in aioredis v1 returns a future instead of using coroutines transaction = current_transaction() if not transaction: @@ -205,6 +213,11 @@ def instrument_aioredis_connection(module): if hasattr(module.Connection, "send_command"): wrap_function_wrapper(module, "Connection.send_command", wrap_Connection_send_command) - if hasattr(module, "RedisConnection"): + # This conditional is for versions of aioredis that are outside + # New Relic's supportability window but will still work. New + # Relic does not provide testing/support for this. In order to + # keep functionality without affecting coverage metrics, this + # segment is excluded from coverage analysis. + if hasattr(module, "RedisConnection"): # pragma: no cover if hasattr(module.RedisConnection, "execute"): wrap_function_wrapper(module, "RedisConnection.execute", wrap_RedisConnection_execute) diff --git a/newrelic/hooks/datastore_redis.py b/newrelic/hooks/datastore_redis.py index 0754c21ed..6854d84f3 100644 --- a/newrelic/hooks/datastore_redis.py +++ b/newrelic/hooks/datastore_redis.py @@ -16,12 +16,67 @@ from newrelic.api.datastore_trace import DatastoreTrace from newrelic.api.transaction import current_transaction -from newrelic.common.object_wrapper import wrap_function_wrapper +from newrelic.common.object_wrapper import function_wrapper, wrap_function_wrapper -_redis_client_methods = { +_redis_client_sync_methods = { + "acl_dryrun", + "auth", + "bgrewriteaof", + "bitfield", + "blmpop", + "bzmpop", + "client", + "command", + "command_docs", + "command_getkeysandflags", + "command_info", + "debug_segfault", + "expiretime", + "failover", + "hello", + "latency_doctor", + "latency_graph", + "latency_histogram", + "lcs", + "lpop", + "lpos", + "memory_doctor", + "memory_help", + "monitor", + "pexpiretime", + "psetex", + "psync", + "pubsub", + "renamenx", + "rpop", + "script_debug", + "sentinel_ckquorum", + "sentinel_failover", + "sentinel_flushconfig", + "sentinel_get_master_addr_by_name", + "sentinel_master", + "sentinel_masters", + "sentinel_monitor", + "sentinel_remove", + "sentinel_reset", + "sentinel_sentinels", + "sentinel_set", + "sentinel_slaves", + "shutdown", + "sort", + "sort_ro", + "spop", + "srandmember", + "unwatch", + "watch", + "zlexcount", + "zrevrangebyscore", +} + + +_redis_client_async_methods = { "acl_cat", "acl_deluser", - "acl_dryrun", "acl_genpass", "acl_getuser", "acl_help", @@ -50,11 +105,8 @@ "arrlen", "arrpop", "arrtrim", - "auth", - "bgrewriteaof", "bgsave", "bitcount", - "bitfield", "bitfield_ro", "bitop_and", "bitop_not", @@ -63,13 +115,11 @@ "bitop", "bitpos", "blmove", - "blmpop", "blpop", "brpop", "brpoplpush", "byrank", "byrevrank", - "bzmpop", "bzpopmax", "bzpopmin", "card", @@ -90,7 +140,6 @@ "client_trackinginfo", "client_unblock", "client_unpause", - "client", "cluster_add_slots", "cluster_addslots", "cluster_count_failure_report", @@ -117,10 +166,7 @@ "cluster_slots", "cluster", "command_count", - "command_docs", "command_getkeys", - "command_getkeysandflags", - "command_info", "command_list", "command", "commit", @@ -136,7 +182,6 @@ "createrule", "dbsize", "debug_object", - "debug_segfault", "debug_sleep", "debug", "decr", @@ -159,10 +204,8 @@ "exists", "expire", "expireat", - "expiretime", "explain_cli", "explain", - "failover", "fcall_ro", "fcall", "flushall", @@ -191,7 +234,6 @@ "getrange", "getset", "hdel", - "hello", "hexists", "hget", "hgetall", @@ -219,13 +261,9 @@ "insertnx", "keys", "lastsave", - "latency_doctor", - "latency_graph", - "latency_histogram", "latency_history", "latency_latest", "latency_reset", - "lcs", "lindex", "linsert", "list", @@ -234,8 +272,6 @@ "lmpop", "loadchunk", "lolwut", - "lpop", - "lpos", "lpush", "lpushx", "lrange", @@ -244,8 +280,6 @@ "ltrim", "madd", "max", - "memory_doctor", - "memory_help", "memory_malloc_stats", "memory_purge", "memory_stats", @@ -260,7 +294,6 @@ "module_load", "module_loadex", "module_unload", - "monitor", "move", "mrange", "mrevrange", @@ -276,21 +309,17 @@ "persist", "pexpire", "pexpireat", - "pexpiretime", "pfadd", "pfcount", "pfmerge", "ping", "profile", - "psetex", "psubscribe", - "psync", "pttl", "publish", "pubsub_channels", "pubsub_numpat", "pubsub_numsub", - "pubsub", "punsubscribe", "quantile", "query", @@ -302,7 +331,6 @@ "readonly", "readwrite", "rename", - "renamenx", "replicaof", "reserve", "reset", @@ -311,7 +339,6 @@ "revrange", "revrank", "role", - "rpop", "rpoplpush", "rpush", "rpushx", @@ -321,7 +348,6 @@ "scan", "scandump", "scard", - "script_debug", "script_exists", "script_flush", "script_kill", @@ -330,24 +356,11 @@ "sdiffstore", "search", "select", - "sentinel_ckquorum", - "sentinel_failover", - "sentinel_flushconfig", - "sentinel_get_master_addr_by_name", - "sentinel_master", - "sentinel_masters", - "sentinel_monitor", - "sentinel_remove", - "sentinel_reset", - "sentinel_sentinels", - "sentinel_set", - "sentinel_slaves", "set", "setbit", "setex", "setnx", "setrange", - "shutdown", "sinter", "sintercard", "sinterstore", @@ -360,11 +373,7 @@ "smembers", "smismember", "smove", - "sort_ro", - "sort", "spellcheck", - "spop", - "srandmember", "srem", "sscan_iter", "sscan", @@ -392,10 +401,8 @@ "type", "unlink", "unsubscribe", - "unwatch", "wait", "waitaof", - "watch", "xack", "xadd", "xautoclaim", @@ -431,7 +438,6 @@ "zinter", "zintercard", "zinterstore", - "zlexcount", "zmpop", "zmscore", "zpopmax", @@ -448,7 +454,6 @@ "zremrangebyscore", "zrevrange", "zrevrangebylex", - "zrevrangebyscore", "zrevrank", "zscan_iter", "zscan", @@ -457,6 +462,8 @@ "zunionstore", } +_redis_client_methods = _redis_client_sync_methods.union(_redis_client_async_methods) + _redis_multipart_commands = set(["client", "cluster", "command", "config", "debug", "sentinel", "slowlog", "script"]) _redis_operation_re = re.compile(r"[-\s]+") @@ -504,6 +511,29 @@ def _nr_wrapper_Redis_method_(wrapped, instance, args, kwargs): wrap_function_wrapper(module, name, _nr_wrapper_Redis_method_) +def _wrap_asyncio_Redis_method_wrapper(module, instance_class_name, operation): + @function_wrapper + async def _nr_wrapper_asyncio_Redis_async_method_(wrapped, instance, args, kwargs): + transaction = current_transaction() + if transaction is None: + return await wrapped(*args, **kwargs) + + with DatastoreTrace(product="Redis", target=None, operation=operation): + return await wrapped(*args, **kwargs) + + def _nr_wrapper_asyncio_Redis_method_(wrapped, instance, args, kwargs): + from redis.asyncio.client import Pipeline + + if isinstance(instance, Pipeline): + return wrapped(*args, **kwargs) + + # Method should be run when awaited, therefore we wrap in an async wrapper. + return _nr_wrapper_asyncio_Redis_async_method_(wrapped)(*args, **kwargs) + + name = "%s.%s" % (instance_class_name, operation) + wrap_function_wrapper(module, name, _nr_wrapper_asyncio_Redis_method_) + + def _nr_Connection_send_command_wrapper_(wrapped, instance, args, kwargs): transaction = current_transaction() @@ -565,6 +595,14 @@ def instrument_redis_client(module): _wrap_Redis_method_wrapper_(module, "Redis", name) +def instrument_asyncio_redis_client(module): + if hasattr(module, "Redis"): + class_ = getattr(module, "Redis") + for operation in _redis_client_async_methods: + if hasattr(class_, operation): + _wrap_asyncio_Redis_method_wrapper(module, "Redis", operation) + + def instrument_redis_commands_core(module): _instrument_redis_commands_module(module, "CoreCommands") diff --git a/tests/datastore_redis/test_asyncio.py b/tests/datastore_redis/test_asyncio.py new file mode 100644 index 000000000..97c1b7853 --- /dev/null +++ b/tests/datastore_redis/test_asyncio.py @@ -0,0 +1,100 @@ +# Copyright 2010 New Relic, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import asyncio + +import pytest +from testing_support.db_settings import redis_settings +from testing_support.fixture.event_loop import event_loop as loop # noqa: F401 +from testing_support.util import instance_hostname +from testing_support.validators.validate_transaction_metrics import ( + validate_transaction_metrics, +) + +from newrelic.api.background_task import background_task +from newrelic.common.package_version_utils import get_package_version_tuple + +# Settings + +DB_SETTINGS = redis_settings()[0] +REDIS_VERSION = get_package_version_tuple("redis") + +# Metrics + +_enable_scoped_metrics = [("Datastore/operation/Redis/publish", 3)] + +_enable_rollup_metrics = [ + ("Datastore/all", 3), + ("Datastore/allOther", 3), + ("Datastore/Redis/all", 3), + ("Datastore/Redis/allOther", 3), + ("Datastore/operation/Redis/publish", 3), + ("Datastore/instance/Redis/%s/%s" % (instance_hostname(DB_SETTINGS["host"]), DB_SETTINGS["port"]), 3), +] + +# Tests + + +@pytest.fixture() +def client(loop): # noqa + import redis.asyncio + + return loop.run_until_complete(redis.asyncio.Redis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0)) + + +@pytest.mark.skipif(REDIS_VERSION < (4, 2), reason="This functionality exists in Redis 4.2+") +@validate_transaction_metrics("test_asyncio:test_async_pipeline", background_task=True) +@background_task() +def test_async_pipeline(client, loop): # noqa + async def _test_pipeline(client): + async with client.pipeline(transaction=True) as pipe: + await pipe.set("key1", "value1") + await pipe.execute() + + loop.run_until_complete(_test_pipeline(client)) + + +@pytest.mark.skipif(REDIS_VERSION < (4, 2), reason="This functionality exists in Redis 4.2+") +@validate_transaction_metrics( + "test_asyncio:test_async_pubsub", + scoped_metrics=_enable_scoped_metrics, + rollup_metrics=_enable_rollup_metrics, + background_task=True, +) +@background_task() +def test_async_pubsub(client, loop): # noqa + messages_received = [] + + async def reader(pubsub): + while True: + message = await pubsub.get_message(ignore_subscribe_messages=True) + if message: + messages_received.append(message["data"].decode()) + if message["data"].decode() == "NOPE": + break + + async def _test_pubsub(): + async with client.pubsub() as pubsub: + await pubsub.psubscribe("channel:*") + + future = asyncio.create_task(reader(pubsub)) + + await client.publish("channel:1", "Hello") + await client.publish("channel:2", "World") + await client.publish("channel:1", "NOPE") + + await future + + loop.run_until_complete(_test_pubsub()) + assert messages_received == ["Hello", "World", "NOPE"] diff --git a/tox.ini b/tox.ini index 95aa841fa..d36edbe73 100644 --- a/tox.ini +++ b/tox.ini @@ -94,12 +94,8 @@ envlist = mssql-datastore_pymssql-{py37,py38,py39,py310,py311}, mysql-datastore_pymysql-{py27,py37,py38,py39,py310,py311,pypy27,pypy38}, solr-datastore_pysolr-{py27,py37,py38,py39,py310,py311,pypy27,pypy38}, - redis-datastore_redis-{py27,py37,py38,pypy27,pypy38}-redis03, redis-datastore_redis-{py37,py38,py39,py310,py311,pypy38}-redis{0400,latest}, rediscluster-datastore_rediscluster-{py37,py311,pypy38}-redis{latest}, - redis-datastore_aioredis-{py37,py38,py39,py310,pypy38}-aioredislatest, - redis-datastore_aioredis-{py37,py38,py39,py310,py311,pypy38}-redislatest, - redis-datastore_aredis-{py37,py38,py39,pypy38}-aredislatest, python-datastore_sqlite-{py27,py37,py38,py39,py310,py311,pypy27,pypy38}, python-external_boto3-{py27,py37,py38,py39,py310,py311}-boto01, python-external_botocore-{py37,py38,py39,py310,py311}-botocorelatest, @@ -257,11 +253,6 @@ deps = datastore_redis-redislatest: redis datastore_rediscluster-redislatest: redis datastore_redis-redis0400: redis<4.1 - datastore_redis-redis03: redis<4.0 - datastore_redis-{py27,pypy27}: rb - datastore_aioredis-redislatest: redis - datastore_aioredis-aioredislatest: aioredis - datastore_aredis-aredislatest: aredis external_boto3-boto01: boto3<2.0 external_boto3-boto01: moto<2.0 external_boto3-py27: rsa<4.7.1 @@ -460,8 +451,6 @@ changedir = datastore_pysolr: tests/datastore_pysolr datastore_redis: tests/datastore_redis datastore_rediscluster: tests/datastore_rediscluster - datastore_aioredis: tests/datastore_aioredis - datastore_aredis: tests/datastore_aredis datastore_sqlite: tests/datastore_sqlite external_boto3: tests/external_boto3 external_botocore: tests/external_botocore