Merge pull request #762 from python-discord/setup-sentry-sdk

Sentry SDK setup

10 files changed, 40 insertions, 145 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/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
diff --git a/bot/__main__.py b/bot/__main__.py
index 84bc7094b..2ed078903 100644
--- a/bot/__main__.py
+++ b/bot/__main__.py
@@ -1,10 +1,14 @@
 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(
+    dsn=BotConfig.sentry_dsn
+)
 
 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 52893b2ee..0abb7e521 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 push_scope
 
 from bot.api import ResponseCodeError
 from bot.bot import Bot
@@ -147,10 +148,26 @@ 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}"
-        )
-        raise e
+
+        with push_scope() as scope:
+            scope.user = {
+                "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_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)
 
 
 def setup(bot: Bot) -> None:
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.
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__)