Skip to content

feat(insight): add instrumentation config #210

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 28, 2025

Conversation

rabidpraxis
Copy link
Contributor

@rabidpraxis rabidpraxis commented May 21, 2025

This PR adds custom config per integration / component. Along with this I reworked honeybadger.config to lean on dataclasses. A few notes:

  • I used nested dataclasses for each kind of config, which allows for simple loading of defaults and key checking
  • Missing keys from explicit configs (not env vars) will log a warning if the key does not exist.
    • Unknown Configuration option: blah for the root
    • Unknown DBConfig option: include_stuff for each invalid dataclass
  • I included basic config options per integration, I am open to suggestions for anything that is missing
  • I decided to break out the database stuff into its own db contrib module, which means it has it's own config as well. That felt better than including db config in both django and flask config namespaces.

This also reworks our config class to use a root dataclass with nested
dataclasses for each `insights_config` option set.
@rabidpraxis rabidpraxis requested review from subzero10, a team and Copilot May 21, 2025 21:28
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 per-integration instrumentation configuration to Honeybadger while reworking the core configuration to use nested dataclasses. Key changes include changes to config management across integrations (Django, Flask, ASGI, Celery, DB), updates to integration tests to verify new behavior, and revised documentation outlining the new configuration options.

Reviewed Changes

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

Show a summary per file
File Description
honeybadger/utils.py Updated filter_dict logic to support optional removal of filter keys
honeybadger/tests/* Added tests to cover new configuration options and instrumentation scenarios
honeybadger/contrib/* Updated instrumentation for Flask, Django, ASGI, Celery, and DB to respect new config flags
honeybadger/config.py and config_types.py Reworked the configuration using dataclasses and improved validation of unknown keys
README.md Updated documentation with configuration examples for each integration
dev-requirements.txt Added dependency on sqlalchemy

Copy link
Member

@stympy stympy left a comment

Choose a reason for hiding this comment

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

Looking good!

@stympy
Copy link
Member

stympy commented May 21, 2025

Speaking of key checking, there is one breaking change: If the config key is not within any dataclasses, the app will error at startup, which it silently ignores currently.

Could this error be logged so the user (if they are paying attention) can see that an invalid configuration option was provided?

@rabidpraxis
Copy link
Contributor Author

Could this error be logged so the user (if they are paying attention) can see that an invalid configuration option was provided?

We could do that.

Also a nice little refactor on the Configuration internals.
@rabidpraxis rabidpraxis merged commit 8fe36a7 into insights-instrumentation May 28, 2025
34 checks passed
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.

3 participants