Instrument RedisCluster (#809)

* Add instrumentation for RedisCluster

* Add tests for redis cluster

Author: hmstepanek

.github/workflows/tests.yml

@@ -46,6 +46,7 @@ jobs:
       - postgres
       - rabbitmq
       - redis
+      - rediscluster
       - solr
 
     steps:
@@ -384,6 +385,105 @@ jobs:
           path: ./**/.coverage.*
           retention-days: 1
 
+  rediscluster:
+    env:
+      TOTAL_GROUPS: 1
+
+    strategy:
+      fail-fast: false
+      matrix:
+        group-number: [1]
+
+    runs-on: ubuntu-20.04
+    container:
+      image: ghcr.io/newrelic/newrelic-python-agent-ci:latest
+      options: >-
+        --add-host=host.docker.internal:host-gateway
+    timeout-minutes: 30
+
+    services:
+      redis1:
+        image: hmstepanek/redis-cluster-node:1.0.0
+        ports:
+          - 6379:6379
+          - 16379:16379
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+      redis2:
+        image: hmstepanek/redis-cluster-node:1.0.0
+        ports:
+          - 6380:6379
+          - 16380:16379
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+      redis3:
+        image: hmstepanek/redis-cluster-node:1.0.0
+        ports:
+          - 6381:6379
+          - 16381:16379
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+      redis4:
+        image: hmstepanek/redis-cluster-node:1.0.0
+        ports:
+          - 6382:6379
+          - 16382:16379
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+      redis5:
+        image: hmstepanek/redis-cluster-node:1.0.0
+        ports:
+          - 6383:6379
+          - 16383:16379
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+      redis6:
+        image: hmstepanek/redis-cluster-node:1.0.0
+        ports:
+          - 6384:6379
+          - 16384:16379
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+      cluster-setup:
+        image: hmstepanek/redis-cluster:1.0.0    
+        options: >-
+          --add-host=host.docker.internal:host-gateway
+
+    steps:
+      - uses: actions/checkout@v3
+
+      - name: Fetch git tags
+        run: |
+          git config --global --add safe.directory "$GITHUB_WORKSPACE"
+          git fetch --tags origin
+
+      - name: Get Environments
+        id: get-envs
+        run: |
+          echo "envs=$(tox -l | grep '^${{ github.job }}\-' | ./.github/workflows/get-envs.py)" >> $GITHUB_OUTPUT
+        env:
+          GROUP_NUMBER: ${{ matrix.group-number }}
+
+      - name: Test
+        run: |
+          tox -vv -e ${{ steps.get-envs.outputs.envs }} -p auto
+        env:
+          TOX_PARALLEL_NO_SPINNER: 1
+          PY_COLORS: 0
+
+      - name: Upload Coverage Artifacts
+        uses: actions/upload-artifact@v3
+        with:
+          name: coverage-${{ github.job }}-${{ strategy.job-index }}
+          path: ./**/.coverage.*
+          retention-days: 1
+
   redis:
     env:
       TOTAL_GROUPS: 2

newrelic/config.py

@@ -2849,6 +2849,10 @@ def _process_module_builtin_defaults():
     )
     _process_module_definition("redis.client", "newrelic.hooks.datastore_redis", "instrument_redis_client")
 
+    _process_module_definition(
+        "redis.commands.cluster", "newrelic.hooks.datastore_redis", "instrument_redis_commands_cluster"
+    )
+
     _process_module_definition(
         "redis.commands.core", "newrelic.hooks.datastore_redis", "instrument_redis_commands_core"
     )

newrelic/hooks/datastore_redis.py

@@ -597,6 +597,10 @@ def instrument_redis_commands_bf_commands(module):
     _instrument_redis_commands_module(module, "TOPKCommands")
 
 
+def instrument_redis_commands_cluster(module):
+    _instrument_redis_commands_module(module, "RedisClusterCommands")
+
+
 def _instrument_redis_commands_module(module, class_name):
     for name in _redis_client_methods:
         if hasattr(module, class_name):

setup.cfg

@@ -5,4 +5,4 @@ license_files =
 
 [flake8]
 max-line-length = 120
-extend-ignore = E122,E126,E127,E128,E203,E501,E722,F841,W504
+extend-ignore = E122,E126,E127,E128,E203,E501,E722,F841,W504,E731

tests/datastore_rediscluster/conftest.py

@@ -0,0 +1,32 @@
+# 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.
+
+from testing_support.fixtures import (  # noqa: F401; pylint: disable=W0611
+    collector_agent_registration_fixture,
+    collector_available_fixture,
+)
+
+_default_settings = {
+    "transaction_tracer.explain_threshold": 0.0,
+    "transaction_tracer.transaction_threshold": 0.0,
+    "transaction_tracer.stack_trace_threshold": 0.0,
+    "debug.log_data_collector_payloads": True,
+    "debug.record_transaction_failure": True,
+}
+
+collector_agent_registration = collector_agent_registration_fixture(
+    app_name="Python Agent Test (datastore_redis)",
+    default_settings=_default_settings,
+    linked_applications=["Python Agent Test (datastore)"],
+)

tests/datastore_rediscluster/test_uninstrumented_rediscluster_methods.py

