From eef691666141acd4c4bd63332deb7861aa2834df Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Mon, 9 Oct 2023 11:14:19 -0700 Subject: [PATCH 1/4] Pin flask version for flask restx tests. (#931) --- tox.ini | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tox.ini b/tox.ini index b142cd840..f64eb82b1 100644 --- a/tox.ini +++ b/tox.ini @@ -208,6 +208,8 @@ deps = component_flask_rest: jinja2 component_flask_rest: itsdangerous component_flask_rest-flaskrestxlatest: flask-restx + ; Pin Flask version until flask-restx is updated to support v3 + component_flask_rest-flaskrestxlatest: flask<3.0 component_flask_rest-flaskrestx051: flask-restx<1.0 component_graphqlserver: graphql-server[sanic,flask]==3.0.0b5 component_graphqlserver: sanic>20 From d577a69eab49fc6d9d4b1eca6feb71a149aed206 Mon Sep 17 00:00:00 2001 From: Uma Annamalai Date: Mon, 9 Oct 2023 11:29:20 -0700 Subject: [PATCH 2/4] Ignore new redis methods. (#932) Co-authored-by: Lalleh Rafeei <84813886+lrafeei@users.noreply.github.com> --- tests/datastore_redis/test_uninstrumented_methods.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/datastore_redis/test_uninstrumented_methods.py b/tests/datastore_redis/test_uninstrumented_methods.py index ccf5a096d..d86f4de95 100644 --- a/tests/datastore_redis/test_uninstrumented_methods.py +++ b/tests/datastore_redis/test_uninstrumented_methods.py @@ -39,6 +39,7 @@ "append_no_scale", "append_values_and_weights", "append_weights", + "auto_close_connection_pool", "batch_indexer", "BatchIndexer", "bulk", @@ -55,6 +56,7 @@ "edges", "execute_command", "flush", + "from_pool", "from_url", "get_connection_kwargs", "get_encoder", From 13e9891e1ef61671c35b6bbd91885de4e9dd48ef Mon Sep 17 00:00:00 2001 From: Timothy Pansino <11214426+TimPansino@users.noreply.github.com> Date: Mon, 9 Oct 2023 12:42:47 -0700 Subject: [PATCH 3/4] Update CI Image (#930) * Update available python versions in CI * Update makefile with overrides * Fix default branch detection for arm builds --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .github/containers/Dockerfile | 2 +- .github/containers/Makefile | 61 +++++++++++++++++++--------- .github/workflows/build-ci-image.yml | 2 +- 3 files changed, 44 insertions(+), 21 deletions(-) diff --git a/.github/containers/Dockerfile b/.github/containers/Dockerfile index d761b6f4a..57d8c234c 100644 --- a/.github/containers/Dockerfile +++ b/.github/containers/Dockerfile @@ -96,7 +96,7 @@ RUN echo 'eval "$(pyenv init -)"' >>$HOME/.bashrc && \ pyenv update # Install Python -ARG PYTHON_VERSIONS="3.10 3.9 3.8 3.7 3.11 2.7 pypy2.7-7.3.12 pypy3.8-7.3.11" +ARG PYTHON_VERSIONS="3.11 3.10 3.9 3.8 3.7 3.12 2.7 pypy2.7-7.3.12 pypy3.8-7.3.11" COPY --chown=1000:1000 --chmod=+x ./install-python.sh /tmp/install-python.sh RUN /tmp/install-python.sh && \ rm /tmp/install-python.sh diff --git a/.github/containers/Makefile b/.github/containers/Makefile index 4c057813d..97b4e7256 100644 --- a/.github/containers/Makefile +++ b/.github/containers/Makefile @@ -12,37 +12,60 @@ # See the License for the specific language governing permissions and # limitations under the License. -# Repository root for mounting into container. -MAKEFILE_DIR:=$(dir $(realpath $(firstword $(MAKEFILE_LIST)))) -REPO_ROOT:=$(realpath $(MAKEFILE_DIR)../../) +# Override constants +PLATFORM_OVERRIDE:= +PYTHON_VERSIONS_OVERRIDE:= + +# Computed variables +IMAGE_NAME:=ghcr.io/newrelic/newrelic-python-agent-ci +MAKEFILE_DIR:=$(dir $(realpath $(firstword ${MAKEFILE_LIST}))) +REPO_ROOT:=$(realpath ${MAKEFILE_DIR}../../) +UNAME_P:=$(shell uname -p) +PLATFORM_AUTOMATIC:=$(if $(findstring arm,${UNAME_P}),linux/arm64,linux/amd64) +PLATFORM:=$(if ${PLATFORM_OVERRIDE},${PLATFORM_OVERRIDE},${PLATFORM_AUTOMATIC}) +PYTHON_VERSIONS_AUTOMATIC:=3.10 2.7 +PYTHON_VERSIONS:=$(if ${PYTHON_VERSIONS_OVERRIDE},${PYTHON_VERSIONS_OVERRIDE},${PYTHON_VERSIONS_AUTOMATIC}) .PHONY: default default: test -# Perform a shortened build for testing .PHONY: build build: - @docker build $(MAKEFILE_DIR) \ - -t ghcr.io/newrelic/newrelic-python-agent-ci:local \ - --build-arg='PYTHON_VERSIONS=3.10 2.7' - -# Ensure python versions are usable -.PHONY: test -test: build - @docker run --rm ghcr.io/newrelic/python-agent-ci:local /bin/bash -c '\ - python3.10 --version && \ - python2.7 --version && \ - touch tox.ini && tox --version && \ - echo "Success! Python versions installed."' + @docker build ${MAKEFILE_DIR} \ + --platform=${PLATFORM} \ + -t ${IMAGE_NAME}:local \ + --build-arg='PYTHON_VERSIONS=${PYTHON_VERSIONS}' +# Run the local tag as a container. .PHONY: run -run: build +run: run.local + +# Run a specific tag as a container. +# Usage: make run. +# Defaults to run.local, but can instead be run.latest or any other tag. +.PHONY: run.% +run.%: +# Build image if local was specified, else pull latest + @if [[ "$*" = "local" ]]; then cd ${MAKEFILE_DIR} && $(MAKE) build; else docker pull ${IMAGE_NAME}:$*; fi @docker run --rm -it \ - --mount type=bind,source="$(REPO_ROOT)",target=/home/github/python-agent \ + --platform=${PLATFORM} \ + --mount type=bind,source="${REPO_ROOT}",target=/home/github/python-agent \ --workdir=/home/github/python-agent \ --add-host=host.docker.internal:host-gateway \ -e NEW_RELIC_HOST="${NEW_RELIC_HOST}" \ -e NEW_RELIC_LICENSE_KEY="${NEW_RELIC_LICENSE_KEY}" \ -e NEW_RELIC_DEVELOPER_MODE="${NEW_RELIC_DEVELOPER_MODE}" \ -e GITHUB_ACTIONS="true" \ - ghcr.io/newrelic/newrelic-python-agent-ci:local /bin/bash + ${IMAGE_NAME}:$* /bin/bash + +# Ensure python versions are usable. Cannot be automatically used with PYTHON_VERSIONS_OVERRIDE. +.PHONY: test +test: build + @docker run --rm \ + --platform=${PLATFORM} \ + ghcr.io/newrelic/python-agent-ci:local \ + /bin/bash -c '\ + python3.10 --version && \ + python2.7 --version && \ + touch tox.ini && tox --version && \ + echo "Success! Python versions installed."' diff --git a/.github/workflows/build-ci-image.yml b/.github/workflows/build-ci-image.yml index 8bd904661..9d60cea8e 100644 --- a/.github/workflows/build-ci-image.yml +++ b/.github/workflows/build-ci-image.yml @@ -63,6 +63,6 @@ jobs: with: push: ${{ github.event_name != 'pull_request' }} context: .github/containers - platforms: ${{ (github.ref == 'refs/head/main') && 'linux/amd64,linux/arm64' || 'linux/amd64' }} + platforms: ${{ (format('refs/heads/{0}', github.event.repository.default_branch) == github.ref) && 'linux/amd64,linux/arm64' || 'linux/amd64' }} tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} From 43160affcba5e598f1d702f1e5b933627976a270 Mon Sep 17 00:00:00 2001 From: Hannah Stepanek Date: Mon, 9 Oct 2023 15:53:06 -0700 Subject: [PATCH 4/4] Only get package version once (#928) * Only get package version once * Add disconnect method * Add disconnect method --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- newrelic/hooks/datastore_aioredis.py | 8 +- .../datastore_redis/test_custom_conn_pool.py | 114 +++++++++--------- 2 files changed, 65 insertions(+), 57 deletions(-) diff --git a/newrelic/hooks/datastore_aioredis.py b/newrelic/hooks/datastore_aioredis.py index 03c0f0900..e27f8d7a9 100644 --- a/newrelic/hooks/datastore_aioredis.py +++ b/newrelic/hooks/datastore_aioredis.py @@ -22,6 +22,8 @@ _redis_operation_re, ) +AIOREDIS_VERSION = get_package_version_tuple("aioredis") + def _conn_attrs_to_dict(connection): host = getattr(connection, "host", None) @@ -58,14 +60,13 @@ def _nr_wrapper_AioRedis_method_(wrapped, instance, args, kwargs): # Check for transaction and return early if found. # Method will return synchronously without executing, # it will be added to the command stack and run later. - aioredis_version = get_package_version_tuple("aioredis") # 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 + 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 @@ -75,7 +76,7 @@ def _nr_wrapper_AioRedis_method_(wrapped, instance, args, kwargs): return wrapped(*args, **kwargs) else: # AioRedis v2 uses a Pipeline object for a client and internally queues up pipeline commands - if aioredis_version: + if AIOREDIS_VERSION: from aioredis.client import Pipeline if isinstance(instance, Pipeline): return wrapped(*args, **kwargs) @@ -139,6 +140,7 @@ async def wrap_Connection_send_command(wrapped, instance, args, kwargs): ): return await wrapped(*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 diff --git a/tests/datastore_redis/test_custom_conn_pool.py b/tests/datastore_redis/test_custom_conn_pool.py index 8e4503b75..b16a77f48 100644 --- a/tests/datastore_redis/test_custom_conn_pool.py +++ b/tests/datastore_redis/test_custom_conn_pool.py @@ -12,22 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. -''' The purpose of these tests is to confirm that using a non-standard +""" The purpose of these tests is to confirm that using a non-standard connection pool that does not have a `connection_kwargs` attribute will not result in an error. -''' +""" import pytest import redis - -from newrelic.api.background_task import background_task -from newrelic.common.package_version_utils import get_package_version_tuple - -from testing_support.fixtures import override_application_settings -from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics 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, +) +from newrelic.api.background_task import background_task +from newrelic.common.package_version_utils import get_package_version_tuple DB_SETTINGS = redis_settings()[0] REDIS_PY_VERSION = get_package_version_tuple("redis") @@ -45,13 +45,17 @@ def get_connection(self, name, *keys, **options): def release(self, connection): self.connection.disconnect() + def disconnect(self): + self.connection.disconnect() + + # Settings _enable_instance_settings = { - 'datastore_tracer.instance_reporting.enabled': True, + "datastore_tracer.instance_reporting.enabled": True, } _disable_instance_settings = { - 'datastore_tracer.instance_reporting.enabled': False, + "datastore_tracer.instance_reporting.enabled": False, } # Metrics @@ -61,98 +65,100 @@ def release(self, connection): datastore_all_metric_count = 5 if REDIS_PY_VERSION >= (5, 0) else 3 _base_scoped_metrics = [ - ('Datastore/operation/Redis/get', 1), - ('Datastore/operation/Redis/set', 1), - ('Datastore/operation/Redis/client_list', 1), + ("Datastore/operation/Redis/get", 1), + ("Datastore/operation/Redis/set", 1), + ("Datastore/operation/Redis/client_list", 1), ] # client_setinfo was introduced in v5.0.0 and assigns info displayed in client_list output if REDIS_PY_VERSION >= (5, 0): - _base_scoped_metrics.append(('Datastore/operation/Redis/client_setinfo', 2),) + _base_scoped_metrics.append( + ("Datastore/operation/Redis/client_setinfo", 2), + ) _base_rollup_metrics = [ - ('Datastore/all', datastore_all_metric_count), - ('Datastore/allOther', datastore_all_metric_count), - ('Datastore/Redis/all', datastore_all_metric_count), - ('Datastore/Redis/allOther', datastore_all_metric_count), - ('Datastore/operation/Redis/get', 1), - ('Datastore/operation/Redis/set', 1), - ('Datastore/operation/Redis/client_list', 1), + ("Datastore/all", datastore_all_metric_count), + ("Datastore/allOther", datastore_all_metric_count), + ("Datastore/Redis/all", datastore_all_metric_count), + ("Datastore/Redis/allOther", datastore_all_metric_count), + ("Datastore/operation/Redis/get", 1), + ("Datastore/operation/Redis/set", 1), + ("Datastore/operation/Redis/client_list", 1), ] if REDIS_PY_VERSION >= (5, 0): - _base_rollup_metrics.append(('Datastore/operation/Redis/client_setinfo', 2),) + _base_rollup_metrics.append( + ("Datastore/operation/Redis/client_setinfo", 2), + ) -_host = instance_hostname(DB_SETTINGS['host']) -_port = DB_SETTINGS['port'] +_host = instance_hostname(DB_SETTINGS["host"]) +_port = DB_SETTINGS["port"] -_instance_metric_name = 'Datastore/instance/Redis/%s/%s' % (_host, _port) +_instance_metric_name = "Datastore/instance/Redis/%s/%s" % (_host, _port) instance_metric_count = 5 if REDIS_PY_VERSION >= (5, 0) else 3 -_enable_rollup_metrics = _base_rollup_metrics.append( - (_instance_metric_name, instance_metric_count) -) +_enable_rollup_metrics = _base_rollup_metrics.append((_instance_metric_name, instance_metric_count)) -_disable_rollup_metrics = _base_rollup_metrics.append( - (_instance_metric_name, None) -) +_disable_rollup_metrics = _base_rollup_metrics.append((_instance_metric_name, None)) # Operations + def exercise_redis(client): - client.set('key', 'value') - client.get('key') - client.execute_command('CLIENT', 'LIST', parse='LIST') + client.set("key", "value") + client.get("key") + client.execute_command("CLIENT", "LIST", parse="LIST") + # Tests -@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7), - reason='Client list command introduced in 2.7') + +@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7), reason="Client list command introduced in 2.7") @override_application_settings(_enable_instance_settings) @validate_transaction_metrics( - 'test_custom_conn_pool:test_fake_conn_pool_enable_instance', - scoped_metrics=_base_scoped_metrics, - rollup_metrics=_enable_rollup_metrics, - background_task=True) + "test_custom_conn_pool:test_fake_conn_pool_enable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_enable_rollup_metrics, + background_task=True, +) @background_task() def test_fake_conn_pool_enable_instance(): - client = redis.StrictRedis(host=DB_SETTINGS['host'], - port=DB_SETTINGS['port'], db=0) + client = redis.StrictRedis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) # Get a real connection - conn = client.connection_pool.get_connection('GET') + conn = client.connection_pool.get_connection("GET") # Replace the original connection pool with one that doesn't # have the `connection_kwargs` attribute. fake_pool = FakeConnectionPool(conn) client.connection_pool = fake_pool - assert not hasattr(client.connection_pool, 'connection_kwargs') + assert not hasattr(client.connection_pool, "connection_kwargs") exercise_redis(client) -@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7), - reason='Client list command introduced in 2.7') + +@pytest.mark.skipif(REDIS_PY_VERSION < (2, 7), reason="Client list command introduced in 2.7") @override_application_settings(_disable_instance_settings) @validate_transaction_metrics( - 'test_custom_conn_pool:test_fake_conn_pool_disable_instance', - scoped_metrics=_base_scoped_metrics, - rollup_metrics=_disable_rollup_metrics, - background_task=True) + "test_custom_conn_pool:test_fake_conn_pool_disable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_disable_rollup_metrics, + background_task=True, +) @background_task() def test_fake_conn_pool_disable_instance(): - client = redis.StrictRedis(host=DB_SETTINGS['host'], - port=DB_SETTINGS['port'], db=0) + client = redis.StrictRedis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) # Get a real connection - conn = client.connection_pool.get_connection('GET') + conn = client.connection_pool.get_connection("GET") # Replace the original connection pool with one that doesn't # have the `connection_kwargs` attribute. fake_pool = FakeConnectionPool(conn) client.connection_pool = fake_pool - assert not hasattr(client.connection_pool, 'connection_kwargs') + assert not hasattr(client.connection_pool, "connection_kwargs") exercise_redis(client)