Fix redis.asyncio sync operations wrapper

newrelic/hooks/datastore_aioredis.py

@@ -11,7 +11,6 @@
 # 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.
-
 from newrelic.api.datastore_trace import DatastoreTrace
 from newrelic.api.time_trace import current_trace
 from newrelic.api.transaction import current_transaction
@@ -60,7 +59,13 @@ 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
 
@@ -134,8 +139,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:
@@ -203,6 +212,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)

newrelic/hooks/datastore_redis.py

@@ -18,10 +18,66 @@
 from newrelic.api.transaction import current_transaction
 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 +106,8 @@
  "arrlen",
  "arrpop",
  "arrtrim",
- "auth",
- "bgrewriteaof",
  "bgsave",
  "bitcount",
- "bitfield",
  "bitfield_ro",
  "bitop_and",
  "bitop_not",
@@ -63,13 +116,11 @@
  "bitop",
  "bitpos",
  "blmove",
- "blmpop",
  "blpop",
  "brpop",
  "brpoplpush",
  "byrank",
  "byrevrank",
- "bzmpop",
  "bzpopmax",
  "bzpopmin",
  "card",
@@ -90,7 +141,6 @@
  "client_trackinginfo",
  "client_unblock",
  "client_unpause",
- "client",
  "cluster_add_slots",
  "cluster_addslots",
  "cluster_count_failure_report",
@@ -118,10 +168,7 @@
  "cluster_slots",
  "cluster",
  "command_count",
- "command_docs",
  "command_getkeys",
- "command_getkeysandflags",
- "command_info",
  "command_list",
  "command",
  "commit",
@@ -137,7 +184,6 @@
  "createrule",
  "dbsize",
  "debug_object",
- "debug_segfault",
  "debug_sleep",
  "debug",
  "decr",
@@ -160,10 +206,8 @@
  "exists",
  "expire",
  "expireat",
- "expiretime",
  "explain_cli",
  "explain",
- "failover",
  "fcall_ro",
  "fcall",
  "flushall",
@@ -193,7 +237,6 @@
  "getrange",
  "getset",
  "hdel",
- "hello",
  "hexists",
  "hget",
  "hgetall",
@@ -221,13 +264,9 @@
  "insertnx",
  "keys",
  "lastsave",
- "latency_doctor",
- "latency_graph",
- "latency_histogram",
  "latency_history",
  "latency_latest",
  "latency_reset",
- "lcs",
  "lindex",
  "linsert",
  "list",
@@ -236,8 +275,6 @@
  "lmpop",
  "loadchunk",
  "lolwut",
- "lpop",
- "lpos",
  "lpush",
  "lpushx",
  "lrange",
@@ -246,8 +283,6 @@
  "ltrim",
  "madd",
  "max",
- "memory_doctor",
- "memory_help",
  "memory_malloc_stats",
  "memory_purge",
  "memory_stats",
@@ -262,7 +297,6 @@
  "module_load",
  "module_loadex",
  "module_unload",
- "monitor",
  "move",
  "mrange",
  "mrevrange",
@@ -278,23 +312,19 @@
  "persist",
  "pexpire",
  "pexpireat",
- "pexpiretime",
  "pfadd",
  "pfcount",
  "pfmerge",
  "ping",
  "profile",
- "psetex",
  "psubscribe",
- "psync",
  "pttl",
  "publish",
  "pubsub_channels",
  "pubsub_numpat",
  "pubsub_numsub",
  "pubsub_shardchannels",
  "pubsub_shardnumsub",
- "pubsub",
  "punsubscribe",
  "quantile",
  "query",
@@ -306,7 +336,6 @@
  "readonly",
  "readwrite",
  "rename",
- "renamenx",
  "replicaof",
  "reserve",
  "reset",
@@ -315,7 +344,6 @@
  "revrange",
  "revrank",
  "role",
- "rpop",
  "rpoplpush",
  "rpush",
  "rpushx",
@@ -325,7 +353,6 @@
  "scan",
  "scandump",
  "scard",
- "script_debug",
  "script_exists",
  "script_flush",
  "script_kill",
@@ -334,24 +361,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",
@@ -364,12 +378,8 @@
  "smembers",
  "smismember",
  "smove",
- "sort_ro",
- "sort",
  "spellcheck",
  "spublish",
- "spop",
- "srandmember",
  "srem",
  "sscan_iter",
  "sscan",
@@ -402,10 +412,8 @@
  "type",
  "unlink",
  "unsubscribe",
- "unwatch",
  "wait",
  "waitaof",
- "watch",
  "xack",
  "xadd",
  "xautoclaim",
@@ -441,7 +449,6 @@
  "zinter",
  "zintercard",
  "zinterstore",
- "zlexcount",
  "zmpop",
  "zmscore",
  "zpopmax",
@@ -458,7 +465,6 @@
  "zremrangebyscore",
  "zrevrange",
  "zrevrangebylex",
- "zrevrangebyscore",
  "zrevrank",
  "zscan_iter",
  "zscan",
@@ -467,6 +473,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]+")
@@ -601,11 +609,10 @@ def instrument_redis_client(module):
 def instrument_asyncio_redis_client(module):
  if hasattr(module, "Redis"):
  class_ = getattr(module, "Redis")
- for operation in _redis_client_methods:
+ 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")
 

tests/datastore_aioredis/test_execute_command.py

@@ -14,14 +14,12 @@
 
 import pytest
 
-# import aioredis
 from conftest import AIOREDIS_VERSION, loop # noqa # pylint: disable=E0611,W0611
 from testing_support.db_settings import redis_settings
 from testing_support.fixtures import override_application_settings
 from testing_support.util import instance_hostname
 from testing_support.validators.validate_transaction_metrics import (
- validate_transaction_metrics,
-)
+ validate_transaction_metrics,)
 
 from newrelic.api.background_task import background_task
 
@@ -122,3 +120,4 @@ def test_redis_execute_command_as_two_args_enable(client, loop): # noqa
 @background_task()
 def test_redis_execute_command_as_two_args_disable(client, loop): # noqa
  loop.run_until_complete(exercise_redis_multi_args(client))
+