From f152d0b90259f1ea919f58d51f009eafb3c5753f Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 4 Nov 2020 16:56:33 -0800 Subject: [PATCH 1/8] test matcher improvements for py2/py3 interoperability --- sentry_sdk/_compat.py | 1 - tests/conftest.py | 86 +++++++++++++++++++++++++++++++++---------- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/sentry_sdk/_compat.py b/sentry_sdk/_compat.py index b7f79c1f48..49a55392a7 100644 --- a/sentry_sdk/_compat.py +++ b/sentry_sdk/_compat.py @@ -7,7 +7,6 @@ from typing import Tuple from typing import Any from typing import Type - from typing import TypeVar T = TypeVar("T") diff --git a/tests/conftest.py b/tests/conftest.py index 6c53e502ef..35631bcd70 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -355,8 +355,14 @@ class StringContaining(object): def __init__(self, substring): self.substring = substring + try: + # unicode only exists in python 2 + self.valid_types = (str, unicode) # noqa + except NameError: + self.valid_types = (str,) + def __eq__(self, test_string): - if not isinstance(test_string, str): + if not isinstance(test_string, self.valid_types): return False if len(self.substring) > len(test_string): @@ -364,9 +370,45 @@ def __eq__(self, test_string): return self.substring in test_string + def __ne__(self, test_string): + return not self.__eq__(test_string) + return StringContaining +def _safe_is_equal(x, y): + """ + Compares two values, preferring to use the first's __eq__ method if it + exists and is implemented. + + Accounts for py2/py3 differences (like ints in py2 not having a __eq__ + method), as well as the incomparability of certain types exposed by using + raw __eq__ () rather than ==. + """ + + # Prefer using __eq__ directly to ensure that examples like + # + # maisey = Dog() + # maisey.name = "Maisey the Dog" + # maisey == ObjectDescribedBy(attrs={"name": StringContaining("Maisey")}) + # + # evaluate to True (in other words, examples where the values in self.attrs + # might also have custom __eq__ methods; this makes sure those methods get + # used if possible) + try: + is_equal = x.__eq__(y) + except AttributeError: + is_equal = NotImplemented + + # this can happen on its own, too (i.e. without an AttributeError being + # thrown), which is why this is separate from the except block above + if is_equal == NotImplemented: + # using == smoothes out weird variations exposed by raw __eq__ + return x == y + + return is_equal + + @pytest.fixture(name="DictionaryContaining") def dictionary_containing_matcher(): """ @@ -397,13 +439,19 @@ def __eq__(self, test_dict): if len(self.subdict) > len(test_dict): return False - # Have to test self == other (rather than vice-versa) in case - # any of the values in self.subdict is another matcher with a custom - # __eq__ method (in LHS == RHS, LHS's __eq__ is tried before RHS's). - # In other words, this order is important so that examples like - # {"dogs": "are great"} == DictionaryContaining({"dogs": StringContaining("great")}) - # evaluate to True - return all(self.subdict[key] == test_dict.get(key) for key in self.subdict) + for key, value in self.subdict.items(): + try: + test_value = test_dict[key] + except KeyError: # missing key + return False + + if not _safe_is_equal(value, test_value): + return False + + return True + + def __ne__(self, test_dict): + return not self.__eq__(test_dict) return DictionaryContaining @@ -442,19 +490,19 @@ def __eq__(self, test_obj): if not isinstance(test_obj, self.type): return False - # all checks here done with getattr rather than comparing to - # __dict__ because __dict__ isn't guaranteed to exist if self.attrs: - # attributes must exist AND values must match - try: - if any( - getattr(test_obj, attr_name) != attr_value - for attr_name, attr_value in self.attrs.items() - ): - return False # wrong attribute value - except AttributeError: # missing attribute - return False + for attr_name, attr_value in self.attrs.items(): + try: + test_value = getattr(test_obj, attr_name) + except AttributeError: # missing attribute + return False + + if not _safe_is_equal(attr_value, test_value): + return False return True + def __ne__(self, test_obj): + return not self.__eq__(test_obj) + return ObjectDescribedBy From 8c5b8deccbd8a75a350369c910f4c17c756d77c4 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 4 Nov 2020 17:03:36 -0800 Subject: [PATCH 2/8] close resources in tests to eliminate warnings --- tests/integrations/aws_lambda/client.py | 12 +++++++++++- tests/integrations/gcp/test_gcp.py | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/integrations/aws_lambda/client.py b/tests/integrations/aws_lambda/client.py index 12b59ca60a..493f60baa7 100644 --- a/tests/integrations/aws_lambda/client.py +++ b/tests/integrations/aws_lambda/client.py @@ -69,9 +69,19 @@ def run_lambda_function( ) @add_finalizer - def delete_function(): + def clean_up(): client.delete_function(FunctionName=fn_name) + # this closes the web socket so we don't get a + # ResourceWarning: unclosed + # warning on every test + # based on https://github.com/boto/botocore/pull/1810 + # (if that's ever merged, this can just become client.close()) + session = client._endpoint.http_session + managers = [session._manager] + list(session._proxy_managers.values()) + for manager in managers: + manager.clear() + response = client.invoke( FunctionName=fn_name, InvocationType="RequestResponse", diff --git a/tests/integrations/gcp/test_gcp.py b/tests/integrations/gcp/test_gcp.py index fa234a0da3..06cc026380 100644 --- a/tests/integrations/gcp/test_gcp.py +++ b/tests/integrations/gcp/test_gcp.py @@ -112,6 +112,8 @@ def inner(code, subprocess_kwargs=()): stream = os.popen("python {}/main.py".format(tmpdir)) stream_data = stream.read() + stream.close() + for line in stream_data.splitlines(): print("GCP:", line) if line.startswith("EVENT: "): From 4e4c774b7eaa5384edfc64ba649f6778fed10310 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 4 Nov 2020 16:54:27 -0800 Subject: [PATCH 3/8] factor code getting lambda bootstrap out into a function --- sentry_sdk/integrations/aws_lambda.py | 42 ++++++++++++++++----------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/sentry_sdk/integrations/aws_lambda.py b/sentry_sdk/integrations/aws_lambda.py index cb7dc38b14..fac2ec2116 100644 --- a/sentry_sdk/integrations/aws_lambda.py +++ b/sentry_sdk/integrations/aws_lambda.py @@ -177,23 +177,8 @@ def __init__(self, timeout_warning=False): def setup_once(): # type: () -> None - # Python 2.7: Everything is in `__main__`. - # - # Python 3.7: If the bootstrap module is *already imported*, it is the - # one we actually want to use (no idea what's in __main__) - # - # On Python 3.8 bootstrap is also importable, but will be the same file - # as __main__ imported under a different name: - # - # sys.modules['__main__'].__file__ == sys.modules['bootstrap'].__file__ - # sys.modules['__main__'] is not sys.modules['bootstrap'] - # - # Such a setup would then make all monkeypatches useless. - if "bootstrap" in sys.modules: - lambda_bootstrap = sys.modules["bootstrap"] # type: Any - elif "__main__" in sys.modules: - lambda_bootstrap = sys.modules["__main__"] - else: + lambda_bootstrap = get_lambda_bootstrap() + if not lambda_bootstrap: logger.warning( "Not running in AWS Lambda environment, " "AwsLambdaIntegration disabled (could not find bootstrap module)" @@ -280,6 +265,29 @@ def inner(*args, **kwargs): ) +def get_lambda_bootstrap(): + # type: () -> Optional[Any] + + # Python 2.7: Everything is in `__main__`. + # + # Python 3.7: If the bootstrap module is *already imported*, it is the + # one we actually want to use (no idea what's in __main__) + # + # On Python 3.8 bootstrap is also importable, but will be the same file + # as __main__ imported under a different name: + # + # sys.modules['__main__'].__file__ == sys.modules['bootstrap'].__file__ + # sys.modules['__main__'] is not sys.modules['bootstrap'] + # + # Such a setup would then make all monkeypatches useless. + if "bootstrap" in sys.modules: + return sys.modules["bootstrap"] + elif "__main__" in sys.modules: + return sys.modules["__main__"] + else: + return None + + def _make_request_event_processor(aws_event, aws_context, configured_timeout): # type: (Any, Any, Any) -> EventProcessor start_time = datetime.utcnow() From fa068aa4f9410b0f81733627d8b12a8e915cc133 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 4 Nov 2020 17:04:00 -0800 Subject: [PATCH 4/8] install mock in aws environment so it can be used in tests --- tests/integrations/aws_lambda/client.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/integrations/aws_lambda/client.py b/tests/integrations/aws_lambda/client.py index 493f60baa7..17181c54ee 100644 --- a/tests/integrations/aws_lambda/client.py +++ b/tests/integrations/aws_lambda/client.py @@ -49,6 +49,13 @@ def run_lambda_function( **subprocess_kwargs ) + subprocess.check_call( + "pip install mock==3.0.0 funcsigs -t .", + cwd=tmpdir, + shell=True, + **subprocess_kwargs + ) + # https://docs.aws.amazon.com/lambda/latest/dg/lambda-python-how-to-create-deployment-package.html subprocess.check_call( "pip install ../*.tar.gz -t .", cwd=tmpdir, shell=True, **subprocess_kwargs From 84e964a83ce7ce1ac62a93090ceea98045b0e2a6 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 4 Nov 2020 17:08:32 -0800 Subject: [PATCH 5/8] add sampling context for aws integration --- sentry_sdk/integrations/aws_lambda.py | 24 ++++--- tests/integrations/aws_lambda/test_aws.py | 80 ++++++++++++++++++++++- 2 files changed, 95 insertions(+), 9 deletions(-) diff --git a/sentry_sdk/integrations/aws_lambda.py b/sentry_sdk/integrations/aws_lambda.py index fac2ec2116..335c08eee7 100644 --- a/sentry_sdk/integrations/aws_lambda.py +++ b/sentry_sdk/integrations/aws_lambda.py @@ -65,7 +65,7 @@ def sentry_init_error(*args, **kwargs): def _wrap_handler(handler): # type: (F) -> F - def sentry_handler(aws_event, context, *args, **kwargs): + def sentry_handler(aws_event, aws_context, *args, **kwargs): # type: (Any, Any, *Any, **Any) -> Any # Per https://docs.aws.amazon.com/lambda/latest/dg/python-handler.html, @@ -94,21 +94,23 @@ def sentry_handler(aws_event, context, *args, **kwargs): hub = Hub.current integration = hub.get_integration(AwsLambdaIntegration) if integration is None: - return handler(aws_event, context, *args, **kwargs) + return handler(aws_event, aws_context, *args, **kwargs) # If an integration is there, a client has to be there. client = hub.client # type: Any - configured_time = context.get_remaining_time_in_millis() + configured_time = aws_context.get_remaining_time_in_millis() with hub.push_scope() as scope: with capture_internal_exceptions(): scope.clear_breadcrumbs() scope.add_event_processor( _make_request_event_processor( - request_data, context, configured_time + request_data, aws_context, configured_time ) ) - scope.set_tag("aws_region", context.invoked_function_arn.split(":")[3]) + scope.set_tag( + "aws_region", aws_context.invoked_function_arn.split(":")[3] + ) if batch_size > 1: scope.set_tag("batch_request", True) scope.set_tag("batch_size", batch_size) @@ -134,11 +136,17 @@ def sentry_handler(aws_event, context, *args, **kwargs): headers = request_data.get("headers", {}) transaction = Transaction.continue_from_headers( - headers, op="serverless.function", name=context.function_name + headers, op="serverless.function", name=aws_context.function_name ) - with hub.start_transaction(transaction): + with hub.start_transaction( + transaction, + custom_sampling_context={ + "aws_event": aws_event, + "aws_context": aws_context, + }, + ): try: - return handler(aws_event, context, *args, **kwargs) + return handler(aws_event, aws_context, *args, **kwargs) except Exception: exc_info = sys.exc_info() sentry_event, hint = event_from_exception( diff --git a/tests/integrations/aws_lambda/test_aws.py b/tests/integrations/aws_lambda/test_aws.py index 41585387b1..320155df45 100644 --- a/tests/integrations/aws_lambda/test_aws.py +++ b/tests/integrations/aws_lambda/test_aws.py @@ -27,7 +27,7 @@ LAMBDA_PRELUDE = """ from __future__ import print_function -from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration +from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration, get_lambda_bootstrap import sentry_sdk import json import time @@ -69,6 +69,7 @@ def envelope_processor(envelope): return envelope_data + class TestTransport(HttpTransport): def _send_event(self, event): event = event_processor(event) @@ -82,6 +83,7 @@ def _send_envelope(self, envelope): envelope = envelope_processor(envelope) print("\\nENVELOPE: {}\\n".format(json.dumps(envelope))) + def init_sdk(timeout_warning=False, **extra_init_args): sentry_sdk.init( dsn="https://123abc@example.com/123", @@ -508,3 +510,79 @@ def test_handler(event, context): assert error_event["tags"]["batch_request"] is True assert envelope["tags"]["batch_size"] == batch_size assert envelope["tags"]["batch_request"] is True + + +def test_traces_sampler_gets_correct_values_in_sampling_context( + run_lambda_function, + DictionaryContaining, # noqa:N803 + ObjectDescribedBy, # noqa:N803 + StringContaining, # noqa:N803 +): + import inspect + + envelopes, events, response = run_lambda_function( + LAMBDA_PRELUDE + + dedent(inspect.getsource(StringContaining)) + + dedent(inspect.getsource(DictionaryContaining)) + + dedent(inspect.getsource(ObjectDescribedBy)) + + dedent( + """ + try: + from unittest import mock # python 3.3 and above + except ImportError: + import mock # python < 3.3 + + def _safe_is_equal(x, y): + # copied from conftest.py - see docstring and comments there + try: + is_equal = x.__eq__(y) + except AttributeError: + is_equal = NotImplemented + + if is_equal == NotImplemented: + # using == smoothes out weird variations exposed by raw __eq__ + return x == y + + return is_equal + + def test_handler(event, context): + # this runs after the transaction has started, which means we + # can make assertions about traces_sampler + try: + traces_sampler.assert_any_call( + DictionaryContaining( + { + "aws_event": DictionaryContaining({ + "httpMethod": "GET", + "path": "/sit/stay/rollover", + "headers": {"Host": "dogs.are.great", "X-Forwarded-Proto": "http"}, + }), + "aws_context": ObjectDescribedBy( + type=get_lambda_bootstrap().LambdaContext, + attrs={ + 'function_name': StringContaining("test_function"), + 'function_version': '$LATEST', + } + ) + } + ) + ) + except AssertionError as e: + # catch the error and print it because the error itself will + # get swallowed by the SDK as an "internal exception" + print("\\nERROR: AssertionError: traces_sampler not called with the specified argument.") + + return "dogs are great" + + + traces_sampler = mock.Mock(return_value=True) + + init_sdk( + traces_sampler=traces_sampler, + ) + """ + ), + b'{"httpMethod": "GET", "path": "/sit/stay/rollover", "headers": {"Host": "dogs.are.great", "X-Forwarded-Proto": "http"}}', + ) + + assert "AssertionError" not in str(response["LogResult"]) From 50f900c9b5e1917fcf051af1a81a92f9b7233f9e Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Wed, 4 Nov 2020 17:17:00 -0800 Subject: [PATCH 6/8] clarify "event" variables, add gcp context for sampler --- sentry_sdk/integrations/gcp.py | 34 ++++++++---- tests/integrations/gcp/test_gcp.py | 89 ++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 16 deletions(-) diff --git a/sentry_sdk/integrations/gcp.py b/sentry_sdk/integrations/gcp.py index 4f5d69bd65..e92422d8b9 100644 --- a/sentry_sdk/integrations/gcp.py +++ b/sentry_sdk/integrations/gcp.py @@ -34,13 +34,13 @@ def _wrap_func(func): # type: (F) -> F - def sentry_func(functionhandler, event, *args, **kwargs): + def sentry_func(functionhandler, gcp_event, *args, **kwargs): # type: (Any, Any, *Any, **Any) -> Any hub = Hub.current integration = hub.get_integration(GcpIntegration) if integration is None: - return func(functionhandler, event, *args, **kwargs) + return func(functionhandler, gcp_event, *args, **kwargs) # If an integration is there, a client has to be there. client = hub.client # type: Any @@ -50,7 +50,7 @@ def sentry_func(functionhandler, event, *args, **kwargs): logger.debug( "The configured timeout could not be fetched from Cloud Functions configuration." ) - return func(functionhandler, event, *args, **kwargs) + return func(functionhandler, gcp_event, *args, **kwargs) configured_time = int(configured_time) @@ -60,7 +60,9 @@ def sentry_func(functionhandler, event, *args, **kwargs): with capture_internal_exceptions(): scope.clear_breadcrumbs() scope.add_event_processor( - _make_request_event_processor(event, configured_time, initial_time) + _make_request_event_processor( + gcp_event, configured_time, initial_time + ) ) scope.set_tag("gcp_region", environ.get("FUNCTION_REGION")) timeout_thread = None @@ -76,22 +78,34 @@ def sentry_func(functionhandler, event, *args, **kwargs): timeout_thread.start() headers = {} - if hasattr(event, "headers"): - headers = event.headers + if hasattr(gcp_event, "headers"): + headers = gcp_event.headers transaction = Transaction.continue_from_headers( headers, op="serverless.function", name=environ.get("FUNCTION_NAME", "") ) - with hub.start_transaction(transaction): + sampling_context = { + "gcp_env": { + "function_name": environ.get("FUNCTION_NAME"), + "function_entry_point": environ.get("ENTRY_POINT"), + "function_identity": environ.get("FUNCTION_IDENTITY"), + "function_region": environ.get("FUNCTION_REGION"), + "function_project": environ.get("GCP_PROJECT"), + }, + "gcp_event": gcp_event, + } + with hub.start_transaction( + transaction, custom_sampling_context=sampling_context + ): try: - return func(functionhandler, event, *args, **kwargs) + return func(functionhandler, gcp_event, *args, **kwargs) except Exception: exc_info = sys.exc_info() - event, hint = event_from_exception( + sentry_event, hint = event_from_exception( exc_info, client_options=client.options, mechanism={"type": "gcp", "handled": False}, ) - hub.capture_event(event, hint=hint) + hub.capture_event(sentry_event, hint=hint) reraise(*exc_info) finally: if timeout_thread: diff --git a/tests/integrations/gcp/test_gcp.py b/tests/integrations/gcp/test_gcp.py index 06cc026380..3c4f1d2055 100644 --- a/tests/integrations/gcp/test_gcp.py +++ b/tests/integrations/gcp/test_gcp.py @@ -64,6 +64,7 @@ def _send_envelope(self, envelope): envelope = envelope_processor(envelope) print("\\nENVELOPE: {}\\n".format(envelope.decode(\"utf-8\"))) + def init_sdk(timeout_warning=False, **extra_init_args): sentry_sdk.init( dsn="https://123abc@example.com/123", @@ -125,13 +126,13 @@ def inner(code, subprocess_kwargs=()): else: continue - return envelope, event + return envelope, event, stream_data return inner def test_handled_exception(run_cloud_function): - envelope, event = run_cloud_function( + envelope, event, stream_data = run_cloud_function( dedent( """ functionhandler = None @@ -157,7 +158,7 @@ def cloud_function(functionhandler, event): def test_unhandled_exception(run_cloud_function): - envelope, event = run_cloud_function( + envelope, event, stream_data = run_cloud_function( dedent( """ functionhandler = None @@ -184,7 +185,7 @@ def cloud_function(functionhandler, event): def test_timeout_error(run_cloud_function): - envelope, event = run_cloud_function( + envelope, event, stream_data = run_cloud_function( dedent( """ functionhandler = None @@ -214,7 +215,7 @@ def cloud_function(functionhandler, event): def test_performance_no_error(run_cloud_function): - envelope, event = run_cloud_function( + envelope, event, stream_data = run_cloud_function( dedent( """ functionhandler = None @@ -239,7 +240,7 @@ def cloud_function(functionhandler, event): def test_performance_error(run_cloud_function): - envelope, event = run_cloud_function( + envelope, event, stream_data = run_cloud_function( dedent( """ functionhandler = None @@ -267,3 +268,79 @@ def cloud_function(functionhandler, event): assert exception["type"] == "Exception" assert exception["value"] == "something went wrong" assert exception["mechanism"] == {"type": "gcp", "handled": False} + + +def test_traces_sampler_gets_correct_values_in_sampling_context( + run_cloud_function, DictionaryContaining # noqa:N803 +): + import inspect + + envelope, event, stream_data = run_cloud_function( + dedent( + """ + functionhandler = None + event = { + "type": "chase", + "chasers": ["Maisey", "Charlie"], + "num_squirrels": 2, + } + def cloud_function(functionhandler, event): + # this runs after the transaction has started, which means we + # can make assertions about traces_sampler + try: + traces_sampler.assert_any_call( + DictionaryContaining({ + "gcp_env": DictionaryContaining({ + "function_name": "chase_into_tree", + "function_region": "dogpark", + "function_project": "SquirrelChasing", + }), + "gcp_event": { + "type": "chase", + "chasers": ["Maisey", "Charlie"], + "num_squirrels": 2, + }, + }) + ) + except AssertionError as e: + # catch the error and print it because the error itself will + # get swallowed by the SDK as an "internal exception" + print("\\nAssertionError: {}\\n".format(e)) + + return "dogs are great" + """ + ) + + FUNCTIONS_PRELUDE + + dedent(inspect.getsource(DictionaryContaining)) + + dedent( + """ + os.environ["FUNCTION_NAME"] = "chase_into_tree" + os.environ["FUNCTION_REGION"] = "dogpark" + os.environ["GCP_PROJECT"] = "SquirrelChasing" + + def _safe_is_equal(x, y): + # copied from conftest.py - see docstring and comments there + try: + is_equal = x.__eq__(y) + except AttributeError: + is_equal = NotImplemented + + if is_equal == NotImplemented: + # using == smoothes out weird variations exposed by raw __eq__ + return x == y + + return is_equal + + traces_sampler = Mock(return_value=True) + + init_sdk( + traces_sampler=traces_sampler, + debug=True + ) + + gcp_functions.worker_v1.FunctionHandler.invoke_user_function(functionhandler, event) + """ + ) + ) + + assert "AssertionError" not in str(stream_data) From bafed905686cac0c1b645247d49a32e8c7a2916e Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 10 Nov 2020 11:21:16 -0800 Subject: [PATCH 7/8] return assertion error rather than printing it --- tests/integrations/aws_lambda/test_aws.py | 12 +++---- tests/integrations/gcp/test_gcp.py | 42 +++++++++++++++-------- 2 files changed, 33 insertions(+), 21 deletions(-) diff --git a/tests/integrations/aws_lambda/test_aws.py b/tests/integrations/aws_lambda/test_aws.py index 320155df45..536debf15b 100644 --- a/tests/integrations/aws_lambda/test_aws.py +++ b/tests/integrations/aws_lambda/test_aws.py @@ -127,7 +127,7 @@ def inner(code, payload, timeout=30, syntax_check=True): # for better debugging response["LogResult"] = base64.b64decode(response["LogResult"]).splitlines() - response["Payload"] = response["Payload"].read() + response["Payload"] = json.loads(response["Payload"].read().decode("utf-8")) del response["ResponseMetadata"] events = [] @@ -567,12 +567,12 @@ def test_handler(event, context): } ) ) - except AssertionError as e: - # catch the error and print it because the error itself will + except AssertionError: + # catch the error and return it because the error itself will # get swallowed by the SDK as an "internal exception" - print("\\nERROR: AssertionError: traces_sampler not called with the specified argument.") + return {"AssertionError raised": True,} - return "dogs are great" + return {"AssertionError raised": False,} traces_sampler = mock.Mock(return_value=True) @@ -585,4 +585,4 @@ def test_handler(event, context): b'{"httpMethod": "GET", "path": "/sit/stay/rollover", "headers": {"Host": "dogs.are.great", "X-Forwarded-Proto": "http"}}', ) - assert "AssertionError" not in str(response["LogResult"]) + assert response["Payload"]["AssertionError raised"] is False diff --git a/tests/integrations/gcp/test_gcp.py b/tests/integrations/gcp/test_gcp.py index 3c4f1d2055..80f4836e7c 100644 --- a/tests/integrations/gcp/test_gcp.py +++ b/tests/integrations/gcp/test_gcp.py @@ -30,9 +30,19 @@ os.environ["FUNCTION_REGION"] = "us-central1" os.environ["GCP_PROJECT"] = "serverless_project" +def log_return_value(func): + def inner(*args, **kwargs): + rv = func(*args, **kwargs) + + print("\\nRETURN VALUE: {}\\n".format(json.dumps(rv))) + + return rv + + return inner + gcp_functions.worker_v1 = Mock() gcp_functions.worker_v1.FunctionHandler = Mock() -gcp_functions.worker_v1.FunctionHandler.invoke_user_function = cloud_function +gcp_functions.worker_v1.FunctionHandler.invoke_user_function = log_return_value(cloud_function) import sentry_sdk @@ -83,6 +93,7 @@ def inner(code, subprocess_kwargs=()): event = [] envelope = [] + return_value = None # STEP : Create a zip of cloud function @@ -123,16 +134,19 @@ def inner(code, subprocess_kwargs=()): elif line.startswith("ENVELOPE: "): line = line[len("ENVELOPE: ") :] envelope = json.loads(line) + elif line.startswith("RETURN VALUE: "): + line = line[len("RETURN VALUE: ") :] + return_value = json.loads(line) else: continue - return envelope, event, stream_data + return envelope, event, return_value return inner def test_handled_exception(run_cloud_function): - envelope, event, stream_data = run_cloud_function( + envelope, event, return_value = run_cloud_function( dedent( """ functionhandler = None @@ -158,7 +172,7 @@ def cloud_function(functionhandler, event): def test_unhandled_exception(run_cloud_function): - envelope, event, stream_data = run_cloud_function( + envelope, event, return_value = run_cloud_function( dedent( """ functionhandler = None @@ -185,7 +199,7 @@ def cloud_function(functionhandler, event): def test_timeout_error(run_cloud_function): - envelope, event, stream_data = run_cloud_function( + envelope, event, return_value = run_cloud_function( dedent( """ functionhandler = None @@ -215,7 +229,7 @@ def cloud_function(functionhandler, event): def test_performance_no_error(run_cloud_function): - envelope, event, stream_data = run_cloud_function( + envelope, event, return_value = run_cloud_function( dedent( """ functionhandler = None @@ -240,7 +254,7 @@ def cloud_function(functionhandler, event): def test_performance_error(run_cloud_function): - envelope, event, stream_data = run_cloud_function( + envelope, event, return_value = run_cloud_function( dedent( """ functionhandler = None @@ -275,7 +289,7 @@ def test_traces_sampler_gets_correct_values_in_sampling_context( ): import inspect - envelope, event, stream_data = run_cloud_function( + envelopes, events, return_value = run_cloud_function( dedent( """ functionhandler = None @@ -302,12 +316,12 @@ def cloud_function(functionhandler, event): }, }) ) - except AssertionError as e: - # catch the error and print it because the error itself will + except AssertionError: + # catch the error and return it because the error itself will # get swallowed by the SDK as an "internal exception" - print("\\nAssertionError: {}\\n".format(e)) + return {"AssertionError raised": True,} - return "dogs are great" + return {"AssertionError raised": False,} """ ) + FUNCTIONS_PRELUDE @@ -326,7 +340,6 @@ def _safe_is_equal(x, y): is_equal = NotImplemented if is_equal == NotImplemented: - # using == smoothes out weird variations exposed by raw __eq__ return x == y return is_equal @@ -335,7 +348,6 @@ def _safe_is_equal(x, y): init_sdk( traces_sampler=traces_sampler, - debug=True ) gcp_functions.worker_v1.FunctionHandler.invoke_user_function(functionhandler, event) @@ -343,4 +355,4 @@ def _safe_is_equal(x, y): ) ) - assert "AssertionError" not in str(stream_data) + assert return_value["AssertionError raised"] is False From fa4bf7666b1817f9464d8ef3436caebf1cbf18e4 Mon Sep 17 00:00:00 2001 From: Katie Byers Date: Tue, 10 Nov 2020 12:26:50 -0800 Subject: [PATCH 8/8] add comments explaining the hackiness of the current tests --- tests/integrations/aws_lambda/test_aws.py | 26 +++++++++++++++++++++++ tests/integrations/gcp/test_gcp.py | 5 +++++ 2 files changed, 31 insertions(+) diff --git a/tests/integrations/aws_lambda/test_aws.py b/tests/integrations/aws_lambda/test_aws.py index 536debf15b..332e5e8ce2 100644 --- a/tests/integrations/aws_lambda/test_aws.py +++ b/tests/integrations/aws_lambda/test_aws.py @@ -518,6 +518,32 @@ def test_traces_sampler_gets_correct_values_in_sampling_context( ObjectDescribedBy, # noqa:N803 StringContaining, # noqa:N803 ): + # TODO: This whole thing is a little hacky, specifically around the need to + # get `conftest.py` code into the AWS runtime, which is why there's both + # `inspect.getsource` and a copy of `_safe_is_equal` included directly in + # the code below. Ideas which have been discussed to fix this: + + # - Include the test suite as a module installed in the package which is + # shot up to AWS + # - In client.py, copy `conftest.py` (or wherever the necessary code lives) + # from the test suite into the main SDK directory so it gets included as + # "part of the SDK" + + # It's also worth noting why it's necessary to run the assertions in the AWS + # runtime rather than asserting on side effects the way we do with events + # and envelopes. The reasons are two-fold: + + # - We're testing against the `LambdaContext` class, which only exists in + # the AWS runtime + # - If we were to transmit call args data they way we transmit event and + # envelope data (through JSON), we'd quickly run into the problem that all + # sorts of stuff isn't serializable by `json.dumps` out of the box, up to + # and including `datetime` objects (so anything with a timestamp is + # automatically out) + + # Perhaps these challenges can be solved in a cleaner and more systematic + # way if we ever decide to refactor the entire AWS testing apparatus. + import inspect envelopes, events, response = run_lambda_function( diff --git a/tests/integrations/gcp/test_gcp.py b/tests/integrations/gcp/test_gcp.py index 80f4836e7c..debcf8386f 100644 --- a/tests/integrations/gcp/test_gcp.py +++ b/tests/integrations/gcp/test_gcp.py @@ -287,6 +287,11 @@ def cloud_function(functionhandler, event): def test_traces_sampler_gets_correct_values_in_sampling_context( run_cloud_function, DictionaryContaining # noqa:N803 ): + # TODO: There are some decent sized hacks below. For more context, see the + # long comment in the test of the same name in the AWS integration. The + # situations there and here aren't identical, but they're similar enough + # that solving one would probably solve both. + import inspect envelopes, events, return_value = run_cloud_function(