feat: event sampling #217
feat: event sampling #217
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces event sampling and extends Honeybadger’s Insights instrumentation across multiple integrations. Key changes include new tests for Flask, Django, ASGI, Celery, and DB, the implementation of deterministic event sampling in core, and updates to configuration handling using dataclasses.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| honeybadger/protocols.py | Updated the send_notice method signature (payload → notice) |
| honeybadger/notice.py | Removed processing of thread_local context and tags |
| honeybadger/core.py | Added event sampling logic and refactored context management |
| honeybadger/contrib/* (Flask, Django, etc.) | Added Insights instrumentation for various integrations |
| honeybadger/config.py | Migrated configuration to dataclasses with recursive merging |
| Tests and Documentation | Added tests and README updates for insights instrumentation |
| GitHub workflows | Updated branch filters to include insights-instrumentation branch |
There was a problem hiding this comment.
Pull Request Overview
This PR adds event sampling to the Honeybadger library, allowing events to be conditionally sent based on a sample rate.
- Added tests to verify event sampling behavior and context overrides
- Introduced _should_sample_event logic in core.py to determine event delivery
- Updated configuration with a new events_sample_rate parameter
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| honeybadger/tests/test_core.py | Added tests for various sampling rates and precedence |
| honeybadger/core.py | Integrated new event sampling logic into the event method |
| honeybadger/config.py | Introduced events_sample_rate configuration parameter |
| if sample_rate <= 0: | ||
| return False | ||
|
|
||
| sampling_key = payload.get("request_id") |
There was a problem hiding this comment.
[nitpick] Consider adding a comment clarifying that if 'request_id' is missing, a new UUID is generated, which results in non-deterministic sampling until the 'request_id' PR is merged.
Copilot uses AI. Check for mistakes.
rabidpraxis
changed the title
This adds event sampling with a similar strategy as our other libraries.
Once the request_id PR is incorporated, this will do deterministic sampling.