From 1dad497e0ee7dcda85137f341db2fdf12edc679e Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 9 Mar 2023 16:22:32 +0100 Subject: [PATCH 1/2] Use sentry_sdk instead of raven and improve Sentry grouping Fixes #1521 by bringing in the changes from: - https://github.com/mozilla/code-review/pull/1253 - https://github.com/mozilla/code-review/pull/1276 - https://github.com/mozilla/code-review/pull/1333 - https://github.com/mozilla/code-review/pull/1365 --- .pre-commit-config.yaml | 1 + tools/code_coverage_tools/log.py | 145 ++++++++++++++++++++----------- tools/requirements.txt | 3 +- 3 files changed, 97 insertions(+), 52 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2e4940641..9c09dfb32 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -90,6 +90,7 @@ repos: pass_filenames: false additional_dependencies: - types-requests==0.1.11 + - types-setuptools==67.6.0.0 - types-pytz==2022.6.0.1 - repo: meta hooks: diff --git a/tools/code_coverage_tools/log.py b/tools/code_coverage_tools/log.py index 4b34771e9..2a6aa2196 100644 --- a/tools/code_coverage_tools/log.py +++ b/tools/code_coverage_tools/log.py @@ -3,29 +3,46 @@ # License, v. 2.0. If a copy of the MPL was not distributed with this # file, You can obtain one at http://mozilla.org/MPL/2.0/. +import logging +import logging.handlers import os +import sys -import logbook -import logbook.more -import raven -import raven.handlers.logbook import structlog +import pkg_resources +import sentry_sdk +from sentry_sdk.integrations.logging import LoggingIntegration -class UnstructuredRenderer(structlog.processors.KeyValueRenderer): - def __call__(self, logger, method_name, event_dict): - event = None - if "event" in event_dict: - event = event_dict.pop("event") - if event_dict or event is None: - # if there are other keys, use the parent class to render them - # and append to the event - rendered = super(UnstructuredRenderer, self).__call__( - logger, method_name, event_dict - ) - return f"{event} ({rendered})" - else: - return event +root = logging.getLogger() + + +class AppNameFilter(logging.Filter): + def __init__(self, project_name, channel, *args, **kwargs): + self.project_name = project_name + self.channel = channel + super().__init__(*args, **kwargs) + + def filter(self, record): + record.app_name = f"code-coverage/{self.channel}/{self.project_name}" + return True + + +class ExtraFormatter(logging.Formatter): + def format(self, record): + log = super().format(record) + + extra = { + key: value + for key, value in record.__dict__.items() + if key + not in list(sentry_sdk.integrations.logging.COMMON_RECORD_ATTRS) + + ["asctime", "app_name"] + } + if len(extra) > 0: + log += " | extra=" + str(extra) + + return log def setup_papertrail(project_name, channel, PAPERTRAIL_HOST, PAPERTRAIL_PORT): @@ -34,14 +51,18 @@ def setup_papertrail(project_name, channel, PAPERTRAIL_HOST, PAPERTRAIL_PORT): """ # Setup papertrail - papertrail = logbook.SyslogHandler( - application_name=f"code-coverage/{channel}/{project_name}", + papertrail = logging.handlers.SysLogHandler( address=(PAPERTRAIL_HOST, int(PAPERTRAIL_PORT)), - level=logbook.INFO, - format_string="{record.time} {record.channel}: {record.message}", - bubble=True, ) - papertrail.push_application() + formatter = ExtraFormatter( + "%(app_name)s: %(asctime)s %(filename)s: %(message)s", + datefmt="%Y-%m-%d %H:%M:%S", + ) + papertrail.setLevel(logging.INFO) + papertrail.setFormatter(formatter) + # This filter is used to add the 'app_name' value to all logs to be formatted + papertrail.addFilter(AppNameFilter(project_name, channel)) + root.addHandler(papertrail) def setup_sentry(name, channel, dsn): @@ -58,67 +79,91 @@ def setup_sentry(name, channel, dsn): else: site = "unknown" - sentry_client = raven.Client( + # This integration allows sentry to catch logs from logging and process them + # By default, the 'event_level' is set to ERROR, we are defining it to WARNING + sentry_logging = LoggingIntegration( + level=logging.INFO, # Capture INFO and above as breadcrumbs + event_level=logging.WARNING, # Send WARNINGs as events + ) + # sentry_sdk will automatically retrieve the 'extra' attribute from logs and + # add contained values as Additional Data on the dashboard of the Sentry issue + sentry_sdk.init( dsn=dsn, - site=site, - name=name, + integrations=[sentry_logging], + server_name=name, environment=channel, - release=raven.fetch_package_version(f"code-coverage-{name}"), + release=pkg_resources.get_distribution(f"code-coverage-{name}").version, ) + sentry_sdk.set_tag("site", site) if task_id is not None: # Add a Taskcluster task id when available - # It will be shown in the Additional Data section on the dashboard - sentry_client.context.merge({"extra": {"task_id": task_id}}) + # It will be shown in a new section called Task on the dashboard + sentry_sdk.set_context("task", {"task_id": task_id}) - sentry_handler = raven.handlers.logbook.SentryHandler( - sentry_client, level=logbook.WARNING, bubble=True - ) - sentry_handler.push_application() + +class RenameAttrsProcessor(structlog.processors.KeyValueRenderer): + """ + Rename event_dict keys that will attempt to overwrite LogRecord common + attributes during structlog.stdlib.render_to_log_kwargs processing + """ + + def __call__(self, logger, method_name, event_dict): + to_rename = [ + key + for key in event_dict + if key in sentry_sdk.integrations.logging.COMMON_RECORD_ATTRS + ] + + for key in to_rename: + event_dict[f"{key}_"] = event_dict[key] + event_dict.pop(key) + + return event_dict def init_logger( project_name, channel=None, - level=logbook.INFO, + level=logging.INFO, PAPERTRAIL_HOST=None, PAPERTRAIL_PORT=None, sentry_dsn=None, ): - if not channel: channel = os.environ.get("APP_CHANNEL") - # Output logs on stderr, with color support on consoles - fmt = "{record.time} [{record.level_name:<8}] {record.channel}: {record.message}" - handler = logbook.more.ColorizedStderrHandler(level=level, format_string=fmt) - handler.push_application() + logging.basicConfig( + format="%(asctime)s.%(msecs)06d [%(levelname)-8s] %(filename)s: %(message)s", + datefmt="%Y-%m-%d %H:%M:%S", + stream=sys.stdout, + level=level, + ) # Log to papertrail if channel and PAPERTRAIL_HOST and PAPERTRAIL_PORT: setup_papertrail(project_name, channel, PAPERTRAIL_HOST, PAPERTRAIL_PORT) - # Log to senty + # Log to sentry if channel and sentry_dsn: setup_sentry(project_name, channel, sentry_dsn) - def logbook_factory(*args, **kwargs): - # Logger given to structlog - logbook.compat.redirect_logging() - return logbook.Logger(level=level, *args, **kwargs) - - # Setup structlog over logbook, with args list at the end + # Setup structlog processors = [ structlog.stdlib.PositionalArgumentsFormatter(), structlog.processors.StackInfoRenderer(), structlog.processors.format_exc_info, - UnstructuredRenderer(), + RenameAttrsProcessor(), + # Transpose the 'event_dict' from structlog into keyword arguments for logging.log + # E.g.: 'event' become 'msg' and, at the end, all remaining values from 'event_dict' + # are added as 'extra' + structlog.stdlib.render_to_log_kwargs, ] structlog.configure( - context_class=structlog.threadlocal.wrap_dict(dict), processors=processors, - logger_factory=logbook_factory, + context_class=structlog.threadlocal.wrap_dict(dict), + logger_factory=structlog.stdlib.LoggerFactory(), wrapper_class=structlog.stdlib.BoundLogger, cache_logger_on_first_use=True, ) diff --git a/tools/requirements.txt b/tools/requirements.txt index 3882dc184..a86b6af1a 100644 --- a/tools/requirements.txt +++ b/tools/requirements.txt @@ -1,8 +1,7 @@ aiohttp==3.8.4 async-timeout==4.0.2 -logbook==1.5.3 pytz==2022.7.1 -raven==6.10.0 +sentry-sdk==1.16.0 structlog==22.3.0 taskcluster==47.1.2 # Please see #1779, this eventually must be removed! From 46dca11209021f92ecaa22e3b5f39902b95463f1 Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Thu, 9 Mar 2023 16:51:29 +0100 Subject: [PATCH 2/2] Rename sentry_dsn parameter to SENTRY_DSN to match the name in code-review and reduce code differences between the log.py files --- backend/code_coverage_backend/backend/__init__.py | 2 +- bot/code_coverage_bot/cli.py | 2 +- events/code_coverage_events/cli.py | 2 +- tools/code_coverage_tools/log.py | 6 +++--- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/backend/code_coverage_backend/backend/__init__.py b/backend/code_coverage_backend/backend/__init__.py index bc8255d56..cc0392bce 100644 --- a/backend/code_coverage_backend/backend/__init__.py +++ b/backend/code_coverage_backend/backend/__init__.py @@ -40,7 +40,7 @@ def create_app(): channel=taskcluster.secrets.get("APP_CHANNEL", "dev"), PAPERTRAIL_HOST=taskcluster.secrets.get("PAPERTRAIL_HOST"), PAPERTRAIL_PORT=taskcluster.secrets.get("PAPERTRAIL_PORT"), - sentry_dsn=taskcluster.secrets.get("SENTRY_DSN"), + SENTRY_DSN=taskcluster.secrets.get("SENTRY_DSN"), ) logger = structlog.get_logger(__name__) diff --git a/bot/code_coverage_bot/cli.py b/bot/code_coverage_bot/cli.py index 605a7b68a..ea31ff837 100644 --- a/bot/code_coverage_bot/cli.py +++ b/bot/code_coverage_bot/cli.py @@ -65,7 +65,7 @@ def setup_cli(ask_repository=True, ask_revision=True): channel=secrets.get("APP_CHANNEL", "dev"), PAPERTRAIL_HOST=secrets.get("PAPERTRAIL_HOST"), PAPERTRAIL_PORT=secrets.get("PAPERTRAIL_PORT"), - sentry_dsn=secrets.get("SENTRY_DSN"), + SENTRY_DSN=secrets.get("SENTRY_DSN"), ) return args diff --git a/events/code_coverage_events/cli.py b/events/code_coverage_events/cli.py index 27f1f6891..34e334a86 100644 --- a/events/code_coverage_events/cli.py +++ b/events/code_coverage_events/cli.py @@ -41,7 +41,7 @@ def main(): channel=taskcluster_config.secrets.get("APP_CHANNEL", "dev"), PAPERTRAIL_HOST=taskcluster_config.secrets.get("PAPERTRAIL_HOST"), PAPERTRAIL_PORT=taskcluster_config.secrets.get("PAPERTRAIL_PORT"), - sentry_dsn=taskcluster_config.secrets.get("SENTRY_DSN"), + SENTRY_DSN=taskcluster_config.secrets.get("SENTRY_DSN"), ) events = Events() diff --git a/tools/code_coverage_tools/log.py b/tools/code_coverage_tools/log.py index 2a6aa2196..26eb0b173 100644 --- a/tools/code_coverage_tools/log.py +++ b/tools/code_coverage_tools/log.py @@ -128,7 +128,7 @@ def init_logger( level=logging.INFO, PAPERTRAIL_HOST=None, PAPERTRAIL_PORT=None, - sentry_dsn=None, + SENTRY_DSN=None, ): if not channel: channel = os.environ.get("APP_CHANNEL") @@ -145,8 +145,8 @@ def init_logger( setup_papertrail(project_name, channel, PAPERTRAIL_HOST, PAPERTRAIL_PORT) # Log to sentry - if channel and sentry_dsn: - setup_sentry(project_name, channel, sentry_dsn) + if channel and SENTRY_DSN: + setup_sentry(project_name, channel, SENTRY_DSN) # Setup structlog processors = [