From 9a0a5468c532b2e7ca3d707221f6dc39f80bc896 Mon Sep 17 00:00:00 2001 From: Akhil Narang Date: Mon, 27 Nov 2023 17:29:32 +0530 Subject: [PATCH] feat: setup sentry integration Inspired primarily from sentry's generic WSGI integration Environment variable `FRAPPE_SENTRY_DSN` needs to be enabled as well as explicit opt-in from the user's side in system sid telemetry settings Conditionally include telemetry JS bundles Signed-off-by: Akhil Narang --- frappe/__init__.py | 8 +++++ frappe/app.py | 6 +++- frappe/hooks.py | 7 ----- frappe/utils/error.py | 6 ++++ frappe/utils/sentry.py | 68 ++++++++++++++++++++++++++++++++++++++++-- frappe/www/app.py | 10 +++++-- pyproject.toml | 1 + 7 files changed, 93 insertions(+), 13 deletions(-) diff --git a/frappe/__init__.py b/frappe/__init__.py index 946665fc9d..462be6962c 100644 --- a/frappe/__init__.py +++ b/frappe/__init__.py @@ -59,6 +59,14 @@ 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 frappe.utils.sentry import before_send + + sentry_sdk.init(dsn=sentry_dsn, before_send=before_send, release=__version__) + class _dict(dict): """dict like object that exposes keys as attributes""" diff --git a/frappe/app.py b/frappe/app.py index 0284968113..99f4bb35a4 100644 --- a/frappe/app.py +++ b/frappe/app.py @@ -22,7 +22,7 @@ import frappe.rate_limiter import frappe.recorder import frappe.utils.response from frappe import _ -from frappe.auth import SAFE_HTTP_METHODS, UNSAFE_HTTP_METHODS, HTTPRequest, validate_auth +from frappe.auth import UNSAFE_HTTP_METHODS, HTTPRequest, validate_auth from frappe.middlewares import StaticDataMiddleware from frappe.utils import CallbackManager, cint, get_site_name from frappe.utils.data import escape_html @@ -60,6 +60,10 @@ if frappe._tune_gc: import frappe.website.router # Website router import frappe.website.website_generator # web page doctypes + # Import sentry only if DSN is set + if os.getenv("FRAPPE_SENTRY_DSN"): + import frappe.utils.sentry + # end: module pre-loading diff --git a/frappe/hooks.py b/frappe/hooks.py index fd80ddbda3..3bb3d2f9a6 100644 --- a/frappe/hooks.py +++ b/frappe/hooks.py @@ -29,13 +29,6 @@ app_include_js = [ "form.bundle.js", "controls.bundle.js", "report.bundle.js", - "telemetry.bundle.js", -] - -# JS code to include for error reporting -# This is only loaded if error reporting is enabled. -error_reporting_js = [ - "sentry.bundle.js", ] app_include_css = [ diff --git a/frappe/utils/error.py b/frappe/utils/error.py index fa78697d88..e826f25296 100644 --- a/frappe/utils/error.py +++ b/frappe/utils/error.py @@ -70,6 +70,12 @@ def log_error( ) capture("error_logged", "frappe", properties={"title": title, "trace_id": trace_id}) + if frappe.get_system_settings("enable_telemetry"): + from frappe.app import capture_exception + + # Capture exception data if telemetry is enabled + capture_exception(message=f"{title}\n{traceback}") + if frappe.flags.read_only or defer_insert: error_log.deferred_insert() else: diff --git a/frappe/utils/sentry.py b/frappe/utils/sentry.py index 6ad110759c..baadec6411 100644 --- a/frappe/utils/sentry.py +++ b/frappe/utils/sentry.py @@ -1,4 +1,67 @@ +import os +import sys + +from sentry_sdk import capture_message as sentry_capture_message +from sentry_sdk.hub import Hub +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 + import frappe +import frappe.monitor + + +def before_send(event, hint): + # Not doing anything here for now - we can add some checks to clean up the data, strip PII, etc. + return event + + +def capture_exception( + exception: ValueError | BaseException | None = None, message: str | None = None +) -> None: + """ + Function to upload exception data to entry + + :param exception: Exception object - if missing, try to get with sys.exc_info() + :param message: A message to be sent if we can't find an exception + """ + # Don't report anything if the user hasn't opted-in to telemetry + if not frappe.get_system_settings("enable_telemetry"): + return + try: + hub = Hub.current + + 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) + + # 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: + if exception is None and ((exception := sys.exc_info()[1]) is None): + if message: + sentry_capture_message(message, level="error") + return + + event, hint = event_from_exception( + exception, + client_options=client.options, + mechanism={"type": "wsgi", "handled": False}, + ) + hub.capture_event(event, hint=hint) + + except Exception: + frappe.logger().error("Failed to capture exception", exc_info=True) + pass def add_bootinfo(bootinfo): @@ -13,9 +76,8 @@ def add_bootinfo(bootinfo): } } """ - if not frappe.get_system_settings("auto_report_errors"): + if not frappe.get_system_settings("enable_telemetry"): return - sentry_info = (frappe.conf.get("error_reporting") or {}).get("sentry") - if sentry_info: + if sentry_info := os.getenv("FRAPPE_SENTRY_DSN"): bootinfo.sentry = sentry_info diff --git a/frappe/www/app.py b/frappe/www/app.py index 2002bada96..412812f9d7 100644 --- a/frappe/www/app.py +++ b/frappe/www/app.py @@ -1,5 +1,7 @@ # Copyright (c) 2015, Frappe Technologies Pvt. Ltd. and Contributors # License: MIT. See LICENSE +import os + no_cache = 1 import json @@ -47,8 +49,12 @@ def get_context(context): include_icons = hooks.get("app_include_icons", []) frappe.local.preload_assets["icons"].extend(include_icons) - if frappe.get_system_settings("auto_report_errors"): - include_js = hooks["error_reporting_js"] + include_js + if frappe.get_system_settings("enable_telemetry"): + if os.getenv("FRAPPE_SENTRY_DSN"): + include_js = ["sentry.bundle.js"] + include_js + + if hasattr(frappe.local, "posthog"): + include_js = ["telemetry.bundle.js"] + include_js context.update( { diff --git a/pyproject.toml b/pyproject.toml index c371e87ac1..2631778a85 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -64,6 +64,7 @@ dependencies = [ "rq~=1.15.1", "rsa>=4.1", "semantic-version~=2.10.0", + "sentry-sdk~=1.37.1", "sqlparse~=0.4.4", "tenacity~=8.2.2", "terminaltables~=3.1.10",