Skip to content

feat(insights): basic framework event hooks #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 19, 2025

Conversation

rabidpraxis
Copy link
Contributor

This includes the basic framework hooks & tests for Django, Flask, ASGI & Celery.

A few notes:

  • I'm still not sure why, but the way I changed the Celery app made the previous testing strategy non-functional. Because of this I reworked the tests to use pytest and a bit more mocking. If anyone knows how to write a unit test that actually runs a Celery task with events (without loading docker containers), please let me know :)
  • I patched each database implementation individually for Django and Flask within the middleware. I created a generic DB shim, but I could not get it to load in the right order, so I did not include it with this PR. If I have time after all the other tasks I might take another stab at getting it to work for apps that are not Django and Flask.
  • I had included template events, but they generated a ton of noise, even for my small test apps, so I decided to not include them at this time. It might be worth doing something like storing all the time for template generation and adding that to the request events for Django and Flask (similar to how Rails does it).

@rabidpraxis rabidpraxis requested review from roelbondoc, subzero10, a team and Copilot May 14, 2025 15:19
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a basic framework for event hooks—referred to as Honeybadger Insights—for Django, Flask, ASGI, and Celery, along with updated tests and documentation.

  • Introduces helper functions like get_duration and extract_honeybadger_config
  • Implements insights event registrations in middleware and extensions
  • Updates tests across supported frameworks and enhances README documentation

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
honeybadger/utils.py Adds helper functions for configuration extraction and timing
honeybadger/tests/contrib/test_flask.py Adds new tests for Flask insights events
honeybadger/tests/contrib/test_django.py Adds new tests for Django insights events
honeybadger/tests/contrib/test_celery.py Updates Celery tests to work with new task event hooks
honeybadger/tests/contrib/test_asgi.py Adds async tests for ASGI insights events
honeybadger/contrib/flask.py Incorporates insights instrumentation and SQLAlchemy event patching
honeybadger/contrib/django.py Adds insights instrumentation through middleware enhancements
honeybadger/contrib/celery.py Introduces task event listener and improves event payload generation
honeybadger/contrib/asgi.py Refactors the ASGI app runner to include insights event reporting
honeybadger/config.py Adds a new insights_enabled configuration option
README.md Updates documentation to include insights automatic instrumentation
Comments suppressed due to low confidence (1)

honeybadger/tests/contrib/test_asgi.py:70

  • The exception SomeError is used without a clear import or definition in this test context. Ensure that SomeError is properly imported or defined to prevent a NameError during test execution.
with self.assertRaises(SomeError):

}


def get_duration(start_time):
Copy link
Preview

Copilot AI May 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring states that the duration is returned in milliseconds, but the return type is a float. Consider clarifying the unit (ms) and data type (float) in the docstring for consistency.

Copilot uses AI. Check for mistakes.

I followed the naming from our notice data. The more common name
for this field is "app", so renaming to that.
Until we have filtering and before_event functionality.
@rabidpraxis rabidpraxis merged commit d776ed0 into insights-instrumentation May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants