diff --git a/sentry/const.py b/sentry/const.py index c3135a9df..9f7eab3af 100644 --- a/sentry/const.py +++ b/sentry/const.py @@ -69,7 +69,11 @@ def get_sentry_logging(level=DEFAULT_LOG_LEVEL): if level not in LOG_LEVEL_MAP: level = DEFAULT_LOG_LEVEL - return LoggingIntegration(level=LOG_LEVEL_MAP[level], event_level=logging.WARNING) + return LoggingIntegration( + # Gather warnings into breadcrumbs regardless of actual logging level + level=logging.WARNING, + event_level=LOG_LEVEL_MAP[level], + ) def get_sentry_options(): diff --git a/sentry/tests/test_client.py b/sentry/tests/test_client.py index efbcb29d6..9be06efd1 100644 --- a/sentry/tests/test_client.py +++ b/sentry/tests/test_client.py @@ -60,19 +60,56 @@ class InMemoryTransport(HttpTransport): pass +class NoopHandler(logging.Handler): + """ + A Handler subclass that does nothing with any given log record. + + Sentry's log patching works by having the integration process things after + the normal log handlers are run, so we use this handler to do nothing and + move to Sentry logic ASAP. + """ + + def emit(self, record): + pass + + class TestClientSetup(TransactionCase): def setUp(self): super(TestClientSetup, self).setUp() self.dsn = "http://public:secret@example.com/1" - config.options["sentry_enabled"] = True - config.options["sentry_dsn"] = self.dsn + self.patch_config( + { + "sentry_enabled": True, + "sentry_dsn": self.dsn, + "sentry_logging_level": "error", + } + ) self.client = initialize_sentry(config)._client self.client.transport = InMemoryTransport({"dsn": self.dsn}) - self.handler = self.client.integrations["logging"]._handler + + # Setup our own logger so we don't flood stderr with error logs + self.logger = logging.getLogger("odoo.sentry.test.logger") + # Do not mutate list while iterating it + handlers = [handler for handler in self.logger.handlers] + for handler in handlers: + self.logger.removeHandler(handler) + self.logger.addHandler(NoopHandler()) + self.logger.propagate = False + + def patch_config(self, options: dict): + """ + Patch Odoo's config with the given `options`, ensuring that the patch + is undone when the test completes. + """ + _config_patcher = patch.dict( + in_dict=config.options, + values=options, + ) + _config_patcher.start() + self.addCleanup(_config_patcher.stop) def log(self, level, msg, exc_info=None): - record = logging.LogRecord(__name__, level, __file__, 42, msg, (), exc_info) - self.handler.emit(record) + self.logger.log(level, msg, exc_info=exc_info) def assertEventCaptured(self, client, event_level, event_msg): self.assertTrue( @@ -89,50 +126,64 @@ class TestClientSetup(TransactionCase): def test_initialize_raven_sets_dsn(self): self.assertEqual(self.client.dsn, self.dsn) - def test_capture_event(self): + def test_ignore_low_level_event(self): level, msg = logging.WARNING, "Test event, can be ignored" self.log(level, msg) level = "warning" + self.assertEventNotCaptured(self.client, level, msg) + + def test_capture_event(self): + level, msg = logging.ERROR, "Test event, should be captured" + self.log(level, msg) + level = "error" self.assertEventCaptured(self.client, level, msg) def test_capture_event_exc(self): - level, msg = logging.WARNING, "Test event, can be ignored exception" + level, msg = logging.ERROR, "Test event, can be ignored exception" try: raise TestException(msg) except TestException: exc_info = sys.exc_info() self.log(level, msg, exc_info) - level = "warning" + level = "error" self.assertEventCaptured(self.client, level, msg) def test_ignore_exceptions(self): - config.options["sentry_ignore_exceptions"] = "odoo.exceptions.UserError" + self.patch_config( + { + "sentry_ignore_exceptions": "odoo.exceptions.UserError", + } + ) client = initialize_sentry(config)._client client.transport = InMemoryTransport({"dsn": self.dsn}) - level, msg = logging.WARNING, "Test exception" + level, msg = logging.ERROR, "Test exception" try: raise exceptions.UserError(msg) except exceptions.UserError: exc_info = sys.exc_info() self.log(level, msg, exc_info) - level = "warning" + level = "error" self.assertEventNotCaptured(client, level, msg) def test_exclude_logger(self): - config.options["sentry_enabled"] = True - config.options["sentry_exclude_loggers"] = __name__ + self.patch_config( + { + "sentry_enabled": True, + "sentry_exclude_loggers": self.logger.name, + } + ) client = initialize_sentry(config)._client client.transport = InMemoryTransport({"dsn": self.dsn}) - level, msg = logging.WARNING, "Test exclude logger %s" % __name__ + level, msg = logging.ERROR, "Test exclude logger %s" % __name__ self.log(level, msg) - level = "warning" + level = "error" # Revert ignored logger so it doesn't affect other tests - remove_handler_ignore(__name__) + remove_handler_ignore(self.logger.name) self.assertEventNotCaptured(client, level, msg) @patch("odoo.addons.sentry.hooks.get_odoo_commit", return_value=GIT_SHA) def test_config_odoo_dir(self, get_odoo_commit): - config.options["sentry_odoo_dir"] = "/opt/odoo/core" + self.patch_config({"sentry_odoo_dir": "/opt/odoo/core"}) client = initialize_sentry(config)._client self.assertEqual( @@ -143,8 +194,12 @@ class TestClientSetup(TransactionCase): @patch("odoo.addons.sentry.hooks.get_odoo_commit", return_value=GIT_SHA) def test_config_release(self, get_odoo_commit): - config.options["sentry_odoo_dir"] = "/opt/odoo/core" - config.options["sentry_release"] = RELEASE + self.patch_config( + { + "sentry_odoo_dir": "/opt/odoo/core", + "sentry_release": RELEASE, + } + ) client = initialize_sentry(config)._client self.assertEqual(