From 067104ca9cf3f5e0ed59f7623edab730b0d796d2 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 18 Dec 2023 12:02:13 +0530 Subject: [PATCH] refactor(sentry): sync up with FC implementation Co-authored-by: Aditya Hase Signed-off-by: Akhil Narang --- frappe/__init__.py | 26 ---------- frappe/app.py | 46 ++++++++++++++++- frappe/hooks.py | 2 + frappe/utils/background_jobs.py | 43 ++++++++++++++++ frappe/utils/sentry.py | 91 ++++++++++++++++++++++++++++----- 5 files changed, 167 insertions(+), 41 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 158d5c9fa4..c450e26578 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -59,32 +59,6 @@ if _dev_server: warnings.simplefilter("always", DeprecationWarning) warnings.simplefilter("always", PendingDeprecationWarning) -# Always initialize sentry SDK if the DSN is sent -if sentry_dsn := os.getenv("FRAPPE_SENTRY_DSN"): - import sentry_sdk - from sentry_sdk.integrations.argv import ArgvIntegration - from sentry_sdk.integrations.atexit import AtexitIntegration - from sentry_sdk.integrations.dedupe import DedupeIntegration - from sentry_sdk.integrations.excepthook import ExcepthookIntegration - from sentry_sdk.integrations.modules import ModulesIntegration - - from frappe.utils.sentry import before_send - - sentry_sdk.init( - dsn=sentry_dsn, - before_send=before_send, - release=__version__, - auto_enabling_integrations=False, - default_integrations=False, - integrations=[ - AtexitIntegration(), - ExcepthookIntegration(), - DedupeIntegration(), - ModulesIntegration(), - ArgvIntegration(), - ], - ) - class _dict(dict): """dict like object that exposes keys as attributes""" diff --git a/frappe/app.py b/frappe/app.py index 5ddabfbfc9..6d6a747119 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -140,7 +140,7 @@ def application(request: Request): try: run_after_request_hooks(request, response) - except Exception as e: + except Exception: # We can not handle exceptions safely here. frappe.logger().error("Failed to run after request hook", exc_info=True) @@ -420,6 +420,50 @@ def sync_database(rollback: bool) -> bool: return rollback +# Always initialize sentry SDK if the DSN is sent +if sentry_dsn := os.getenv("FRAPPE_SENTRY_DSN"): + import sentry_sdk + from sentry_sdk.integrations.argv import ArgvIntegration + from sentry_sdk.integrations.atexit import AtexitIntegration + from sentry_sdk.integrations.dedupe import DedupeIntegration + from sentry_sdk.integrations.excepthook import ExcepthookIntegration + from sentry_sdk.integrations.modules import ModulesIntegration + from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware + + from frappe.utils.sentry import FrappeIntegration, before_send + + integrations = [ + AtexitIntegration(), + ExcepthookIntegration(), + DedupeIntegration(), + ModulesIntegration(), + ArgvIntegration(), + ] + + experiments = {} + kwargs = {} + + if os.getenv("ENABLE_SENTRY_DB_MONITORING"): + integrations.append(FrappeIntegration()) + experiments["record_sql_params"] = True + + if tracing_sample_rate := os.getenv("SENTRY_TRACING_SAMPLE_RATE"): + kwargs["traces_sample_rate"] = float(tracing_sample_rate) + application = SentryWsgiMiddleware(application) + + sentry_sdk.init( + dsn=sentry_dsn, + before_send=before_send, + attach_stacktrace=True, + release=frappe.__version__, + auto_enabling_integrations=False, + default_integrations=False, + integrations=integrations, + _experiments=experiments, + **kwargs, + ) + + def serve( port=8000, profile=False, diff --git a/frappe/hooks.py b/frappe/hooks.py index ec7436ac19..de4254584f 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -422,11 +422,13 @@ before_request = [ "frappe.recorder.record", "frappe.monitor.start", "frappe.rate_limiter.apply", + "frappe.utils.sentry.set_sentry_context", ] # Background Job Hooks before_job = [ "frappe.monitor.start", + "frappe.utils.sentry.set_sentry_context", ] after_job = [ "frappe.monitor.stop", diff --git a/frappe/utils/background_jobs.py b/frappe/utils/background_jobs.py index d89003a81d..8ab529284d 100755 --- a/frappe/utils/background_jobs.py +++ b/frappe/utils/background_jobs.py @@ -288,6 +288,49 @@ def start_worker( if quiet: logging_level = "WARNING" + # Always initialize sentry SDK if the DSN is sent + if sentry_dsn := os.getenv("FRAPPE_SENTRY_DSN"): + import sentry_sdk + from sentry_sdk.integrations.argv import ArgvIntegration + from sentry_sdk.integrations.atexit import AtexitIntegration + from sentry_sdk.integrations.dedupe import DedupeIntegration + from sentry_sdk.integrations.excepthook import ExcepthookIntegration + from sentry_sdk.integrations.modules import ModulesIntegration + from sentry_sdk.integrations.rq import RqIntegration + + from frappe.utils.sentry import FrappeIntegration, before_send + + integrations = [ + AtexitIntegration(), + ExcepthookIntegration(), + DedupeIntegration(), + ModulesIntegration(), + ArgvIntegration(), + RqIntegration(), + ] + + experiments = {} + kwargs = {} + + if os.getenv("ENABLE_SENTRY_DB_MONITORING"): + integrations.append(FrappeIntegration()) + experiments["record_sql_params"] = True + + if tracing_sample_rate := os.getenv("SENTRY_TRACING_SAMPLE_RATE"): + kwargs["traces_sample_rate"] = float(tracing_sample_rate) + + sentry_sdk.init( + dsn=sentry_dsn, + before_send=before_send, + attach_stacktrace=True, + release=frappe.__version__, + auto_enabling_integrations=False, + default_integrations=False, + integrations=integrations, + _experiments=experiments, + **kwargs, + ) + worker = Worker(queues, name=get_worker_name(queue_name), connection=redis_connection) worker.work( logging_level=logging_level, diff --git a/frappe/utils/sentry.py b/frappe/utils/sentry.py index 8ca66a15d7..bcc6a3ab4b 100644 --- a/frappe/utils/sentry.py +++ b/frappe/utils/sentry.py @@ -1,18 +1,93 @@ import os import sys +from datetime import datetime +import rq from sentry_sdk import capture_message as sentry_capture_message +from sentry_sdk import configure_scope from sentry_sdk.hub import Hub +from sentry_sdk.integrations import Integration from sentry_sdk.integrations.wsgi import _make_wsgi_event_processor from sentry_sdk.tracing import SOURCE_FOR_STYLE -from sentry_sdk.utils import event_from_exception +from sentry_sdk.tracing_utils import record_sql_queries +from sentry_sdk.utils import capture_internal_exceptions, event_from_exception import frappe import frappe.monitor +from frappe.database.database import Database, EmptyQueryValues + + +class FrappeIntegration(Integration): + identifier = "frappe" + + @staticmethod + def setup_once(): + real_connect = Database.connect + real_sql = Database.sql + + def sql(self, query, values=None, *args, **kwargs): + hub = Hub.current + + if not self._conn: + self.connect() + + with record_sql_queries( + hub, self._cursor, query, values, paramstyle="pyformat", executemany=False + ): + return real_sql(self, query, values or EmptyQueryValues, *args, **kwargs) + + def connect(self): + hub = Hub.current + with capture_internal_exceptions(): + hub.add_breadcrumb(message="connect", category="query") + + with hub.start_span(op="db", description="connect"): + return real_connect(self) + + Database.connect = connect + Database.sql = sql + + +def set_sentry_context(): + with configure_scope() as scope: + if job := rq.get_current_job(): + kwargs = job._kwargs + transaction_name = str(kwargs["method"]) + context = frappe._dict({"scheduled": False, "wait": 0}) + if "run_scheduled_job" in transaction_name: + transaction_name = kwargs.get("kwargs", {}).get("job_type", "") + context.scheduled = True + + waitdiff = datetime.utcnow() - job.enqueued_at + context.uuid = job.id + context.wait = waitdiff.total_seconds() + + scope.set_extra("job", context) + scope.set_transaction_name(transaction_name) + else: + if frappe.form_dict.cmd: + path = f"/api/method/{frappe.form_dict.cmd}" + else: + path = frappe.request.path + + scope.set_transaction_name( + path, + source=SOURCE_FOR_STYLE["endpoint"], + ) + + scope.set_tag("site", frappe.local.site) + user = getattr(frappe.session, "user", "Unidentified") + if "@" not in user: + user = f"{user}@{frappe.local.site}" + scope.set_user({"id": user, "email": user}) + # Extract `X-Frappe-Request-ID` to store as a separate field if its present + if trace_id := frappe.monitor.get_trace_id(): + scope.set_tag("frappe_trace_id", trace_id) def before_send(event, hint): - # Not doing anything here for now - we can add some checks to clean up the data, strip PII, etc. + if event.get("logger", "") == "CSSUTILS": + return None return event @@ -30,20 +105,8 @@ def capture_exception(message: str | None = None) -> None: if frappe.request: with hub.configure_scope() as scope: - scope.set_transaction_name( - frappe.request.path, - source=SOURCE_FOR_STYLE["endpoint"], - ) - evt_processor = _make_wsgi_event_processor(frappe.request.environ, False) scope.add_event_processor(evt_processor) - scope.set_tag("site", frappe.local.site) - user = getattr(frappe.session, "user", "Unidentified") - scope.set_user({"id": user, "email": user}) - - # Extract `X-Frappe-Request-ID` to store as a separate field if its present - if trace_id := frappe.monitor.get_trace_id(): - scope.set_tag("frappe_trace_id", trace_id) if client := hub.client: exc_info = sys.exc_info()