From 7cfb72f0b25647da2b2b5e83daedc22ca5bdfe23 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 02:51:21 +0000 Subject: Implement basic Sentry SDK usage --- Pipfile | 1 + Pipfile.lock | 10 +++++++++- bot/__main__.py | 2 ++ bot/constants.py | 2 +- config-default.yml | 1 + 5 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Pipfile b/Pipfile index 11d5e7deb..400e64c18 100644 --- a/Pipfile +++ b/Pipfile @@ -18,6 +18,7 @@ deepdiff = "~=4.0" requests = "~=2.22" more_itertools = "~=7.2" urllib3 = ">=1.24.2,<1.25" +sentry-sdk = "~=0.14" [dev-packages] coverage = "~=4.5" diff --git a/Pipfile.lock b/Pipfile.lock index 633e62887..fa29bf995 100644 --- a/Pipfile.lock +++ b/Pipfile.lock @@ -1,7 +1,7 @@ { "_meta": { "hash": { - "sha256": "560a5168f32bafd9e5173d49502e10aeb278fe64ed50262da29be208f4a66739" + "sha256": "c7706a61eb96c06d073898018ea2dbcf5bd3b15d007496e2d60120a65647f31e" }, "pipfile-spec": 6, "requires": { @@ -413,6 +413,14 @@ "index": "pypi", "version": "==2.23.0" }, + "sentry-sdk": { + "hashes": [ + "sha256:b06dd27391fd11fb32f84fe054e6a64736c469514a718a99fb5ce1dff95d6b28", + "sha256:e023da07cfbead3868e1e2ba994160517885a32dfd994fc455b118e37989479b" + ], + "index": "pypi", + "version": "==0.14.1" + }, "six": { "hashes": [ "sha256:236bdbdce46e6e6a3d61a337c0f8b763ca1e8717c03b369e87a7ec7ce1319c0a", diff --git a/bot/__main__.py b/bot/__main__.py index 84bc7094b..4f51a08cc 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,10 +1,12 @@ import discord +import sentry_sdk from discord.ext.commands import when_mentioned_or from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE +sentry_sdk.init(BotConfig.sentry_dsn) bot = Bot( command_prefix=when_mentioned_or(BotConfig.prefix), diff --git a/bot/constants.py b/bot/constants.py index fe8e57322..a4c65a1f8 100644 --- a/bot/constants.py +++ b/bot/constants.py @@ -193,7 +193,7 @@ class Bot(metaclass=YAMLGetter): prefix: str token: str - + sentry_dsn: str class Filter(metaclass=YAMLGetter): section = "filter" diff --git a/config-default.yml b/config-default.yml index 3345e6f2a..2eaf8ee06 100644 --- a/config-default.yml +++ b/config-default.yml @@ -1,6 +1,7 @@ bot: prefix: "!" token: !ENV "BOT_TOKEN" + sentry_dsn: !ENV "BOT_SENTRY_DSN" cooldowns: # Per channel, per tag. -- cgit v1.2.3 From 23829807867c37c293f7ab4d998b86fbc548b980 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 03:04:58 +0000 Subject: Set BOT_SENTRY_DSN environment variable for tests --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 0400ac4d2..874364a6f 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -30,7 +30,7 @@ jobs: - script: python -m flake8 displayName: 'Run linter' - - script: BOT_API_KEY=foo BOT_TOKEN=bar WOLFRAM_API_KEY=baz REDDIT_CLIENT_ID=spam REDDIT_SECRET=ham coverage run -m xmlrunner + - script: BOT_API_KEY=foo BOT_SENTRY_DSN=blah BOT_TOKEN=bar WOLFRAM_API_KEY=baz REDDIT_CLIENT_ID=spam REDDIT_SECRET=ham coverage run -m xmlrunner displayName: Run tests - script: coverage report -m && coverage xml -o coverage.xml -- cgit v1.2.3 From 03175ae8bfcb70b1b2c798267fdd23b48534b2b7 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 03:05:23 +0000 Subject: Attach extra information on a command error --- bot/cogs/error_handler.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 52893b2ee..021753081 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,6 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context +from sentry_sdk import configure_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -150,7 +151,18 @@ class ErrorHandler(Cog): log.error( f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}" ) - raise e + + with configure_scope() as scope: + scope.user = { + "username": ctx.author.username, + "id": ctx.author.id + } + + scope.set_tag("command", ctx.command.qualified_name) + scope.set_tag("message_id", ctx.message.id) + scope.set_extra("full_message", ctx.message.content) + + raise e def setup(bot: Bot) -> None: -- cgit v1.2.3 From 215483e390b7f5636cf1d46c96c216ef0d05cb03 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 11:14:03 +0000 Subject: Add basic sentry usage for command errors giving user information and command context. --- bot/__main__.py | 6 +++- bot/api.py | 74 ----------------------------------------------- bot/bot.py | 2 -- bot/cogs/error_handler.py | 13 ++++----- 4 files changed, 10 insertions(+), 85 deletions(-) diff --git a/bot/__main__.py b/bot/__main__.py index 4f51a08cc..51bba6191 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,12 +1,16 @@ import discord import sentry_sdk from discord.ext.commands import when_mentioned_or +from sentry_sdk.integrations.aiohttp import AioHttpIntegration from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE -sentry_sdk.init(BotConfig.sentry_dsn) +sentry_sdk.init( + dsn=BotConfig.sentry_dsn, + integrations=[AioHttpIntegration()] +) bot = Bot( command_prefix=when_mentioned_or(BotConfig.prefix), diff --git a/bot/api.py b/bot/api.py index 56db99828..fb126b384 100644 --- a/bot/api.py +++ b/bot/api.py @@ -141,77 +141,3 @@ def loop_is_running() -> bool: except RuntimeError: return False return True - - -class APILoggingHandler(logging.StreamHandler): - """Site API logging handler.""" - - def __init__(self, client: APIClient): - logging.StreamHandler.__init__(self) - self.client = client - - # internal batch of shipoff tasks that must not be scheduled - # on the event loop yet - scheduled when the event loop is ready. - self.queue = [] - - async def ship_off(self, payload: dict) -> None: - """Ship log payload to the logging API.""" - try: - await self.client.post('logs', json=payload) - except ResponseCodeError as err: - log.warning( - "Cannot send logging record to the site, got code %d.", - err.response.status, - extra={'via_handler': True} - ) - except Exception as err: - log.warning( - "Cannot send logging record to the site: %r", - err, - extra={'via_handler': True} - ) - - def emit(self, record: logging.LogRecord) -> None: - """ - Determine if a log record should be shipped to the logging API. - - If the asyncio event loop is not yet running, log records will instead be put in a queue - which will be consumed once the event loop is running. - - The following two conditions are set: - 1. Do not log anything below DEBUG (only applies to the monkeypatched `TRACE` level) - 2. Ignore log records originating from this logging handler itself to prevent infinite recursion - """ - if ( - record.levelno >= logging.DEBUG - and not record.__dict__.get('via_handler') - ): - payload = { - 'application': 'bot', - 'logger_name': record.name, - 'level': record.levelname.lower(), - 'module': record.module, - 'line': record.lineno, - 'message': self.format(record) - } - - task = self.ship_off(payload) - if not loop_is_running(): - self.queue.append(task) - else: - asyncio.create_task(task) - self.schedule_queued_tasks() - - def schedule_queued_tasks(self) -> None: - """Consume the queue and schedule the logging of each queued record.""" - for task in self.queue: - asyncio.create_task(task) - - if self.queue: - log.debug( - "Scheduled %d pending logging tasks.", - len(self.queue), - extra={'via_handler': True} - ) - - self.queue.clear() diff --git a/bot/bot.py b/bot/bot.py index 8f808272f..cecee7b68 100644 --- a/bot/bot.py +++ b/bot/bot.py @@ -27,8 +27,6 @@ class Bot(commands.Bot): self.http_session: Optional[aiohttp.ClientSession] = None self.api_client = api.APIClient(loop=self.loop, connector=self.connector) - log.addHandler(api.APILoggingHandler(self.api_client)) - def add_cog(self, cog: commands.Cog) -> None: """Adds a "cog" to the bot and logs the operation.""" super().add_cog(cog) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 021753081..e76f1a086 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,7 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context -from sentry_sdk import configure_scope +from sentry_sdk import capture_exception, configure_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -148,21 +148,18 @@ class ErrorHandler(Cog): f"Sorry, an unexpected error occurred. Please let us know!\n\n" f"```{e.__class__.__name__}: {e}```" ) - log.error( - f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}" - ) with configure_scope() as scope: scope.user = { - "username": ctx.author.username, - "id": ctx.author.id + "id": ctx.author.id, + "username": str(ctx.author) } scope.set_tag("command", ctx.command.qualified_name) scope.set_tag("message_id", ctx.message.id) scope.set_extra("full_message", ctx.message.content) - - raise e + + capture_exception(e) def setup(bot: Bot) -> None: -- cgit v1.2.3 From aa12d72c7ad6ad54f82f113da1e916cee1903eaf Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 17:22:23 +0000 Subject: Remove tests for custom bot log --- tests/bot/test_api.py | 64 ++------------------------------------------------- 1 file changed, 2 insertions(+), 62 deletions(-) diff --git a/tests/bot/test_api.py b/tests/bot/test_api.py index 5a88adc5c..bdfcc73e4 100644 --- a/tests/bot/test_api.py +++ b/tests/bot/test_api.py @@ -1,9 +1,7 @@ -import logging import unittest -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock from bot import api -from tests.base import LoggingTestCase from tests.helpers import async_test @@ -34,7 +32,7 @@ class APIClientTests(unittest.TestCase): self.assertEqual(error.response_text, "") self.assertIs(error.response, self.error_api_response) - def test_responde_code_error_string_representation_default_initialization(self): + def test_response_code_error_string_representation_default_initialization(self): """Test the string representation of `ResponseCodeError` initialized without text or json.""" error = api.ResponseCodeError(response=self.error_api_response) self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: ") @@ -76,61 +74,3 @@ class APIClientTests(unittest.TestCase): response_text=text_data ) self.assertEqual(str(error), f"Status: {self.error_api_response.status} Response: {text_data}") - - -class LoggingHandlerTests(LoggingTestCase): - """Tests the bot's API Log Handler.""" - - @classmethod - def setUpClass(cls): - cls.debug_log_record = logging.LogRecord( - name='my.logger', level=logging.DEBUG, - pathname='my/logger.py', lineno=666, - msg="Lemon wins", args=(), - exc_info=None - ) - - cls.trace_log_record = logging.LogRecord( - name='my.logger', level=logging.TRACE, - pathname='my/logger.py', lineno=666, - msg="This will not be logged", args=(), - exc_info=None - ) - - def setUp(self): - self.log_handler = api.APILoggingHandler(None) - - def test_emit_appends_to_queue_with_stopped_event_loop(self): - """Test if `APILoggingHandler.emit` appends to queue when the event loop is not running.""" - with patch("bot.api.APILoggingHandler.ship_off") as ship_off: - # Patch `ship_off` to ease testing against the return value of this coroutine. - ship_off.return_value = 42 - self.log_handler.emit(self.debug_log_record) - - self.assertListEqual(self.log_handler.queue, [42]) - - def test_emit_ignores_less_than_debug(self): - """`APILoggingHandler.emit` should not queue logs with a log level lower than DEBUG.""" - self.log_handler.emit(self.trace_log_record) - self.assertListEqual(self.log_handler.queue, []) - - def test_schedule_queued_tasks_for_empty_queue(self): - """`APILoggingHandler` should not schedule anything when the queue is empty.""" - with self.assertNotLogs(level=logging.DEBUG): - self.log_handler.schedule_queued_tasks() - - def test_schedule_queued_tasks_for_nonempty_queue(self): - """`APILoggingHandler` should schedule logs when the queue is not empty.""" - log = logging.getLogger("bot.api") - - with self.assertLogs(logger=log, level=logging.DEBUG) as logs, patch('asyncio.create_task') as create_task: - self.log_handler.queue = [555] - self.log_handler.schedule_queued_tasks() - self.assertListEqual(self.log_handler.queue, []) - create_task.assert_called_once_with(555) - - [record] = logs.records - self.assertEqual(record.message, "Scheduled 1 pending logging tasks.") - self.assertEqual(record.levelno, logging.DEBUG) - self.assertEqual(record.name, 'bot.api') - self.assertIn('via_handler', record.__dict__) -- cgit v1.2.3 From 5c0819890259e446273707ec2dbb4b8312c9e19f Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 17:36:05 +0000 Subject: Remove AIOHTTP integration from Sentry since it is intended for AIOHTTP servers. --- bot/__main__.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bot/__main__.py b/bot/__main__.py index 51bba6191..2ed078903 100644 --- a/bot/__main__.py +++ b/bot/__main__.py @@ -1,15 +1,13 @@ import discord import sentry_sdk from discord.ext.commands import when_mentioned_or -from sentry_sdk.integrations.aiohttp import AioHttpIntegration from bot import patches from bot.bot import Bot from bot.constants import Bot as BotConfig, DEBUG_MODE sentry_sdk.init( - dsn=BotConfig.sentry_dsn, - integrations=[AioHttpIntegration()] + dsn=BotConfig.sentry_dsn ) bot = Bot( -- cgit v1.2.3 From 24ddab52ac1db77f4dd061d22ab4d622e60da237 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 17:51:09 +0000 Subject: Use log.error instead of capture_exception --- bot/cogs/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index e76f1a086..8261633ee 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,7 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context -from sentry_sdk import capture_exception, configure_scope +from sentry_sdk import configure_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -159,7 +159,7 @@ class ErrorHandler(Cog): scope.set_tag("message_id", ctx.message.id) scope.set_extra("full_message", ctx.message.content) - capture_exception(e) + log.error(f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}", exc_info=e) def setup(bot: Bot) -> None: -- cgit v1.2.3 From cc623e96965d511e7408590d36b7017ea9d6b495 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 18:01:12 +0000 Subject: Use push_scope instead of configure_scope --- bot/cogs/error_handler.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 8261633ee..589448b9a 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -15,7 +15,7 @@ from discord.ext.commands import ( UserInputError, ) from discord.ext.commands import Cog, Context -from sentry_sdk import configure_scope +from sentry_sdk import push_scope from bot.api import ResponseCodeError from bot.bot import Bot @@ -149,7 +149,7 @@ class ErrorHandler(Cog): f"```{e.__class__.__name__}: {e}```" ) - with configure_scope() as scope: + with push_scope() as scope: scope.user = { "id": ctx.author.id, "username": str(ctx.author) -- cgit v1.2.3 From 969f1eea95a9bd44de65bd08b8a551538b7b6581 Mon Sep 17 00:00:00 2001 From: Joseph Banks Date: Fri, 21 Feb 2020 18:01:35 +0000 Subject: Attach jump_to url to the event --- bot/cogs/error_handler.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/bot/cogs/error_handler.py b/bot/cogs/error_handler.py index 589448b9a..0abb7e521 100644 --- a/bot/cogs/error_handler.py +++ b/bot/cogs/error_handler.py @@ -157,8 +157,16 @@ class ErrorHandler(Cog): scope.set_tag("command", ctx.command.qualified_name) scope.set_tag("message_id", ctx.message.id) + scope.set_tag("channel_id", ctx.channel.id) + scope.set_extra("full_message", ctx.message.content) + if ctx.guild is not None: + scope.set_extra( + "jump_to", + f"https://discordapp.com/channels/{ctx.guild.id}/{ctx.channel.id}/{ctx.message.id}" + ) + log.error(f"Error executing command invoked by {ctx.message.author}: {ctx.message.content}", exc_info=e) -- cgit v1.2.3