From d776ed02108fdedaa8792a65b2cbe37fd1020ae6 Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Mon, 19 May 2025 09:35:32 -0700 Subject: [PATCH 01/17] feat(insights): basic framework event hooks (#208) * Instrumenting ASGI * Add celery events & reworked tests * Django request & db events * Flask request & db instrumentation * ASGI instrumentation * Syntax update * Updated README * Change "component" to "app" for insights event I followed the naming from our notice data. The more common name for this field is "app", so renaming to that. * Remove url & add blueprint to flask events * Removing args from celery event Until we have filtering and before_event functionality. * Add duration to celery events * PR fixes --- README.md | 21 ++ honeybadger/config.py | 4 + honeybadger/contrib/asgi.py | 33 +++- honeybadger/contrib/celery.py | 98 +++++++-- honeybadger/contrib/django.py | 58 +++++- honeybadger/contrib/flask.py | 93 ++++++++- honeybadger/tests/contrib/test_asgi.py | 47 +++++ honeybadger/tests/contrib/test_celery.py | 241 +++++++++-------------- honeybadger/tests/contrib/test_django.py | 44 +++++ honeybadger/tests/contrib/test_flask.py | 53 +++++ honeybadger/utils.py | 20 ++ 11 files changed, 533 insertions(+), 179 deletions(-) diff --git a/README.md b/README.md index 904e56b..be37f10 100644 --- a/README.md +++ b/README.md @@ -277,6 +277,26 @@ That's it! For additional configuration options, keep reading. **Note:** By default, honeybadger reports errors in separate threads. For platforms that disallows threading (such as serving a flask/django app with uwsgi and disabling threading), Honeybadger will fail to report errors. You can either enable threading if you have the option, or set `force_sync` config option to `True`. This causes Honeybadger to report errors in a single thread. +## Insights Automatic Instrumentation + +Honeybadger Insights allows you to automatically track various events in your +application. To enable Insights automatic instrumentation, add the following to +your configuration: + +```python +honeybadger.configure(insights_enabled=True) +``` + +### Supported Libraries + +After integration with our middleware or extensions, Honeybadger will +automatically instrument the following libraries: + +- Django requests & database queries +- Flask requests & database queries +- ASGI requests +- Celery tasks + ## Logging By default, Honeybadger uses the `logging.NullHandler` for logging so it doesn't make any assumptions about your logging setup. In Django, add a `honeybadger` section to your `LOGGING` config to enable Honeybadger logging. For example: @@ -373,6 +393,7 @@ The following options are available to you: | excluded_exceptions | `list` | `[]` | `['Http404', 'MyCustomIgnoredError']` | `HONEYBADGER_EXCLUDED_EXCEPTIONS` | | force_sync | `bool` | `False` | `True` | `HONEYBADGER_FORCE_SYNC` | | report_local_variables | `bool` | `False` | `True` | `HONEYBADGER_REPORT_LOCAL_VARIABLES` | +| insights_enabled | `bool` | `False` | `True` | `HONEYBADGER_INSIGHTS_ENABLED` | | events_batch_size | `int` | `1000` | `50` | `HONEYBADGER_EVENTS_BATCH_SIZE` | | events_max_queue_size | `int` | `10000` | `5000` | `HONEYBADGER_EVENTS_MAX_QUEUE_SIZE` | | events_timeout | `float`| `5.0` | `1.0` | `HONEYBADGER_EVENTS_TIMEOUT` | diff --git a/honeybadger/config.py b/honeybadger/config.py index 4a049fa..a2d427d 100644 --- a/honeybadger/config.py +++ b/honeybadger/config.py @@ -20,6 +20,8 @@ class Configuration(object): ("report_local_variables", bool), ("before_notify", callable), # Insights options + ("insights_enabled", bool), + # Events options ("events_batch_size", int), ("events_max_queue_size", int), ("events_timeout", float), @@ -45,6 +47,8 @@ def __init__(self, *args, **kwargs): self.report_local_variables = False self.before_notify = lambda notice: notice + self.insights_enabled = False + self.events_max_batch_retries = 3 self.events_max_queue_size = 10_000 self.events_batch_size = 1000 diff --git a/honeybadger/contrib/asgi.py b/honeybadger/contrib/asgi.py index 7f8b3dc..793b116 100644 --- a/honeybadger/contrib/asgi.py +++ b/honeybadger/contrib/asgi.py @@ -1,4 +1,6 @@ from honeybadger import honeybadger, plugins, utils +from honeybadger.utils import get_duration +import time import urllib import inspect import asyncio @@ -104,22 +106,43 @@ def __init__(self, app, **kwargs): def _run_asgi2(self, scope): async def inner(receive, send): - return await self._run_app(scope, lambda: self.app(scope)(receive, send)) + return await self._run_request(scope, receive, send, self.app(scope)) return inner async def _run_asgi3(self, scope, receive, send): - return await self._run_app(scope, lambda: self.app(scope, receive, send)) + return await self._run_request( + scope, receive, send, lambda recv, snd: self.app(scope, recv, snd) + ) - async def _run_app(self, scope, callback): + async def _run_request(self, scope, receive, send, app_callable): # TODO: Should we check recursive middleware stacks? # See: https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/integrations/asgi.py#L112 + start = time.time() + status = None + + async def send_wrapper(message): + nonlocal status + if message.get("type") == "http.response.start": + status = message.get("status") + await send(message) + try: - return await callback() + return await app_callable(receive, send_wrapper) except Exception as exc: honeybadger.notify(exception=exc, context=_as_context(scope)) - raise exc from None + raise finally: + if honeybadger.config.insights_enabled: + honeybadger.event( + "asgi.request", + { + "method": scope.get("method"), + "path": scope.get("path"), + "status": status, + "duration": get_duration(start), + }, + ) honeybadger.reset_context() def supports(self, config, context): diff --git a/honeybadger/contrib/celery.py b/honeybadger/contrib/celery.py index 7c37df1..b92f091 100644 --- a/honeybadger/contrib/celery.py +++ b/honeybadger/contrib/celery.py @@ -1,5 +1,11 @@ +import threading +import time + from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager +from honeybadger.utils import extract_honeybadger_config, get_duration + +_listener_started = False class CeleryPlugin(Plugin): @@ -61,17 +67,80 @@ def init_app(self): """ Initialize honeybadger and listen for errors. """ - from celery.signals import task_failure, task_postrun + from celery.signals import task_failure, task_postrun, task_prerun, worker_ready + self._task_starts = {} self._initialize_honeybadger(self.app.conf) + if self.report_exceptions: task_failure.connect(self._on_task_failure, weak=False) task_postrun.connect(self._on_task_postrun, weak=False) - def _on_task_postrun(self, *args, **kwargs): + if honeybadger.config.insights_enabled: + # Enable task events, as we need to listen to + # task-finished events + self.app.conf.worker_send_task_events = True + task_prerun.connect(self._on_task_prerun, weak=False) + worker_ready.connect(self._start_task_event_listener, weak=False) + + def _initialize_honeybadger(self, config): + """ + Initializes honeybadger using the given config object. + :param dict config: a dict or dict-like object that contains honeybadger configuration properties. + """ + config_kwargs = extract_honeybadger_config(config) + + if not config_kwargs.get("api_key"): + return + + honeybadger.configure(**config_kwargs) + honeybadger.config.set_12factor_config() # environment should override celery settings + + def _start_task_event_listener(self, *args, **kwargs): + # only start the listener once + global _listener_started + if _listener_started: + return + _listener_started = True + + from celery.events import EventReceiver # type: ignore[import] + + def run(): + with self.app.connection() as conn: + recv = EventReceiver( + conn, handlers={"task-finished": self._on_task_finished} + ) + recv.capture(limit=None, timeout=None, wakeup=False) + + self._listen_thread = threading.Thread(target=run, daemon=True) + self._listen_thread.start() + + def _on_task_finished(self, payload, **kwargs): + honeybadger.event("celery.task_finished", payload["payload"]) + + def _on_task_prerun(self, task_id=None, task=None, *args, **kwargs): + self._task_starts[task_id] = time.time() + + def _on_task_postrun(self, task_id=None, task=None, *args, **kwargs): """ Callback executed after a task is finished. """ + + if honeybadger.config.insights_enabled: + payload = { + "task_id": task_id, + "task_name": task.name, + "retries": task.request.retries, + "group": task.request.group, + "state": kwargs["state"], + "duration": get_duration(self._task_starts.pop(task_id, None)), + # TODO: allow filtering before sending args + # "args": kwargs["args"], + # "kwargs": kwargs["kwargs"], + } + + task.send_event("task-finished", payload=payload) + honeybadger.reset_context() def _on_task_failure(self, *args, **kwargs): @@ -80,22 +149,6 @@ def _on_task_failure(self, *args, **kwargs): """ honeybadger.notify(exception=kwargs["exception"]) - def _initialize_honeybadger(self, config): - """ - Initializes honeybadger using the given config object. - :param dict config: a dict or dict-like object that contains honeybadger configuration properties. - """ - api_key = config.get("HONEYBADGER_API_KEY") - if not api_key: - return - honeybadger_config = { - "api_key": api_key, - "environment": config.get("HONEYBADGER_ENVIRONMENT", "development"), - "force_report_data": config.get("HONEYBADGER_FORCE_REPORT_DATA", False), - } - honeybadger.configure(**honeybadger_config) - honeybadger.config.set_12factor_config() # environment should override celery settings - def tearDown(self): """ Disconnects celery signals. @@ -106,6 +159,15 @@ def tearDown(self): if self.report_exceptions: task_failure.disconnect(self._on_task_failure) + if honeybadger.config.insights_enabled: + from celery.signals import worker_ready, task_prerun + + task_prerun.disconnect(self._on_task_prerun) + worker_ready.disconnect(self._start_task_event_listener) + + if hasattr(self, "_listen_thread"): + self._listen_thread.join(timeout=1) + # Keep the misspelled method for backward compatibility def tearDowm(self): """ diff --git a/honeybadger/contrib/django.py b/honeybadger/contrib/django.py index 052cbdf..85beb68 100644 --- a/honeybadger/contrib/django.py +++ b/honeybadger/contrib/django.py @@ -1,18 +1,18 @@ from __future__ import absolute_import import re +import time from six import iteritems from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager -from honeybadger.utils import filter_dict, filter_env_vars +from honeybadger.utils import filter_dict, filter_env_vars, get_duration try: from threading import local # type: ignore[no-redef] except ImportError: from django.utils._threading_local import local # type: ignore[no-redef,import] - _thread_locals = local() REQUEST_LOCAL_KEY = "__django_current_request" @@ -130,18 +130,70 @@ def __init__(self, get_response=None): honeybadger.configure(**config_kwargs) honeybadger.config.set_12factor_config() # environment should override Django settings default_plugin_manager.register(DjangoPlugin()) + if honeybadger.config.insights_enabled: + self._patch_cursor() def __call__(self, request): set_request(request) + start_time = time.time() honeybadger.begin_request(request) - response = self.get_response(request) + if honeybadger.config.insights_enabled: + self._send_request_event(request, response, start_time) + honeybadger.reset_context() clear_request() return response + def _patch_cursor(self): + from django.db.backends.utils import CursorWrapper + + orig_exec = CursorWrapper.execute + + def hb_execute(self, sql, params=None): + start = time.time() + try: + return orig_exec(self, sql, params) + finally: + honeybadger.event( + "db.query", {"query": sql, "duration": get_duration(start)} + ) + + CursorWrapper.execute = hb_execute + + def _send_request_event(self, request, response, start_time): + # Get resolver data + resolver_match = getattr(request, "resolver_match", None) + view_name = None + view_module = None + app_name = None + + if resolver_match: + view_name = getattr(resolver_match, "view_name", None) + if not view_name and hasattr(resolver_match, "func"): + view_name = resolver_match.func.__name__ + + if hasattr(resolver_match, "func"): + view_module = resolver_match.func.__module__ + + app_name = getattr(resolver_match, "app_name", None) + + request_data = { + "path": request.path_info if hasattr(request, "path_info") else None, + "method": request.method if hasattr(request, "method") else None, + "status": ( + response.status_code if hasattr(response, "status_code") else None + ), + "view": view_name, + "module": view_module, + "app": app_name, + "duration": get_duration(start_time), + } + + honeybadger.event("django.request", request_data) + def process_exception(self, request, exception): self.__set_user_from_context(request) honeybadger.notify(exception) diff --git a/honeybadger/contrib/flask.py b/honeybadger/contrib/flask.py index 7fde384..fb0ab29 100644 --- a/honeybadger/contrib/flask.py +++ b/honeybadger/contrib/flask.py @@ -1,14 +1,23 @@ from __future__ import absolute_import +from contextvars import ContextVar import logging +import time from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager -from honeybadger.utils import filter_dict, filter_env_vars +from honeybadger.utils import ( + filter_dict, + filter_env_vars, + get_duration, + extract_honeybadger_config, +) from six import iteritems logger = logging.getLogger(__name__) +_request_info: ContextVar[dict] = ContextVar("_request_info") + class FlaskPlugin(Plugin): """ @@ -80,8 +89,6 @@ class FlaskHoneybadger(object): Flask extension for Honeybadger. Initializes Honeybadger and adds a request information to payload. """ - CONFIG_PREFIX = "HONEYBADGER_" - def __init__( self, app=None, report_exceptions=False, reset_context_after_request=False ): @@ -96,6 +103,7 @@ def __init__( self.report_exceptions = False self.reset_context_after_request = False default_plugin_manager.register(FlaskPlugin()) + if app is not None: self.init_app( app, @@ -111,7 +119,12 @@ def init_app(self, app, report_exceptions=False, reset_context_after_request=Fal (i.e. by calling abort) or not. :param bool reset_context_after_request: whether to reset honeybadger context after each request. """ - from flask import request_tearing_down, got_request_exception + from flask import ( + request_tearing_down, + got_request_exception, + request_finished, + request_started, + ) self.app = app @@ -129,6 +142,20 @@ def init_app(self, app, report_exceptions=False, reset_context_after_request=Fal self._handle_exception, ) + if honeybadger.config.insights_enabled: + self._install_sqlalchemy_instrumentation() + self._register_signal_handler( + "insights on request end", + request_finished, + self._handle_request_finished, + ) + + self._register_signal_handler( + "insights on request end", + request_started, + self._handle_request_started, + ) + if self.reset_context_after_request: self._register_signal_handler( "auto clear context on request end", @@ -166,14 +193,40 @@ def _initialize_honeybadger(self, config): if config.get("DEBUG", False): honeybadger.configure(environment="development") - honeybadger_config = {} - for key, value in iteritems(config): - if key.startswith(self.CONFIG_PREFIX): - honeybadger_config[key[len(self.CONFIG_PREFIX) :].lower()] = value - + honeybadger_config = extract_honeybadger_config(config) honeybadger.configure(**honeybadger_config) honeybadger.config.set_12factor_config() # environment should override Flask settings + def _handle_request_started(self, sender, *args, **kwargs): + from flask import request + + _request_info.set( + { + "start_time": time.time(), + "request": request, + } + ) + + def _handle_request_finished(self, sender, *args, **kwargs): + info = _request_info.get({}) + request = info.get("request") + start = info.get("start_time") + response = kwargs.get("response") + + honeybadger.event( + "flask.request", + { + "path": request.path, + "method": request.method, + "status": response.status_code, + "view": request.endpoint, + "blueprint": request.blueprint, + "duration": get_duration(start), + }, + ) + + _request_info.set({}) + def _reset_context(self, *args, **kwargs): """ Resets context when request is done. @@ -189,3 +242,25 @@ def _handle_exception(self, sender, exception=None): honeybadger.notify(exception) if self.reset_context_after_request: self._reset_context() + + def _install_sqlalchemy_instrumentation(self): + """ + Attach SQLAlchemy Engine events: before/after_cursor_execute => honeybadger.event + """ + try: + import sqlalchemy # type: ignore + except ImportError: + return + # immediate patch + from sqlalchemy import event # type: ignore + from sqlalchemy.engine import Engine # type: ignore + + @event.listens_for(Engine, "before_cursor_execute", propagate=True) + def _before(conn, cursor, stmt, params, ctx, executemany): + ctx._hb_start = time.time() + + @event.listens_for(Engine, "after_cursor_execute", propagate=True) + def _after(conn, cursor, stmt, params, ctx, executemany): + honeybadger.event( + "db.query", {"query": stmt, "duration": get_duration(ctx._hb_start)} + ) diff --git a/honeybadger/tests/contrib/test_asgi.py b/honeybadger/tests/contrib/test_asgi.py index 3b435d4..603675d 100644 --- a/honeybadger/tests/contrib/test_asgi.py +++ b/honeybadger/tests/contrib/test_asgi.py @@ -50,3 +50,50 @@ async def test_should_notify_exception(self, hb): async def test_should_not_notify_exception(self, hb): response = await self.client.get("/") hb.notify.assert_not_called() + + @aiounittest.async_test + @mock.patch("honeybadger.contrib.asgi.honeybadger") + async def test_should_send_request_event_on_success(self, hb): + response = await self.client.get("/test") + self.assertEqual(response.status_code, 200) + hb.event.assert_called_once() + name, payload = hb.event.call_args.args + self.assertEqual(name, "asgi.request") + self.assertEqual(payload["method"], "GET") + self.assertEqual(payload["path"], "/test") + self.assertEqual(payload["status"], 200) + self.assertIsInstance(payload["duration"], float) + + @aiounittest.async_test + @mock.patch("honeybadger.contrib.asgi.honeybadger") + async def test_should_send_request_event_on_exception(self, hb): + with self.assertRaises(SomeError): + await self.client.get("/error") + hb.event.assert_called_once() + name, payload = hb.event.call_args.args + self.assertEqual(name, "asgi.request") + self.assertEqual(payload["method"], "GET") + self.assertEqual(payload["path"], "/error") + self.assertIsNone(payload["status"]) + self.assertIsInstance(payload["duration"], float) + + +class ASGIEventPayloadTestCase(unittest.TestCase): + def setUp(self): + # wrap your ASGI app in the plugin + app = contrib.ASGIHoneybadger(asgi_app(), api_key="abcd", insights_enabled=True) + self.client = TestClient(app) + + @aiounittest.async_test + @mock.patch("honeybadger.contrib.asgi.honeybadger.event") + async def test_success_event_payload(self, event): + # even if there’s a query, url stays just the path + await self.client.get("/hello?x=1") + event.assert_called_once() + name, payload = event.call_args.args + + self.assertEqual(name, "asgi.request") + self.assertEqual(payload["method"], "GET") + self.assertEqual(payload["path"], "/hello") + self.assertEqual(payload["status"], 200) + self.assertIsInstance(payload["duration"], float) diff --git a/honeybadger/tests/contrib/test_celery.py b/honeybadger/tests/contrib/test_celery.py index 3b61fba..324c164 100644 --- a/honeybadger/tests/contrib/test_celery.py +++ b/honeybadger/tests/contrib/test_celery.py @@ -1,150 +1,103 @@ -import unittest -from unittest.mock import patch +import pytest +import time +from unittest.mock import patch, MagicMock + from celery import Celery +from celery.app.task import Context from honeybadger import honeybadger -from honeybadger.contrib.celery import CeleryHoneybadger +from honeybadger.contrib.celery import CeleryHoneybadger, CeleryPlugin import honeybadger.connection as connection -class CeleryPluginTestCase(unittest.TestCase): - def setUp(self): - super().setUp() - self.app = Celery(__name__) - self.app.conf.update( - CELERY_ALWAYS_EAGER=True, - HONEYBADGER_API_KEY="test", - HONEYBADGER_ENVIRONMENT="celery_test", - HONEYBADGER_FORCE_REPORT_DATA=True, - ) - self.celery_hb = None - - def get_mock_notice_payload(self, mock): - return mock.call_args[0][1].payload - - def tearDown(self): - super().tearDown() - if self.celery_hb: - self.celery_hb.tearDown() - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - def test_celery_task_with_exception(self, mock, mock_request): - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=True) - - @self.app.task - def error(): - return 1 / 0 - - error.delay() - mock.assert_called_once() - self.assertEqual( - self.get_mock_notice_payload(mock)["error"]["class"], "ZeroDivisionError" - ) - self.assertEqual( - self.get_mock_notice_payload(mock)["error"]["message"], "division by zero" - ) - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - def test_celery_task_with_params(self, mock, mock_request): - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=True) - - @self.app.task - def error(a, b, c): - return a / b - - error.delay(1, 0, c=3) - mock.assert_called_once() - self.assertEqual( - self.get_mock_notice_payload(mock)["request"]["params"]["args"], [1, 0] - ) - self.assertEqual( - self.get_mock_notice_payload(mock)["request"]["params"]["kwargs"], {"c": 3} - ) - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - def test_celery_task_without_retries(self, mock, mock_request): - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=True) - - @self.app.task - def error(): - return 1 / 0 - - error.delay() - mock.assert_called_once() - self.assertEqual( - self.get_mock_notice_payload(mock)["request"]["context"]["retries"], 0 - ) - self.assertEqual( - self.get_mock_notice_payload(mock)["request"]["context"]["max_retries"], 3 - ) - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - def test_celery_task_with_retries(self, mock, mock_request): - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=True) - - @self.app.task(bind=True, max_retries=5, autoretry_for=(ZeroDivisionError,)) - def error(self): - return 1 / 0 - - error.delay() - mock.assert_called_once() - self.assertEqual( - self.get_mock_notice_payload(mock)["request"]["context"]["retries"], 5 - ) - self.assertEqual( - self.get_mock_notice_payload(mock)["request"]["context"]["max_retries"], 5 - ) - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - @patch("honeybadger.honeybadger.reset_context") - def test_celery_task_with_reset_context(self, mock_reset, mock_send, mock_request): - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=True) - - @self.app.task - def error(): - return 1 / 0 - - error.delay() - mock_send.assert_called_once() - mock_reset.assert_called_once() - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - def test_without_auto_report_exceptions(self, mock, mock_request): - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=False) - - @self.app.task - def error(): - return 1 / 0 - - error.delay() - mock.assert_not_called() - - @patch("honeybadger.connection._make_http_request") - @patch("honeybadger.connection.send_notice", wraps=connection.send_notice) - def test_context_merging(self, mock, mock_request): - """Test that custom context is merged with task context rather than being replaced""" - self.celery_hb = CeleryHoneybadger(self.app, report_exceptions=True) - - @self.app.task - def error(): - with honeybadger.context(user_id=123, custom_data="test"): - return 1 / 0 - - error.delay() - mock.assert_called_once() - - # Verify task context is present - context = self.get_mock_notice_payload(mock)["request"]["context"] - self.assertIn("task_id", context) - self.assertIn("retries", context) - self.assertIn("max_retries", context) - - # Verify custom context is also present - self.assertEqual(context["user_id"], 123) - self.assertEqual(context["custom_data"], "test") +@patch("honeybadger.honeybadger.reset_context") +@patch("honeybadger.honeybadger.notify") +def test_notify_from_task_failure(notify, reset_context): + from celery.signals import task_failure, task_postrun + + app = Celery(__name__, broker="memory://") + exception = Exception("Test exception") + hb = CeleryHoneybadger(app, report_exceptions=True) + + # Send task_failure event + task_failure.send(sender=app, task_id="hi", task_name="tasks.add", exception=exception) + + assert notify.call_count == 1 + assert notify.call_args[1]["exception"] == exception + + task_postrun.send(sender=app, task_id="hi", task_name="tasks.add", task={}) + + assert reset_context.call_count == 1 + hb.tearDown() + + +@patch("honeybadger.honeybadger.notify") +def test_notify_not_called_from_task_failure(mock): + from celery.signals import task_failure + + app = Celery(__name__, broker="memory://") + hb = CeleryHoneybadger(app, report_exceptions=False) + + # Send task_failure event + task_failure.send( + sender=app, task_name="tasks.add", exception=Exception("Test exception") + ) + + assert mock.call_count == 0 + hb.tearDown() + + +def test_plugin_payload(): + test_task = MagicMock() + test_task.name = "test_task" + test_task.max_retries = 10 + test_task.request = Context( + id="test_id", + name="test_task", + args=(1, 2), + kwargs={"foo": "bar"}, + ) + + with patch("celery.current_task", test_task): + plugin = CeleryPlugin() + payload = plugin.generate_payload({"request": {}}, {}, {}) + request = payload["request"] + assert request["component"] == "unittest.mock" + assert request["action"] == "test_task" + assert request["params"]["args"] == [1, 2] + assert request["params"]["kwargs"] == {"foo": "bar"} + assert request["context"]["task_id"] == "test_id" + assert request["context"]["retries"] == 0 + assert request["context"]["max_retries"] == 10 + + +@patch("celery.events.EventReceiver") +@patch("honeybadger.honeybadger.event") +def test_finished_task_event(mock_event, mock_event_receiver): + from celery.signals import worker_ready, task_postrun + + app = Celery(__name__, broker="memory://localhost/") + app.conf.update( + HONEYBADGER_INSIGHTS_ENABLED=True, + HONEYBADGER_API_KEY="test_api_key", + ) + hb = CeleryHoneybadger(app) + + worker_ready.send(sender=app) + + # Give time for the thread to start + time.sleep(0.5) + + assert mock_event_receiver.call_count == 1 + assert ( + mock_event_receiver.call_args[1]["handlers"]["task-finished"] + == hb._on_task_finished + ) + + hb._on_task_finished({"payload": {"data": "test_task_id"}}) + + assert mock_event.call_count == 1 + assert mock_event.call_args[0][0] == "celery.task_finished" + assert mock_event.call_args[0][1] == {"data": "test_task_id"} + + hb.tearDown() diff --git a/honeybadger/tests/contrib/test_django.py b/honeybadger/tests/contrib/test_django.py index 9597130..e6265f9 100644 --- a/honeybadger/tests/contrib/test_django.py +++ b/honeybadger/tests/contrib/test_django.py @@ -244,3 +244,47 @@ def assert_payload(req): except: pass self.assertTrue(request_mock.called) + + +@override_settings(HONEYBADGER={"INSIGHTS_ENABLED": True}) +class DjangoMiddlewareEventTestCase(SimpleTestCase): + def setUp(self): + self.rf = RequestFactory() + # point at your URLconf so resolver_match works + self.url = re_path(r"plain_view/?$", plain_view, name="plain_view") + + def tearDown(self): + clear_request() + + @patch("honeybadger.contrib.django.honeybadger.event") + def test_event_sent_on_successful_request(self, mock_event): + # arrange + request = self.rf.get("/plain_view/") + request.resolver_match = self.url.resolve("plain_view") + # force a known status code + response = Mock(status_code=418) + mw = DjangoHoneybadgerMiddleware(lambda req: response) + + # act + mw(request) + + # assert + mock_event.assert_called_once() + event_name, data = mock_event.call_args[0] + self.assertEqual(event_name, "django.request") + self.assertEqual(data["method"], "GET") + self.assertEqual(data["status"], 418) + self.assertEqual(data["path"], "/plain_view/") + self.assertEqual(data["view"], "plain_view") + # duration should be a float > 0 + self.assertIsInstance(data["duration"], float) + self.assertGreater(data["duration"], 0) + + @patch("honeybadger.contrib.django.honeybadger.event") + def test_no_request_left_in_thread_locals(self, mock_event): + # ensure that clear_request() always runs + request = self.rf.get("/plain_view/") + request.resolver_match = self.url.resolve("plain_view") + mw = DjangoHoneybadgerMiddleware(lambda req: Mock()) + mw(request) + self.assertIsNone(current_request()) diff --git a/honeybadger/tests/contrib/test_flask.py b/honeybadger/tests/contrib/test_flask.py index a531541..e4bcbc7 100644 --- a/honeybadger/tests/contrib/test_flask.py +++ b/honeybadger/tests/contrib/test_flask.py @@ -353,3 +353,56 @@ def get(self): self.assert_called_with_exception_type(mock_hb, ZeroDivisionError) self.assertEqual(2, mock_hb.reset_context.call_count) + + +class FlaskHoneybadgerInsightsTestCase(unittest.TestCase): + def setUp(self): + import flask + + self.app = flask.Flask(__name__) + # minimal honeybadger config + self.app.config.update( + { + "HONEYBADGER_INSIGHTS_ENABLED": True, + "HONEYBADGER_API_KEY": "test", + "HONEYBADGER_ENVIRONMENT": "test", + } + ) + # install the extension (hooks get registered here) + FlaskHoneybadger(self.app) + + # a simple endpoint to drive requests + @self.app.route("/ping", methods=["GET", "POST"]) + def ping(): + return "pong", 201 + + self.client = self.app.test_client() + + @patch("honeybadger.contrib.flask.honeybadger.event") + def test_insights_event_on_get(self, mock_event): + resp = self.client.get("/ping?foo=bar") + self.assertEqual(resp.status_code, 201) + # should fire exactly once per request + self.assertEqual(mock_event.call_count, 1) + + name, payload = mock_event.call_args[0] + self.assertEqual(name, "flask.request") + self.assertEqual(payload["method"], "GET") + self.assertEqual(payload["path"], "/ping") + self.assertEqual(payload["status"], 201) + self.assertEqual(payload["view"], "ping") + self.assertTrue(isinstance(payload["duration"], float)) + + @patch("honeybadger.contrib.flask.honeybadger.event") + def test_insights_event_on_post(self, mock_event): + resp = self.client.post("/ping", data={"a": "1", "b": "2"}) + self.assertEqual(resp.status_code, 201) + self.assertEqual(mock_event.call_count, 1) + + _, payload = mock_event.call_args[0] + self.assertEqual(payload["method"], "POST") + self.assertEqual(payload["path"], "/ping") + # query‐string not present → still only path + self.assertEqual(payload["view"], "ping") + # duration should be non‐negative + self.assertGreaterEqual(payload["duration"], 0.0) diff --git a/honeybadger/utils.py b/honeybadger/utils.py index 006a3c8..174ec65 100644 --- a/honeybadger/utils.py +++ b/honeybadger/utils.py @@ -1,4 +1,5 @@ import json +import time class StringReprJSONEncoder(json.JSONEncoder): @@ -64,3 +65,22 @@ def filter_dict(data, filter_keys): data[key] = filter_dict(data[key], filter_keys) return data + + +PREFIX = "HONEYBADGER_" + + +def extract_honeybadger_config(kwargs): + return { + key[len(PREFIX) :].lower(): value + for key, value in kwargs.items() + if key.startswith(PREFIX) + } + + +def get_duration(start_time): + """Get the duration in milliseconds since start_time.""" + if start_time is None: + return None + + return round((time.time() - start_time) * 1000, 4) From eadb4fdc4b40ea259e0c66143d42030a766acb91 Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Wed, 28 May 2025 09:07:12 -0700 Subject: [PATCH 02/17] feat(insights): before_event callback (#213) Also fix config bleeding issue with asgi test --- honeybadger/config.py | 2 ++ honeybadger/core.py | 11 +++++++ honeybadger/tests/contrib/test_asgi.py | 10 +++--- honeybadger/tests/test_core.py | 44 ++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 5 deletions(-) diff --git a/honeybadger/config.py b/honeybadger/config.py index a2d427d..2fb44e3 100644 --- a/honeybadger/config.py +++ b/honeybadger/config.py @@ -22,6 +22,7 @@ class Configuration(object): # Insights options ("insights_enabled", bool), # Events options + ("before_event", callable), ("events_batch_size", int), ("events_max_queue_size", int), ("events_timeout", float), @@ -48,6 +49,7 @@ def __init__(self, *args, **kwargs): self.before_notify = lambda notice: notice self.insights_enabled = False + self.before_event = lambda event: event self.events_max_batch_retries = 3 self.events_max_queue_size = 10_000 diff --git a/honeybadger/core.py b/honeybadger/core.py index 658e401..303d953 100644 --- a/honeybadger/core.py +++ b/honeybadger/core.py @@ -107,6 +107,17 @@ def event(self, event_type=None, data=None, **kwargs): "The first argument must be either a string or a dictionary" ) + if callable(self.config.before_event): + try: + next_payload = self.config.before_event(payload) + if next_payload is False: + return # Skip sending the event + elif next_payload is not payload and next_payload is not None: + payload = next_payload # Overwrite payload + # else: assume in-place mutation; keep payload as-is + except Exception as e: + logger.error("Error in before_event callback: %s", e) + # Add a timestamp to the payload if not provided if "ts" not in payload: payload["ts"] = datetime.datetime.now(datetime.timezone.utc) diff --git a/honeybadger/tests/contrib/test_asgi.py b/honeybadger/tests/contrib/test_asgi.py index 603675d..63dab1d 100644 --- a/honeybadger/tests/contrib/test_asgi.py +++ b/honeybadger/tests/contrib/test_asgi.py @@ -3,7 +3,9 @@ from async_asgi_testclient import TestClient # type: ignore import aiounittest import mock +from honeybadger import honeybadger from honeybadger import contrib +from honeybadger.config import Configuration class SomeError(Exception): @@ -79,14 +81,11 @@ async def test_should_send_request_event_on_exception(self, hb): class ASGIEventPayloadTestCase(unittest.TestCase): - def setUp(self): - # wrap your ASGI app in the plugin - app = contrib.ASGIHoneybadger(asgi_app(), api_key="abcd", insights_enabled=True) - self.client = TestClient(app) - @aiounittest.async_test @mock.patch("honeybadger.contrib.asgi.honeybadger.event") async def test_success_event_payload(self, event): + app = contrib.ASGIHoneybadger(asgi_app(), api_key="abcd", insights_enabled=True) + self.client = TestClient(app) # even if there’s a query, url stays just the path await self.client.get("/hello?x=1") event.assert_called_once() @@ -97,3 +96,4 @@ async def test_success_event_payload(self, event): self.assertEqual(payload["path"], "/hello") self.assertEqual(payload["status"], 200) self.assertIsInstance(payload["duration"], float) + honeybadger.config = Configuration() diff --git a/honeybadger/tests/test_core.py b/honeybadger/tests/test_core.py index 37a8a2b..cdc9df8 100644 --- a/honeybadger/tests/test_core.py +++ b/honeybadger/tests/test_core.py @@ -6,6 +6,7 @@ from .utils import mock_urlopen from honeybadger import Honeybadger from mock import MagicMock, patch +from honeybadger.config import Configuration def test_set_context(): @@ -260,6 +261,49 @@ def test_event_without_event_type(): assert payload["email"] == "user@example.com" +def test_event_with_before_event_mutated_changes(): + def before_event(event): + if "ignore" in event: + return False + event["new_key"] = "new_value" + + mock_events_worker = MagicMock() + hb = Honeybadger() + hb.events_worker = mock_events_worker + hb.configure(api_key="aaa", force_report_data=True, before_event=before_event) + hb.event(dict(email="user@example.com")) + hb.event(dict(ignore="yeah")) + + mock_events_worker.push.assert_called_once() + payload = mock_events_worker.push.call_args[0][0] + assert len(mock_events_worker.push.call_args[0]) == 1 + + assert "ts" in payload + assert payload["new_key"] == "new_value" + hb.config = Configuration() + + +def test_event_with_before_event_returned_changes(): + def before_event(event): + return { + "new_key": "new_value", + } + + mock_events_worker = MagicMock() + hb = Honeybadger() + hb.events_worker = mock_events_worker + hb.configure(api_key="aaa", force_report_data=True, before_event=before_event) + hb.event(dict(a="b")) + + mock_events_worker.push.assert_called_once() + payload = mock_events_worker.push.call_args[0][0] + assert "ts" in payload + assert payload["new_key"] == "new_value" + assert "a" not in payload + + hb.config = Configuration() + + def test_notify_with_before_notify_changes(): def before_notify(notice): notice.payload["error"]["tags"] = ["tag1-updated"] From 8fe36a7fc1285b9ec4081f1167c3274631ef59f6 Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Wed, 28 May 2025 11:44:18 -0700 Subject: [PATCH 03/17] feat(insight): add instrumentation config (#210) * feat(insights): add config for insights contrib This also reworks our config class to use a root dataclass with nested dataclasses for each `insights_config` option set. * Refactor the missing key error handling * Add ASGI config * README fixes * Add comments back and fixed readme, remove cruft * Log errors Also a nice little refactor on the Configuration internals. * Wrap insights event code in a try * How about some test results * Fix 3.8 object is not subscriptable error * Don't reference private Context * Check code-quality as well --- .github/workflows/code-quality.yml | 2 +- .github/workflows/python.yml | 2 +- README.md | 106 ++++++++++ dev-requirements.txt | 1 + honeybadger/config.py | 242 ++++++++++++++++------- honeybadger/contrib/asgi.py | 34 +++- honeybadger/contrib/celery.py | 30 ++- honeybadger/contrib/db.py | 40 ++++ honeybadger/contrib/django.py | 29 +-- honeybadger/contrib/flask.py | 53 +++-- honeybadger/tests/contrib/test_asgi.py | 64 +++--- honeybadger/tests/contrib/test_celery.py | 97 +++++++-- honeybadger/tests/contrib/test_db.py | 62 ++++++ honeybadger/tests/contrib/test_django.py | 49 ++++- honeybadger/tests/contrib/test_flask.py | 32 +++ honeybadger/tests/test_config.py | 66 ++++++- honeybadger/tests/utils.py | 39 ++++ honeybadger/utils.py | 11 +- 18 files changed, 796 insertions(+), 163 deletions(-) create mode 100644 honeybadger/contrib/db.py create mode 100644 honeybadger/tests/contrib/test_db.py diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 28d9833..179ab8f 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -2,7 +2,7 @@ name: Code Quality on: pull_request: - branches: [ master ] + branches: [ master, insights-instrumentation ] types: [opened, edited, synchronize, reopened] jobs: diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index 9ef80db..52d47a2 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -4,7 +4,7 @@ on: push: branches: [ master ] pull_request: - branches: [ master ] + branches: [ master, insights-instrumentation ] jobs: build: diff --git a/README.md b/README.md index be37f10..6e2c7b1 100644 --- a/README.md +++ b/README.md @@ -297,6 +297,112 @@ automatically instrument the following libraries: - ASGI requests - Celery tasks +### Configuration + +You can configure the instrumentation for specific libraries / components by +passing a dictionary to a specialized `insights_config` parameter. + +By default, all keyword dict params are run through our `params_filters`. + +The following instrumentation configs are available: + +#### Django + +```python + honeybadger.configure( + insights_config={ + "django": { + # Disable instrumentation for Django, defaults to False + "disabled": True, + # include GET/POST params in events, defaults to False + "include_params": True, + } + } + ) +``` + +#### Flask + +```python + honeybadger.configure( + insights_config={ + "flask": { + # Disable instrumentation for Flask, defaults to False + "disabled": True, + # Include GET/POST params in events, defaults to False + "include_params": True, + } + } + ) +``` + +#### ASGI + +```python + honeybadger.configure( + insights_config={ + "asgi": { + # Disable instrumentation for ASGI, defaults to False + "disabled": True, + # Include query params in events, defaults to False + "include_params": True, + } + } + ) +``` + +#### Celery + +```python + honeybadger.configure( + insights_config={ + "celery": { + # Disable instrumentation for Celery, defaults to False + "disabled": True, + # Include task args/kwargs, defaults to False + "include_args": True, + # List of task names or regexes to exclude, defaults to [] + "exclude_tasks": [ + "tasks.cleanup", + re.compile("^internal_"), + ], + } + } + ) +``` + +#### DB + +To configure database instrumentation, you can pass a dictionary to the +"db" insights config. This will affect all supported libraries that use capture +database events. + +```python + honeybadger.configure( + insights_config={ + "db": { + # Disable instrumentation for DB, defaults to False + "disabled": True, + # Include SQL params in events, defaults to False + "include_params": True, + + # List of task names or regexes to exclude, defaults to + # `honeybadger.config.default_excluded_queries()` + "exclude_queries": [ + "django_admin_log", # Matches any query containing this string + re.compile(r".*auth_permission.*"), # Regex pattern + ], + + # To add to the default excluded queries, use the `honeybadger.config.default_excluded_queries()` method + "exclude_queries": honeybadger.config.default_excluded_queries() + [ + re.compile(r".*django_admin_log.*"), + re.compile(r".*auth_permission.*"), + ], + } + } + ) +``` + ## Logging By default, Honeybadger uses the `logging.NullHandler` for logging so it doesn't make any assumptions about your logging setup. In Django, add a `honeybadger` section to your `LOGGING` config to enable Honeybadger logging. For example: diff --git a/dev-requirements.txt b/dev-requirements.txt index 55239c9..5dc426f 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -16,6 +16,7 @@ pylint pytest pytest-cov six +sqlalchemy testfixtures typing-extensions diff --git a/honeybadger/config.py b/honeybadger/config.py index 2fb44e3..4009b01 100644 --- a/honeybadger/config.py +++ b/honeybadger/config.py @@ -1,88 +1,155 @@ import os import socket -from six.moves import zip -from six import iteritems - - -class Configuration(object): - DEVELOPMENT_ENVIRONMENTS = ["development", "dev", "test"] - - OPTIONS = ( - ("api_key", str), - ("project_root", str), - ("environment", str), - ("hostname", str), - ("endpoint", str), - ("params_filters", list), - ("force_report_data", bool), - ("force_sync", bool), - ("excluded_exceptions", list), - ("report_local_variables", bool), - ("before_notify", callable), - # Insights options - ("insights_enabled", bool), - # Events options - ("before_event", callable), - ("events_batch_size", int), - ("events_max_queue_size", int), - ("events_timeout", float), - ("events_max_batch_retries", int), - ("events_throttle_wait", float), +import re +import logging + +from dataclasses import is_dataclass, dataclass, field, fields, MISSING +from typing import List, Callable, Any, Dict, Optional, ClassVar, Union, Pattern, Tuple + +logger = logging.getLogger(__name__) + + +def default_excluded_queries() -> List[Union[str, Pattern[Any]]]: + return [ + re.compile(r"^PRAGMA"), + re.compile(r"^SHOW\s"), + re.compile(r"^SELECT .* FROM information_schema\."), + re.compile(r"^SELECT .* FROM pg_catalog\."), + re.compile(r"^BEGIN"), + re.compile(r"^COMMIT"), + re.compile(r"^ROLLBACK"), + re.compile(r"^SAVEPOINT"), + re.compile(r"^RELEASE SAVEPOINT"), + re.compile(r"^ROLLBACK TO SAVEPOINT"), + re.compile(r"^VACUUM"), + re.compile(r"^ANALYZE"), + re.compile(r"^SET\s"), + re.compile(r".*django_migrations.*"), + re.compile(r".*django_admin_log.*"), + re.compile(r".*auth_permission.*"), + re.compile(r".*auth_group.*"), + re.compile(r".*auth_group_permissions.*"), + re.compile(r".*django_session.*"), + ] + + +@dataclass +class DBConfig: + disabled: bool = False + exclude_queries: List[Union[str, Pattern]] = field( + default_factory=default_excluded_queries ) + include_params: bool = False - def __init__(self, *args, **kwargs): - self.api_key = "" - self.project_root = os.getcwd() - self.environment = "production" - self.hostname = socket.gethostname() - self.endpoint = "https://api.honeybadger.io" - self.params_filters = [ + +@dataclass +class DjangoConfig: + disabled: bool = False + include_params: bool = False + + +@dataclass +class FlaskConfig: + disabled: bool = False + include_params: bool = False + + +@dataclass +class ASGIConfig: + disabled: bool = False + include_params: bool = False + + +@dataclass +class CeleryConfig: + disabled: bool = False + exclude_tasks: List[Union[str, Pattern]] = field(default_factory=list) + include_args: bool = False + + +@dataclass +class InsightsConfig: + db: DBConfig = field(default_factory=DBConfig) + django: DjangoConfig = field(default_factory=DjangoConfig) + flask: FlaskConfig = field(default_factory=FlaskConfig) + celery: CeleryConfig = field(default_factory=CeleryConfig) + asgi: ASGIConfig = field(default_factory=ASGIConfig) + + +@dataclass +class BaseConfig: + DEVELOPMENT_ENVIRONMENTS: ClassVar[List[str]] = ["development", "dev", "test"] + + api_key: str = "" + project_root: str = field(default_factory=os.getcwd) + environment: str = "production" + hostname: str = field(default_factory=socket.gethostname) + endpoint: str = "https://api.honeybadger.io" + params_filters: List[str] = field( + default_factory=lambda: [ "password", "password_confirmation", "credit_card", "CSRF_COOKIE", ] - self.force_report_data = False - self.force_sync = self.is_aws_lambda_environment - self.excluded_exceptions = [] - self.report_local_variables = False - self.before_notify = lambda notice: notice + ) + force_report_data: bool = False + force_sync: bool = False + excluded_exceptions: List[str] = field(default_factory=list) + report_local_variables: bool = False + before_notify: Callable[[Any], Any] = lambda notice: notice - self.insights_enabled = False - self.before_event = lambda event: event + insights_enabled: bool = False + insights_config: InsightsConfig = field(default_factory=InsightsConfig) - self.events_max_batch_retries = 3 - self.events_max_queue_size = 10_000 - self.events_batch_size = 1000 - self.events_timeout = 5.0 - self.events_throttle_wait = 60.0 + before_event: Callable[[Any], Any] = lambda _: None + events_batch_size: int = 1000 + events_max_queue_size: int = 10_000 + events_timeout: float = 5.0 + events_max_batch_retries: int = 3 + events_throttle_wait: float = 60.0 + + +class Configuration(BaseConfig): + def __init__(self, **kwargs): + super().__init__() self.set_12factor_config() self.set_config_from_dict(kwargs) def set_12factor_config(self): - for option in list(zip(*self.OPTIONS))[0]: - val = os.environ.get( - "HONEYBADGER_{}".format(option.upper()), getattr(self, option) - ) - option_types = dict(self.OPTIONS) - - try: - if option_types[option] is list: - val = val.split(",") - elif option_types[option] is int: - val = int(val) - elif option_types[option] is bool: - val = bool(val) - except: - pass - - setattr(self, option, val) - - def set_config_from_dict(self, config): - for key, value in iteritems(config): - if key in list(zip(*self.OPTIONS))[0]: - setattr(self, key, value) + for f in fields(self): + env_val = os.environ.get(f"HONEYBADGER_{f.name.upper()}") + if env_val is not None: + typ = f.type + try: + if typ == list or typ == List[str]: + val = env_val.split(",") + elif typ == int: + val = int(env_val) + elif typ == bool: + val = env_val.lower() in ("true", "1", "yes") + else: + val = env_val + setattr(self, f.name, val) + except Exception: + pass + + def set_config_from_dict(self, config: Dict[str, Any]): + filtered = filter_and_warn_unknown(config, self.__class__) + for k, v in filtered.items(): + current_val = getattr(self, k) + # If current_val is a dataclass and v is a dict, merge recursively + if hasattr(current_val, "__dataclass_fields__") and isinstance(v, dict): + # Merge current values and updates + current_dict = { + f.name: getattr(current_val, f.name) for f in fields(current_val) + } + merged = {**current_dict, **v} + hydrated = dataclass_from_dict(type(current_val), merged) + setattr(self, k, hydrated) + else: + setattr(self, k, v) def is_dev(self): """Returns wether you are in a dev environment or not @@ -102,3 +169,40 @@ def is_aws_lambda_environment(self): :rtype: bool """ return os.environ.get("AWS_LAMBDA_FUNCTION_NAME") is not None + + +def filter_and_warn_unknown(opts: Dict[str, Any], schema: Any) -> Dict[str, Any]: + if is_dataclass(schema): + if isinstance(schema, type): # It's a class + schema_name = schema.__name__ + else: # It's an instance + schema_name = type(schema).__name__ + allowed = {f.name for f in fields(schema)} + else: + raise TypeError(f"Expected a dataclass type or instance, got: {schema!r}") + + unknown = set(opts) - allowed + if unknown: + logger.warning( + "Unknown %s option(s): %s", + schema_name, + ", ".join(sorted(unknown)), + ) + return {k: opts[k] for k in opts.keys() & allowed} + + +def dataclass_from_dict(klass, d): + """ + Recursively build a dataclass instance from a dict. + """ + if not is_dataclass(klass): + return d + filtered = filter_and_warn_unknown(d, klass) + kwargs = {} + for f in fields(klass): + if f.name in d: + val = d[f.name] + if is_dataclass(f.type) and isinstance(val, dict): + val = dataclass_from_dict(f.type, val) + kwargs[f.name] = val + return klass(**kwargs) diff --git a/honeybadger/contrib/asgi.py b/honeybadger/contrib/asgi.py index 793b116..198e2d4 100644 --- a/honeybadger/contrib/asgi.py +++ b/honeybadger/contrib/asgi.py @@ -1,5 +1,6 @@ from honeybadger import honeybadger, plugins, utils from honeybadger.utils import get_duration +import logging import time import urllib import inspect @@ -7,6 +8,8 @@ import json from typing import Dict, Any, Optional, Callable, Awaitable, Union, Tuple, List, cast +logger = logging.getLogger(__name__) + def _looks_like_asgi3(app) -> bool: # https://github.com/encode/uvicorn/blob/bf1c64e2c141971c546671c7dc91b8ccf0afeb7d/uvicorn/config.py#L327 @@ -133,17 +136,36 @@ async def send_wrapper(message): honeybadger.notify(exception=exc, context=_as_context(scope)) raise finally: - if honeybadger.config.insights_enabled: - honeybadger.event( - "asgi.request", - { + try: + asgi_config = honeybadger.config.insights_config.asgi + if honeybadger.config.insights_enabled and not asgi_config.disabled: + payload = { "method": scope.get("method"), "path": scope.get("path"), "status": status, "duration": get_duration(start), - }, + } + + if asgi_config.include_params: + raw_qs = scope.get("query_string", b"") + params = {} + if raw_qs: + parsed = urllib.parse.parse_qs(raw_qs.decode()) + for key, values in parsed.items(): + params[key] = values[0] if len(values) == 1 else values + + payload["params"] = utils.filter_dict( + params, + honeybadger.config.params_filters, + remove_keys=True, + ) + + honeybadger.event("asgi.request", payload) + honeybadger.reset_context() + except Exception as e: + logger.warning( + f"Exception while sending Honeybadger event: {e}", exc_info=True ) - honeybadger.reset_context() def supports(self, config, context): return context.get("asgi") is not None diff --git a/honeybadger/contrib/celery.py b/honeybadger/contrib/celery.py index b92f091..a43cae9 100644 --- a/honeybadger/contrib/celery.py +++ b/honeybadger/contrib/celery.py @@ -3,7 +3,7 @@ from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager -from honeybadger.utils import extract_honeybadger_config, get_duration +from honeybadger.utils import filter_dict, extract_honeybadger_config, get_duration _listener_started = False @@ -125,8 +125,23 @@ def _on_task_postrun(self, task_id=None, task=None, *args, **kwargs): """ Callback executed after a task is finished. """ + insights_config = honeybadger.config.insights_config - if honeybadger.config.insights_enabled: + exclude = insights_config.celery.exclude_tasks + should_exclude = exclude and any( + ( + pattern.search(task.name) + if hasattr(pattern, "search") + else pattern == task.name + ) + for pattern in exclude + ) + + if ( + honeybadger.config.insights_enabled + and not insights_config.celery.disabled + and not should_exclude + ): payload = { "task_id": task_id, "task_name": task.name, @@ -134,11 +149,16 @@ def _on_task_postrun(self, task_id=None, task=None, *args, **kwargs): "group": task.request.group, "state": kwargs["state"], "duration": get_duration(self._task_starts.pop(task_id, None)), - # TODO: allow filtering before sending args - # "args": kwargs["args"], - # "kwargs": kwargs["kwargs"], } + if insights_config.celery.include_args: + payload["args"] = task.request.args + payload["kwargs"] = filter_dict( + task.request.kwargs, + honeybadger.config.params_filters, + remove_keys=True, + ) + task.send_event("task-finished", payload=payload) honeybadger.reset_context() diff --git a/honeybadger/contrib/db.py b/honeybadger/contrib/db.py new file mode 100644 index 0000000..9e2d177 --- /dev/null +++ b/honeybadger/contrib/db.py @@ -0,0 +1,40 @@ +import time +import re +from honeybadger import honeybadger +from honeybadger.utils import get_duration + + +class DBHoneybadger: + @staticmethod + def django_execute(orig_exec): + def wrapper(self, sql, params=None): + start = time.time() + try: + return orig_exec(self, sql, params) + finally: + DBHoneybadger.execute(sql, start, params) + + return wrapper + + @staticmethod + def execute(sql, start, params=None): + db_config = honeybadger.config.insights_config.db + if db_config.disabled: + return + + q = db_config.exclude_queries + if q and any( + (pattern.search(sql) if hasattr(pattern, "search") else pattern in sql) + for pattern in q + ): + return + + data = { + "query": sql, + "duration": get_duration(start), + } + + if params and db_config.include_params: + data["params"] = params + + honeybadger.event("db.query", data) diff --git a/honeybadger/contrib/django.py b/honeybadger/contrib/django.py index 85beb68..22d2476 100644 --- a/honeybadger/contrib/django.py +++ b/honeybadger/contrib/django.py @@ -7,6 +7,7 @@ from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager from honeybadger.utils import filter_dict, filter_env_vars, get_duration +from honeybadger.contrib.db import DBHoneybadger try: from threading import local # type: ignore[no-redef] @@ -139,7 +140,10 @@ def __call__(self, request): honeybadger.begin_request(request) response = self.get_response(request) - if honeybadger.config.insights_enabled: + if ( + honeybadger.config.insights_enabled + and not honeybadger.config.insights_config.django.disabled + ): self._send_request_event(request, response, start_time) honeybadger.reset_context() @@ -151,17 +155,7 @@ def _patch_cursor(self): from django.db.backends.utils import CursorWrapper orig_exec = CursorWrapper.execute - - def hb_execute(self, sql, params=None): - start = time.time() - try: - return orig_exec(self, sql, params) - finally: - honeybadger.event( - "db.query", {"query": sql, "duration": get_duration(start)} - ) - - CursorWrapper.execute = hb_execute + CursorWrapper.execute = DBHoneybadger.django_execute(orig_exec) def _send_request_event(self, request, response, start_time): # Get resolver data @@ -192,6 +186,17 @@ def _send_request_event(self, request, response, start_time): "duration": get_duration(start_time), } + if honeybadger.config.insights_config.django.include_params: + params = {} + for qd in [request.GET, request.POST]: + for key in qd: + values = qd.getlist(key) + params[key] = values[0] if len(values) == 1 else values + filtered = filter_dict( + params, honeybadger.config.params_filters, remove_keys=True + ) + request_data["params"] = filtered + honeybadger.event("django.request", request_data) def process_exception(self, request, exception): diff --git a/honeybadger/contrib/flask.py b/honeybadger/contrib/flask.py index fb0ab29..ddb2866 100644 --- a/honeybadger/contrib/flask.py +++ b/honeybadger/contrib/flask.py @@ -12,6 +12,7 @@ get_duration, extract_honeybadger_config, ) +from honeybadger.contrib.db import DBHoneybadger from six import iteritems logger = logging.getLogger(__name__) @@ -208,22 +209,48 @@ def _handle_request_started(self, sender, *args, **kwargs): ) def _handle_request_finished(self, sender, *args, **kwargs): + if honeybadger.config.insights_config.flask.disabled: + return + info = _request_info.get({}) request = info.get("request") start = info.get("start_time") response = kwargs.get("response") - honeybadger.event( - "flask.request", - { - "path": request.path, - "method": request.method, - "status": response.status_code, - "view": request.endpoint, - "blueprint": request.blueprint, - "duration": get_duration(start), - }, - ) + payload = { + "path": request.path, + "method": request.method, + "status": response.status_code, + "view": request.endpoint, + "blueprint": request.blueprint, + "duration": get_duration(start), + } + + if honeybadger.config.insights_config.flask.include_params: + params = {} + + # Add query params (from URL) + for key in request.args: + values = request.args.getlist(key) + params[key] = values[0] if len(values) == 1 else values + + # Add form params (from POST body) + for key in request.form: + values = request.form.getlist(key) + # Combine with existing values if key present + if key in params: + existing = ( + params[key] if isinstance(params[key], list) else [params[key]] + ) + params[key] = existing + values + else: + params[key] = values[0] if len(values) == 1 else values + + payload["params"] = filter_dict( + params, honeybadger.config.params_filters, remove_keys=True + ) + + honeybadger.event("flask.request", payload) _request_info.set({}) @@ -261,6 +288,4 @@ def _before(conn, cursor, stmt, params, ctx, executemany): @event.listens_for(Engine, "after_cursor_execute", propagate=True) def _after(conn, cursor, stmt, params, ctx, executemany): - honeybadger.event( - "db.query", {"query": stmt, "duration": get_duration(ctx._hb_start)} - ) + DBHoneybadger.execute(stmt, ctx._hb_start, params) diff --git a/honeybadger/tests/contrib/test_asgi.py b/honeybadger/tests/contrib/test_asgi.py index 63dab1d..9dca228 100644 --- a/honeybadger/tests/contrib/test_asgi.py +++ b/honeybadger/tests/contrib/test_asgi.py @@ -3,9 +3,11 @@ from async_asgi_testclient import TestClient # type: ignore import aiounittest import mock + from honeybadger import honeybadger from honeybadger import contrib from honeybadger.config import Configuration +from honeybadger.tests.utils import with_config class SomeError(Exception): @@ -53,41 +55,17 @@ async def test_should_not_notify_exception(self, hb): response = await self.client.get("/") hb.notify.assert_not_called() - @aiounittest.async_test - @mock.patch("honeybadger.contrib.asgi.honeybadger") - async def test_should_send_request_event_on_success(self, hb): - response = await self.client.get("/test") - self.assertEqual(response.status_code, 200) - hb.event.assert_called_once() - name, payload = hb.event.call_args.args - self.assertEqual(name, "asgi.request") - self.assertEqual(payload["method"], "GET") - self.assertEqual(payload["path"], "/test") - self.assertEqual(payload["status"], 200) - self.assertIsInstance(payload["duration"], float) - - @aiounittest.async_test - @mock.patch("honeybadger.contrib.asgi.honeybadger") - async def test_should_send_request_event_on_exception(self, hb): - with self.assertRaises(SomeError): - await self.client.get("/error") - hb.event.assert_called_once() - name, payload = hb.event.call_args.args - self.assertEqual(name, "asgi.request") - self.assertEqual(payload["method"], "GET") - self.assertEqual(payload["path"], "/error") - self.assertIsNone(payload["status"]) - self.assertIsInstance(payload["duration"], float) - class ASGIEventPayloadTestCase(unittest.TestCase): @aiounittest.async_test + @with_config({"insights_enabled": True}) @mock.patch("honeybadger.contrib.asgi.honeybadger.event") async def test_success_event_payload(self, event): - app = contrib.ASGIHoneybadger(asgi_app(), api_key="abcd", insights_enabled=True) - self.client = TestClient(app) + app = TestClient( + contrib.ASGIHoneybadger(asgi_app(), api_key="abcd", insights_enabled=True) + ) # even if there’s a query, url stays just the path - await self.client.get("/hello?x=1") + await app.get("/hello?x=1") event.assert_called_once() name, payload = event.call_args.args @@ -96,4 +74,30 @@ async def test_success_event_payload(self, event): self.assertEqual(payload["path"], "/hello") self.assertEqual(payload["status"], 200) self.assertIsInstance(payload["duration"], float) - honeybadger.config = Configuration() + + @aiounittest.async_test + @with_config( + {"insights_enabled": True, "insights_config": {"asgi": {"disabled": True}}} + ) + @mock.patch("honeybadger.contrib.asgi.honeybadger.event") + async def test_disabled_by_insights_config(self, event): + app = TestClient(contrib.ASGIHoneybadger(asgi_app(), api_key="abcd")) + await app.get("/hello?x=1") + event.assert_not_called() + + @aiounittest.async_test + @with_config( + { + "insights_enabled": True, + "insights_config": {"asgi": {"include_params": True}}, + } + ) + @mock.patch("honeybadger.contrib.asgi.honeybadger.event") + async def test_disable_insights(self, event): + app = TestClient( + contrib.ASGIHoneybadger(asgi_app(), api_key="abcd", insights_enabled=True) + ) + await app.get("/hello?x=1&password=secret&y=2&y=3") + event.assert_called_once() + name, payload = event.call_args.args + self.assertEqual(payload["params"], {"x": "1", "y": ["2", "3"]}) diff --git a/honeybadger/tests/contrib/test_celery.py b/honeybadger/tests/contrib/test_celery.py index 324c164..94df887 100644 --- a/honeybadger/tests/contrib/test_celery.py +++ b/honeybadger/tests/contrib/test_celery.py @@ -1,13 +1,10 @@ -import pytest import time +import re from unittest.mock import patch, MagicMock from celery import Celery -from celery.app.task import Context -from honeybadger import honeybadger from honeybadger.contrib.celery import CeleryHoneybadger, CeleryPlugin - -import honeybadger.connection as connection +from honeybadger.tests.utils import with_config @patch("honeybadger.honeybadger.reset_context") @@ -20,14 +17,19 @@ def test_notify_from_task_failure(notify, reset_context): hb = CeleryHoneybadger(app, report_exceptions=True) # Send task_failure event - task_failure.send(sender=app, task_id="hi", task_name="tasks.add", exception=exception) + task_failure.send( + sender=app, task_id="hi", task_name="tasks.add", exception=exception + ) assert notify.call_count == 1 assert notify.call_args[1]["exception"] == exception - task_postrun.send(sender=app, task_id="hi", task_name="tasks.add", task={}) + task_postrun.send( + sender=app, task_id="hi", task_name="tasks.add", task={"name": "tasks.add"} + ) assert reset_context.call_count == 1 + hb.tearDown() @@ -51,10 +53,12 @@ def test_plugin_payload(): test_task = MagicMock() test_task.name = "test_task" test_task.max_retries = 10 - test_task.request = Context( + test_task.request = MagicMock( id="test_id", name="test_task", args=(1, 2), + retries=0, + max_retries=10, kwargs={"foo": "bar"}, ) @@ -71,10 +75,8 @@ def test_plugin_payload(): assert request["context"]["max_retries"] == 10 -@patch("celery.events.EventReceiver") -@patch("honeybadger.honeybadger.event") -def test_finished_task_event(mock_event, mock_event_receiver): - from celery.signals import worker_ready, task_postrun +def setup_celery_hb(): + from celery.signals import worker_ready app = Celery(__name__, broker="memory://localhost/") app.conf.update( @@ -82,11 +84,15 @@ def test_finished_task_event(mock_event, mock_event_receiver): HONEYBADGER_API_KEY="test_api_key", ) hb = CeleryHoneybadger(app) - worker_ready.send(sender=app) + time.sleep(0.2) + return app, hb + - # Give time for the thread to start - time.sleep(0.5) +@patch("celery.events.EventReceiver") +@patch("honeybadger.honeybadger.event") +def test_finished_task_event(mock_event, mock_event_receiver): + app, hb = setup_celery_hb() assert mock_event_receiver.call_count == 1 assert ( @@ -101,3 +107,64 @@ def test_finished_task_event(mock_event, mock_event_receiver): assert mock_event.call_args[0][1] == {"data": "test_task_id"} hb.tearDown() + + +@with_config({"insights_config": {"celery": {"include_args": True}}}) +def test_includes_task_args(): + app, hb = setup_celery_hb() + task = MagicMock() + task.request.group = None + task.name = "test_task" + task.request.name = "test_task" + task.request.retries = 1 + task.request.args = [1, 2] + task.request.kwargs = {"foo": "bar", "password": "secret"} + + hb._on_task_prerun("test_task_id") + hb._on_task_postrun("test_task_id", task, None, state="SUCCESS") + + task_arg = lambda x: task.send_event.call_args[1]["payload"][x] + + assert task.send_event.call_count == 1 + assert task.send_event.call_args[0][0] == "task-finished" + assert task_arg("task_id") == "test_task_id" + assert task_arg("task_name") == "test_task" + assert task_arg("args") == [1, 2] + assert task_arg("kwargs") == {"foo": "bar"} + assert task_arg("retries") == 1 + assert task_arg("state") == "SUCCESS" + assert task_arg("group") is None + assert task_arg("duration") > 0 + + hb.tearDown() + + +@with_config({"insights_config": {"celery": {"disabled": True}}}) +def test_can_disable(): + app, hb = setup_celery_hb() + task = MagicMock() + hb._on_task_postrun("test_task_id", task, None, state="SUCCESS") + assert task.send_event.call_count == 0 + hb.tearDown() + + +@with_config({"insights_config": {"celery": {"exclude_tasks": ["test_task"]}}}) +def test_exclude_tasks_with_string(): + app, hb = setup_celery_hb() + task = MagicMock() + task.name = "test_task" + hb._on_task_postrun("test_task_id", task, None, state="SUCCESS") + assert task.send_event.call_count == 0 + hb.tearDown() + + +@with_config( + {"insights_config": {"celery": {"exclude_tasks": [re.compile(r"test_.*_task")]}}} +) +def test_exclude_tasks_with_regex(): + app, hb = setup_celery_hb() + task = MagicMock() + task.name = "test_the_task" + hb._on_task_postrun("test_task_id", task, None, state="SUCCESS") + assert task.send_event.call_count == 0 + hb.tearDown() diff --git a/honeybadger/tests/contrib/test_db.py b/honeybadger/tests/contrib/test_db.py new file mode 100644 index 0000000..6cc6860 --- /dev/null +++ b/honeybadger/tests/contrib/test_db.py @@ -0,0 +1,62 @@ +import time +import re +from functools import wraps +from unittest.mock import patch, MagicMock +from honeybadger.tests.utils import with_config + +import pytest + +from honeybadger import honeybadger +from honeybadger.config import Configuration +from honeybadger.contrib.db import DBHoneybadger + + +@patch("honeybadger.honeybadger.event") +def test_execute_sends_event_when_enabled(mock_event): + DBHoneybadger.execute("SELECT 1", start=0) + mock_event.assert_called_once() + args, kwargs = mock_event.call_args + assert args[0] == "db.query" + assert args[1]["query"] == "SELECT 1" + assert args[1]["duration"] > 0 + assert "params" not in args[1] + + +@with_config({"insights_config": {"db": {"disabled": True}}}) +@patch("honeybadger.honeybadger.event") +def test_execute_does_not_send_event_when_disabled(mock_event): + DBHoneybadger.execute("SELECT 1", start=0) + mock_event.assert_not_called() + + +@with_config({"insights_config": {"db": {"include_params": True}}}) +@patch("honeybadger.honeybadger.event") +def test_execute_includes_params(mock_event): + params = (123, "abc") + DBHoneybadger.execute("SELECT x FROM t WHERE a=%s AND b=%s", start=0, params=params) + args, kwargs = mock_event.call_args + assert args[1]["params"] == params + + +@patch("honeybadger.honeybadger.event") +def test_execute_does_not_include_params_when_not_configured(mock_event): + params = (1, 2) + DBHoneybadger.execute("SELECT 1", start=0, params=params) + args, kwargs = mock_event.call_args + assert "params" not in args[1] + + +@with_config( + {"insights_config": {"db": {"exclude_queries": [re.compile(r"SELECT 1")]}}} +) +@patch("honeybadger.honeybadger.event") +def test_execute_excludes_queries_for_regexes(mock_event): + DBHoneybadger.execute("SELECT 1 abc", start=0) + mock_event.assert_not_called() + + +@with_config({"insights_config": {"db": {"exclude_queries": ["PRAGMA"]}}}) +@patch("honeybadger.honeybadger.event") +def test_execute_excludes_queries_for_strings(mock_event): + DBHoneybadger.execute("PRAGMA (*)", start=0) + mock_event.assert_not_called() diff --git a/honeybadger/tests/contrib/test_django.py b/honeybadger/tests/contrib/test_django.py index e6265f9..f10de56 100644 --- a/honeybadger/tests/contrib/test_django.py +++ b/honeybadger/tests/contrib/test_django.py @@ -246,7 +246,15 @@ def assert_payload(req): self.assertTrue(request_mock.called) -@override_settings(HONEYBADGER={"INSIGHTS_ENABLED": True}) +class FakeCursorWrapper: + def __init__(self, *a, **kw): + pass + + def execute(self, sql, params=None): + return f"original execute: {sql}" + + +@override_settings(HONEYBADGER={"INSIGHTS_ENABLED": True, "INSIGHTS_CONFIG": {}}) class DjangoMiddlewareEventTestCase(SimpleTestCase): def setUp(self): self.rf = RequestFactory() @@ -288,3 +296,42 @@ def test_no_request_left_in_thread_locals(self, mock_event): mw = DjangoHoneybadgerMiddleware(lambda req: Mock()) mw(request) self.assertIsNone(current_request()) + + @patch("django.db.backends.utils.CursorWrapper", new=FakeCursorWrapper) + @patch("honeybadger.contrib.django.honeybadger.event") + def test_patch_cursor_and_execute_sends_event(self, mock_event): + mw = DjangoHoneybadgerMiddleware(lambda req: None) + cur = FakeCursorWrapper() + res = cur.execute("SELECT something") + assert res == "original execute: SELECT something" + mock_event.assert_called_once() + + @override_settings( + HONEYBADGER={ + "INSIGHTS_ENABLED": True, + "INSIGHTS_CONFIG": {"django": {"disabled": True}}, + } + ) + @patch("honeybadger.contrib.django.honeybadger.event") + def test_event_disabled_with_disabled_config(self, mock_event): + request = self.rf.get("/plain_view/") + request.resolver_match = self.url.resolve("plain_view") + mw = DjangoHoneybadgerMiddleware(lambda req: Mock()) + mw(request) + mock_event.assert_not_called() + + @override_settings( + HONEYBADGER={ + "INSIGHTS_ENABLED": True, + "INSIGHTS_CONFIG": {"django": {"include_params": True}}, + } + ) + @patch("honeybadger.contrib.django.honeybadger.event") + def test_event_includes_filtered_params(self, mock_event): + request = self.rf.get("/plain_view/", {"password": "hide", "b": 2}) + request.resolver_match = self.url.resolve("plain_view") + mw = DjangoHoneybadgerMiddleware(lambda req: Mock()) + mw(request) + mock_event.assert_called_once() + event_name, data = mock_event.call_args[0] + self.assertEqual(data["params"], {"b": "2"}) diff --git a/honeybadger/tests/contrib/test_flask.py b/honeybadger/tests/contrib/test_flask.py index e4bcbc7..f2d50a1 100644 --- a/honeybadger/tests/contrib/test_flask.py +++ b/honeybadger/tests/contrib/test_flask.py @@ -7,6 +7,7 @@ from honeybadger import honeybadger from honeybadger.config import Configuration from honeybadger.contrib.flask import FlaskPlugin, FlaskHoneybadger +from honeybadger.tests.utils import with_config PYTHON_VERSION = sys.version_info[0:2] @@ -406,3 +407,34 @@ def test_insights_event_on_post(self, mock_event): self.assertEqual(payload["view"], "ping") # duration should be non‐negative self.assertGreaterEqual(payload["duration"], 0.0) + + @patch("honeybadger.contrib.flask.honeybadger.event") + def test_insights_db_event(self, mock_event): + from sqlalchemy import create_engine, text + + engine = create_engine("sqlite:///:memory:") + with engine.connect() as conn: + conn.execute(text("SELECT 1")) + + assert mock_event.called + event_name, payload = mock_event.call_args[0] + assert event_name == "db.query" + assert "SELECT 1" in payload["query"] + assert "duration" in payload + + @with_config({"insights_config": {"flask": {"disabled": True}}}) + @patch("honeybadger.contrib.flask.honeybadger.event") + def test_insights_no_event_when_disabled(self, mock_event): + resp = self.client.get("/ping?foo=bar") + self.assertEqual(resp.status_code, 201) + mock_event.assert_not_called() + + @with_config({"insights_config": {"flask": {"include_params": True}}}) + @patch("honeybadger.contrib.flask.honeybadger.event") + def test_insights_event_includes_filtered_params(self, mock_event): + resp = self.client.post("/ping", data={"password": "ha!", "id": "abc123"}) + self.assertEqual(resp.status_code, 201) + self.assertEqual(mock_event.call_count, 1) + + _, payload = mock_event.call_args[0] + self.assertEqual(payload["params"], {"id": "abc123"}) diff --git a/honeybadger/tests/test_config.py b/honeybadger/tests/test_config.py index 88d000d..4c469d3 100644 --- a/honeybadger/tests/test_config.py +++ b/honeybadger/tests/test_config.py @@ -2,6 +2,7 @@ import os import pytest +import logging from honeybadger.config import Configuration @@ -36,11 +37,39 @@ def test_config_bool_types_are_accurate(): assert c.force_report_data == True -def test_can_only_set_valid_options(): - c = Configuration(foo="bar") - with pytest.raises(AttributeError): - # pylint: disable-next=no-member - print(c.foo) +def test_can_only_set_valid_options(caplog): + with caplog.at_level(logging.WARNING): + try: + Configuration(foo="bar") + except AttributeError: + pass + assert any( + "Unknown Configuration option" in msg for msg in caplog.text.splitlines() + ) + + +def test_is_okay_with_unknown_env_var(): + os.environ["HONEYBADGER_FOO"] = "bar" + try: + Configuration() + except Exception: + pytest.fail("This should fail silently.") + + +def test_nested_dataclass_raises_for_invalid_key(caplog): + c = Configuration(insights_config={}) + with caplog.at_level(logging.WARNING): + c.set_config_from_dict({"insights_config": {"db": {"bogus": True}}}) + assert any("Unknown DBConfig option" in msg for msg in caplog.text.splitlines()) + + +def test_set_config_from_dict_raises_for_unknown_key(caplog): + c = Configuration() + with caplog.at_level(logging.WARNING): + c.set_config_from_dict({"does_not_exist": 123}) + assert any( + "Unknown Configuration option" in msg for msg in caplog.text.splitlines() + ) def test_valid_dev_environments(): @@ -72,3 +101,30 @@ def before_notify_callback(notice): c = Configuration(before_notify=before_notify_callback) assert c.before_notify == before_notify_callback + + +def test_configure_nested_insights_config(): + c = Configuration(insights_config={"db": {"disabled": True}}) + assert c.insights_config.db.disabled == True + + +def test_configure_throws_for_invalid_insights_config(caplog): + with caplog.at_level(logging.WARNING): + Configuration(insights_config={"foo": "bar"}) + assert any( + "Unknown InsightsConfig option" in msg for msg in caplog.text.splitlines() + ) + + +def test_configure_merges_insights_config(): + c = Configuration(api_key="test", insights_config={}) + + c.set_config_from_dict({"insights_config": {"db": {"include_params": True}}}) + assert hasattr(c.insights_config, "db") + assert c.insights_config.db.include_params is True + + c.set_config_from_dict({"insights_config": {"celery": {"disabled": True}}}) + assert hasattr(c.insights_config, "celery") + assert c.insights_config.celery.disabled is True + + assert c.insights_config.db.include_params is True diff --git a/honeybadger/tests/utils.py b/honeybadger/tests/utils.py index a67b1f0..030ad8e 100644 --- a/honeybadger/tests/utils.py +++ b/honeybadger/tests/utils.py @@ -1,9 +1,13 @@ from contextlib import contextmanager from mock import patch from mock import DEFAULT +import inspect import six import time +from functools import wraps from threading import Event +from honeybadger import honeybadger +from honeybadger.config import Configuration @contextmanager @@ -21,3 +25,38 @@ def mock_was_called(*args, **kwargs): mock_called_event.wait(0.5) ((request_object,), mock_kwargs) = request_mock.call_args func(request_object) + + +def with_config(config): + """ + Decorator to set honeybadger.config for a test, and restore it after. + Usage: + @with_config({"a": "b"}) + def test_...(): + ... + """ + + def decorator(fn): + if inspect.iscoroutinefunction(fn): + + @wraps(fn) + async def wrapper(*args, **kwargs): + honeybadger.configure(**config) + try: + return await fn(*args, **kwargs) + finally: + honeybadger.config = Configuration() + + else: + + @wraps(fn) + def wrapper(*args, **kwargs): + honeybadger.configure(**config) + try: + return fn(*args, **kwargs) + finally: + honeybadger.config = Configuration() + + return wrapper + + return decorator diff --git a/honeybadger/utils.py b/honeybadger/utils.py index 174ec65..2a823ff 100644 --- a/honeybadger/utils.py +++ b/honeybadger/utils.py @@ -45,7 +45,7 @@ def filter_env_vars(data): return filtered_data -def filter_dict(data, filter_keys): +def filter_dict(data, filter_keys, remove_keys=False): if type(data) != dict: return data @@ -58,12 +58,15 @@ def filter_dict(data, filter_keys): data.pop(key) continue - if key in filter_keys: - data[key] = "[FILTERED]" - if type(data[key]) == dict: data[key] = filter_dict(data[key], filter_keys) + if key in filter_keys: + if remove_keys: + data.pop(key) + else: + data[key] = "[FILTERED]" + return data From ee20e94df25ed2d093abbd723288aa14c7616bb7 Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Wed, 28 May 2025 12:18:16 -0700 Subject: [PATCH 04/17] feat(insights): event_context (#214) * refactor threadlocals This now uses the ContextVar and a helper ContextStore class. * Basic event_context * More tests * Fix other Notice usage --- dev-requirements.txt | 1 + honeybadger/connection.py | 1 - honeybadger/context_store.py | 47 +++++++ honeybadger/core.py | 86 +++++++----- honeybadger/notice.py | 41 +++--- honeybadger/protocols.py | 4 +- honeybadger/tests/test_core.py | 218 +++++++++++++++++++++++++++++-- honeybadger/tests/test_notice.py | 24 +--- honeybadger/types.py | 2 +- pytest.ini | 1 + 10 files changed, 330 insertions(+), 95 deletions(-) create mode 100644 honeybadger/context_store.py diff --git a/dev-requirements.txt b/dev-requirements.txt index 5dc426f..24ea21c 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -15,6 +15,7 @@ psutil pylint pytest pytest-cov +pytest-asyncio six sqlalchemy testfixtures diff --git a/honeybadger/connection.py b/honeybadger/connection.py index a604e5b..dc4e9a6 100644 --- a/honeybadger/connection.py +++ b/honeybadger/connection.py @@ -3,7 +3,6 @@ import threading from urllib.error import HTTPError, URLError -from typing import Protocol from six.moves.urllib import request from six import b diff --git a/honeybadger/context_store.py b/honeybadger/context_store.py new file mode 100644 index 0000000..f843e6a --- /dev/null +++ b/honeybadger/context_store.py @@ -0,0 +1,47 @@ +from contextvars import ContextVar +from contextlib import contextmanager +from typing import Any, Dict, Optional + + +class ContextStore: + def __init__(self, name: str): + self._ctx: ContextVar[Optional[Dict[str, Any]]] = ContextVar(name, default=None) + + def get(self) -> Dict[str, Any]: + data = self._ctx.get() + return {} if data is None else data.copy() + + def clear(self) -> None: + self._ctx.set({}) + + def update(self, ctx: Optional[Dict[str, Any]] = None, **kwargs: Any) -> None: + """ + Merge into the current context. Accepts either: + - update({'foo': 'bar'}) + - update(foo='bar', baz=123) + - or both: update({'foo': 'bar'}, baz=123) + """ + to_merge: Dict[str, Any] = {} + if ctx: + to_merge.update(ctx) + to_merge.update(kwargs) + + new = self.get() + new.update(to_merge) + self._ctx.set(new) + + @contextmanager + def override(self, ctx: Optional[Dict[str, Any]] = None, **kwargs: Any): + """ + Temporarily merge these into context for the duration of the with-block. + """ + to_merge: Dict[str, Any] = {} + if ctx: + to_merge.update(ctx) + to_merge.update(kwargs) + + token = self._ctx.set({**self.get(), **to_merge}) + try: + yield + finally: + self._ctx.reset(token) diff --git a/honeybadger/core.py b/honeybadger/core.py index 303d953..279b806 100644 --- a/honeybadger/core.py +++ b/honeybadger/core.py @@ -2,10 +2,9 @@ from contextlib import contextmanager import sys import logging -import copy -import time import datetime import atexit +from typing import Optional, Dict, Any, List from honeybadger.plugins import default_plugin_manager import honeybadger.connection as connection @@ -13,18 +12,23 @@ from .events_worker import EventsWorker from .config import Configuration from .notice import Notice +from .context_store import ContextStore logger = logging.getLogger("honeybadger") logger.addHandler(logging.NullHandler()) +error_context = ContextStore("honeybadger_error_context") +event_context = ContextStore("honeybadger_event_context") + class Honeybadger(object): TS_FORMAT = "%Y-%m-%dT%H:%M:%S.%fZ" def __init__(self): + error_context.clear() + event_context.clear() + self.config = Configuration() - self.thread_local = threading.local() - self.thread_local.context = {} self.events_worker = EventsWorker( self._connection(), self.config, logger=logging.getLogger("honeybadger") ) @@ -47,21 +51,16 @@ def _send_notice(self, notice): self._connection().send_notice(self.config, notice) - def _get_context(self): - return getattr(self.thread_local, "context", {}) - - def begin_request(self, request): - self.thread_local.context = self._get_context() + def begin_request(self, _): + error_context.clear() + event_context.clear() def wrap_excepthook(self, func): self.existing_except_hook = func sys.excepthook = self.exception_hook def exception_hook(self, type, exception, exc_traceback): - notice = Notice( - exception=exception, thread_local=self.thread_local, config=self.config - ) - self._send_notice(notice) + self.notify(exception=exception) self.existing_except_hook(type, exception, exc_traceback) def shutdown(self): @@ -72,18 +71,22 @@ def notify( exception=None, error_class=None, error_message=None, - context={}, + context: Optional[Dict[str, Any]] = None, fingerprint=None, - tags=[], + tags: Optional[List[str]] = None, ): + base = error_context.get() + tag_ctx = base.pop("_tags", []) + merged_ctx = {**base, **(context or {})} + merged_tags = list({*tag_ctx, *(tags or [])}) + notice = Notice( exception=exception, error_class=error_class, error_message=error_message, - context=context, + context=merged_ctx, fingerprint=fingerprint, - tags=tags, - thread_local=self.thread_local, + tags=merged_tags, config=self.config, ) return self._send_notice(notice) @@ -124,7 +127,9 @@ def event(self, event_type=None, data=None, **kwargs): if isinstance(payload["ts"], datetime.datetime): payload["ts"] = payload["ts"].strftime(self.TS_FORMAT) - return self.events_worker.push(payload) + final_payload = {**self._get_event_context(), **payload} + + return self.events_worker.push(final_payload) def configure(self, **kwargs): self.config.set_config_from_dict(kwargs) @@ -141,28 +146,37 @@ def auto_discover_plugins(self): if self.config.is_aws_lambda_environment: default_plugin_manager.register(contrib.AWSLambdaPlugin()) - def set_context(self, ctx=None, **kwargs): - # This operation is an update, not a set! - if not ctx: - ctx = kwargs - else: - ctx.update(kwargs) - self.thread_local.context = self._get_context() - self.thread_local.context.update(ctx) + # Error context + # + def _get_context(self): + return error_context.get() + + def set_context(self, ctx: Optional[Dict[str, Any]] = None, **kwargs): + error_context.update(ctx, **kwargs) def reset_context(self): - self.thread_local.context = {} + error_context.clear() @contextmanager - def context(self, **kwargs): - original_context = copy.copy(self._get_context()) - self.set_context(**kwargs) - try: + def context(self, ctx: Optional[Dict[str, Any]] = None, **kwargs): + with error_context.override(ctx, **kwargs): + yield + + # Event context + # + def _get_event_context(self): + return event_context.get() + + def set_event_context(self, ctx: Optional[Dict[str, Any]] = None, **kwargs): + event_context.update(ctx, **kwargs) + + def reset_event_context(self): + event_context.clear() + + @contextmanager + def event_context(self, ctx: Optional[Dict[str, Any]] = None, **kwargs): + with event_context.override(ctx, **kwargs): yield - except: - raise - else: - self.thread_local.context = original_context def _connection(self): if self.config.is_dev() and not self.config.force_report_data: diff --git a/honeybadger/notice.py b/honeybadger/notice.py index b21c0fd..cf6d222 100644 --- a/honeybadger/notice.py +++ b/honeybadger/notice.py @@ -9,14 +9,11 @@ def __init__(self, *args, **kwargs): self.error_message = kwargs.get("error_message", None) self.exc_traceback = kwargs.get("exc_traceback", None) self.fingerprint = kwargs.get("fingerprint", None) - self.thread_local = kwargs.get("thread_local", None) self.config = kwargs.get("config", None) self.context = kwargs.get("context", {}) self.tags = self._construct_tags(kwargs.get("tags", [])) self._process_exception() - self._process_context() - self._process_tags() def _process_exception(self): if self.exception is None and self.error_class: @@ -28,15 +25,6 @@ def _process_exception(self): elif self.exception and self.error_message: self.context["error_message"] = self.error_message - def _process_context(self): - self.context = dict(**self._get_thread_context(), **self.context) - - def _process_tags(self): - tags_from_context = self._construct_tags( - self._get_thread_context().get("_tags", []) - ) - self.tags = list(set(tags_from_context + self.tags)) - @cached_property def payload(self): return create_payload( @@ -61,15 +49,24 @@ def excluded_exception(self): return True return False - def _get_thread_context(self): - if self.thread_local is None: - return {} - return getattr(self.thread_local, "context", {}) - def _construct_tags(self, tags): - constructed_tags = [] + """ + Accepts either: + - a single string (possibly comma-separated) + - a list of strings (each possibly comma-separated) + and returns a flat list of stripped tags. + """ + raw = [] if isinstance(tags, str): - constructed_tags = [tag.strip() for tag in tags.split(",")] - elif isinstance(tags, list): - constructed_tags = tags - return constructed_tags + raw = [tags] + elif isinstance(tags, (list, tuple)): + raw = tags + out = [] + for item in raw: + if not isinstance(item, str): + continue + for part in item.split(","): + t = part.strip() + if t: + out.append(t) + return out diff --git a/honeybadger/protocols.py b/honeybadger/protocols.py index 7fc4139..dfcbb86 100644 --- a/honeybadger/protocols.py +++ b/honeybadger/protocols.py @@ -1,9 +1,9 @@ -from typing import Protocol, Dict, Any, Optional, List +from typing import Protocol, Any, Optional, List from .types import EventsSendResult, Notice, Event class Connection(Protocol): - def send_notice(self, config: Any, payload: Notice) -> Optional[str]: + def send_notice(self, config: Any, notice: Notice) -> Optional[str]: """ Send an error notice to Honeybadger. diff --git a/honeybadger/tests/test_core.py b/honeybadger/tests/test_core.py index cdc9df8..a337ab0 100644 --- a/honeybadger/tests/test_core.py +++ b/honeybadger/tests/test_core.py @@ -2,6 +2,7 @@ import threading import pytest +import asyncio from .utils import mock_urlopen from honeybadger import Honeybadger @@ -9,20 +10,115 @@ from honeybadger.config import Configuration -def test_set_context(): - honeybadger = Honeybadger() - honeybadger.set_context(foo="bar") - assert honeybadger.thread_local.context == dict(foo="bar") - honeybadger.set_context(bar="foo") - assert honeybadger.thread_local.context == dict(foo="bar", bar="foo") +def test_set_and_get_context_merges_values(): + hb = Honeybadger() + assert hb._get_context() == {} + + hb.set_context(foo="bar") + hb.set_context(baz=123) + assert hb._get_context() == {"foo": "bar", "baz": 123} + + hb.set_context({"a": 1}) + assert hb._get_context() == {"foo": "bar", "baz": 123, "a": 1} + + +def test_reset_context_clears_all(): + hb = Honeybadger() + hb.set_context(temp="value") + assert hb._get_context() # non-empty + hb.reset_context() + assert hb._get_context() == {} + + +def test_context_manager_pushes_and_pops(): + hb = Honeybadger() + hb.set_context(x=1) + original = hb._get_context() + + with hb.context(y=2): + # inside block, we see both x and y + assert hb._get_context() == {"x": 1, "y": 2} + + # after block, y is gone + assert hb._get_context() == original + + +def test_thread_isolation(): + hb = Honeybadger() + hb.set_context(main=True) + + def worker(): + # new thread should start with empty context + assert hb._get_context() == {} + hb.set_context(thread="worker") + assert hb._get_context() == {"thread": "worker"} + + t = threading.Thread(target=worker) + t.start() + t.join() + + # main thread context is untouched + assert hb._get_context() == {"main": True} + + +@pytest.mark.asyncio +async def test_context_async_isolation(): + hb = Honeybadger() + hb.set_context(main=True) + assert hb._get_context() == {"main": True} + + async def worker(): + assert hb._get_context() == {"main": True} + hb.set_context(thread="worker") + assert hb._get_context() == {"main": True, "thread": "worker"} + tasks = [asyncio.create_task(worker()) for _ in range(2)] + await asyncio.gather(*tasks) + assert hb._get_context() == {"main": True} -def test_set_context_with_dict(): - honeybadger = Honeybadger() - honeybadger.set_context(dict(foo="bar")) - assert honeybadger.thread_local.context == dict(foo="bar") - honeybadger.set_context(dict(foo="bar", bar="foo")) - assert honeybadger.thread_local.context == dict(foo="bar", bar="foo") + +def test_notify_merges_context_and_tags(monkeypatch): + hb = Honeybadger() + hb.set_context(user="alice", _tags=["from_ctx, another_tag"]) + captured = {} + + def fake_send(notice): + captured["context"] = notice.context + captured["tags"] = notice.tags + + monkeypatch.setattr(hb, "_send_notice", fake_send) + + hb.notify( + exception=RuntimeError("oops"), + context={"action": "save"}, + tags=["explicit"], + ) + + # should merge store + explicit + assert captured["context"] == {"user": "alice", "action": "save"} + # tags deduped and merged + assert set(captured["tags"]) == {"from_ctx", "another_tag", "explicit"} + + +def test_exception_hook_calls_notify(monkeypatch): + hb = Honeybadger() + captured = {} + + def fake_send(notice=None, **kwargs): + captured["notified"] = notice + return "sent" + + def fake_existing_except_hook(*args, **kwargs): + captured["hook"] = True + + monkeypatch.setattr(hb, "_send_notice", fake_send) + + exc = ValueError("fail!") + hb.wrap_excepthook(fake_existing_except_hook) + hb.exception_hook(ValueError, exc, None) + + assert captured["notified"].exception == exc + assert captured["hook"] is True def test_threading(): @@ -261,6 +357,104 @@ def test_event_without_event_type(): assert payload["email"] == "user@example.com" +def test_event_with_event_context(): + mock_events_worker = MagicMock() + + hb = Honeybadger() + hb.events_worker = mock_events_worker + hb.configure(api_key="aaa", force_report_data=True) + hb.set_event_context(service="web") + hb.event(event_type="order.completed", data=dict(email="user@example.com")) + + mock_events_worker.push.assert_called_once() + payload = mock_events_worker.push.call_args[0][0] + + assert payload["service"] == "web" + assert payload["email"] == "user@example.com" + + +def test_event_with_event_context_does_not_override(): + mock_events_worker = MagicMock() + + hb = Honeybadger() + hb.events_worker = mock_events_worker + hb.configure(api_key="aaa", force_report_data=True) + hb.set_event_context(service="web") + hb.event(event_type="order.completed", data=dict(service="my-service!")) + + mock_events_worker.push.assert_called_once() + payload = mock_events_worker.push.call_args[0][0] + + assert payload["service"] == "my-service!" + + +def test_set_and_get_event_context_merges_values(): + hb = Honeybadger() + assert hb._get_event_context() == {} + + hb.set_event_context(foo="bar") + hb.set_event_context(baz=123) + assert hb._get_event_context() == {"foo": "bar", "baz": 123} + + hb.set_event_context({"a": 1}) + assert hb._get_event_context() == {"foo": "bar", "baz": 123, "a": 1} + + +def test_reset_event_context_clears_all(): + hb = Honeybadger() + hb.set_event_context(temp="value") + assert hb._get_event_context() # non-empty + hb.reset_event_context() + assert hb._get_event_context() == {} + + +def test_event_context_manager_pushes_and_pops(): + hb = Honeybadger() + hb.set_event_context(x=1) + original = hb._get_event_context() + + with hb.event_context(y=2): + # inside block, we see both x and y + assert hb._get_event_context() == {"x": 1, "y": 2} + + # after block, y is gone + assert hb._get_event_context() == original + + +def test_event_context_thread_isolation(): + hb = Honeybadger() + hb.set_event_context(main=True) + + def worker(): + # new thread should start with empty event_context + assert hb._get_event_context() == {} + hb.set_event_context(thread="worker") + assert hb._get_event_context() == {"thread": "worker"} + + t = threading.Thread(target=worker) + t.start() + t.join() + + # main thread event_context is untouched + assert hb._get_event_context() == {"main": True} + + +@pytest.mark.asyncio +async def test_event_context_async_isolation(): + hb = Honeybadger() + hb.set_event_context(main=True) + assert hb._get_event_context() == {"main": True} + + async def worker(): + assert hb._get_event_context() == {"main": True} + hb.set_event_context(thread="worker") + assert hb._get_event_context() == {"main": True, "thread": "worker"} + + tasks = [asyncio.create_task(worker()) for _ in range(2)] + await asyncio.gather(*tasks) + assert hb._get_event_context() == {"main": True} + + def test_event_with_before_event_mutated_changes(): def before_event(event): if "ignore" in event: diff --git a/honeybadger/tests/test_notice.py b/honeybadger/tests/test_notice.py index c2d49d3..8b09c77 100644 --- a/honeybadger/tests/test_notice.py +++ b/honeybadger/tests/test_notice.py @@ -85,34 +85,16 @@ def test_notice_with_tags(): assert "tag2" in payload["error"]["tags"] -def test_notify_with_context_tags(): +def test_notice_with_multiple_tags(): config = Configuration() - thread_local = threading.local() - thread_local.context = {"_tags": "tag1, tag2"} notice = Notice( error_class="TestError", error_message="Test message", - thread_local=thread_local, - config=config, - ) - payload = notice.payload - assert "tag1" in payload["error"]["tags"] - assert "tag2" in payload["error"]["tags"] - - -def test_notify_with_context_merging_tags(): - config = Configuration() - thread_local = threading.local() - thread_local.context = {"_tags": "tag1"} - - notice = Notice( - error_class="TestError", - error_message="Test message", - thread_local=thread_local, - tags="tag2", + tags=["tag1, tag2", "tag3"], config=config, ) payload = notice.payload assert "tag1" in payload["error"]["tags"] assert "tag2" in payload["error"]["tags"] + assert "tag3" in payload["error"]["tags"] diff --git a/honeybadger/types.py b/honeybadger/types.py index bbe1253..6ef5fc4 100644 --- a/honeybadger/types.py +++ b/honeybadger/types.py @@ -1,6 +1,6 @@ from enum import Enum from dataclasses import dataclass -from typing import Optional, Protocol, Any, Dict +from typing import Optional, Any, Dict class EventsSendStatus(Enum): diff --git a/pytest.ini b/pytest.ini index 33f6067..a6cde27 100644 --- a/pytest.ini +++ b/pytest.ini @@ -2,3 +2,4 @@ testpaths = honeybadger/tests python_files = test_*.py addopts = --mypy +asyncio_default_fixture_loop_scope = function From 06e1eec30f7a15021c1e0db4bd5c99c0082ca9cf Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Mon, 2 Jun 2025 12:20:13 -0700 Subject: [PATCH 05/17] feat: add request id to django and flask libraries and propagate event context to celery tasks (#216) * feat(insights) add request_id * Add request_id to notice * Fix test * Fix formatted string * Update honeybadger/contrib/celery.py Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Add logger to celelry * Sanitize request_id from inputs --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- honeybadger/contrib/celery.py | 71 ++++++++-------- honeybadger/contrib/django.py | 22 ++++- honeybadger/contrib/flask.py | 8 ++ honeybadger/core.py | 3 + honeybadger/events_worker.py | 20 ++++- honeybadger/notice.py | 7 ++ honeybadger/payload.py | 4 + honeybadger/tests/contrib/test_celery.py | 100 +++++++++++++++++------ honeybadger/tests/contrib/test_django.py | 30 +++++++ honeybadger/tests/contrib/test_flask.py | 17 ++++ honeybadger/tests/test_core.py | 13 +++ honeybadger/tests/test_notice.py | 13 +++ honeybadger/tests/test_payload.py | 10 +++ honeybadger/tests/test_utils.py | 13 ++- honeybadger/utils.py | 11 +++ 15 files changed, 280 insertions(+), 62 deletions(-) diff --git a/honeybadger/contrib/celery.py b/honeybadger/contrib/celery.py index a43cae9..14fd5c8 100644 --- a/honeybadger/contrib/celery.py +++ b/honeybadger/contrib/celery.py @@ -1,11 +1,11 @@ -import threading import time +import logging from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager from honeybadger.utils import filter_dict, extract_honeybadger_config, get_duration -_listener_started = False +logger = logging.getLogger(__name__) class CeleryPlugin(Plugin): @@ -67,7 +67,13 @@ def init_app(self): """ Initialize honeybadger and listen for errors. """ - from celery.signals import task_failure, task_postrun, task_prerun, worker_ready + from celery.signals import ( + task_failure, + task_postrun, + task_prerun, + before_task_publish, + worker_process_init, + ) self._task_starts = {} self._initialize_honeybadger(self.app.conf) @@ -79,9 +85,9 @@ def init_app(self): if honeybadger.config.insights_enabled: # Enable task events, as we need to listen to # task-finished events - self.app.conf.worker_send_task_events = True + worker_process_init.connect(self._on_worker_process_init, weak=False) task_prerun.connect(self._on_task_prerun, weak=False) - worker_ready.connect(self._start_task_event_listener, weak=False) + before_task_publish.connect(self._on_before_task_publish, weak=False) def _initialize_honeybadger(self, config): """ @@ -96,35 +102,34 @@ def _initialize_honeybadger(self, config): honeybadger.configure(**config_kwargs) honeybadger.config.set_12factor_config() # environment should override celery settings - def _start_task_event_listener(self, *args, **kwargs): - # only start the listener once - global _listener_started - if _listener_started: - return - _listener_started = True - - from celery.events import EventReceiver # type: ignore[import] - - def run(): - with self.app.connection() as conn: - recv = EventReceiver( - conn, handlers={"task-finished": self._on_task_finished} - ) - recv.capture(limit=None, timeout=None, wakeup=False) - - self._listen_thread = threading.Thread(target=run, daemon=True) - self._listen_thread.start() - - def _on_task_finished(self, payload, **kwargs): - honeybadger.event("celery.task_finished", payload["payload"]) + def _on_worker_process_init(self, *args, **kwargs): + # Restart the events worker to ensure it is running in the new worker + # process. + try: + honeybadger.events_worker.restart() + except Exception as e: + logger.warning(f"Warning: Failed to restart Honeybadger events worker: {e}") + + def _on_before_task_publish(self, sender=None, body=None, headers=None, **kwargs): + # Inject Honeybadger event context into task headers + if headers is not None: + current_context = honeybadger._get_event_context() + if current_context: + headers["honeybadger_event_context"] = current_context def _on_task_prerun(self, task_id=None, task=None, *args, **kwargs): self._task_starts[task_id] = time.time() + if task: + context = getattr(task.request, "honeybadger_event_context", None) + if context: + honeybadger.set_event_context(context) + def _on_task_postrun(self, task_id=None, task=None, *args, **kwargs): """ Callback executed after a task is finished. """ + insights_config = honeybadger.config.insights_config exclude = insights_config.celery.exclude_tasks @@ -159,7 +164,7 @@ def _on_task_postrun(self, task_id=None, task=None, *args, **kwargs): remove_keys=True, ) - task.send_event("task-finished", payload=payload) + honeybadger.event("celery.task_finished", payload=payload) honeybadger.reset_context() @@ -180,13 +185,15 @@ def tearDown(self): task_failure.disconnect(self._on_task_failure) if honeybadger.config.insights_enabled: - from celery.signals import worker_ready, task_prerun + from celery.signals import ( + task_prerun, + worker_process_init, + before_task_publish, + ) task_prerun.disconnect(self._on_task_prerun) - worker_ready.disconnect(self._start_task_event_listener) - - if hasattr(self, "_listen_thread"): - self._listen_thread.join(timeout=1) + worker_process_init.disconnect(self._on_worker_process_init, weak=False) + before_task_publish.disconnect(self._on_before_task_publish, weak=False) # Keep the misspelled method for backward compatibility def tearDowm(self): diff --git a/honeybadger/contrib/django.py b/honeybadger/contrib/django.py index 22d2476..4710afd 100644 --- a/honeybadger/contrib/django.py +++ b/honeybadger/contrib/django.py @@ -1,12 +1,18 @@ from __future__ import absolute_import import re import time +import uuid from six import iteritems from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager -from honeybadger.utils import filter_dict, filter_env_vars, get_duration +from honeybadger.utils import ( + filter_dict, + filter_env_vars, + get_duration, + sanitize_request_id, +) from honeybadger.contrib.db import DBHoneybadger try: @@ -138,6 +144,7 @@ def __call__(self, request): set_request(request) start_time = time.time() honeybadger.begin_request(request) + self._set_request_id(request) response = self.get_response(request) if ( @@ -151,6 +158,19 @@ def __call__(self, request): return response + def _set_request_id(self, request): + # Attempt to get request ID from various sources + request_id = ( + getattr(request, "id", None) + or getattr(request, "request_id", None) + or request.headers.get("X-Request-ID", None) + ) + request_id = sanitize_request_id(request_id) + if not request_id: + request_id = str(uuid.uuid4()) + + honeybadger.set_event_context(request_id=request_id) + def _patch_cursor(self): from django.db.backends.utils import CursorWrapper diff --git a/honeybadger/contrib/flask.py b/honeybadger/contrib/flask.py index ddb2866..9dd5c12 100644 --- a/honeybadger/contrib/flask.py +++ b/honeybadger/contrib/flask.py @@ -3,6 +3,7 @@ import logging import time +import uuid from honeybadger import honeybadger from honeybadger.plugins import Plugin, default_plugin_manager @@ -11,6 +12,7 @@ filter_env_vars, get_duration, extract_honeybadger_config, + sanitize_request_id, ) from honeybadger.contrib.db import DBHoneybadger from six import iteritems @@ -201,6 +203,12 @@ def _initialize_honeybadger(self, config): def _handle_request_started(self, sender, *args, **kwargs): from flask import request + request_id = sanitize_request_id(request.headers.get("X-Request-ID")) + if not request_id: + request_id = str(uuid.uuid4()) + + honeybadger.set_event_context(request_id=request_id) + _request_info.set( { "start_time": time.time(), diff --git a/honeybadger/core.py b/honeybadger/core.py index 279b806..85af163 100644 --- a/honeybadger/core.py +++ b/honeybadger/core.py @@ -80,6 +80,8 @@ def notify( merged_ctx = {**base, **(context or {})} merged_tags = list({*tag_ctx, *(tags or [])}) + request_id = self._get_event_context().get("request_id", None) + notice = Notice( exception=exception, error_class=error_class, @@ -88,6 +90,7 @@ def notify( fingerprint=fingerprint, tags=merged_tags, config=self.config, + request_id=request_id, ) return self._send_notice(notice) diff --git a/honeybadger/events_worker.py b/honeybadger/events_worker.py index 9cfadbd..6da348a 100644 --- a/honeybadger/events_worker.py +++ b/honeybadger/events_worker.py @@ -1,3 +1,4 @@ +import os import time import threading import logging @@ -41,12 +42,29 @@ def __init__( self._thread = threading.Thread( target=self._run, - name="honeybadger-events-worker", + name=f"honeybadger-events-worker-{os.getpid()}", daemon=True, ) self._thread.start() self.log.debug("Events worker started") + def restart(self): + """Restart the batch worker thread (useful after process forking)""" + if hasattr(self, "_thread") and self._thread and self._thread.is_alive(): + self.shutdown() + + # Reset state + self._stop = False + + self._thread = threading.Thread( + target=self._run, + name=f"honeybadger-events-worker-{os.getpid()}", + daemon=True, + ) + self._thread.start() + + return self._thread.is_alive() + def push(self, event: Event) -> bool: with self._cond: if self._all_events_queued_len() >= self.config.events_max_queue_size: diff --git a/honeybadger/notice.py b/honeybadger/notice.py index cf6d222..5d8edef 100644 --- a/honeybadger/notice.py +++ b/honeybadger/notice.py @@ -11,6 +11,7 @@ def __init__(self, *args, **kwargs): self.fingerprint = kwargs.get("fingerprint", None) self.config = kwargs.get("config", None) self.context = kwargs.get("context", {}) + self.request_id = kwargs.get("request_id", None) self.tags = self._construct_tags(kwargs.get("tags", [])) self._process_exception() @@ -34,6 +35,7 @@ def payload(self): context=self.context, tags=self.tags, config=self.config, + correlation_context=self._correlation_context(), ) def excluded_exception(self): @@ -49,6 +51,11 @@ def excluded_exception(self): return True return False + def _correlation_context(self): + if self.request_id: + return {"request_id": self.request_id} + return None + def _construct_tags(self, tags): """ Accepts either: diff --git a/honeybadger/payload.py b/honeybadger/payload.py index 4d08b35..6c95601 100644 --- a/honeybadger/payload.py +++ b/honeybadger/payload.py @@ -158,6 +158,7 @@ def create_payload( config=None, context=None, fingerprint=None, + correlation_context=None, tags=None, ): # if using local_variables get them @@ -188,4 +189,7 @@ def create_payload( "request": {"context": context, "local_variables": local_variables}, } + if correlation_context: + payload["correlation_context"] = correlation_context + return default_plugin_manager.generate_payload(payload, config, context) diff --git a/honeybadger/tests/contrib/test_celery.py b/honeybadger/tests/contrib/test_celery.py index 94df887..5fe1efd 100644 --- a/honeybadger/tests/contrib/test_celery.py +++ b/honeybadger/tests/contrib/test_celery.py @@ -89,52 +89,98 @@ def setup_celery_hb(): return app, hb -@patch("celery.events.EventReceiver") @patch("honeybadger.honeybadger.event") -def test_finished_task_event(mock_event, mock_event_receiver): - app, hb = setup_celery_hb() +def test_finished_task_event(mock_event): + _, hb = setup_celery_hb() - assert mock_event_receiver.call_count == 1 - assert ( - mock_event_receiver.call_args[1]["handlers"]["task-finished"] - == hb._on_task_finished - ) + task = MagicMock() + task.name = "test_task" + task.request.retries = 0 + task.request.group = None + task.request.args = [] + task.request.kwargs = {} - hb._on_task_finished({"payload": {"data": "test_task_id"}}) + hb._on_task_prerun("test_task_id", task) + hb._on_task_postrun("test_task_id", task, state="SUCCESS") + # Verify honeybadger.event was called directly assert mock_event.call_count == 1 assert mock_event.call_args[0][0] == "celery.task_finished" - assert mock_event.call_args[0][1] == {"data": "test_task_id"} + + payload = mock_event.call_args[1]["payload"] + assert payload["task_id"] == "test_task_id" + assert payload["task_name"] == "test_task" + assert payload["state"] == "SUCCESS" hb.tearDown() @with_config({"insights_config": {"celery": {"include_args": True}}}) -def test_includes_task_args(): - app, hb = setup_celery_hb() +@patch("honeybadger.honeybadger.event") +def test_includes_task_args(mock_event): + _, hb = setup_celery_hb() + task = MagicMock() task.request.group = None task.name = "test_task" - task.request.name = "test_task" task.request.retries = 1 task.request.args = [1, 2] task.request.kwargs = {"foo": "bar", "password": "secret"} - hb._on_task_prerun("test_task_id") - hb._on_task_postrun("test_task_id", task, None, state="SUCCESS") + hb._on_task_prerun("test_task_id", task) + hb._on_task_postrun("test_task_id", task, state="SUCCESS") + + assert mock_event.call_count == 1 + assert mock_event.call_args[0][0] == "celery.task_finished" + + payload = mock_event.call_args[1]["payload"] + + assert payload["task_id"] == "test_task_id" + assert payload["task_name"] == "test_task" + assert payload["args"] == [1, 2] + assert payload["kwargs"] == {"foo": "bar"} # password should be filtered out + assert payload["retries"] == 1 + assert payload["state"] == "SUCCESS" + assert payload["group"] is None + assert payload["duration"] > 0 + + hb.tearDown() + + +# Test context propagation +@patch("honeybadger.honeybadger.event") +@patch("honeybadger.honeybadger._get_event_context") +@patch("honeybadger.honeybadger.set_event_context") +def test_context_propagation(mock_set_context, mock_get_context, mock_event): + """Test that context is properly propagated from publish to execution""" + _, hb = setup_celery_hb() + + test_context = {"request_id": "test-123", "user_id": "456"} + mock_get_context.return_value = test_context + + headers = {} + hb._on_before_task_publish(headers=headers) + + assert headers["honeybadger_event_context"] == test_context + + task = MagicMock() + task.request.honeybadger_event_context = test_context + + hb._on_task_prerun("test_task_id", task) + + mock_set_context.assert_called_once_with(test_context) + + hb.tearDown() + + +@patch("honeybadger.honeybadger.events_worker") +def test_worker_process_init(mock_events_worker): + """Test that events worker is restarted in new worker process""" + _, hb = setup_celery_hb() + + hb._on_worker_process_init() - task_arg = lambda x: task.send_event.call_args[1]["payload"][x] - - assert task.send_event.call_count == 1 - assert task.send_event.call_args[0][0] == "task-finished" - assert task_arg("task_id") == "test_task_id" - assert task_arg("task_name") == "test_task" - assert task_arg("args") == [1, 2] - assert task_arg("kwargs") == {"foo": "bar"} - assert task_arg("retries") == 1 - assert task_arg("state") == "SUCCESS" - assert task_arg("group") is None - assert task_arg("duration") > 0 + mock_events_worker.restart.assert_called_once() hb.tearDown() diff --git a/honeybadger/tests/contrib/test_django.py b/honeybadger/tests/contrib/test_django.py index f10de56..97f38b1 100644 --- a/honeybadger/tests/contrib/test_django.py +++ b/honeybadger/tests/contrib/test_django.py @@ -1,6 +1,7 @@ import unittest import json import importlib +import uuid from mock import patch from mock import Mock import sys @@ -335,3 +336,32 @@ def test_event_includes_filtered_params(self, mock_event): mock_event.assert_called_once() event_name, data = mock_event.call_args[0] self.assertEqual(data["params"], {"b": "2"}) + + @patch("honeybadger.contrib.django.honeybadger.set_event_context") + def test_existing_request_id_header(self, mock_set_event_context): + req_id = "abc-123" + request = self.rf.get("/foo", headers={"x-request-id": req_id}) + + def get_response(req): + return object() + + mw = DjangoHoneybadgerMiddleware(get_response) + mw(request) + + # Assert request_id propagated + mock_set_event_context.assert_called_once_with(request_id=req_id) + + @patch("honeybadger.contrib.django.honeybadger.set_event_context") + def test_missing_request_id_header(self, mock_set_event_context): + request = self.rf.get("/foo") + + def get_response(req): + return object() + + mw = DjangoHoneybadgerMiddleware(get_response) + mw(request) + + # Should be a UUID + request_id = mock_set_event_context.call_args[1]["request_id"] + uuid_obj = uuid.UUID(request_id) + assert isinstance(uuid_obj, uuid.UUID) diff --git a/honeybadger/tests/contrib/test_flask.py b/honeybadger/tests/contrib/test_flask.py index f2d50a1..d1d9c56 100644 --- a/honeybadger/tests/contrib/test_flask.py +++ b/honeybadger/tests/contrib/test_flask.py @@ -2,6 +2,7 @@ import importlib import sys +import uuid from mock import patch from honeybadger import honeybadger @@ -438,3 +439,19 @@ def test_insights_event_includes_filtered_params(self, mock_event): _, payload = mock_event.call_args[0] self.assertEqual(payload["params"], {"id": "abc123"}) + + @patch("honeybadger.contrib.flask.honeybadger.set_event_context") + def test_request_id_set_from_header(self, mock_set_event_context): + resp = self.client.get("/ping", headers={"X-Request-ID": "test-request-123"}) + self.assertEqual(resp.status_code, 201) + + mock_set_event_context.assert_called_once_with(request_id="test-request-123") + + @patch("honeybadger.contrib.flask.honeybadger.set_event_context") + def test_request_id_generated_when_missing(self, mock_set_event_context): + resp = self.client.get("/ping") + self.assertEqual(resp.status_code, 201) + + request_id = mock_set_event_context.call_args[1]["request_id"] + uuid_obj = uuid.UUID(request_id) + assert isinstance(uuid_obj, uuid.UUID) diff --git a/honeybadger/tests/test_core.py b/honeybadger/tests/test_core.py index a337ab0..6d4224c 100644 --- a/honeybadger/tests/test_core.py +++ b/honeybadger/tests/test_core.py @@ -310,6 +310,19 @@ def test_payload(request): ) +def test_notify_with_request_id(): + def test_payload(request): + payload = json.loads(request.data.decode("utf-8")) + assert payload["correlation_context"]["request_id"] == "12345" + + hb = Honeybadger() + hb.set_event_context(request_id="12345") + + with mock_urlopen(test_payload): + hb.configure(api_key="aaa", force_report_data=True) + hb.notify(error_class="Exception", error_message="Test message.") + + def test_event_with_two_params(): mock_events_worker = MagicMock() diff --git a/honeybadger/tests/test_notice.py b/honeybadger/tests/test_notice.py index 8b09c77..d533e6c 100644 --- a/honeybadger/tests/test_notice.py +++ b/honeybadger/tests/test_notice.py @@ -98,3 +98,16 @@ def test_notice_with_multiple_tags(): assert "tag1" in payload["error"]["tags"] assert "tag2" in payload["error"]["tags"] assert "tag3" in payload["error"]["tags"] + + +def test_notice_with_request_id(): + config = Configuration() + + notice = Notice( + error_class="TestError", + error_message="Test message", + request_id="12345", + config=config, + ) + payload = notice.payload + assert payload["correlation_context"]["request_id"] == "12345" diff --git a/honeybadger/tests/test_payload.py b/honeybadger/tests/test_payload.py index 99d2db3..57b833e 100644 --- a/honeybadger/tests/test_payload.py +++ b/honeybadger/tests/test_payload.py @@ -237,3 +237,13 @@ def test_create_payload_with_local_variables(): exception = Exception("Test") payload = create_payload(exception, config=config) assert payload["request"]["local_variables"] == test_local_variable + + +def test_create_payload_with_correlation_context(): + config = Configuration() + exception = Exception("Test") + request_id = "12345" + payload = create_payload( + exception, config=config, correlation_context={"request_id": request_id} + ) + assert payload["correlation_context"]["request_id"] == request_id diff --git a/honeybadger/tests/test_utils.py b/honeybadger/tests/test_utils.py index 5960c3d..8e67691 100644 --- a/honeybadger/tests/test_utils.py +++ b/honeybadger/tests/test_utils.py @@ -1,4 +1,4 @@ -from honeybadger.utils import filter_dict, filter_env_vars +from honeybadger.utils import filter_dict, filter_env_vars, sanitize_request_id def test_filter_dict(): @@ -69,3 +69,14 @@ def test_filter_env_vars_with_non_dict(): def test_filter_env_vars_empty_dict(): assert filter_env_vars({}) == {} + + +def test_sanitize_request_id(): + assert sanitize_request_id("abc123-def456") == "abc123-def456" + assert sanitize_request_id("abc_123@def#456") == "abc123def456" + assert sanitize_request_id("a" * 300) == "a" * 255 + assert sanitize_request_id(" abc123 ") == "abc123" + assert sanitize_request_id("@#$%^&*()") is None + assert sanitize_request_id(None) is None + assert sanitize_request_id("") is None + assert sanitize_request_id(" ") is None diff --git a/honeybadger/utils.py b/honeybadger/utils.py index 2a823ff..0388577 100644 --- a/honeybadger/utils.py +++ b/honeybadger/utils.py @@ -1,5 +1,6 @@ import json import time +import re class StringReprJSONEncoder(json.JSONEncoder): @@ -87,3 +88,13 @@ def get_duration(start_time): return None return round((time.time() - start_time) * 1000, 4) + + +def sanitize_request_id(request_id): + """Sanitize a Request ID by keeping only alphanumeric characters and hyphens.""" + if not request_id: + return None + + sanitized = re.sub(r"[^a-zA-Z0-9-]", "", request_id.strip())[:255] + + return sanitized or None From af1281378df93cd90d006e24b33aa44108cb9c76 Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Tue, 3 Jun 2025 08:57:01 -0700 Subject: [PATCH 06/17] feat(insights) event sampling (#217) --- honeybadger/config.py | 1 + honeybadger/core.py | 31 ++++++++++++++++++++++++ honeybadger/tests/test_core.py | 44 ++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+) diff --git a/honeybadger/config.py b/honeybadger/config.py index 4009b01..5375a01 100644 --- a/honeybadger/config.py +++ b/honeybadger/config.py @@ -104,6 +104,7 @@ class BaseConfig: before_event: Callable[[Any], Any] = lambda _: None + events_sample_rate: int = 100 events_batch_size: int = 1000 events_max_queue_size: int = 10_000 events_timeout: float = 5.0 diff --git a/honeybadger/core.py b/honeybadger/core.py index 85af163..011c0f2 100644 --- a/honeybadger/core.py +++ b/honeybadger/core.py @@ -4,6 +4,9 @@ import logging import datetime import atexit +import uuid +import hashlib + from typing import Optional, Dict, Any, List from honeybadger.plugins import default_plugin_manager @@ -132,6 +135,13 @@ def event(self, event_type=None, data=None, **kwargs): final_payload = {**self._get_event_context(), **payload} + # Check sampling on the final merged payload + if not self._should_sample_event(final_payload): + return + + # Strip internal _hb metadata before sending + final_payload.pop("_hb", None) + return self.events_worker.push(final_payload) def configure(self, **kwargs): @@ -149,6 +159,27 @@ def auto_discover_plugins(self): if self.config.is_aws_lambda_environment: default_plugin_manager.register(contrib.AWSLambdaPlugin()) + def _should_sample_event(self, payload): + """ + Determine if an event should be sampled based on sample rate and payload metadata. + Returns True if the event should be sent, False if it should be skipped. + """ + # Get sample rate from payload _hb override or global config + hb_metadata = payload.get("_hb", {}) + sample_rate = hb_metadata.get("sample_rate", self.config.events_sample_rate) + + if sample_rate >= 100: + return True + + if sample_rate <= 0: + return False + + sampling_key = payload.get("request_id") + if not sampling_key: + sampling_key = str(uuid.uuid4()) + hash_value = int(hashlib.md5(sampling_key.encode()).hexdigest(), 16) + return (hash_value % 100) < sample_rate + # Error context # def _get_context(self): diff --git a/honeybadger/tests/test_core.py b/honeybadger/tests/test_core.py index 6d4224c..88dac82 100644 --- a/honeybadger/tests/test_core.py +++ b/honeybadger/tests/test_core.py @@ -530,3 +530,47 @@ def test_payload(request): context=dict(bar="foo"), tags="tag1", ) + + +@pytest.mark.parametrize( + "sample_rate,expected_calls", + [ + (0, 0), # Never send + (100, 1), # Always send + (-10, 0), # Negative = never send + ], +) +def test_event_sampling_rates(sample_rate, expected_calls): + """Test various sample rates""" + mock_events_worker = MagicMock() + hb = Honeybadger() + hb.events_worker = mock_events_worker + hb.configure(api_key="aaa", force_report_data=True, events_sample_rate=sample_rate) + + hb.event("test.event", {"data": "test"}) + assert mock_events_worker.push.call_count == expected_calls + + +def test_event_sampling_precedence_and_cleanup(): + """Test that _hb overrides work and get stripped from payload""" + mock_events_worker = MagicMock() + hb = Honeybadger() + hb.events_worker = mock_events_worker + hb.configure( + api_key="aaa", force_report_data=True, events_sample_rate=0 + ) # Global: never send + + # Context override: should send + hb.set_event_context(_hb={"sample_rate": 100}, service="web") + hb.event("test.event1", {"data": "test1"}) + assert mock_events_worker.push.call_count == 1 + + # Event data override: should not send (overrides context) + hb.event("test.event2", {"data": "test2", "_hb": {"sample_rate": 0}}) + assert mock_events_worker.push.call_count == 1 # Still 1, no new call + + # Verify _hb is stripped and other data preserved + payload = mock_events_worker.push.call_args[0][0] + assert "_hb" not in payload + assert payload["service"] == "web" + assert payload["data"] == "test1" From 8f9c795e84d9f8e4aa9a9ab84aeb1fe75114fcce Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Mon, 9 Jun 2025 14:08:21 -0700 Subject: [PATCH 07/17] Formatter update --- honeybadger/tests/test_core.py | 1 + 1 file changed, 1 insertion(+) diff --git a/honeybadger/tests/test_core.py b/honeybadger/tests/test_core.py index 2033876..b5569be 100644 --- a/honeybadger/tests/test_core.py +++ b/honeybadger/tests/test_core.py @@ -575,6 +575,7 @@ def test_event_sampling_precedence_and_cleanup(): assert payload["service"] == "web" assert payload["data"] == "test1" + def test_notify_with_error_message_only(): """Test the case where only an error message is provided""" From e240f37969ea08cb40bacf97610c2b117e3cbd30 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Wed, 10 Sep 2025 08:53:18 +0300 Subject: [PATCH 08/17] chore: typo fix --- honeybadger/contrib/flask.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/honeybadger/contrib/flask.py b/honeybadger/contrib/flask.py index 9dd5c12..21676c2 100644 --- a/honeybadger/contrib/flask.py +++ b/honeybadger/contrib/flask.py @@ -154,7 +154,7 @@ def init_app(self, app, report_exceptions=False, reset_context_after_request=Fal ) self._register_signal_handler( - "insights on request end", + "insights on request start", request_started, self._handle_request_started, ) From 0da666af158c8854a4c9729dfaac9951c649a4ca Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Wed, 10 Sep 2025 21:55:08 +0300 Subject: [PATCH 09/17] fix: send payload for celery events --- honeybadger/contrib/celery.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/honeybadger/contrib/celery.py b/honeybadger/contrib/celery.py index 14fd5c8..efcd880 100644 --- a/honeybadger/contrib/celery.py +++ b/honeybadger/contrib/celery.py @@ -164,7 +164,7 @@ def _on_task_postrun(self, task_id=None, task=None, *args, **kwargs): remove_keys=True, ) - honeybadger.event("celery.task_finished", payload=payload) + honeybadger.event("celery.task_finished", payload) honeybadger.reset_context() From 8eb887815e7ba01697ba5a57a6c2890b6c73eb2f Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Wed, 10 Sep 2025 21:58:53 +0300 Subject: [PATCH 10/17] test: read correct arg --- honeybadger/tests/contrib/test_celery.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/honeybadger/tests/contrib/test_celery.py b/honeybadger/tests/contrib/test_celery.py index 5fe1efd..8cb054f 100644 --- a/honeybadger/tests/contrib/test_celery.py +++ b/honeybadger/tests/contrib/test_celery.py @@ -107,7 +107,7 @@ def test_finished_task_event(mock_event): assert mock_event.call_count == 1 assert mock_event.call_args[0][0] == "celery.task_finished" - payload = mock_event.call_args[1]["payload"] + payload = mock_event.call_args[1]["data"] assert payload["task_id"] == "test_task_id" assert payload["task_name"] == "test_task" assert payload["state"] == "SUCCESS" @@ -133,7 +133,7 @@ def test_includes_task_args(mock_event): assert mock_event.call_count == 1 assert mock_event.call_args[0][0] == "celery.task_finished" - payload = mock_event.call_args[1]["payload"] + payload = mock_event.call_args[1]["data"] assert payload["task_id"] == "test_task_id" assert payload["task_name"] == "test_task" From 094da4279ac249d792c350548ee62bdce3a49c50 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Thu, 11 Sep 2025 08:43:55 +0300 Subject: [PATCH 11/17] test: read correct arg --- honeybadger/tests/contrib/test_celery.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/honeybadger/tests/contrib/test_celery.py b/honeybadger/tests/contrib/test_celery.py index 8cb054f..fcff6e9 100644 --- a/honeybadger/tests/contrib/test_celery.py +++ b/honeybadger/tests/contrib/test_celery.py @@ -107,7 +107,7 @@ def test_finished_task_event(mock_event): assert mock_event.call_count == 1 assert mock_event.call_args[0][0] == "celery.task_finished" - payload = mock_event.call_args[1]["data"] + payload = mock_event.call_args[0][1] assert payload["task_id"] == "test_task_id" assert payload["task_name"] == "test_task" assert payload["state"] == "SUCCESS" @@ -133,7 +133,7 @@ def test_includes_task_args(mock_event): assert mock_event.call_count == 1 assert mock_event.call_args[0][0] == "celery.task_finished" - payload = mock_event.call_args[1]["data"] + payload = mock_event.call_args[0][1] assert payload["task_id"] == "test_task_id" assert payload["task_name"] == "test_task" From b46413affeaf81e23c7247defd13265b613a7867 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Mon, 6 Oct 2025 08:32:24 +0300 Subject: [PATCH 12/17] perf: remove lock (#227) --- honeybadger/events_worker.py | 50 ++++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 22 deletions(-) diff --git a/honeybadger/events_worker.py b/honeybadger/events_worker.py index 6da348a..082da5d 100644 --- a/honeybadger/events_worker.py +++ b/honeybadger/events_worker.py @@ -29,7 +29,7 @@ def __init__( self.log = logger or logging.getLogger(__name__) self._lock = threading.RLock() - self._cond = threading.Condition(self._lock) + self._batch_ready_event = threading.Event() self._queue: Deque[Event] = deque() self._batches: Deque[Tuple[List[Event], int]] = deque() @@ -66,22 +66,24 @@ def restart(self): return self._thread.is_alive() def push(self, event: Event) -> bool: - with self._cond: - if self._all_events_queued_len() >= self.config.events_max_queue_size: - self._drop() - return False + # Small race condition is acceptable - may slightly exceed max size briefly + current_size = self._all_events_queued_len() + if current_size >= self.config.events_max_queue_size: + self._drop() + return False - self._queue.append(event) - if len(self._queue) >= self.config.events_batch_size: - self._cond.notify() + self._queue.append(event) + + # Signal worker thread if batch size reached + if len(self._queue) >= self.config.events_batch_size: + self._batch_ready_event.set() return True def shutdown(self) -> None: self.log.debug("Shutting down events worker") - with self._cond: - self._stop = True - self._cond.notify() + self._stop = True + self._batch_ready_event.set() # Wake up the worker thread if self._thread.is_alive(): timeout = ( @@ -109,14 +111,12 @@ def _run(self) -> None: Main loop: wait until stop or enough events to batch, then flush. """ while True: - with self._cond: - # Wake on stop flag, full batch, or after compute_timeout - self._cond.wait_for( - lambda: self._stop - or len(self._queue) >= self.config.events_batch_size, - timeout=self._compute_timeout(), - ) - # Exit when shutdown requested and all work is done + # Wait for batch ready signal or timeout + self._batch_ready_event.wait(timeout=self._compute_timeout()) + self._batch_ready_event.clear() + + # Check if we should exit (need consistent view of state) + with self._lock: if self._stop and not self._queue and not self._batches: break @@ -130,9 +130,15 @@ def _flush(self) -> None: """ with self._lock: # If there are new queued events, package them as a fresh batch - if self._queue: - batch = list(self._queue) - self._queue.clear() + # Use popleft() which is atomic/thread-safe (unlike list() or clear()) + batch = [] + while True: + try: + batch.append(self._queue.popleft()) + except IndexError: + break + + if batch: self._batches.append((batch, 0)) self._start_time = time.monotonic() From d627123b67e8611e9558f756ec5ce7e167993739 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Mon, 6 Oct 2025 08:35:14 +0300 Subject: [PATCH 13/17] chode: revert temp workflow changes --- .github/workflows/code-quality.yml | 2 +- .github/workflows/python.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/code-quality.yml b/.github/workflows/code-quality.yml index 90528ee..487cee3 100644 --- a/.github/workflows/code-quality.yml +++ b/.github/workflows/code-quality.yml @@ -6,7 +6,7 @@ concurrency: on: pull_request: - branches: [ master, insights-instrumentation ] + branches: [ master ] types: [opened, edited, synchronize, reopened] jobs: diff --git a/.github/workflows/python.yml b/.github/workflows/python.yml index d3b6485..4bf6215 100644 --- a/.github/workflows/python.yml +++ b/.github/workflows/python.yml @@ -8,7 +8,7 @@ on: push: branches: [ master ] pull_request: - branches: [ master, insights-instrumentation ] + branches: [ master ] jobs: build: From c92c6ec494e55cc7c06cc6bd64e87b730a70fd0c Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Mon, 6 Oct 2025 09:05:32 +0300 Subject: [PATCH 14/17] chore: capture logs to debug failing test --- pytest.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/pytest.ini b/pytest.ini index a6cde27..e68b994 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,3 +3,4 @@ testpaths = honeybadger/tests python_files = test_*.py addopts = --mypy asyncio_default_fixture_loop_scope = function +log_cli = True # capture live logs (known as "live logging") From 28d6ad0aff5d7762d29b0b0f3da42a4a4a307e73 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Mon, 6 Oct 2025 09:08:35 +0300 Subject: [PATCH 15/17] chore: capture logs to debug failing test --- pytest.ini | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index e68b994..ea237f2 100644 --- a/pytest.ini +++ b/pytest.ini @@ -3,4 +3,5 @@ testpaths = honeybadger/tests python_files = test_*.py addopts = --mypy asyncio_default_fixture_loop_scope = function -log_cli = True # capture live logs (known as "live logging") +# capture live logs (known as "live logging") +log_cli = True From adf640c9402000baf3ddab687176b707e7387954 Mon Sep 17 00:00:00 2001 From: Pangratios Cosma Date: Mon, 6 Oct 2025 09:18:47 +0300 Subject: [PATCH 16/17] chore: allow more time for the batch to be sent --- honeybadger/tests/test_events_worker.py | 2 +- pytest.ini | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/honeybadger/tests/test_events_worker.py b/honeybadger/tests/test_events_worker.py index e79c93a..45c4652 100644 --- a/honeybadger/tests/test_events_worker.py +++ b/honeybadger/tests/test_events_worker.py @@ -288,7 +288,7 @@ def test_interleave_new_events_during_throttle_backoff(base_config): w.push(e) assert wait_for( - lambda: len(conn.batches) >= 1, timeout=cfg.events_timeout + lambda: len(conn.batches) >= 1, timeout=cfg.events_timeout * 1.1 ), f"Expected first batch within {cfg.events_timeout}s" second = [{"id": 4}, {"id": 5}, {"id": 6}] diff --git a/pytest.ini b/pytest.ini index ea237f2..771d628 100644 --- a/pytest.ini +++ b/pytest.ini @@ -4,4 +4,4 @@ python_files = test_*.py addopts = --mypy asyncio_default_fixture_loop_scope = function # capture live logs (known as "live logging") -log_cli = True +# log_cli = True From b1d31f71841c1a3fb53603cf423a6116851a75ad Mon Sep 17 00:00:00 2001 From: Kevin Webster Date: Tue, 7 Oct 2025 11:15:22 -0700 Subject: [PATCH 17/17] chore: add changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97a15ae..b66f54f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ CHANGELOG](http://keepachangelog.com/) for how to update this file. This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] +- Add Insights automatic instrumentation (#215) ## [1.0.3] - 2025-07-21 - Fix: Register signal handler only in main thread