From cc3e285c7075eb4aa45de45228af1fb15b2eca30 Mon Sep 17 00:00:00 2001 From: Timothy Pansino <11214426+TimPansino@users.noreply.github.com> Date: Thu, 19 Oct 2023 09:54:06 -0700 Subject: [PATCH 01/15] Cache Package Version Lookups (#946) * Cache _get_package_version * Add Python 2.7 support to get_package_version caching * [Mega-Linter] Apply linters fixes * Bump tests --------- Co-authored-by: SlavaSkvortsov <29122694+SlavaSkvortsov@users.noreply.github.com> Co-authored-by: TimPansino --- newrelic/common/package_version_utils.py | 41 ++++++++++++++++++- .../test_package_version_utils.py | 24 ++++++++++- 2 files changed, 62 insertions(+), 3 deletions(-) diff --git a/newrelic/common/package_version_utils.py b/newrelic/common/package_version_utils.py index 3152342b4d..68320b897f 100644 --- a/newrelic/common/package_version_utils.py +++ b/newrelic/common/package_version_utils.py @@ -14,6 +14,44 @@ import sys +try: + from functools import cache as _cache_package_versions +except ImportError: + from functools import wraps + from threading import Lock + + _package_version_cache = {} + _package_version_cache_lock = Lock() + + def _cache_package_versions(wrapped): + """ + Threadsafe implementation of caching for _get_package_version. + + Python 2.7 does not have the @functools.cache decorator, and + must be reimplemented with support for clearing the cache. + """ + + @wraps(wrapped) + def _wrapper(name): + if name in _package_version_cache: + return _package_version_cache[name] + + with _package_version_cache_lock: + if name in _package_version_cache: + return _package_version_cache[name] + + version = _package_version_cache[name] = wrapped(name) + return version + + def cache_clear(): + """Cache clear function to mimic @functools.cache""" + with _package_version_cache_lock: + _package_version_cache.clear() + + _wrapper.cache_clear = cache_clear + return _wrapper + + # Need to account for 4 possible variations of version declaration specified in (rejected) PEP 396 VERSION_ATTRS = ("__version__", "version", "__version_tuple__", "version_tuple") # nosec NULL_VERSIONS = frozenset((None, "", "0", "0.0", "0.0.0", "0.0.0.0", (0,), (0, 0), (0, 0, 0), (0, 0, 0, 0))) # nosec @@ -67,6 +105,7 @@ def int_or_str(value): return version +@_cache_package_versions def _get_package_version(name): module = sys.modules.get(name, None) version = None @@ -75,7 +114,7 @@ def _get_package_version(name): if "importlib" in sys.modules and hasattr(sys.modules["importlib"], "metadata"): try: # In Python3.10+ packages_distribution can be checked for as well - if hasattr(sys.modules["importlib"].metadata, "packages_distributions"): # pylint: disable=E1101 + if hasattr(sys.modules["importlib"].metadata, "packages_distributions"): # pylint: disable=E1101 distributions = sys.modules["importlib"].metadata.packages_distributions() # pylint: disable=E1101 distribution_name = distributions.get(name, name) else: diff --git a/tests/agent_unittests/test_package_version_utils.py b/tests/agent_unittests/test_package_version_utils.py index 30c22cff18..5ed689ea2a 100644 --- a/tests/agent_unittests/test_package_version_utils.py +++ b/tests/agent_unittests/test_package_version_utils.py @@ -20,6 +20,7 @@ from newrelic.common.package_version_utils import ( NULL_VERSIONS, VERSION_ATTRS, + _get_package_version, get_package_version, get_package_version_tuple, ) @@ -31,7 +32,7 @@ # such as distribution_packages and removed pkg_resources. IS_PY38_PLUS = sys.version_info[:2] >= (3, 8) -IS_PY310_PLUS = sys.version_info[:2] >= (3,10) +IS_PY310_PLUS = sys.version_info[:2] >= (3, 10) SKIP_IF_NOT_IMPORTLIB_METADATA = pytest.mark.skipif(not IS_PY38_PLUS, reason="importlib.metadata is not supported.") SKIP_IF_IMPORTLIB_METADATA = pytest.mark.skipif( IS_PY38_PLUS, reason="importlib.metadata is preferred over pkg_resources." @@ -46,7 +47,13 @@ def patched_pytest_module(monkeypatch): monkeypatch.delattr(pytest, attr) yield pytest - + + +@pytest.fixture(scope="function", autouse=True) +def cleared_package_version_cache(): + """Ensure cache is empty before every test to exercise code paths.""" + _get_package_version.cache_clear() + # This test only works on Python 3.7 @SKIP_IF_IMPORTLIB_METADATA @@ -123,3 +130,16 @@ def test_mapping_import_to_distribution_packages(): def test_pkg_resources_metadata(): version = get_package_version("pytest") assert version not in NULL_VERSIONS, version + + +def test_version_caching(monkeypatch): + # Add fake module to be deleted later + sys.modules["mymodule"] = sys.modules["pytest"] + setattr(pytest, "__version__", "1.0.0") + version = get_package_version("mymodule") + assert version not in NULL_VERSIONS, version + + # Ensure after deleting that the call to _get_package_version still completes because of caching + del sys.modules["mymodule"] + version = get_package_version("mymodule") + assert version not in NULL_VERSIONS, version From 5996de662544f77310b5dd5da0ed425989f96357 Mon Sep 17 00:00:00 2001 From: Timothy Pansino <11214426+TimPansino@users.noreply.github.com> Date: Thu, 19 Oct 2023 10:46:27 -0700 Subject: [PATCH 02/15] Fix Redis Generator Methods (#947) * Fix scan_iter for redis * Replace generator methods * Update instance info instrumentation * Remove mistake from uninstrumented methods * Add skip condition to asyncio generator tests * Add skip condition to asyncio generator tests --------- Co-authored-by: Lalleh Rafeei Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- newrelic/hooks/datastore_redis.py | 74 +++-- tests/datastore_redis/conftest.py | 1 + tests/datastore_redis/test_generators.py | 258 ++++++++++++++++++ .../test_uninstrumented_methods.py | 1 - 4 files changed, 294 insertions(+), 40 deletions(-) create mode 100644 tests/datastore_redis/test_generators.py diff --git a/newrelic/hooks/datastore_redis.py b/newrelic/hooks/datastore_redis.py index 6ba1920029..bbc517586d 100644 --- a/newrelic/hooks/datastore_redis.py +++ b/newrelic/hooks/datastore_redis.py @@ -14,10 +14,11 @@ import re -from newrelic.api.datastore_trace import DatastoreTrace +from newrelic.api.datastore_trace import DatastoreTrace, DatastoreTraceWrapper, wrap_datastore_trace from newrelic.api.time_trace import current_trace from newrelic.api.transaction import current_transaction -from newrelic.common.object_wrapper import function_wrapper, wrap_function_wrapper +from newrelic.common.object_wrapper import wrap_function_wrapper +from newrelic.common.async_wrapper import coroutine_wrapper, async_generator_wrapper, generator_wrapper _redis_client_sync_methods = { "acl_dryrun", @@ -136,6 +137,7 @@ "client_no_evict", "client_pause", "client_reply", + "client_setinfo", "client_setname", "client_tracking", "client_trackinginfo", @@ -162,7 +164,6 @@ "cluster_reset", "cluster_save_config", "cluster_set_config_epoch", - "client_setinfo", "cluster_setslot", "cluster_slaves", "cluster_slots", @@ -248,7 +249,7 @@ "hmset_dict", "hmset", "hrandfield", - "hscan_inter", + "hscan_iter", "hscan", "hset", "hsetnx", @@ -399,8 +400,8 @@ "syndump", "synupdate", "tagvals", - "tfcall", "tfcall_async", + "tfcall", "tfunction_delete", "tfunction_list", "tfunction_load", @@ -473,6 +474,13 @@ "zunionstore", } +_redis_client_gen_methods = { + "scan_iter", + "hscan_iter", + "sscan_iter", + "zscan_iter", +} + _redis_client_methods = _redis_client_sync_methods.union(_redis_client_async_methods) _redis_multipart_commands = set(["client", "cluster", "command", "config", "debug", "sentinel", "slowlog", "script"]) @@ -498,50 +506,31 @@ def _instance_info(kwargs): def _wrap_Redis_method_wrapper_(module, instance_class_name, operation): - def _nr_wrapper_Redis_method_(wrapped, instance, args, kwargs): - transaction = current_transaction() - - if transaction is None: - return wrapped(*args, **kwargs) - - dt = DatastoreTrace(product="Redis", target=None, operation=operation, source=wrapped) - - transaction._nr_datastore_instance_info = (None, None, None) - - with dt: - result = wrapped(*args, **kwargs) - - host, port_path_or_id, db = transaction._nr_datastore_instance_info - dt.host = host - dt.port_path_or_id = port_path_or_id - dt.database_name = db - - return result - name = "%s.%s" % (instance_class_name, operation) - wrap_function_wrapper(module, name, _nr_wrapper_Redis_method_) + if operation in _redis_client_gen_methods: + async_wrapper = generator_wrapper + else: + async_wrapper = None + wrap_datastore_trace(module, name, product="Redis", target=None, operation=operation, async_wrapper=async_wrapper) -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 _wrap_asyncio_Redis_method_wrapper(module, instance_class_name, operation): 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) + # Method should be run when awaited or iterated, therefore we wrap in an async wrapper. + return DatastoreTraceWrapper(wrapped, product="Redis", target=None, operation=operation, async_wrapper=async_wrapper)(*args, **kwargs) name = "%s.%s" % (instance_class_name, operation) + if operation in _redis_client_gen_methods: + async_wrapper = async_generator_wrapper + else: + async_wrapper = coroutine_wrapper + wrap_function_wrapper(module, name, _nr_wrapper_asyncio_Redis_method_) @@ -614,7 +603,15 @@ def _nr_Connection_send_command_wrapper_(wrapped, instance, args, kwargs): except: pass - transaction._nr_datastore_instance_info = (host, port_path_or_id, db) + # Find DatastoreTrace no matter how many other traces are inbetween + trace = current_trace() + while trace is not None and not isinstance(trace, DatastoreTrace): + trace = getattr(trace, "parent", None) + + if trace is not None: + trace.host = host + trace.port_path_or_id = port_path_or_id + trace.database_name = db # Older Redis clients would when sending multi part commands pass # them in as separate arguments to send_command(). Need to therefore @@ -666,7 +663,6 @@ def instrument_asyncio_redis_client(module): 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/conftest.py b/tests/datastore_redis/conftest.py index 53ff2658de..6747039b47 100644 --- a/tests/datastore_redis/conftest.py +++ b/tests/datastore_redis/conftest.py @@ -15,6 +15,7 @@ import pytest from testing_support.fixtures import collector_agent_registration_fixture, collector_available_fixture # noqa: F401; pylint: disable=W0611 +from testing_support.fixture.event_loop import event_loop as loop # noqa: F401; pylint: disable=W0611 _default_settings = { diff --git a/tests/datastore_redis/test_generators.py b/tests/datastore_redis/test_generators.py new file mode 100644 index 0000000000..f747838e19 --- /dev/null +++ b/tests/datastore_redis/test_generators.py @@ -0,0 +1,258 @@ +# 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 pytest +import redis +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.api.datastore_trace import DatastoreTrace +from newrelic.api.time_trace import current_trace +from newrelic.common.package_version_utils import get_package_version_tuple + +DB_SETTINGS = redis_settings()[0] +REDIS_PY_VERSION = get_package_version_tuple("redis") + +# Settings + +_enable_instance_settings = { + "datastore_tracer.instance_reporting.enabled": True, +} +_disable_instance_settings = { + "datastore_tracer.instance_reporting.enabled": False, +} + +# Metrics + +_base_scoped_metrics = ( + ("Datastore/operation/Redis/scan_iter", 1), + ("Datastore/operation/Redis/sscan_iter", 1), + ("Datastore/operation/Redis/zscan_iter", 1), + ("Datastore/operation/Redis/hscan_iter", 1), + ("Datastore/operation/Redis/set", 1), + ("Datastore/operation/Redis/sadd", 1), + ("Datastore/operation/Redis/zadd", 1), + ("Datastore/operation/Redis/hset", 1), +) + +_base_rollup_metrics = ( + ("Datastore/all", 8), + ("Datastore/allOther", 8), + ("Datastore/Redis/all", 8), + ("Datastore/Redis/allOther", 8), + ("Datastore/operation/Redis/scan_iter", 1), + ("Datastore/operation/Redis/sscan_iter", 1), + ("Datastore/operation/Redis/zscan_iter", 1), + ("Datastore/operation/Redis/hscan_iter", 1), + ("Datastore/operation/Redis/set", 1), + ("Datastore/operation/Redis/sadd", 1), + ("Datastore/operation/Redis/zadd", 1), + ("Datastore/operation/Redis/hset", 1), +) + +_disable_rollup_metrics = list(_base_rollup_metrics) +_enable_rollup_metrics = list(_base_rollup_metrics) + +_host = instance_hostname(DB_SETTINGS["host"]) +_port = DB_SETTINGS["port"] + +_instance_metric_name = "Datastore/instance/Redis/%s/%s" % (_host, _port) + +_enable_rollup_metrics.append((_instance_metric_name, 8)) + +_disable_rollup_metrics.append((_instance_metric_name, None)) + +# Operations + + +def exercise_redis(client): + """ + Exercise client generators by iterating on various methods and ensuring they are + non-empty, and that traces are started and stopped with the generator. + """ + + # Set existing values + client.set("scan-key", "value") + client.sadd("sscan-key", "value") + client.zadd("zscan-key", {"value": 1}) + client.hset("hscan-key", "field", "value") + + # Check generators + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + for k in client.scan_iter("scan-*"): + assert k == b"scan-key" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + for k in client.sscan_iter("sscan-key"): + assert k == b"value" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + for k, _ in client.zscan_iter("zscan-key"): + assert k == b"value" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + for f, v in client.hscan_iter("hscan-key"): + assert f == b"field" + assert v == b"value" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + +async def exercise_redis_async(client): + """ + Exercise client generators by iterating on various methods and ensuring they are + non-empty, and that traces are started and stopped with the generator. + """ + + # Set existing values + await client.set("scan-key", "value") + await client.sadd("sscan-key", "value") + await client.zadd("zscan-key", {"value": 1}) + await client.hset("hscan-key", "field", "value") + + # Check generators + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + async for k in client.scan_iter("scan-*"): + assert k == b"scan-key" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + async for k in client.sscan_iter("sscan-key"): + assert k == b"value" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + async for k, _ in client.zscan_iter("zscan-key"): + assert k == b"value" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + flag = False + assert not isinstance(current_trace(), DatastoreTrace) # Assert no active DatastoreTrace + async for f, v in client.hscan_iter("hscan-key"): + assert f == b"field" + assert v == b"value" + assert isinstance(current_trace(), DatastoreTrace) # Assert DatastoreTrace now active + flag = True + assert flag + + +# Tests + + +@override_application_settings(_enable_instance_settings) +@validate_transaction_metrics( + "test_generators:test_strict_redis_generator_enable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_enable_rollup_metrics, + background_task=True, +) +@background_task() +def test_strict_redis_generator_enable_instance(): + client = redis.StrictRedis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) + exercise_redis(client) + + +@override_application_settings(_disable_instance_settings) +@validate_transaction_metrics( + "test_generators:test_strict_redis_generator_disable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_disable_rollup_metrics, + background_task=True, +) +@background_task() +def test_strict_redis_generator_disable_instance(): + client = redis.StrictRedis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) + exercise_redis(client) + + +@override_application_settings(_enable_instance_settings) +@validate_transaction_metrics( + "test_generators:test_redis_generator_enable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_enable_rollup_metrics, + background_task=True, +) +@background_task() +def test_redis_generator_enable_instance(): + client = redis.Redis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) + exercise_redis(client) + + +@override_application_settings(_disable_instance_settings) +@validate_transaction_metrics( + "test_generators:test_redis_generator_disable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_disable_rollup_metrics, + background_task=True, +) +@background_task() +def test_redis_generator_disable_instance(): + client = redis.Redis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) + exercise_redis(client) + + +@pytest.mark.skipif(REDIS_PY_VERSION < (4, 2), reason="Redis.asyncio was not added until v4.2") +@override_application_settings(_enable_instance_settings) +@validate_transaction_metrics( + "test_generators:test_redis_async_generator_enable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_enable_rollup_metrics, + background_task=True, +) +@background_task() +def test_redis_async_generator_enable_instance(loop): + client = redis.asyncio.Redis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) + loop.run_until_complete(exercise_redis_async(client)) + + +@pytest.mark.skipif(REDIS_PY_VERSION < (4, 2), reason="Redis.asyncio was not added until v4.2") +@override_application_settings(_disable_instance_settings) +@validate_transaction_metrics( + "test_generators:test_redis_async_generator_disable_instance", + scoped_metrics=_base_scoped_metrics, + rollup_metrics=_disable_rollup_metrics, + background_task=True, +) +@background_task() +def test_redis_async_generator_disable_instance(loop): + client = redis.asyncio.Redis(host=DB_SETTINGS["host"], port=DB_SETTINGS["port"], db=0) + loop.run_until_complete(exercise_redis_async(client)) diff --git a/tests/datastore_redis/test_uninstrumented_methods.py b/tests/datastore_redis/test_uninstrumented_methods.py index d86f4de955..c0be684b2f 100644 --- a/tests/datastore_redis/test_uninstrumented_methods.py +++ b/tests/datastore_redis/test_uninstrumented_methods.py @@ -65,7 +65,6 @@ "get_property", "get_relation", "get_retry", - "hscan_iter", "index_name", "labels", "list_keys", From 47210252a77b41223601e7bc562dd1f5d31ab708 Mon Sep 17 00:00:00 2001 From: Timothy Pansino <11214426+TimPansino@users.noreply.github.com> Date: Mon, 23 Oct 2023 14:39:49 -0700 Subject: [PATCH 03/15] Automatic RPM System Updates (#948) * Checkout old action * Adding RPM action * Add dry run * Incorporating action into workflow * Wire secret into custom action * Enable action * Correct action name * Fix syntax * Fix quoting issues * Drop pre-verification. Does not work on python * Fix merge artifact --- .github/actions/update-rpm-config/action.yml | 109 +++++++++++++++++++ .github/workflows/deploy-python.yml | 10 ++ .github/workflows/tests.yml | 1 - 3 files changed, 119 insertions(+), 1 deletion(-) create mode 100644 .github/actions/update-rpm-config/action.yml diff --git a/.github/actions/update-rpm-config/action.yml b/.github/actions/update-rpm-config/action.yml new file mode 100644 index 0000000000..9d19ebba0b --- /dev/null +++ b/.github/actions/update-rpm-config/action.yml @@ -0,0 +1,109 @@ +name: "update-rpm-config" +description: "Set current version of agent in rpm config using API." +inputs: + agent-language: + description: "Language agent to configure (eg. python)" + required: true + default: "python" + target-system: + description: "Target System: prod|staging|all" + required: true + default: "all" + agent-version: + description: "3-4 digit agent version number (eg. 1.2.3) with optional leading v (ignored)" + required: true + dry-run: + description: "Dry Run" + required: true + default: "false" + production-api-key: + description: "API key for New Relic Production" + required: false + staging-api-key: + description: "API key for New Relic Staging" + required: false + +runs: + using: "composite" + steps: + - name: Trim potential leading v from agent version + shell: bash + run: | + AGENT_VERSION=${{ inputs.agent-version }} + echo "AGENT_VERSION=${AGENT_VERSION#"v"}" >> $GITHUB_ENV + + - name: Generate Payload + shell: bash + run: | + echo "PAYLOAD='{ \"system_configuration\": { \"key\": \"${{ inputs.agent-language }}_agent_version\", \"value\": \"${{ env.AGENT_VERSION }}\" } }'" >> $GITHUB_ENV + + - name: Generate Content-Type + shell: bash + run: | + echo "CONTENT_TYPE='Content-Type: application/json'" >> $GITHUB_ENV + + - name: Update Staging system configuration page + shell: bash + if: ${{ inputs.dry-run == 'false' && (inputs.target-system == 'staging' || inputs.target-system == 'all') }} + run: | + curl -X POST 'https://staging-api.newrelic.com/v2/system_configuration.json' \ + -H "X-Api-Key:${{ inputs.staging-api-key }}" -i \ + -H ${{ env.CONTENT_TYPE }} \ + -d ${{ env.PAYLOAD }} + + - name: Update Production system configuration page + shell: bash + if: ${{ inputs.dry-run == 'false' && (inputs.target-system == 'prod' || inputs.target-system == 'all') }} + run: | + curl -X POST 'https://api.newrelic.com/v2/system_configuration.json' \ + -H "X-Api-Key:${{ inputs.production-api-key }}" -i \ + -H ${{ env.CONTENT_TYPE }} \ + -d ${{ env.PAYLOAD }} + + - name: Verify Staging system configuration update + shell: bash + if: ${{ inputs.dry-run == 'false' && (inputs.target-system == 'staging' || inputs.target-system == 'all') }} + run: | + STAGING_VERSION=$(curl -X GET 'https://staging-api.newrelic.com/v2/system_configuration.json' \ + -H "X-Api-Key:${{ inputs.staging-api-key }}" \ + -H "${{ env.CONTENT_TYPE }}" | jq ".system_configurations | from_entries | .${{inputs.agent-language}}_agent_version") + + if [ "${{ env.AGENT_VERSION }}" != "$STAGING_VERSION" ]; then + echo "Staging version mismatch: $STAGING_VERSION" + exit 1 + fi + + - name: Verify Production system configuration update + shell: bash + if: ${{ inputs.dry-run == 'false' && (inputs.target-system == 'prod' || inputs.target-system == 'all') }} + run: | + PROD_VERSION=$(curl -X GET 'https://api.newrelic.com/v2/system_configuration.json' \ + -H "X-Api-Key:${{ inputs.production-api-key }}" \ + -H "${{ env.CONTENT_TYPE }}" | jq ".system_configurations | from_entries | .${{inputs.agent-language}}_agent_version") + + if [ "${{ env.AGENT_VERSION }}" != "$PROD_VERSION" ]; then + echo "Production version mismatch: $PROD_VERSION" + exit 1 + fi + + - name: (dry-run) Update Staging system configuration page + shell: bash + if: ${{ inputs.dry-run != 'false' && (inputs.target-system == 'staging' || inputs.target-system == 'all') }} + run: | + cat << EOF + curl -X POST 'https://staging-api.newrelic.com/v2/system_configuration.json' \ + -H "X-Api-Key:**REDACTED**" -i \ + -H ${{ env.CONTENT_TYPE }} \ + -d ${{ env.PAYLOAD }} + EOF + + - name: (dry-run) Update Production system configuration page + shell: bash + if: ${{ inputs.dry-run != 'false' && (inputs.target-system == 'prod' || inputs.target-system == 'all') }} + run: | + cat << EOF + curl -X POST 'https://api.newrelic.com/v2/system_configuration.json' \ + -H "X-Api-Key:**REDACTED**" -i \ + -H ${{ env.CONTENT_TYPE }} \ + -d ${{ env.PAYLOAD }} + EOF diff --git a/.github/workflows/deploy-python.yml b/.github/workflows/deploy-python.yml index fe16ee4854..ca908b8250 100644 --- a/.github/workflows/deploy-python.yml +++ b/.github/workflows/deploy-python.yml @@ -80,3 +80,13 @@ jobs: env: TWINE_USERNAME: __token__ TWINE_PASSWORD: ${{ secrets.PYPI_TOKEN }} + + - name: Update RPM Config + uses: ./.github/actions/update-rpm-config + with: + agent-language: "python" + target-system: "all" + agent-version: "${{ github.ref_name }}" + dry-run: "false" + production-api-key: ${{ secrets.NEW_RELIC_API_KEY_PRODUCTION }}" + staging-api-key: ${{ secrets.NEW_RELIC_API_KEY_STAGING }}" diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index e3b264a9fc..402d0c629c 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -62,7 +62,6 @@ jobs: steps: - uses: actions/checkout@v3 - - uses: actions/setup-python@v4 with: python-version: "3.10" From 21916848ab6c0f7f4a818718598f63ed788d7ae9 Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei <84813886+lrafeei@users.noreply.github.com> Date: Mon, 30 Oct 2023 15:43:30 -0700 Subject: [PATCH 04/15] Drop python 3.7 tests for Hypercorn (#954) --- tox.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.ini b/tox.ini index f64eb82b14..bdcaa745ab 100644 --- a/tox.ini +++ b/tox.ini @@ -49,7 +49,7 @@ envlist = python-adapter_daphne-{py37,py38,py39,py310,py311}-daphnelatest, python-adapter_gevent-{py27,py37,py38,py310,py311}, python-adapter_gunicorn-{py37,py38,py39,py310,py311}-aiohttp3-gunicornlatest, - python-adapter_hypercorn-{py37,py38,py39,py310,py311}-hypercornlatest, + python-adapter_hypercorn-{py38,py39,py310,py311}-hypercornlatest, python-adapter_hypercorn-py38-hypercorn{0010,0011,0012,0013}, python-adapter_uvicorn-{py37,py38,py39,py310,py311}-uvicorn{014,latest}, python-adapter_waitress-{py37,py38,py39,py310}-waitress02, From b2512eb145f0681c09c9743f63a3af5fe97f3780 Mon Sep 17 00:00:00 2001 From: Timothy Pansino <11214426+TimPansino@users.noreply.github.com> Date: Thu, 2 Nov 2023 14:49:30 -0700 Subject: [PATCH 05/15] Fix pyenv installation for devcontainer (#936) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- .devcontainer/Dockerfile | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index 7a7c6087d5..bc4a5324a1 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -11,4 +11,12 @@ RUN mkdir -p ${HOME} && \ groupadd --gid ${GID} vscode && \ useradd --uid ${UID} --gid ${GID} --home ${HOME} vscode && \ chown -R ${UID}:${GID} /home/vscode + +# Move pyenv installation +ENV PYENV_ROOT="${HOME}/.pyenv" +ENV PATH="$PYENV_ROOT/bin:$PYENV_ROOT/shims:${PATH}" +RUN mv /root/.pyenv /home/vscode/.pyenv && \ + chown -R vscode:vscode /home/vscode/.pyenv + +# Set user USER ${UID}:${GID} From b12f7be1430649505952336fc92dfb5e76348c5a Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei <84813886+lrafeei@users.noreply.github.com> Date: Thu, 2 Nov 2023 15:14:39 -0700 Subject: [PATCH 06/15] Remove duplicate kafka import hook (#956) Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- newrelic/config.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/newrelic/config.py b/newrelic/config.py index 6816c43b5b..5c4c52464f 100644 --- a/newrelic/config.py +++ b/newrelic/config.py @@ -2448,11 +2448,6 @@ def _process_module_builtin_defaults(): "newrelic.hooks.messagebroker_kafkapython", "instrument_kafka_heartbeat", ) - _process_module_definition( - "kafka.consumer.group", - "newrelic.hooks.messagebroker_kafkapython", - "instrument_kafka_consumer_group", - ) _process_module_definition( "logging", From cad06ffe51d758740b16d99ce951607dbce31cc0 Mon Sep 17 00:00:00 2001 From: Lalleh Rafeei Date: Fri, 3 Nov 2023 17:25:00 -0700 Subject: [PATCH 07/15] Initial bedrock error tracing commit --- newrelic/hooks/external_botocore.py | 43 ++++- .../test_bedrock_chat_completion_error.py | 147 ++++++++++++++++++ 2 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 tests/external_botocore/test_bedrock_chat_completion_error.py diff --git a/newrelic/hooks/external_botocore.py b/newrelic/hooks/external_botocore.py index c99293a63f..607c182b83 100644 --- a/newrelic/hooks/external_botocore.py +++ b/newrelic/hooks/external_botocore.py @@ -55,6 +55,38 @@ def extractor_string(*args, **kwargs): return extractor_list +def bedrock_error_attributes(exception, request_args): + + response_body = json.loads(request_args.get("body", "")) + # "api_key_last_four_digits": api_key_last_four_digits, + # "request.model": request_args.get("model") or request_args.get("engine") or "", + # "request.temperature": request_args.get("temperature", ""), + # "request.max_tokens": request_args.get("max_tokens", ""), + # "vendor": "openAI", + # "ingest_source": "Python", + # "response.organization": getattr(exception, "organization", ""), + # "response.number_of_messages": number_of_messages, + # "http.statusCode": getattr(exception, "http_status", ""), + # "error.message": getattr(exception, "_message", ""), + # "error.code": getattr(getattr(exception, "error", ""), "code", ""), + # "error.param": getattr(exception, "param", ""), + + breakpoint() + error_attributes = { + "request.id": exception.response.get("ResponseMetadata", "").get("RequestId", ""), + "api_key_last_four_digits": None, + "request.model": request_args.get("modelId", ""), + "request.temperature": response_body.get("textGenerationConfig", "").get("temperature", ""), + "request.max_tokens": response_body.get("textGenerationConfig", "").get("maxTokenCount", ""), + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": exception.response.get("ResponseMetadata", "").get("HTTPStatusCode", ""), + "error.message": exception.response.get("Error", "").get("Message", ""), + "error.code": exception.response.get("Error", "").get("Code", ""), + } + return error_attributes + + def create_chat_completion_message_event( transaction, app_name, @@ -217,7 +249,16 @@ def wrap_bedrock_runtime_invoke_model(wrapped, instance, args, kwargs): ft_name = callable_name(wrapped) with FunctionTrace(ft_name) as ft: - response = wrapped(*args, **kwargs) + try: + response = wrapped(*args, **kwargs) + except Exception as exc: + breakpoint() + error_attributes = bedrock_error_attributes(exc, kwargs) + # exc._nr_message = error_attributes.pop("error.message") + ft.notice_error( + attributes=error_attributes, + ) + raise if not response: return response diff --git a/tests/external_botocore/test_bedrock_chat_completion_error.py b/tests/external_botocore/test_bedrock_chat_completion_error.py new file mode 100644 index 0000000000..209b5ea95b --- /dev/null +++ b/tests/external_botocore/test_bedrock_chat_completion_error.py @@ -0,0 +1,147 @@ +# 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 json +from io import BytesIO + +import botocore +import pytest +from _test_bedrock_chat_completion import ( + chat_completion_expected_events, + chat_completion_payload_templates, +) +from test_bedrock_chat_completion import ( + exercise_model, + is_file_payload, + model_id, +) +from testing_support.fixtures import ( + dt_enabled, + override_application_settings, + reset_core_stats_engine, +) +from testing_support.validators.validate_error_trace_attributes import ( + validate_error_trace_attributes, +) +from testing_support.validators.validate_span_events import validate_span_events + +from newrelic.api.background_task import background_task +from newrelic.api.time_trace import current_trace +from newrelic.api.transaction import add_custom_attribute, current_transaction +from newrelic.common.object_names import callable_name + +_test_bedrock_chat_completion_prompt = "What is 212 degrees Fahrenheit converted to Celsius?" + +chat_completion_payload_templates_no_prompt = { + "amazon.titan-text-express-v1": '{ "textGenerationConfig": {"temperature": %f, "maxTokenCount": %d }}', + "ai21.j2-mid-v1": '{"temperature": %f, "maxTokens": %d}', + "cohere.command-text-v14": '{"temperature": %f, "max_tokens": %d}', +} + + +@pytest.fixture(scope="function") +def exercise_model_no_prompt(bedrock_server, model_id, is_file_payload): + payload_template = chat_completion_payload_templates_no_prompt[model_id] + + def _exercise_model(temperature=0.7, max_tokens=100): + breakpoint() + body = (payload_template % (temperature, max_tokens)).encode("utf-8") + if is_file_payload: + body = BytesIO(body) + + bedrock_server.invoke_model( + body=body, + modelId=model_id, + accept="application/json", + contentType="application/json", + ) + + return _exercise_model + + +# No prompt provided +@dt_enabled +@reset_core_stats_engine() +# @validate_error_trace_attributes( +# callable_name(botocore.InvalidRequestError), +# exact_attrs={ +# "agent": {}, +# "intrinsic": {}, +# "user": { +# # "api_key_last_four_digits": "sk-CRET", +# # "request.temperature": 0.7, +# # "request.max_tokens": 100, +# # "vendor": "openAI", +# # "ingest_source": "Python", +# # "response.number_of_messages": 2, +# # "error.param": "engine", +# }, +# }, +# ) +# @validate_span_events( +# exact_agents={ +# # "error.message": "Must provide an 'engine' or 'model' parameter to create a ", +# } +# ) +def test_bedrock_chat_completion_no_prompt(exercise_model_no_prompt): + @background_task() + def _test(): + set_trace_info() + add_custom_attribute("conversation_id", "my-awesome-id") + exercise_model_no_prompt(temperature=0.7, max_tokens=100) + + _test() + + +@dt_enabled +@reset_core_stats_engine() +@validate_error_trace_attributes( + callable_name(botocore.InvalidSignatureException), + exact_attrs={ + "agent": {}, + "intrinsic": {}, + "user": { + # "request.id": "b61f5406-5955-4dc9-915c-9ae1bedda182", # This will change + # "api_key_last_four_digits": "sk-CRET", + # "request.model": None, # Grab from payload templates + "request.temperature": 0.7, + "request.max_tokens": 100, + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.", + "error.code": "InvalidSignatureException", + }, + }, +) +def test_bedrock_chat_completion_incorrect_secret_access_key(exercise_model): + @background_task() + def _test(): + with pytest.raises(botocore.InvalidSignatureException): # not sure where this exception actually comes from + set_trace_info() + add_custom_attribute("conversation_id", "my-awesome-id") + exercise_model(prompt=_test_bedrock_chat_completion_prompt, temperature=0.7, max_tokens=100) + + _test() + + +# @reset_core_stats_engine() +# def test_bedrock_chat_completion_in_txn(exercise_model, expected_events): +# @background_task() +# def _test(): +# set_trace_info() +# add_custom_attribute("conversation_id", "my-awesome-id") +# exercise_model(prompt=_test_bedrock_chat_completion_prompt, temperature=0.7, max_tokens=100) + +# _test() From b2e9e7404d4d26b98161cdb7c0fb0fda6449ab74 Mon Sep 17 00:00:00 2001 From: Hannah Stepanek Date: Mon, 6 Nov 2023 13:56:25 -0800 Subject: [PATCH 08/15] Handle 0.32.0.post1 version in tests (#963) --- tests/framework_starlette/test_application.py | 30 +++++++++++-------- tests/framework_starlette/test_bg_tasks.py | 5 ++-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/tests/framework_starlette/test_application.py b/tests/framework_starlette/test_application.py index 7d36d66ccc..bd89bb9a9c 100644 --- a/tests/framework_starlette/test_application.py +++ b/tests/framework_starlette/test_application.py @@ -17,13 +17,21 @@ import pytest import starlette from testing_support.fixtures import override_ignore_status_codes +from testing_support.validators.validate_code_level_metrics import ( + validate_code_level_metrics, +) +from testing_support.validators.validate_transaction_errors import ( + validate_transaction_errors, +) +from testing_support.validators.validate_transaction_metrics import ( + validate_transaction_metrics, +) from newrelic.common.object_names import callable_name -from testing_support.validators.validate_code_level_metrics import validate_code_level_metrics -from testing_support.validators.validate_transaction_errors import validate_transaction_errors -from testing_support.validators.validate_transaction_metrics import validate_transaction_metrics +from newrelic.common.package_version_utils import get_package_version_tuple + +starlette_version = get_package_version_tuple("starlette")[:3] -starlette_version = tuple(int(x) for x in starlette.__version__.split(".")) @pytest.fixture(scope="session") def target_application(): @@ -78,6 +86,7 @@ def test_application_non_async(target_application, app_name): response = app.get("/non_async") assert response.status == 200 + # Starting in Starlette v0.20.1, the ExceptionMiddleware class # has been moved to the starlette.middleware.exceptions from # starlette.exceptions @@ -96,8 +105,10 @@ def test_application_non_async(target_application, app_name): ), ) + @pytest.mark.parametrize( - "app_name, transaction_name", middleware_test, + "app_name, transaction_name", + middleware_test, ) def test_application_nonexistent_route(target_application, app_name, transaction_name): @validate_transaction_metrics( @@ -117,10 +128,6 @@ def _test(): def test_exception_in_middleware(target_application, app_name): app = target_application[app_name] - from starlette import __version__ as version - - starlette_version = tuple(int(v) for v in version.split(".")) - # Starlette >=0.15 and <0.17 raises an exception group instead of reraising the ValueError # This only occurs on Python versions >=3.8 if sys.version_info[0:2] > (3, 7) and starlette_version >= (0, 15, 0) and starlette_version < (0, 17, 0): @@ -272,9 +279,8 @@ def _test(): ), ) -@pytest.mark.parametrize( - "app_name,scoped_metrics", middleware_test_exception -) + +@pytest.mark.parametrize("app_name,scoped_metrics", middleware_test_exception) def test_starlette_http_exception(target_application, app_name, scoped_metrics): @validate_transaction_errors(errors=["starlette.exceptions:HTTPException"]) @validate_transaction_metrics( diff --git a/tests/framework_starlette/test_bg_tasks.py b/tests/framework_starlette/test_bg_tasks.py index 5e30fe32e5..9ad8fe61be 100644 --- a/tests/framework_starlette/test_bg_tasks.py +++ b/tests/framework_starlette/test_bg_tasks.py @@ -15,7 +15,6 @@ import sys import pytest -from starlette import __version__ from testing_support.validators.validate_transaction_count import ( validate_transaction_count, ) @@ -23,7 +22,9 @@ validate_transaction_metrics, ) -starlette_version = tuple(int(x) for x in __version__.split(".")) +from newrelic.common.package_version_utils import get_package_version_tuple + +starlette_version = get_package_version_tuple("starlette")[:3] try: from starlette.middleware import Middleware # noqa: F401 From 302972061c38f234d3d8858d4241b382a1d717d4 Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Tue, 7 Nov 2023 11:14:50 -0800 Subject: [PATCH 09/15] Add status code to mock bedrock server --- .../external_botocore/_mock_external_bedrock_server.py | 10 ++++++++-- tests/external_botocore/conftest.py | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/external_botocore/_mock_external_bedrock_server.py b/tests/external_botocore/_mock_external_bedrock_server.py index c9149e3f1b..19ccf733cd 100644 --- a/tests/external_botocore/_mock_external_bedrock_server.py +++ b/tests/external_botocore/_mock_external_bedrock_server.py @@ -31,6 +31,7 @@ RESPONSES = { "amazon.titan-text-express-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ {"content-type": "application/json", "x-amzn-requestid": "660d4de9-6804-460e-8556-4ab2a019d1e3"}, + 200, { "inputTextTokenCount": 12, "results": [ @@ -44,6 +45,7 @@ ], "anthropic.claude-instant-v1::Human: What is 212 degrees Fahrenheit converted to Celsius? Assistant:": [ {"content-type": "application/json", "x-amzn-requestid": "f354b9a7-9eac-4f50-a8d7-7d5d23566176"}, + 200, { "completion": " Here are the step-by-step workings:\n1) 212 degrees Fahrenheit \n2) To convert to Celsius, use the formula: C = (F - 32) * 5/9\n3) Plug in the values: C = (212 - 32) * 5/9 = 100 * 5/9 = 100 degrees Celsius\n\nSo, 212 degrees Fahrenheit converted to Celsius is 100 degrees Celsius.", "stop_reason": "stop_sequence", @@ -52,6 +54,7 @@ ], "cohere.command-text-v14::What is 212 degrees Fahrenheit converted to Celsius?": [ {"content-type": "application/json", "x-amzn-requestid": "c5188fb5-dc58-4cbe-948d-af173c69ce0d"}, + 200, { "generations": [ { @@ -66,6 +69,7 @@ ], "ai21.j2-mid-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ {"content-type": "application/json", "x-amzn-requestid": "3bf1bb6b-b6f0-4901-85a1-2fa0e814440e"}, + 200, { "id": 1234, "prompt": { @@ -240,6 +244,7 @@ ], "amazon.titan-embed-text-v1::This is an embedding test.": [ {"content-type": "application/json", "x-amzn-requestid": "75f1d3fe-6cde-4cf5-bdaf-7101f746ccfe"}, + 200, { "embedding": [ -0.14160156, @@ -1784,6 +1789,7 @@ ], "amazon.titan-embed-g1-text-02::This is an embedding test.": [ {"content-type": "application/json", "x-amzn-requestid": "f7e78265-6b7c-4b3a-b750-0c1d00347258"}, + 200, { "embedding": [ -0.14160156, @@ -3346,7 +3352,7 @@ def simple_get(self): headers, response = ({}, "") for k, v in RESPONSES.items(): if prompt.startswith(k): - headers, response = v + headers, status_code, response = v break else: # If no matches found self.send_response(500) @@ -3355,7 +3361,7 @@ def simple_get(self): return # Send response code - self.send_response(200) + self.send_response(status_code) # Send headers for k, v in headers.items(): diff --git a/tests/external_botocore/conftest.py b/tests/external_botocore/conftest.py index 67a2058239..82f415f10b 100644 --- a/tests/external_botocore/conftest.py +++ b/tests/external_botocore/conftest.py @@ -134,9 +134,10 @@ def wrap_botocore_client_BaseClient__make_api_call(wrapped, instance, args, kwar headers.items(), ) ) + status_code = result["ResponseMetadata"]["HTTPStatusCode"] # Log response - BEDROCK_AUDIT_LOG_CONTENTS[prompt] = headers, data # Append response data to audit log + BEDROCK_AUDIT_LOG_CONTENTS[prompt] = headers, status_code, data # Append response data to audit log return result From 79b46261a96149d4bb5d12a0c9de7245a78f6586 Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Tue, 7 Nov 2023 14:21:51 -0800 Subject: [PATCH 10/15] Updating error response recording logic --- .../_mock_external_bedrock_server.py | 127 ++++++++++-------- tests/external_botocore/conftest.py | 38 +++--- 2 files changed, 93 insertions(+), 72 deletions(-) diff --git a/tests/external_botocore/_mock_external_bedrock_server.py b/tests/external_botocore/_mock_external_bedrock_server.py index 19ccf733cd..378a3756bd 100644 --- a/tests/external_botocore/_mock_external_bedrock_server.py +++ b/tests/external_botocore/_mock_external_bedrock_server.py @@ -29,46 +29,13 @@ # 3) This app runs on a separate thread meaning it won't block the test app. RESPONSES = { - "amazon.titan-text-express-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ - {"content-type": "application/json", "x-amzn-requestid": "660d4de9-6804-460e-8556-4ab2a019d1e3"}, - 200, - { - "inputTextTokenCount": 12, - "results": [ - { - "tokenCount": 55, - "outputText": "\nUse the formula,\n\u00b0C = (\u00b0F - 32) x 5/9\n= 212 x 5/9\n= 100 degrees Celsius\n212 degrees Fahrenheit is 100 degrees Celsius.", - "completionReason": "FINISH", - } - ], - }, - ], - "anthropic.claude-instant-v1::Human: What is 212 degrees Fahrenheit converted to Celsius? Assistant:": [ - {"content-type": "application/json", "x-amzn-requestid": "f354b9a7-9eac-4f50-a8d7-7d5d23566176"}, - 200, - { - "completion": " Here are the step-by-step workings:\n1) 212 degrees Fahrenheit \n2) To convert to Celsius, use the formula: C = (F - 32) * 5/9\n3) Plug in the values: C = (212 - 32) * 5/9 = 100 * 5/9 = 100 degrees Celsius\n\nSo, 212 degrees Fahrenheit converted to Celsius is 100 degrees Celsius.", - "stop_reason": "stop_sequence", - "stop": "\n\nHuman:", - }, - ], - "cohere.command-text-v14::What is 212 degrees Fahrenheit converted to Celsius?": [ - {"content-type": "application/json", "x-amzn-requestid": "c5188fb5-dc58-4cbe-948d-af173c69ce0d"}, - 200, - { - "generations": [ - { - "finish_reason": "MAX_TOKENS", - "id": "0730f5c0-9a49-4f35-af94-cf8f77327740", - "text": " To convert 212 degrees Fahrenheit to Celsius, we can use the conversion factor that Celsius is equal to (Fahrenheit - 32) x 5/9. \\n\\nApplying this formula, we have:\\n212°F = (212°F - 32) x 5/9\\n= (180) x 5/9\\n= 100°C.\\n\\nTherefore, 212 degrees F", - } - ], - "id": "a9cc8ce6-50b6-40b6-bf77-cf24561d8de7", - "prompt": "What is 212 degrees Fahrenheit converted to Celsius?", - }, + "ai21.j2-mid-v1::Invalid Token": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "b393ba08-c838-4503-a489-a43bf9bbaccd"}, + 403, + {"message": "The security token included in the request is invalid."}, ], "ai21.j2-mid-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ - {"content-type": "application/json", "x-amzn-requestid": "3bf1bb6b-b6f0-4901-85a1-2fa0e814440e"}, + {"Content-Type": "application/json", "x-amzn-RequestId": "c863d9fc-888b-421c-a175-ac5256baec62"}, 200, { "id": 1234, @@ -77,7 +44,7 @@ "tokens": [ { "generatedToken": { - "token": "\u2581What\u2581is", + "token": "▁What▁is", "logprob": -7.446773529052734, "raw_logprob": -7.446773529052734, }, @@ -86,7 +53,7 @@ }, { "generatedToken": { - "token": "\u2581", + "token": "▁", "logprob": -3.8046724796295166, "raw_logprob": -3.8046724796295166, }, @@ -104,7 +71,7 @@ }, { "generatedToken": { - "token": "\u2581degrees\u2581Fahrenheit", + "token": "▁degrees▁Fahrenheit", "logprob": -7.953181743621826, "raw_logprob": -7.953181743621826, }, @@ -113,7 +80,7 @@ }, { "generatedToken": { - "token": "\u2581converted\u2581to", + "token": "▁converted▁to", "logprob": -6.168096542358398, "raw_logprob": -6.168096542358398, }, @@ -122,7 +89,7 @@ }, { "generatedToken": { - "token": "\u2581Celsius", + "token": "▁Celsius", "logprob": -0.09790332615375519, "raw_logprob": -0.09790332615375519, }, @@ -156,7 +123,7 @@ }, { "generatedToken": { - "token": "\u2581", + "token": "▁", "logprob": -0.03473362699151039, "raw_logprob": -0.11261807382106781, }, @@ -174,7 +141,7 @@ }, { "generatedToken": { - "token": "\u2581degrees\u2581Fahrenheit", + "token": "▁degrees▁Fahrenheit", "logprob": -0.003579758107662201, "raw_logprob": -0.03144374489784241, }, @@ -183,7 +150,7 @@ }, { "generatedToken": { - "token": "\u2581is\u2581equal\u2581to", + "token": "▁is▁equal▁to", "logprob": -0.0027733694296330214, "raw_logprob": -0.027207009494304657, }, @@ -192,7 +159,7 @@ }, { "generatedToken": { - "token": "\u2581", + "token": "▁", "logprob": -0.0003392120997887105, "raw_logprob": -0.005458095110952854, }, @@ -210,7 +177,7 @@ }, { "generatedToken": { - "token": "\u2581degrees\u2581Celsius", + "token": "▁degrees▁Celsius", "logprob": -0.31207239627838135, "raw_logprob": -0.402545303106308, }, @@ -242,8 +209,8 @@ ], }, ], - "amazon.titan-embed-text-v1::This is an embedding test.": [ - {"content-type": "application/json", "x-amzn-requestid": "75f1d3fe-6cde-4cf5-bdaf-7101f746ccfe"}, + "amazon.titan-embed-g1-text-02::This is an embedding test.": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "b10ac895-eae3-4f07-b926-10b2866c55ed"}, 200, { "embedding": [ @@ -1787,8 +1754,8 @@ "inputTextTokenCount": 6, }, ], - "amazon.titan-embed-g1-text-02::This is an embedding test.": [ - {"content-type": "application/json", "x-amzn-requestid": "f7e78265-6b7c-4b3a-b750-0c1d00347258"}, + "amazon.titan-embed-text-v1::This is an embedding test.": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "11233989-07e8-4ecb-9ba6-79601ba6d8cc"}, 200, { "embedding": [ @@ -3332,8 +3299,62 @@ "inputTextTokenCount": 6, }, ], + "amazon.titan-text-express-v1::Invalid Token": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "041a580c-c3a4-4d99-aafc-00dc0698da5a"}, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "amazon.titan-text-express-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "03524118-8d77-430f-9e08-63b5c03a40cf"}, + 200, + { + "inputTextTokenCount": 12, + "results": [ + { + "tokenCount": 75, + "outputText": "\nUse the formula,\n°C = °F - 32) x (5/9)\n= 212 - 32 x (5/9)\n= 212 - 16.11\n= 195.89\n\nThe answer is 195.89 degrees Celsius.", + "completionReason": "FINISH", + } + ], + }, + ], + "anthropic.claude-instant-v1::Human: Invalid Token Assistant:": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "cdc4ea3e-8724-45e3-97a3-9c2dec3376ca"}, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "anthropic.claude-instant-v1::Human: What is 212 degrees Fahrenheit converted to Celsius? Assistant:": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "7b0b37c6-85fb-4664-8f5b-361ca7b1aa18"}, + 200, + { + "completion": " Okay, here are the conversion steps:\n212 degrees Fahrenheit\n- Subtract 32 from 212 to get 180 (to convert from Fahrenheit to Celsius scale)\n- Multiply by 5/9 (because the formula is °C = (°F - 32) × 5/9)\n- 180 × 5/9 = 100\n\nSo 212 degrees Fahrenheit converted to Celsius is 100 degrees Celsius.", + "stop_reason": "stop_sequence", + "stop": "\n\nHuman:", + }, + ], + "cohere.command-text-v14::Invalid Token": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "cc797330-6fc2-4570-a3c2-f60ef63d37b0"}, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "cohere.command-text-v14::What is 212 degrees Fahrenheit converted to Celsius?": [ + {"Content-Type": "application/json", "x-amzn-RequestId": "e77422c8-fbbf-4e17-afeb-c758425c9f97"}, + 200, + { + "generations": [ + { + "finish_reason": "MAX_TOKENS", + "id": "d20c06b0-aafe-4230-b2c7-200f4069355e", + "text": " 212°F is equivalent to 100°C. \n\nFahrenheit and Celsius are two temperature scales commonly used in everyday life. The Fahrenheit scale is based on 32°F for the freezing point of water and 212°F for the boiling point of water. On the other hand, the Celsius scale uses 0°C and 100°C as the freezing and boiling points of water, respectively. \n\nTo convert from Fahrenheit to Celsius, we subtract 32 from the Fahrenheit temperature and multiply the result", + } + ], + "id": "e77422c8-fbbf-4e17-afeb-c758425c9f97", + "prompt": "What is 212 degrees Fahrenheit converted to Celsius?", + }, + ], } + MODEL_PATH_RE = re.compile(r"/model/([^/]+)/invoke") @@ -3374,7 +3395,7 @@ def simple_get(self): def extract_shortened_prompt(content, model): - prompt = content.get("inputText", None) or content.get("prompt", None) + prompt = content.get("inputText", "") or content.get("prompt", "") prompt = "::".join((model, prompt)) # Prepend model name to prompt key to keep separate copies return prompt.lstrip().split("\n")[0] diff --git a/tests/external_botocore/conftest.py b/tests/external_botocore/conftest.py index 82f415f10b..b39a13c53e 100644 --- a/tests/external_botocore/conftest.py +++ b/tests/external_botocore/conftest.py @@ -14,6 +14,7 @@ import json import os +import re import pytest from _mock_external_bedrock_server import ( @@ -92,11 +93,12 @@ def bedrock_server(): # Apply function wrappers to record data wrap_function_wrapper( - "botocore.client", "BaseClient._make_api_call", wrap_botocore_client_BaseClient__make_api_call + "botocore.endpoint", "Endpoint._do_get_response", wrap_botocore_endpoint_Endpoint__do_get_response ) yield client # Run tests # Write responses to audit log + BEDROCK_AUDIT_LOG_CONTENTS = dict(sorted(BEDROCK_AUDIT_LOG_CONTENTS.items(), key=lambda i: i[0])) with open(BEDROCK_AUDIT_LOG_FILE, "w") as audit_log_fp: json.dump(BEDROCK_AUDIT_LOG_CONTENTS, fp=audit_log_fp, indent=4) @@ -105,44 +107,42 @@ def bedrock_server(): RECORDED_HEADERS = set(["x-amzn-requestid", "content-type"]) -def wrap_botocore_client_BaseClient__make_api_call(wrapped, instance, args, kwargs): - from io import BytesIO - - from botocore.response import StreamingBody - - params = bind_make_api_call_params(*args, **kwargs) - if not params: +def wrap_botocore_endpoint_Endpoint__do_get_response(wrapped, instance, args, kwargs): + request = bind__do_get_response(*args, **kwargs) + if not request: return wrapped(*args, **kwargs) - body = json.loads(params["body"]) - model = params["modelId"] + body = json.loads(request.body) + + match = re.search(r"/model/([0-9a-zA-Z.-]+)/", request.url) + model = match.group(1) prompt = extract_shortened_prompt(body, model) # Send request result = wrapped(*args, **kwargs) - # Intercept body data, and replace stream - streamed_body = result["body"].read() - result["body"] = StreamingBody(BytesIO(streamed_body), len(streamed_body)) + # Unpack response + success, exception = result + response = (success or exception)[0] # Clean up data - data = json.loads(streamed_body.decode("utf-8")) - headers = dict(result["ResponseMetadata"]["HTTPHeaders"].items()) + data = json.loads(response.content.decode("utf-8")) + headers = dict(response.headers.items()) headers = dict( filter( - lambda k: k[0] in RECORDED_HEADERS or k[0].startswith("x-ratelimit"), + lambda k: k[0].lower() in RECORDED_HEADERS or k[0].startswith("x-ratelimit"), headers.items(), ) ) - status_code = result["ResponseMetadata"]["HTTPStatusCode"] + status_code = response.status_code # Log response BEDROCK_AUDIT_LOG_CONTENTS[prompt] = headers, status_code, data # Append response data to audit log return result -def bind_make_api_call_params(operation_name, api_params): - return api_params +def bind__do_get_response(request, operation_model, context): + return request @pytest.fixture(scope="session") From 8ebd9c58043018799adc4c925453aeb4ab72e2df Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Wed, 8 Nov 2023 09:22:45 -0800 Subject: [PATCH 11/15] Work on bedrock errror tracing --- newrelic/hooks/external_botocore.py | 45 +++++------ .../_test_bedrock_chat_completion.py | 29 +++++++ .../test_bedrock_chat_completion.py | 76 +++++++++++++++++++ 3 files changed, 122 insertions(+), 28 deletions(-) diff --git a/newrelic/hooks/external_botocore.py b/newrelic/hooks/external_botocore.py index 607c182b83..0d8a327443 100644 --- a/newrelic/hooks/external_botocore.py +++ b/newrelic/hooks/external_botocore.py @@ -55,34 +55,23 @@ def extractor_string(*args, **kwargs): return extractor_list -def bedrock_error_attributes(exception, request_args): - +def bedrock_error_attributes(exception, request_args, client): + response = getattr(exception, "response", None) + if not response: + return {} + response_body = json.loads(request_args.get("body", "")) - # "api_key_last_four_digits": api_key_last_four_digits, - # "request.model": request_args.get("model") or request_args.get("engine") or "", - # "request.temperature": request_args.get("temperature", ""), - # "request.max_tokens": request_args.get("max_tokens", ""), - # "vendor": "openAI", - # "ingest_source": "Python", - # "response.organization": getattr(exception, "organization", ""), - # "response.number_of_messages": number_of_messages, - # "http.statusCode": getattr(exception, "http_status", ""), - # "error.message": getattr(exception, "_message", ""), - # "error.code": getattr(getattr(exception, "error", ""), "code", ""), - # "error.param": getattr(exception, "param", ""), - - breakpoint() error_attributes = { - "request.id": exception.response.get("ResponseMetadata", "").get("RequestId", ""), - "api_key_last_four_digits": None, + "request.id": response.get("ResponseMetadata", "").get("RequestId", ""), + "api_key_last_four_digits": client._request_signer._credentials.access_key[-4:], "request.model": request_args.get("modelId", ""), "request.temperature": response_body.get("textGenerationConfig", "").get("temperature", ""), "request.max_tokens": response_body.get("textGenerationConfig", "").get("maxTokenCount", ""), "vendor": "Bedrock", "ingest_source": "Python", - "http.statusCode": exception.response.get("ResponseMetadata", "").get("HTTPStatusCode", ""), - "error.message": exception.response.get("Error", "").get("Message", ""), - "error.code": exception.response.get("Error", "").get("Code", ""), + "http.statusCode": response.get("ResponseMetadata", "").get("HTTPStatusCode", ""), + "error.message": response.get("Error", "").get("Message", ""), + "error.code": response.get("Error", "").get("Code", ""), } return error_attributes @@ -252,13 +241,13 @@ def wrap_bedrock_runtime_invoke_model(wrapped, instance, args, kwargs): try: response = wrapped(*args, **kwargs) except Exception as exc: - breakpoint() - error_attributes = bedrock_error_attributes(exc, kwargs) - # exc._nr_message = error_attributes.pop("error.message") - ft.notice_error( - attributes=error_attributes, - ) - raise + try: + error_attributes = bedrock_error_attributes(exc, kwargs, instance) + ft.notice_error( + attributes=error_attributes, + ) + finally: + raise if not response: return response diff --git a/tests/external_botocore/_test_bedrock_chat_completion.py b/tests/external_botocore/_test_bedrock_chat_completion.py index fc69b1ff89..3987919bd0 100644 --- a/tests/external_botocore/_test_bedrock_chat_completion.py +++ b/tests/external_botocore/_test_bedrock_chat_completion.py @@ -260,3 +260,32 @@ ), ], } + +chat_completion_expected_client_errors = { + "amazon.titan-text-express-v1": { + "conversation_id": "my-awesome-id", + "request.id": "041a580c-c3a4-4d99-aafc-00dc0698da5a", + "api_key_last_four_digits": "-KEY", + "request.model": "amazon.titan-text-express-v1", + "request.temperature": 0.7, + "request.max_tokens": 100, + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The security token included in the request is invalid.", + "error.code": "403", + }, + "ai21.j2-mid-v1": { + "conversation_id": "my-awesome-id", + "request.id": "041a580c-c3a4-4d99-aafc-00dc0698da5a", + "api_key_last_four_digits": "-KEY", + "request.model": "amazon.titan-text-express-v1", + "request.temperature": 0.7, + "request.max_tokens": 100, + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The security token included in the request is invalid.", + "error.code": "403", + } +} diff --git a/tests/external_botocore/test_bedrock_chat_completion.py b/tests/external_botocore/test_bedrock_chat_completion.py index 995a931633..9a99f7a6a0 100644 --- a/tests/external_botocore/test_bedrock_chat_completion.py +++ b/tests/external_botocore/test_bedrock_chat_completion.py @@ -12,6 +12,8 @@ # See the License for the specific language governing permissions and # limitations under the License. +import botocore.exceptions + import copy import json from io import BytesIO @@ -20,18 +22,25 @@ from _test_bedrock_chat_completion import ( chat_completion_expected_events, chat_completion_payload_templates, + chat_completion_expected_client_errors, ) from testing_support.fixtures import ( + dt_enabled, override_application_settings, reset_core_stats_engine, ) from testing_support.validators.validate_ml_event_count import validate_ml_event_count from testing_support.validators.validate_ml_events import validate_ml_events +from testing_support.validators.validate_span_events import validate_span_events +from testing_support.validators.validate_error_trace_attributes import ( + validate_error_trace_attributes, +) from newrelic.api.background_task import background_task from newrelic.api.time_trace import current_trace from newrelic.api.transaction import add_custom_attribute, current_transaction +from newrelic.common.object_names import callable_name @pytest.fixture(scope="session", params=[False, True], ids=["Bytes", "Stream"]) def is_file_payload(request): @@ -85,6 +94,11 @@ def expected_events_no_convo_id(model_id): return events +@pytest.fixture(scope="module") +def expected_client_error(model_id): + return chat_completion_expected_client_errors[model_id] + + _test_bedrock_chat_completion_prompt = "What is 212 degrees Fahrenheit converted to Celsius?" @@ -154,3 +168,65 @@ def test_bedrock_chat_completion_outside_txn(set_trace_info, exercise_model): def test_bedrock_chat_completion_disabled_settings(set_trace_info, exercise_model): set_trace_info() exercise_model(prompt=_test_bedrock_chat_completion_prompt, temperature=0.7, max_tokens=100) + + + +_client_error = botocore.exceptions.ClientError +_client_error_name = callable_name(_client_error) + + +# No prompt provided +@dt_enabled +@reset_core_stats_engine() +# @validate_error_trace_attributes( +# callable_name(botocore.InvalidRequestError), +# exact_attrs={ +# "agent": {}, +# "intrinsic": {}, +# "user": { +# # "api_key_last_four_digits": "sk-CRET", +# # "request.temperature": 0.7, +# # "request.max_tokens": 100, +# # "vendor": "openAI", +# # "ingest_source": "Python", +# # "response.number_of_messages": 2, +# # "error.param": "engine", +# }, +# }, +# ) +# @validate_span_events( +# exact_agents={ +# # "error.message": "Must provide an 'engine' or 'model' parameter to create a ", +# } +# ) +def test_bedrock_chat_completion_error_no_prompt(exercise_model_no_prompt, set_trace_info): + @background_task() + def _test(): + set_trace_info() + add_custom_attribute("conversation_id", "my-awesome-id") + exercise_model_no_prompt(temperature=0.7, max_tokens=100) + + _test() + + +@dt_enabled +@reset_core_stats_engine() +def test_bedrock_chat_completion_error_incorrect_access_key(monkeypatch, bedrock_server, exercise_model, set_trace_info, expected_client_error): + @validate_error_trace_attributes( + _client_error_name, + exact_attrs={ + "agent": {}, + "intrinsic": {}, + "user": expected_client_error, + }, + ) + @background_task() + def _test(): + monkeypatch.setattr(bedrock_server._request_signer._credentials, "access_key", "INVALID-ACCESS-KEY") + + with pytest.raises(_client_error): # not sure where this exception actually comes from + set_trace_info() + add_custom_attribute("conversation_id", "my-awesome-id") + exercise_model(prompt="Invalid Token", temperature=0.7, max_tokens=100) + + _test() From 58694dc41b6e6abd033152049e68703fc6997b62 Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Wed, 8 Nov 2023 10:42:16 -0800 Subject: [PATCH 12/15] Chat completion error tracing --- newrelic/hooks/external_botocore.py | 177 +++++++++++------- .../_mock_external_bedrock_server.py | 59 ++++-- .../_test_bedrock_chat_completion.py | 70 ++++--- .../_test_bedrock_embeddings.py | 4 +- tests/external_botocore/conftest.py | 6 +- .../test_bedrock_chat_completion.py | 59 +++--- .../test_bedrock_chat_completion_error.py | 147 --------------- 7 files changed, 231 insertions(+), 291 deletions(-) delete mode 100644 tests/external_botocore/test_bedrock_chat_completion_error.py diff --git a/newrelic/hooks/external_botocore.py b/newrelic/hooks/external_botocore.py index 0d8a327443..a28082e4a8 100644 --- a/newrelic/hooks/external_botocore.py +++ b/newrelic/hooks/external_botocore.py @@ -55,24 +55,24 @@ def extractor_string(*args, **kwargs): return extractor_list -def bedrock_error_attributes(exception, request_args, client): +def bedrock_error_attributes(exception, request_args, client, extractor): response = getattr(exception, "response", None) if not response: return {} - response_body = json.loads(request_args.get("body", "")) - error_attributes = { + request_body = request_args.get("body", "") + error_attributes = extractor(request_body)[1] + + error_attributes.update({ "request.id": response.get("ResponseMetadata", "").get("RequestId", ""), "api_key_last_four_digits": client._request_signer._credentials.access_key[-4:], "request.model": request_args.get("modelId", ""), - "request.temperature": response_body.get("textGenerationConfig", "").get("temperature", ""), - "request.max_tokens": response_body.get("textGenerationConfig", "").get("maxTokenCount", ""), "vendor": "Bedrock", "ingest_source": "Python", "http.statusCode": response.get("ResponseMetadata", "").get("HTTPStatusCode", ""), "error.message": response.get("Error", "").get("Message", ""), "error.code": response.get("Error", "").get("Code", ""), - } + }) return error_attributes @@ -116,37 +116,47 @@ def create_chat_completion_message_event( transaction.record_ml_event("LlmChatCompletionMessage", chat_completion_message_dict) -def extract_bedrock_titan_text_model(request_body, response_body): - response_body = json.loads(response_body) +def extract_bedrock_titan_text_model(request_body, response_body=None): request_body = json.loads(request_body) - - input_tokens = response_body["inputTextTokenCount"] - completion_tokens = sum(result["tokenCount"] for result in response_body.get("results", [])) - total_tokens = input_tokens + completion_tokens + if response_body: + response_body = json.loads(response_body) request_config = request_body.get("textGenerationConfig", {}) - message_list = [{"role": "user", "content": request_body.get("inputText", "")}] - message_list.extend( - {"role": "assistant", "content": result["outputText"]} for result in response_body.get("results", []) - ) chat_completion_summary_dict = { "request.max_tokens": request_config.get("maxTokenCount", ""), "request.temperature": request_config.get("temperature", ""), - "response.choices.finish_reason": response_body["results"][0]["completionReason"], - "response.usage.completion_tokens": completion_tokens, - "response.usage.prompt_tokens": input_tokens, - "response.usage.total_tokens": total_tokens, - "response.number_of_messages": len(message_list), } + + if response_body: + input_tokens = response_body["inputTextTokenCount"] + completion_tokens = sum(result["tokenCount"] for result in response_body.get("results", [])) + total_tokens = input_tokens + completion_tokens + + message_list = [{"role": "user", "content": request_body.get("inputText", "")}] + message_list.extend( + {"role": "assistant", "content": result["outputText"]} for result in response_body.get("results", []) + ) + + chat_completion_summary_dict.update({ + "response.choices.finish_reason": response_body["results"][0]["completionReason"], + "response.usage.completion_tokens": completion_tokens, + "response.usage.prompt_tokens": input_tokens, + "response.usage.total_tokens": total_tokens, + "response.number_of_messages": len(message_list), + }) + else: + message_list = [] + return message_list, chat_completion_summary_dict -def extract_bedrock_titan_embedding_model(request_body, response_body): - response_body = json.loads(response_body) +def extract_bedrock_titan_embedding_model(request_body, response_body=None): request_body = json.loads(request_body) + if response_body: + response_body = json.loads(response_body) - input_tokens = response_body["inputTextTokenCount"] + input_tokens = response_body.get("inputTextTokenCount", None) embedding_dict = { "input": request_body.get("inputText", ""), @@ -156,59 +166,85 @@ def extract_bedrock_titan_embedding_model(request_body, response_body): return embedding_dict -def extract_bedrock_ai21_j2_model(request_body, response_body): - response_body = json.loads(response_body) +def extract_bedrock_ai21_j2_model(request_body, response_body=None): request_body = json.loads(request_body) - - message_list = [{"role": "user", "content": request_body.get("prompt", "")}] - message_list.extend( - {"role": "assistant", "content": result["data"]["text"]} for result in response_body.get("completions", []) - ) + if response_body: + response_body = json.loads(response_body) chat_completion_summary_dict = { "request.max_tokens": request_body.get("maxTokens", ""), "request.temperature": request_body.get("temperature", ""), - "response.choices.finish_reason": response_body["completions"][0]["finishReason"]["reason"], - "response.number_of_messages": len(message_list), - "response_id": str(response_body.get("id", "")), } + + if response_body: + message_list = [{"role": "user", "content": request_body.get("prompt", "")}] + message_list.extend( + {"role": "assistant", "content": result["data"]["text"]} for result in response_body.get("completions", []) + ) + + chat_completion_summary_dict.update({ + "response.choices.finish_reason": response_body["completions"][0]["finishReason"]["reason"], + "response.number_of_messages": len(message_list), + "response_id": str(response_body.get("id", "")), + }) + else: + message_list = [] + return message_list, chat_completion_summary_dict -def extract_bedrock_claude_model(request_body, response_body): - response_body = json.loads(response_body) +def extract_bedrock_claude_model(request_body, response_body=None): request_body = json.loads(request_body) - - message_list = [ - {"role": "user", "content": request_body.get("prompt", "")}, - {"role": "assistant", "content": response_body.get("completion", "")}, - ] + if response_body: + response_body = json.loads(response_body) chat_completion_summary_dict = { "request.max_tokens": request_body.get("max_tokens_to_sample", ""), "request.temperature": request_body.get("temperature", ""), - "response.choices.finish_reason": response_body.get("stop_reason", ""), - "response.number_of_messages": len(message_list), } + + if response_body: + message_list = [ + {"role": "user", "content": request_body.get("prompt", "")}, + {"role": "assistant", "content": response_body.get("completion", "")}, + ] + + chat_completion_summary_dict.update({ + "response.choices.finish_reason": response_body.get("stop_reason", ""), + "response.number_of_messages": len(message_list), + }) + else: + message_list = [] + return message_list, chat_completion_summary_dict -def extract_bedrock_cohere_model(request_body, response_body): - response_body = json.loads(response_body) +def extract_bedrock_cohere_model(request_body, response_body=None): request_body = json.loads(request_body) - - message_list = [{"role": "user", "content": request_body.get("prompt", "")}] - message_list.extend( - {"role": "assistant", "content": result["text"]} for result in response_body.get("generations", []) - ) + if response_body: + response_body = json.loads(response_body) chat_completion_summary_dict = { "request.max_tokens": request_body.get("max_tokens", ""), "request.temperature": request_body.get("temperature", ""), - "response.choices.finish_reason": response_body["generations"][0]["finish_reason"], - "response.number_of_messages": len(message_list), - "response_id": str(response_body.get("id", "")), } + + if response_body: + message_list = [{"role": "user", "content": request_body.get("prompt", "")}] + message_list.extend( + {"role": "assistant", "content": result["text"]} for result in response_body.get("generations", []) + ) + + chat_completion_summary_dict.update({ + "request.max_tokens": request_body.get("max_tokens", ""), + "request.temperature": request_body.get("temperature", ""), + "response.choices.finish_reason": response_body["generations"][0]["finish_reason"], + "response.number_of_messages": len(message_list), + "response_id": str(response_body.get("id", "")), + }) + else: + message_list = [] + return message_list, chat_completion_summary_dict @@ -236,26 +272,10 @@ def wrap_bedrock_runtime_invoke_model(wrapped, instance, args, kwargs): request_body = request_body.read() kwargs["body"] = request_body - ft_name = callable_name(wrapped) - with FunctionTrace(ft_name) as ft: - try: - response = wrapped(*args, **kwargs) - except Exception as exc: - try: - error_attributes = bedrock_error_attributes(exc, kwargs, instance) - ft.notice_error( - attributes=error_attributes, - ) - finally: - raise - - if not response: - return response - # Determine model to be used with extractor model = kwargs.get("modelId") if not model: - return response + return wrapped(*args, **kwargs) # Determine extractor by model type for extractor_name, extractor in MODEL_EXTRACTORS: @@ -271,7 +291,24 @@ def wrap_bedrock_runtime_invoke_model(wrapped, instance, args, kwargs): model, ) UNSUPPORTED_MODEL_WARNING_SENT = True + + extractor = lambda *args: ([], {}) # Empty extractor that returns nothing + ft_name = callable_name(wrapped) + with FunctionTrace(ft_name) as ft: + try: + response = wrapped(*args, **kwargs) + except Exception as exc: + try: + error_attributes = extractor(request_body) + error_attributes = bedrock_error_attributes(exc, kwargs, instance, extractor) + ft.notice_error( + attributes=error_attributes, + ) + finally: + raise + + if not response: return response # Read and replace response streaming bodies diff --git a/tests/external_botocore/_mock_external_bedrock_server.py b/tests/external_botocore/_mock_external_bedrock_server.py index 378a3756bd..42e430a124 100644 --- a/tests/external_botocore/_mock_external_bedrock_server.py +++ b/tests/external_botocore/_mock_external_bedrock_server.py @@ -30,10 +30,50 @@ RESPONSES = { "ai21.j2-mid-v1::Invalid Token": [ - {"Content-Type": "application/json", "x-amzn-RequestId": "b393ba08-c838-4503-a489-a43bf9bbaccd"}, + { + "Content-Type": "application/json", + "x-amzn-RequestId": "9021791d-3797-493d-9277-e33aa6f6d544", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "amazon.titan-text-express-v1::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "15b39c8b-8e85-42c9-9623-06720301bda3", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "anthropic.claude-instant-v1::Human: Invalid Token Assistant:": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "37396f55-b721-4bae-9461-4c369f5a080d", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, 403, {"message": "The security token included in the request is invalid."}, ], + "cohere.command-text-v14::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "22476490-a0d6-42db-b5ea-32d0b8a7f751", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "does-not-exist::": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "f4908827-3db9-4742-9103-2bbc34578b03", + "x-amzn-ErrorType": "ValidationException:http://internal.amazon.com/coral/com.amazon.bedrock/", + }, + 400, + {"message": "The provided model identifier is invalid."}, + ], "ai21.j2-mid-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ {"Content-Type": "application/json", "x-amzn-RequestId": "c863d9fc-888b-421c-a175-ac5256baec62"}, 200, @@ -3299,11 +3339,6 @@ "inputTextTokenCount": 6, }, ], - "amazon.titan-text-express-v1::Invalid Token": [ - {"Content-Type": "application/json", "x-amzn-RequestId": "041a580c-c3a4-4d99-aafc-00dc0698da5a"}, - 403, - {"message": "The security token included in the request is invalid."}, - ], "amazon.titan-text-express-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ {"Content-Type": "application/json", "x-amzn-RequestId": "03524118-8d77-430f-9e08-63b5c03a40cf"}, 200, @@ -3312,17 +3347,12 @@ "results": [ { "tokenCount": 75, - "outputText": "\nUse the formula,\n°C = °F - 32) x (5/9)\n= 212 - 32 x (5/9)\n= 212 - 16.11\n= 195.89\n\nThe answer is 195.89 degrees Celsius.", + "outputText": "\nUse the formula,\n°C = (°F - 32) x 5/9\n= 212 x 5/9\n= 100 degrees Celsius\n212 degrees Fahrenheit is 100 degrees Celsius.", "completionReason": "FINISH", } ], }, ], - "anthropic.claude-instant-v1::Human: Invalid Token Assistant:": [ - {"Content-Type": "application/json", "x-amzn-RequestId": "cdc4ea3e-8724-45e3-97a3-9c2dec3376ca"}, - 403, - {"message": "The security token included in the request is invalid."}, - ], "anthropic.claude-instant-v1::Human: What is 212 degrees Fahrenheit converted to Celsius? Assistant:": [ {"Content-Type": "application/json", "x-amzn-RequestId": "7b0b37c6-85fb-4664-8f5b-361ca7b1aa18"}, 200, @@ -3332,11 +3362,6 @@ "stop": "\n\nHuman:", }, ], - "cohere.command-text-v14::Invalid Token": [ - {"Content-Type": "application/json", "x-amzn-RequestId": "cc797330-6fc2-4570-a3c2-f60ef63d37b0"}, - 403, - {"message": "The security token included in the request is invalid."}, - ], "cohere.command-text-v14::What is 212 degrees Fahrenheit converted to Celsius?": [ {"Content-Type": "application/json", "x-amzn-RequestId": "e77422c8-fbbf-4e17-afeb-c758425c9f97"}, 200, diff --git a/tests/external_botocore/_test_bedrock_chat_completion.py b/tests/external_botocore/_test_bedrock_chat_completion.py index 3987919bd0..bb4e479bdd 100644 --- a/tests/external_botocore/_test_bedrock_chat_completion.py +++ b/tests/external_botocore/_test_bedrock_chat_completion.py @@ -16,13 +16,13 @@ "transaction_id": None, "span_id": "span-id", "trace_id": "trace-id", - "request_id": "660d4de9-6804-460e-8556-4ab2a019d1e3", + "request_id": "03524118-8d77-430f-9e08-63b5c03a40cf", "api_key_last_four_digits": "CRET", "duration": None, # Response time varies each test run "request.model": "amazon.titan-text-express-v1", "response.model": "amazon.titan-text-express-v1", - "response.usage.completion_tokens": 55, - "response.usage.total_tokens": 67, + "response.usage.completion_tokens": 75, + "response.usage.total_tokens": 87, "response.usage.prompt_tokens": 12, "request.temperature": 0.7, "request.max_tokens": 100, @@ -38,7 +38,7 @@ "id": None, # UUID that varies with each run "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "660d4de9-6804-460e-8556-4ab2a019d1e3", + "request_id": "03524118-8d77-430f-9e08-63b5c03a40cf", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, @@ -57,7 +57,7 @@ "id": None, # UUID that varies with each run "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "660d4de9-6804-460e-8556-4ab2a019d1e3", + "request_id": "03524118-8d77-430f-9e08-63b5c03a40cf", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, @@ -81,7 +81,7 @@ "transaction_id": None, "span_id": "span-id", "trace_id": "trace-id", - "request_id": "3bf1bb6b-b6f0-4901-85a1-2fa0e814440e", + "request_id": "c863d9fc-888b-421c-a175-ac5256baec62", "response_id": "1234", "api_key_last_four_digits": "CRET", "duration": None, # Response time varies each test run @@ -101,7 +101,7 @@ "id": "1234-0", "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "3bf1bb6b-b6f0-4901-85a1-2fa0e814440e", + "request_id": "c863d9fc-888b-421c-a175-ac5256baec62", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, @@ -120,7 +120,7 @@ "id": "1234-1", "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "3bf1bb6b-b6f0-4901-85a1-2fa0e814440e", + "request_id": "c863d9fc-888b-421c-a175-ac5256baec62", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, @@ -144,7 +144,7 @@ "transaction_id": None, "span_id": "span-id", "trace_id": "trace-id", - "request_id": "f354b9a7-9eac-4f50-a8d7-7d5d23566176", + "request_id": "7b0b37c6-85fb-4664-8f5b-361ca7b1aa18", "api_key_last_four_digits": "CRET", "duration": None, # Response time varies each test run "request.model": "anthropic.claude-instant-v1", @@ -163,7 +163,7 @@ "id": None, # UUID that varies with each run "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "f354b9a7-9eac-4f50-a8d7-7d5d23566176", + "request_id": "7b0b37c6-85fb-4664-8f5b-361ca7b1aa18", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, @@ -182,11 +182,11 @@ "id": None, # UUID that varies with each run "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "f354b9a7-9eac-4f50-a8d7-7d5d23566176", + "request_id": "7b0b37c6-85fb-4664-8f5b-361ca7b1aa18", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, - "content": " Here are the step-by-step workings:\n1) 212 degrees Fahrenheit \n2) To convert to Celsius, use the formula: C = (F - 32) * 5/9\n3) Plug in the values: C = (212 - 32) * 5/9 = 100 * 5/9 = 100 degrees Celsius\n\nSo, 212 degrees Fahrenheit converted to Celsius is", + "content": " Okay, here are the conversion steps:\n212 degrees Fahrenheit\n- Subtract 32 from 212 to get 180 (to convert from Fahrenheit to Celsius scale)\n- Multiply by 5/9 (because the formula is °C = (°F - 32) × 5/9)\n- 180 × 5/9 = 100\n\nSo 212 degrees Fahrenheit c", "role": "assistant", "completion_id": None, "sequence": 1, @@ -206,7 +206,7 @@ "transaction_id": None, "span_id": "span-id", "trace_id": "trace-id", - "request_id": "c5188fb5-dc58-4cbe-948d-af173c69ce0d", + "request_id": "e77422c8-fbbf-4e17-afeb-c758425c9f97", "response_id": None, # UUID that varies with each run "api_key_last_four_digits": "CRET", "duration": None, # Response time varies each test run @@ -226,7 +226,7 @@ "id": None, # UUID that varies with each run "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "c5188fb5-dc58-4cbe-948d-af173c69ce0d", + "request_id": "e77422c8-fbbf-4e17-afeb-c758425c9f97", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, @@ -245,11 +245,11 @@ "id": None, # UUID that varies with each run "appName": "Python Agent Test (external_botocore)", "conversation_id": "my-awesome-id", - "request_id": "c5188fb5-dc58-4cbe-948d-af173c69ce0d", + "request_id": "e77422c8-fbbf-4e17-afeb-c758425c9f97", "span_id": "span-id", "trace_id": "trace-id", "transaction_id": None, - "content": " To convert 212 degrees Fahrenheit to Celsius, we can use the conversion factor that Celsius is equal to (Fahrenheit - 32) x 5/9. \\n\\nApplying this formula, we have:\\n212°F = (212°F - 32) x 5/9\\n= (180) x 5/9\\n= 100°C.\\n\\nTherefore, 212 degrees F", + "content": " 212°F is equivalent to 100°C. \n\nFahrenheit and Celsius are two temperature scales commonly used in everyday life. The Fahrenheit scale is based on 32°F for the freezing point of water and 212°F for the boiling point of water. On the other hand, the C", "role": "assistant", "completion_id": None, "sequence": 1, @@ -264,7 +264,7 @@ chat_completion_expected_client_errors = { "amazon.titan-text-express-v1": { "conversation_id": "my-awesome-id", - "request.id": "041a580c-c3a4-4d99-aafc-00dc0698da5a", + "request.id": "15b39c8b-8e85-42c9-9623-06720301bda3", "api_key_last_four_digits": "-KEY", "request.model": "amazon.titan-text-express-v1", "request.temperature": 0.7, @@ -273,19 +273,45 @@ "ingest_source": "Python", "http.statusCode": 403, "error.message": "The security token included in the request is invalid.", - "error.code": "403", + "error.code": "UnrecognizedClientException", }, "ai21.j2-mid-v1": { "conversation_id": "my-awesome-id", - "request.id": "041a580c-c3a4-4d99-aafc-00dc0698da5a", + "request.id": "9021791d-3797-493d-9277-e33aa6f6d544", "api_key_last_four_digits": "-KEY", - "request.model": "amazon.titan-text-express-v1", + "request.model": "ai21.j2-mid-v1", + "request.temperature": 0.7, + "request.max_tokens": 100, + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The security token included in the request is invalid.", + "error.code": "UnrecognizedClientException", + }, + "anthropic.claude-instant-v1": { + "conversation_id": "my-awesome-id", + "request.id": "37396f55-b721-4bae-9461-4c369f5a080d", + "api_key_last_four_digits": "-KEY", + "request.model": "anthropic.claude-instant-v1", "request.temperature": 0.7, "request.max_tokens": 100, "vendor": "Bedrock", "ingest_source": "Python", "http.statusCode": 403, "error.message": "The security token included in the request is invalid.", - "error.code": "403", - } + "error.code": "UnrecognizedClientException", + }, + "cohere.command-text-v14": { + "conversation_id": "my-awesome-id", + "request.id": "22476490-a0d6-42db-b5ea-32d0b8a7f751", + "api_key_last_four_digits": "-KEY", + "request.model": "cohere.command-text-v14", + "request.temperature": 0.7, + "request.max_tokens": 100, + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The security token included in the request is invalid.", + "error.code": "UnrecognizedClientException", + }, } diff --git a/tests/external_botocore/_test_bedrock_embeddings.py b/tests/external_botocore/_test_bedrock_embeddings.py index fe4b4b839a..ddb35565b7 100644 --- a/tests/external_botocore/_test_bedrock_embeddings.py +++ b/tests/external_botocore/_test_bedrock_embeddings.py @@ -18,7 +18,7 @@ "duration": None, # Response time varies each test run "response.model": "amazon.titan-embed-text-v1", "request.model": "amazon.titan-embed-text-v1", - "request_id": "75f1d3fe-6cde-4cf5-bdaf-7101f746ccfe", + "request_id": "11233989-07e8-4ecb-9ba6-79601ba6d8cc", "response.usage.total_tokens": 6, "response.usage.prompt_tokens": 6, "vendor": "bedrock", @@ -40,7 +40,7 @@ "duration": None, # Response time varies each test run "response.model": "amazon.titan-embed-g1-text-02", "request.model": "amazon.titan-embed-g1-text-02", - "request_id": "f7e78265-6b7c-4b3a-b750-0c1d00347258", + "request_id": "b10ac895-eae3-4f07-b926-10b2866c55ed", "response.usage.total_tokens": 6, "response.usage.prompt_tokens": 6, "vendor": "bedrock", diff --git a/tests/external_botocore/conftest.py b/tests/external_botocore/conftest.py index b39a13c53e..e0e83329b2 100644 --- a/tests/external_botocore/conftest.py +++ b/tests/external_botocore/conftest.py @@ -98,13 +98,13 @@ def bedrock_server(): yield client # Run tests # Write responses to audit log - BEDROCK_AUDIT_LOG_CONTENTS = dict(sorted(BEDROCK_AUDIT_LOG_CONTENTS.items(), key=lambda i: i[0])) + bedrock_audit_log_contents = dict(sorted(BEDROCK_AUDIT_LOG_CONTENTS.items(), key=lambda i: i[0])) with open(BEDROCK_AUDIT_LOG_FILE, "w") as audit_log_fp: - json.dump(BEDROCK_AUDIT_LOG_CONTENTS, fp=audit_log_fp, indent=4) + json.dump(bedrock_audit_log_contents, fp=audit_log_fp, indent=4) # Intercept outgoing requests and log to file for mocking -RECORDED_HEADERS = set(["x-amzn-requestid", "content-type"]) +RECORDED_HEADERS = set(["x-amzn-requestid", "x-amzn-errortype", "content-type"]) def wrap_botocore_endpoint_Endpoint__do_get_response(wrapped, instance, args, kwargs): diff --git a/tests/external_botocore/test_bedrock_chat_completion.py b/tests/external_botocore/test_bedrock_chat_completion.py index 9a99f7a6a0..1ad87b0a4b 100644 --- a/tests/external_botocore/test_bedrock_chat_completion.py +++ b/tests/external_botocore/test_bedrock_chat_completion.py @@ -176,37 +176,36 @@ def test_bedrock_chat_completion_disabled_settings(set_trace_info, exercise_mode # No prompt provided -@dt_enabled -@reset_core_stats_engine() -# @validate_error_trace_attributes( -# callable_name(botocore.InvalidRequestError), -# exact_attrs={ -# "agent": {}, -# "intrinsic": {}, -# "user": { -# # "api_key_last_four_digits": "sk-CRET", -# # "request.temperature": 0.7, -# # "request.max_tokens": 100, -# # "vendor": "openAI", -# # "ingest_source": "Python", -# # "response.number_of_messages": 2, -# # "error.param": "engine", -# }, -# }, -# ) -# @validate_span_events( -# exact_agents={ -# # "error.message": "Must provide an 'engine' or 'model' parameter to create a ", -# } -# ) -def test_bedrock_chat_completion_error_no_prompt(exercise_model_no_prompt, set_trace_info): - @background_task() - def _test(): - set_trace_info() - add_custom_attribute("conversation_id", "my-awesome-id") - exercise_model_no_prompt(temperature=0.7, max_tokens=100) - _test() +@validate_error_trace_attributes( + "botocore.errorfactory:ValidationException", + exact_attrs={ + "agent": {}, + "intrinsic": {}, + "user": { + "conversation_id": "my-awesome-id", + "request.id": "f4908827-3db9-4742-9103-2bbc34578b03", + "api_key_last_four_digits": "CRET", + "request.model": "does-not-exist", + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 400, + "error.message": "The provided model identifier is invalid.", + "error.code": "ValidationException", + }, + }, +) +@background_task() +def test_bedrock_chat_completion_error_invalid_model(bedrock_server, set_trace_info): + set_trace_info() + add_custom_attribute("conversation_id", "my-awesome-id") + with pytest.raises(_client_error): + bedrock_server.invoke_model( + body=b"{}", + modelId="does-not-exist", + accept="application/json", + contentType="application/json", + ) @dt_enabled diff --git a/tests/external_botocore/test_bedrock_chat_completion_error.py b/tests/external_botocore/test_bedrock_chat_completion_error.py deleted file mode 100644 index 209b5ea95b..0000000000 --- a/tests/external_botocore/test_bedrock_chat_completion_error.py +++ /dev/null @@ -1,147 +0,0 @@ -# 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 json -from io import BytesIO - -import botocore -import pytest -from _test_bedrock_chat_completion import ( - chat_completion_expected_events, - chat_completion_payload_templates, -) -from test_bedrock_chat_completion import ( - exercise_model, - is_file_payload, - model_id, -) -from testing_support.fixtures import ( - dt_enabled, - override_application_settings, - reset_core_stats_engine, -) -from testing_support.validators.validate_error_trace_attributes import ( - validate_error_trace_attributes, -) -from testing_support.validators.validate_span_events import validate_span_events - -from newrelic.api.background_task import background_task -from newrelic.api.time_trace import current_trace -from newrelic.api.transaction import add_custom_attribute, current_transaction -from newrelic.common.object_names import callable_name - -_test_bedrock_chat_completion_prompt = "What is 212 degrees Fahrenheit converted to Celsius?" - -chat_completion_payload_templates_no_prompt = { - "amazon.titan-text-express-v1": '{ "textGenerationConfig": {"temperature": %f, "maxTokenCount": %d }}', - "ai21.j2-mid-v1": '{"temperature": %f, "maxTokens": %d}', - "cohere.command-text-v14": '{"temperature": %f, "max_tokens": %d}', -} - - -@pytest.fixture(scope="function") -def exercise_model_no_prompt(bedrock_server, model_id, is_file_payload): - payload_template = chat_completion_payload_templates_no_prompt[model_id] - - def _exercise_model(temperature=0.7, max_tokens=100): - breakpoint() - body = (payload_template % (temperature, max_tokens)).encode("utf-8") - if is_file_payload: - body = BytesIO(body) - - bedrock_server.invoke_model( - body=body, - modelId=model_id, - accept="application/json", - contentType="application/json", - ) - - return _exercise_model - - -# No prompt provided -@dt_enabled -@reset_core_stats_engine() -# @validate_error_trace_attributes( -# callable_name(botocore.InvalidRequestError), -# exact_attrs={ -# "agent": {}, -# "intrinsic": {}, -# "user": { -# # "api_key_last_four_digits": "sk-CRET", -# # "request.temperature": 0.7, -# # "request.max_tokens": 100, -# # "vendor": "openAI", -# # "ingest_source": "Python", -# # "response.number_of_messages": 2, -# # "error.param": "engine", -# }, -# }, -# ) -# @validate_span_events( -# exact_agents={ -# # "error.message": "Must provide an 'engine' or 'model' parameter to create a ", -# } -# ) -def test_bedrock_chat_completion_no_prompt(exercise_model_no_prompt): - @background_task() - def _test(): - set_trace_info() - add_custom_attribute("conversation_id", "my-awesome-id") - exercise_model_no_prompt(temperature=0.7, max_tokens=100) - - _test() - - -@dt_enabled -@reset_core_stats_engine() -@validate_error_trace_attributes( - callable_name(botocore.InvalidSignatureException), - exact_attrs={ - "agent": {}, - "intrinsic": {}, - "user": { - # "request.id": "b61f5406-5955-4dc9-915c-9ae1bedda182", # This will change - # "api_key_last_four_digits": "sk-CRET", - # "request.model": None, # Grab from payload templates - "request.temperature": 0.7, - "request.max_tokens": 100, - "vendor": "Bedrock", - "ingest_source": "Python", - "http.statusCode": 403, - "error.message": "The request signature we calculated does not match the signature you provided. Check your AWS Secret Access Key and signing method. Consult the service documentation for details.", - "error.code": "InvalidSignatureException", - }, - }, -) -def test_bedrock_chat_completion_incorrect_secret_access_key(exercise_model): - @background_task() - def _test(): - with pytest.raises(botocore.InvalidSignatureException): # not sure where this exception actually comes from - set_trace_info() - add_custom_attribute("conversation_id", "my-awesome-id") - exercise_model(prompt=_test_bedrock_chat_completion_prompt, temperature=0.7, max_tokens=100) - - _test() - - -# @reset_core_stats_engine() -# def test_bedrock_chat_completion_in_txn(exercise_model, expected_events): -# @background_task() -# def _test(): -# set_trace_info() -# add_custom_attribute("conversation_id", "my-awesome-id") -# exercise_model(prompt=_test_bedrock_chat_completion_prompt, temperature=0.7, max_tokens=100) - -# _test() From b37668df57c0621b1afa9d3cbb4234639a8d486c Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Wed, 8 Nov 2023 11:53:56 -0800 Subject: [PATCH 13/15] Adding embedding error tracing --- newrelic/hooks/external_botocore.py | 12 +- .../_mock_external_bedrock_server.py | 112 +++++++++++------- .../_test_bedrock_chat_completion.py | 8 +- .../_test_bedrock_embeddings.py | 23 ++++ tests/external_botocore/conftest.py | 2 +- .../test_bedrock_chat_completion.py | 2 +- .../test_bedrock_embeddings.py | 46 ++++++- 7 files changed, 145 insertions(+), 60 deletions(-) diff --git a/newrelic/hooks/external_botocore.py b/newrelic/hooks/external_botocore.py index a28082e4a8..618c3a32a6 100644 --- a/newrelic/hooks/external_botocore.py +++ b/newrelic/hooks/external_botocore.py @@ -64,7 +64,7 @@ def bedrock_error_attributes(exception, request_args, client, extractor): error_attributes = extractor(request_body)[1] error_attributes.update({ - "request.id": response.get("ResponseMetadata", "").get("RequestId", ""), + "request_id": response.get("ResponseMetadata", {}).get("RequestId", ""), "api_key_last_four_digits": client._request_signer._credentials.access_key[-4:], "request.model": request_args.get("modelId", ""), "vendor": "Bedrock", @@ -152,9 +152,11 @@ def extract_bedrock_titan_text_model(request_body, response_body=None): def extract_bedrock_titan_embedding_model(request_body, response_body=None): + if not response_body: + return [], {} # No extracted information necessary for embedding + request_body = json.loads(request_body) - if response_body: - response_body = json.loads(response_body) + response_body = json.loads(response_body) input_tokens = response_body.get("inputTextTokenCount", None) @@ -163,7 +165,7 @@ def extract_bedrock_titan_embedding_model(request_body, response_body=None): "response.usage.prompt_tokens": input_tokens, "response.usage.total_tokens": input_tokens, } - return embedding_dict + return [], embedding_dict def extract_bedrock_ai21_j2_model(request_body, response_body=None): @@ -332,7 +334,7 @@ def handle_embedding_event(client, transaction, extractor, model, response_body, request_id = response_headers.get("x-amzn-requestid", "") settings = transaction.settings if transaction.settings is not None else global_settings() - embedding_dict = extractor(request_body, response_body) + _, embedding_dict = extractor(request_body, response_body) embedding_dict.update({ "vendor": "bedrock", diff --git a/tests/external_botocore/_mock_external_bedrock_server.py b/tests/external_botocore/_mock_external_bedrock_server.py index 42e430a124..da5ff68dd9 100644 --- a/tests/external_botocore/_mock_external_bedrock_server.py +++ b/tests/external_botocore/_mock_external_bedrock_server.py @@ -29,51 +29,6 @@ # 3) This app runs on a separate thread meaning it won't block the test app. RESPONSES = { - "ai21.j2-mid-v1::Invalid Token": [ - { - "Content-Type": "application/json", - "x-amzn-RequestId": "9021791d-3797-493d-9277-e33aa6f6d544", - "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", - }, - 403, - {"message": "The security token included in the request is invalid."}, - ], - "amazon.titan-text-express-v1::Invalid Token": [ - { - "Content-Type": "application/json", - "x-amzn-RequestId": "15b39c8b-8e85-42c9-9623-06720301bda3", - "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", - }, - 403, - {"message": "The security token included in the request is invalid."}, - ], - "anthropic.claude-instant-v1::Human: Invalid Token Assistant:": [ - { - "Content-Type": "application/json", - "x-amzn-RequestId": "37396f55-b721-4bae-9461-4c369f5a080d", - "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", - }, - 403, - {"message": "The security token included in the request is invalid."}, - ], - "cohere.command-text-v14::Invalid Token": [ - { - "Content-Type": "application/json", - "x-amzn-RequestId": "22476490-a0d6-42db-b5ea-32d0b8a7f751", - "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", - }, - 403, - {"message": "The security token included in the request is invalid."}, - ], - "does-not-exist::": [ - { - "Content-Type": "application/json", - "x-amzn-RequestId": "f4908827-3db9-4742-9103-2bbc34578b03", - "x-amzn-ErrorType": "ValidationException:http://internal.amazon.com/coral/com.amazon.bedrock/", - }, - 400, - {"message": "The provided model identifier is invalid."}, - ], "ai21.j2-mid-v1::What is 212 degrees Fahrenheit converted to Celsius?": [ {"Content-Type": "application/json", "x-amzn-RequestId": "c863d9fc-888b-421c-a175-ac5256baec62"}, 200, @@ -3377,9 +3332,71 @@ "prompt": "What is 212 degrees Fahrenheit converted to Celsius?", }, ], + "does-not-exist::": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "f4908827-3db9-4742-9103-2bbc34578b03", + "x-amzn-ErrorType": "ValidationException:http://internal.amazon.com/coral/com.amazon.bedrock/", + }, + 400, + {"message": "The provided model identifier is invalid."}, + ], + "ai21.j2-mid-v1::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "9021791d-3797-493d-9277-e33aa6f6d544", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "amazon.titan-embed-g1-text-02::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "73328313-506e-4da8-af0f-51017fa6ca3f", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "amazon.titan-embed-text-v1::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "aece6ad7-e2ff-443b-a953-ba7d385fd0cc", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "amazon.titan-text-express-v1::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "15b39c8b-8e85-42c9-9623-06720301bda3", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "anthropic.claude-instant-v1::Human: Invalid Token Assistant:": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "37396f55-b721-4bae-9461-4c369f5a080d", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], + "cohere.command-text-v14::Invalid Token": [ + { + "Content-Type": "application/json", + "x-amzn-RequestId": "22476490-a0d6-42db-b5ea-32d0b8a7f751", + "x-amzn-ErrorType": "UnrecognizedClientException:http://internal.amazon.com/coral/com.amazon.coral.service/", + }, + 403, + {"message": "The security token included in the request is invalid."}, + ], } - MODEL_PATH_RE = re.compile(r"/model/([^/]+)/invoke") @@ -3435,6 +3452,9 @@ def __init__(self, handler=simple_get, port=None, *args, **kwargs): if __name__ == "__main__": + # Use this to sort dict for easier future incremental updates + print("RESPONSES = %s" % dict(sorted(RESPONSES.items(), key=lambda i: (i[1][1], i[0])))) + with MockExternalBedrockServer() as server: print("MockExternalBedrockServer serving on port %s" % str(server.port)) while True: diff --git a/tests/external_botocore/_test_bedrock_chat_completion.py b/tests/external_botocore/_test_bedrock_chat_completion.py index bb4e479bdd..1a66d74e43 100644 --- a/tests/external_botocore/_test_bedrock_chat_completion.py +++ b/tests/external_botocore/_test_bedrock_chat_completion.py @@ -264,7 +264,7 @@ chat_completion_expected_client_errors = { "amazon.titan-text-express-v1": { "conversation_id": "my-awesome-id", - "request.id": "15b39c8b-8e85-42c9-9623-06720301bda3", + "request_id": "15b39c8b-8e85-42c9-9623-06720301bda3", "api_key_last_four_digits": "-KEY", "request.model": "amazon.titan-text-express-v1", "request.temperature": 0.7, @@ -277,7 +277,7 @@ }, "ai21.j2-mid-v1": { "conversation_id": "my-awesome-id", - "request.id": "9021791d-3797-493d-9277-e33aa6f6d544", + "request_id": "9021791d-3797-493d-9277-e33aa6f6d544", "api_key_last_four_digits": "-KEY", "request.model": "ai21.j2-mid-v1", "request.temperature": 0.7, @@ -290,7 +290,7 @@ }, "anthropic.claude-instant-v1": { "conversation_id": "my-awesome-id", - "request.id": "37396f55-b721-4bae-9461-4c369f5a080d", + "request_id": "37396f55-b721-4bae-9461-4c369f5a080d", "api_key_last_four_digits": "-KEY", "request.model": "anthropic.claude-instant-v1", "request.temperature": 0.7, @@ -303,7 +303,7 @@ }, "cohere.command-text-v14": { "conversation_id": "my-awesome-id", - "request.id": "22476490-a0d6-42db-b5ea-32d0b8a7f751", + "request_id": "22476490-a0d6-42db-b5ea-32d0b8a7f751", "api_key_last_four_digits": "-KEY", "request.model": "cohere.command-text-v14", "request.temperature": 0.7, diff --git a/tests/external_botocore/_test_bedrock_embeddings.py b/tests/external_botocore/_test_bedrock_embeddings.py index ddb35565b7..8fb2ceecee 100644 --- a/tests/external_botocore/_test_bedrock_embeddings.py +++ b/tests/external_botocore/_test_bedrock_embeddings.py @@ -49,3 +49,26 @@ ), ] } + +embedding_expected_client_errors = { + "amazon.titan-embed-text-v1": { + "request_id": "aece6ad7-e2ff-443b-a953-ba7d385fd0cc", + "api_key_last_four_digits": "-KEY", + "request.model": "amazon.titan-embed-text-v1", + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The security token included in the request is invalid.", + "error.code": "UnrecognizedClientException", + }, + "amazon.titan-embed-g1-text-02": { + "request_id": "73328313-506e-4da8-af0f-51017fa6ca3f", + "api_key_last_four_digits": "-KEY", + "request.model": "amazon.titan-embed-g1-text-02", + "vendor": "Bedrock", + "ingest_source": "Python", + "http.statusCode": 403, + "error.message": "The security token included in the request is invalid.", + "error.code": "UnrecognizedClientException", + }, +} diff --git a/tests/external_botocore/conftest.py b/tests/external_botocore/conftest.py index e0e83329b2..8b19d3ce75 100644 --- a/tests/external_botocore/conftest.py +++ b/tests/external_botocore/conftest.py @@ -98,7 +98,7 @@ def bedrock_server(): yield client # Run tests # Write responses to audit log - bedrock_audit_log_contents = dict(sorted(BEDROCK_AUDIT_LOG_CONTENTS.items(), key=lambda i: i[0])) + bedrock_audit_log_contents = dict(sorted(BEDROCK_AUDIT_LOG_CONTENTS.items(), key=lambda i: (i[1][1], i[0]))) with open(BEDROCK_AUDIT_LOG_FILE, "w") as audit_log_fp: json.dump(bedrock_audit_log_contents, fp=audit_log_fp, indent=4) diff --git a/tests/external_botocore/test_bedrock_chat_completion.py b/tests/external_botocore/test_bedrock_chat_completion.py index 1ad87b0a4b..9dcbbfdfab 100644 --- a/tests/external_botocore/test_bedrock_chat_completion.py +++ b/tests/external_botocore/test_bedrock_chat_completion.py @@ -184,7 +184,7 @@ def test_bedrock_chat_completion_disabled_settings(set_trace_info, exercise_mode "intrinsic": {}, "user": { "conversation_id": "my-awesome-id", - "request.id": "f4908827-3db9-4742-9103-2bbc34578b03", + "request_id": "f4908827-3db9-4742-9103-2bbc34578b03", "api_key_last_four_digits": "CRET", "request.model": "does-not-exist", "vendor": "Bedrock", diff --git a/tests/external_botocore/test_bedrock_embeddings.py b/tests/external_botocore/test_bedrock_embeddings.py index 022eb07599..c374dd69c5 100644 --- a/tests/external_botocore/test_bedrock_embeddings.py +++ b/tests/external_botocore/test_bedrock_embeddings.py @@ -12,12 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy +import botocore.exceptions + import json from io import BytesIO import pytest -from testing_support.fixtures import ( # override_application_settings, +from testing_support.fixtures import ( + dt_enabled, override_application_settings, reset_core_stats_engine, ) @@ -26,10 +28,17 @@ from testing_support.validators.validate_transaction_metrics import ( validate_transaction_metrics, ) +from testing_support.validators.validate_span_events import validate_span_events +from testing_support.validators.validate_error_trace_attributes import ( + validate_error_trace_attributes, +) from newrelic.api.background_task import background_task -from _test_bedrock_embeddings import embedding_expected_events, embedding_payload_templates +from _test_bedrock_embeddings import embedding_expected_events, embedding_payload_templates, embedding_expected_client_errors + +from newrelic.common.object_names import callable_name + disabled_ml_insights_settings = {"ml_insights_events.enabled": False} @@ -76,6 +85,11 @@ def expected_events(model_id): return embedding_expected_events[model_id] +@pytest.fixture(scope="module") +def expected_client_error(model_id): + return embedding_expected_client_errors[model_id] + + @reset_core_stats_engine() def test_bedrock_embedding(set_trace_info, exercise_model, expected_events): @validate_ml_events(expected_events) @@ -101,6 +115,10 @@ def test_bedrock_embedding_outside_txn(exercise_model): exercise_model(prompt="This is an embedding test.") +_client_error = botocore.exceptions.ClientError +_client_error_name = callable_name(_client_error) + + @override_application_settings(disabled_ml_insights_settings) @reset_core_stats_engine() @validate_ml_event_count(count=0) @@ -115,3 +133,25 @@ def test_bedrock_embedding_outside_txn(exercise_model): def test_bedrock_embedding_disabled_settings(set_trace_info, exercise_model): set_trace_info() exercise_model(prompt="This is an embedding test.") + + +@dt_enabled +@reset_core_stats_engine() +def test_bedrock_embedding_error_incorrect_access_key(monkeypatch, bedrock_server, exercise_model, set_trace_info, expected_client_error): + @validate_error_trace_attributes( + _client_error_name, + exact_attrs={ + "agent": {}, + "intrinsic": {}, + "user": expected_client_error, + }, + ) + @background_task() + def _test(): + monkeypatch.setattr(bedrock_server._request_signer._credentials, "access_key", "INVALID-ACCESS-KEY") + + with pytest.raises(_client_error): # not sure where this exception actually comes from + set_trace_info() + exercise_model(prompt="Invalid Token", temperature=0.7, max_tokens=100) + + _test() From 1eaca3766868493645fc9810ad5eff2ce21a03db Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Wed, 8 Nov 2023 12:36:34 -0800 Subject: [PATCH 14/15] Delete comment --- tests/external_botocore/test_bedrock_chat_completion.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/external_botocore/test_bedrock_chat_completion.py b/tests/external_botocore/test_bedrock_chat_completion.py index 9dcbbfdfab..a1eb881cc1 100644 --- a/tests/external_botocore/test_bedrock_chat_completion.py +++ b/tests/external_botocore/test_bedrock_chat_completion.py @@ -175,8 +175,6 @@ def test_bedrock_chat_completion_disabled_settings(set_trace_info, exercise_mode _client_error_name = callable_name(_client_error) -# No prompt provided - @validate_error_trace_attributes( "botocore.errorfactory:ValidationException", exact_attrs={ From a976e75888d44c46f83eb7be0cffc4f987d51741 Mon Sep 17 00:00:00 2001 From: Tim Pansino Date: Wed, 8 Nov 2023 15:31:29 -0800 Subject: [PATCH 15/15] Update moto --- tests/external_botocore/test_botocore_dynamodb.py | 2 +- tox.ini | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/tests/external_botocore/test_botocore_dynamodb.py b/tests/external_botocore/test_botocore_dynamodb.py index 6ce9f12c33..932fb1743a 100644 --- a/tests/external_botocore/test_botocore_dynamodb.py +++ b/tests/external_botocore/test_botocore_dynamodb.py @@ -80,7 +80,7 @@ background_task=True, ) @background_task() -@moto.mock_dynamodb2 +@moto.mock_dynamodb def test_dynamodb(): session = botocore.session.get_session() client = session.create_client( diff --git a/tox.ini b/tox.ini index 720c301dcd..bb6102d895 100644 --- a/tox.ini +++ b/tox.ini @@ -252,12 +252,11 @@ deps = datastore_redis-redis0400: redis<4.1 external_botocore-botocorelatest: botocore external_botocore-botocorelatest: boto3 - external_botocore-botocorelatest: moto external_botocore-botocore128: botocore<1.29 external_botocore-botocore0125: botocore<1.26 - external_botocore-{py37,py38,py39,py310,py311}: moto[awslambda,ec2,iam]<3.0 + external_botocore-{py37,py38,py39,py310,py311}: moto[awslambda,ec2,iam,sqs] external_botocore-py27: rsa<4.7.1 - external_botocore-py27: moto[awslambda,ec2,iam]<2.0 + external_botocore-py27: moto[awslambda,ec2,iam,sqs]<2.0 external_feedparser-feedparser05: feedparser<6 external_feedparser-feedparser06: feedparser<7 external_httplib2: httplib2<1.0