From 072d5a365bfe8b04d280d4185834dc29ad082bf4 Mon Sep 17 00:00:00 2001 From: Justin Richert Date: Tue, 3 Jan 2023 17:04:39 -0600 Subject: [PATCH] Add aiohttp support for expected status codes --- newrelic/api/time_trace.py | 6 ++++- newrelic/hooks/framework_aiohttp.py | 18 ++++++++++++- .../framework_aiohttp/_target_application.py | 5 ++++ tests/framework_aiohttp/test_server.py | 7 +++++- tests/testing_support/fixtures.py | 25 +++++++++++++++++++ .../validators/validate_transaction_errors.py | 18 ++++++++----- 6 files changed, 70 insertions(+), 9 deletions(-) diff --git a/newrelic/api/time_trace.py b/newrelic/api/time_trace.py index dc010674cb..657332af06 100644 --- a/newrelic/api/time_trace.py +++ b/newrelic/api/time_trace.py @@ -327,6 +327,10 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c if is_expected is None and callable(expected): is_expected = expected(exc, value, tb) + # Callable on transaction + if is_expected is None and hasattr(transaction, "_is_expected"): + is_expected = transaction._is_expected(exc, value, tb) + # List of class names if is_expected is None and expected is not None and not callable(expected): # Do not set is_expected to False @@ -631,7 +635,7 @@ def get_service_linking_metadata(application=None, settings=None): if application is None: from newrelic.api.application import application_instance application = application_instance(activate=False) - + if application is not None: settings = application.settings diff --git a/newrelic/hooks/framework_aiohttp.py b/newrelic/hooks/framework_aiohttp.py index de72ae0c5b..cc13894350 100644 --- a/newrelic/hooks/framework_aiohttp.py +++ b/newrelic/hooks/framework_aiohttp.py @@ -26,7 +26,7 @@ function_wrapper, wrap_function_wrapper, ) -from newrelic.core.config import should_ignore_error +from newrelic.core.config import is_expected_error, should_ignore_error SUPPORTED_METHODS = ("connect", "head", "get", "delete", "options", "patch", "post", "put", "trace") @@ -61,6 +61,19 @@ def _should_ignore(exc, value, tb): return _should_ignore +def is_expected(transaction): + settings = transaction.settings + + def _is_expected(exc, value, tb): + from aiohttp import web + + if isinstance(value, web.HTTPException): + status_code = value.status_code + return is_expected_error((exc, value, tb), status_code, settings=settings) + + return _is_expected + + def _nr_process_response_proxy(response, transaction): nr_headers = transaction.process_response(response.status, response.headers) @@ -338,6 +351,9 @@ async def _coro(*_args, **_kwargs): # Patch in should_ignore to all notice_error calls transaction._ignore_errors = should_ignore(transaction) + # Patch in is_expected to all notice_error calls + transaction._is_expected = is_expected(transaction) + import aiohttp transaction.add_framework_info(name="aiohttp", version=aiohttp.__version__) diff --git a/tests/framework_aiohttp/_target_application.py b/tests/framework_aiohttp/_target_application.py index 77d6fef6c9..f15e7fd65b 100644 --- a/tests/framework_aiohttp/_target_application.py +++ b/tests/framework_aiohttp/_target_application.py @@ -40,6 +40,10 @@ async def non_500_error(request): raise web.HTTPGone() +async def raise_403(request): + raise web.HTTPForbidden() + + async def raise_404(request): raise web.HTTPNotFound() @@ -167,6 +171,7 @@ def make_app(middlewares=None): app.router.add_route("*", "/error", error) app.router.add_route("*", "/known_error", KnownErrorView) app.router.add_route("*", "/non_500_error", non_500_error) + app.router.add_route("*", "/raise_403", raise_403) app.router.add_route("*", "/raise_404", raise_404) app.router.add_route("*", "/hang", hang) app.router.add_route("*", "/background", background) diff --git a/tests/framework_aiohttp/test_server.py b/tests/framework_aiohttp/test_server.py index aa6218c28a..6a5ef0d10e 100644 --- a/tests/framework_aiohttp/test_server.py +++ b/tests/framework_aiohttp/test_server.py @@ -19,6 +19,7 @@ from testing_support.fixtures import ( count_transactions, override_application_settings, + override_expected_status_codes, override_generic_settings, override_ignore_status_codes, ) @@ -64,6 +65,7 @@ ("/error?hello=world", "_target_application:error", "builtins:ValueError", 500), ("/non_500_error?hello=world", "_target_application:non_500_error", "aiohttp.web_exceptions:HTTPGone", 410), ("/raise_404?hello=world", "_target_application:raise_404", None, 404), + ("/raise_403?hello=world", "_target_application:raise_403", "aiohttp.web_exceptions:HTTPForbidden", 403), ], ) def test_error_exception(method, uri, metric_name, error, status, nr_enabled, aiohttp_app): @@ -79,7 +81,9 @@ async def fetch(): if error: errors.append(error) - @validate_transaction_errors(errors=errors) + @validate_transaction_errors( + errors=errors, expected_errors=["aiohttp.web_exceptions:HTTPForbidden"] + ) @validate_transaction_metrics( metric_name, scoped_metrics=[ @@ -111,6 +115,7 @@ async def fetch(): ) @validate_code_level_metrics(*metric_name.split(":")) @override_ignore_status_codes([404]) + @override_expected_status_codes([403]) def _test(): aiohttp_app.loop.run_until_complete(fetch()) diff --git a/tests/testing_support/fixtures.py b/tests/testing_support/fixtures.py index f642d1f6f1..e0a2f9abd3 100644 --- a/tests/testing_support/fixtures.py +++ b/tests/testing_support/fixtures.py @@ -1479,6 +1479,31 @@ def _override_ignore_status_codes(wrapped, instance, args, kwargs): return _override_ignore_status_codes +def override_expected_status_codes(status_codes): + @function_wrapper + def _override_expected_status_codes(wrapped, instance, args, kwargs): + # Updates can be made to expected status codes in server + # side configs. Changes will be applied to application + # settings so we first check there and if they don't + # exist, we default to global settings + + application = application_instance() + settings = application and application.settings + + if not settings: + settings = global_settings() + + original = settings.error_collector.expected_status_codes + + try: + settings.error_collector.expected_status_codes = status_codes + return wrapped(*args, **kwargs) + finally: + settings.error_collector.expected_status_codes = original + + return _override_expected_status_codes + + def code_coverage_fixture(source=None): if source is None: source = ["newrelic"] diff --git a/tests/testing_support/validators/validate_transaction_errors.py b/tests/testing_support/validators/validate_transaction_errors.py index b00b7facd1..75415da5a7 100644 --- a/tests/testing_support/validators/validate_transaction_errors.py +++ b/tests/testing_support/validators/validate_transaction_errors.py @@ -12,15 +12,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -import copy - from newrelic.common.object_wrapper import ( - function_wrapper, - transient_function_wrapper, -) + function_wrapper, + transient_function_wrapper, + ) from testing_support.fixtures import catch_background_exceptions -def validate_transaction_errors(errors=None, required_params=None, forgone_params=None): + +def validate_transaction_errors( + errors=None, required_params=None, forgone_params=None, expected_errors=None +): errors = errors or [] required_params = required_params or [] forgone_params = forgone_params or [] @@ -71,6 +72,11 @@ def _validate_transaction_errors(wrapped, instance, args, kwargs): for name, value in forgone_params: assert name not in e.custom_params, "name=%r, params=%r" % (name, e.custom_params) + if e.type in expected_errors: + assert e.expected is True + else: + assert e.expected is False + return output return _validate_transaction_errors