Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add aiohttp support for expected status codes #735

Merged
merged 5 commits into from
Feb 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion newrelic/api/time_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -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, "_expect_errors"):
is_expected = transaction._expect_errors(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
Expand Down Expand Up @@ -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

Expand Down
18 changes: 17 additions & 1 deletion newrelic/hooks/framework_aiohttp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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._expect_errors = is_expected(transaction)

import aiohttp

transaction.add_framework_info(name="aiohttp", version=aiohttp.__version__)
Expand Down
12 changes: 8 additions & 4 deletions tests/agent_features/test_ignore_expected_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,9 @@ def test_classes_error_event_inside_transaction(settings, expected, ignore):

error_count = 1 if not ignore else 0
errors = _test_runtime_error if not ignore else []
expected_errors = _runtime_error_name if expected and not ignore else None

@validate_transaction_errors(errors=errors)
@validate_transaction_errors(errors=errors, expected_errors=expected_errors)
@validate_error_event_sample_data(
required_attrs=attributes,
required_user_attrs=False,
Expand Down Expand Up @@ -268,8 +269,9 @@ def test_status_codes_inside_transaction(settings, expected, ignore, status_code

error_count = 1 if not ignore else 0
errors = _test_teapot_error if not ignore else []
expected_errors = _teapot_error_name if expected and not ignore else None

@validate_transaction_errors(errors=errors)
@validate_transaction_errors(errors=errors, expected_errors=expected_errors)
@validate_error_event_sample_data(
required_attrs=attributes,
required_user_attrs=False,
Expand Down Expand Up @@ -359,8 +361,9 @@ def test_mixed_ignore_expected_settings_inside_transaction(

error_count = 1 if not ignore else 0
errors = _test_runtime_error if not ignore else []
expected_errors = _runtime_error_name if expected and not ignore else None

@validate_transaction_errors(errors=errors)
@validate_transaction_errors(errors=errors, expected_errors=expected_errors)
@validate_error_event_sample_data(
required_attrs=attributes,
required_user_attrs=False,
Expand Down Expand Up @@ -428,8 +431,9 @@ def test_overrides_inside_transaction(override, result, parameter):

error_count = 1 if not ignore else 0
errors = _test_runtime_error if not ignore else []
expected_errors = _runtime_error_name if expected and not ignore else None

@validate_transaction_errors(errors=errors)
@validate_transaction_errors(errors=errors, expected_errors=expected_errors)
@validate_error_event_sample_data(
required_attrs=attributes,
required_user_attrs=False,
Expand Down
5 changes: 5 additions & 0 deletions tests/framework_aiohttp/_target_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()

Expand Down Expand Up @@ -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)
Expand Down
7 changes: 6 additions & 1 deletion tests/framework_aiohttp/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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):
Expand All @@ -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=[
Expand Down Expand Up @@ -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())

Expand Down
25 changes: 25 additions & 0 deletions tests/testing_support/fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -1468,6 +1468,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"]
Expand Down
13 changes: 10 additions & 3 deletions tests/testing_support/validators/validate_transaction_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,20 @@
# 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,
)
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
TimPansino marked this conversation as resolved.
Show resolved Hide resolved
):
errors = errors or []
required_params = required_params or []
forgone_params = forgone_params or []
expected_errors = expected_errors or []
captured_errors = []

@transient_function_wrapper("newrelic.core.stats_engine", "StatsEngine.record_transaction")
Expand Down Expand Up @@ -71,6 +73,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