@@ -0,0 +1,168 @@
+# 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 redis
+from testing_support.db_settings import redis_cluster_settings
+
+DB_CLUSTER_SETTINGS = redis_cluster_settings()[0]
+
+# Set socket_timeout to 5s for fast fail, otherwise the default is to wait forever.
+client = redis.RedisCluster(host=DB_CLUSTER_SETTINGS["host"], port=DB_CLUSTER_SETTINGS["port"], socket_timeout=5)
+
+IGNORED_METHODS = {
+    "MODULE_CALLBACKS",
+    "MODULE_VERSION",
+    "NAME",
+    "add_edge",
+    "add_node",
+    "append_bucket_size",
+    "append_capacity",
+    "append_error",
+    "append_expansion",
+    "append_items_and_increments",
+    "append_items",
+    "append_max_iterations",
+    "append_no_create",
+    "append_no_scale",
+    "append_values_and_weights",
+    "append_weights",
+    "batch_indexer",
+    "BatchIndexer",
+    "bulk",
+    "call_procedure",
+    "client_tracking_off",
+    "client_tracking_on",
+    "client",
+    "close",
+    "commandmixin",
+    "connection_pool",
+    "connection",
+    "debug_segfault",
+    "edges",
+    "execute_command",
+    "flush",
+    "from_url",
+    "get_connection_kwargs",
+    "get_encoder",
+    "get_label",
+    "get_params_args",
+    "get_property",
+    "get_relation",
+    "get_retry",
+    "hscan_iter",
+    "index_name",
+    "labels",
+    "list_keys",
+    "load_document",
+    "load_external_module",
+    "lock",
+    "name",
+    "nodes",
+    "parse_response",
+    "pipeline",
+    "property_keys",
+    "register_script",
+    "relationship_types",
+    "response_callbacks",
+    "RESPONSE_CALLBACKS",
+    "sentinel",
+    "set_file",
+    "set_path",
+    "set_response_callback",
+    "set_retry",
+    "transaction",
+    "version",
+    "ALL_NODES",
+    "CLUSTER_COMMANDS_RESPONSE_CALLBACKS",
+    "COMMAND_FLAGS",
+    "DEFAULT_NODE",
+    "ERRORS_ALLOW_RETRY",
+    "NODE_FLAGS",
+    "PRIMARIES",
+    "RANDOM",
+    "REPLICAS",
+    "RESULT_CALLBACKS",
+    "RedisClusterRequestTTL",
+    "SEARCH_COMMANDS",
+    "client_no_touch",
+    "cluster_addslotsrange",
+    "cluster_bumpepoch",
+    "cluster_delslotsrange",
+    "cluster_error_retry_attempts",
+    "cluster_flushslots",
+    "cluster_links",
+    "cluster_myid",
+    "cluster_myshardid",
+    "cluster_replicas",
+    "cluster_response_callbacks",
+    "cluster_setslot_stable",
+    "cluster_shards",
+    "command_flags",
+    "commands_parser",
+    "determine_slot",
+    "disconnect_connection_pools",
+    "encoder",
+    "get_default_node",
+    "get_node",
+    "get_node_from_key",
+    "get_nodes",
+    "get_primaries",
+    "get_random_node",
+    "get_redis_connection",
+    "get_replicas",
+    "keyslot",
+    "mget_nonatomic",
+    "monitor",
+    "mset_nonatomic",
+    "node_flags",
+    "nodes_manager",
+    "on_connect",
+    "pubsub",
+    "read_from_replicas",
+    "reinitialize_counter",
+    "reinitialize_steps",
+    "replace_default_node",
+    "result_callbacks",
+    "set_default_node",
+    "user_on_connect_func",
+}
+
+REDIS_MODULES = {
+    "bf",
+    "cf",
+    "cms",
+    "ft",
+    "graph",
+    "json",
+    "tdigest",
+    "topk",
+    "ts",
+}
+
+IGNORED_METHODS |= REDIS_MODULES
+
+
+def test_uninstrumented_methods():
+    methods = {m for m in dir(client) if not m[0] == "_"}
+    is_wrapped = lambda m: hasattr(getattr(client, m), "__wrapped__")
+    uninstrumented = {m for m in methods - IGNORED_METHODS if not is_wrapped(m)}
+
+    for module in REDIS_MODULES:
+        if hasattr(client, module):
+            module_client = getattr(client, module)()
+            module_methods = {m for m in dir(module_client) if not m[0] == "_"}
+            is_wrapped = lambda m: hasattr(getattr(module_client, m), "__wrapped__")
+            uninstrumented |= {m for m in module_methods - IGNORED_METHODS if not is_wrapped(m)}
+
+    assert not uninstrumented, "Uninstrumented methods: %s" % sorted(uninstrumented)

tests/testing_support/db_settings.py

@@ -121,6 +121,31 @@ def redis_settings():
     return settings
 
 
+def redis_cluster_settings():
+    """Return a list of dict of settings for connecting to redis cluster.
+
+    Will return the correct settings, depending on which of the environments it
+    is running in. It attempts to set variables in the following order, where
+    later environments override earlier ones.
+
+        1. Local
+        2. Github Actions
+    """
+
+    host = "host.docker.internal" if "GITHUB_ACTIONS" in os.environ else "localhost"
+    instances = 1
+    base_port = 6379
+
+    settings = [
+        {
+            "host": host,
+            "port": base_port + instance_num,
+        }
+        for instance_num in range(instances)
+    ]
+    return settings
+
+
 def memcached_settings():
     """Return a list of dict of settings for connecting to memcached.
 

tox.ini

@@ -96,6 +96,7 @@ envlist =
     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,
@@ -254,6 +255,7 @@ deps =
     datastore_pymysql: PyMySQL<0.11
     datastore_pysolr: pysolr<4.0
     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
@@ -457,6 +459,7 @@ changedir =
     datastore_pymysql: tests/datastore_pymysql
     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