feat(code_review): control: include github_event & action info#109995
feat(code_review): control: include github_event & action info#109995
Conversation
We want to slice the data with more granularity.
github-actions
bot
added
the
Scope: Backend
|
|
||
| def _get_github_delivery_time_tags(payload: WebhookPayload) -> dict[str, str]: | ||
| """Extract GitHub event type and action from payload for delivery_time_ms metric tags.""" | ||
| if payload.provider != "github": |
There was a problem hiding this comment.
At the moment we only care about GitHub for code reviews.
| def _record_delivery_time_metrics(payload: WebhookPayload) -> None: | ||
| """Record delivery time metrics for a successfully delivered webhook payload.""" | ||
| exclude_mailboxes = set( | ||
| options.get("hybridcloud.deliver_webhooks.delivery_time_exclude_mailboxes") or () |
There was a problem hiding this comment.
Orthogonal: We don't need this flag anymore.
| flags=FLAG_AUTOMATOR_MODIFIABLE, | ||
| ) | ||
| register( | ||
| "hybridcloud.deliver_webhooks.delivery_time_exclude_mailboxes", |
| mailbox_name="github:123", | ||
| mailbox_name="stripe:123", | ||
| region_name="us", | ||
| provider="stripe", |
There was a problem hiding this comment.
Since the provider is stripe we will not have the extra two tags.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Test doesn't enable feature flag, never exercises provider filtering
- Added missing @override_options decorator to enable the feature flag so the test actually exercises the provider filtering logic.
Or push these changes by commenting:
@cursor push 3e8628efd8
Preview (3e8628efd8)
diff --git a/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py b/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py
--- a/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py
+++ b/tests/sentry/hybridcloud/tasks/test_deliver_webhooks.py
@@ -1106,6 +1106,7 @@
@responses.activate
@override_regions(region_config)
+ @override_options({"hybridcloud.deliver_webhooks.delivery_time_include_github_tags": True})
@patch("sentry.hybridcloud.tasks.deliver_webhooks.metrics")
def test_delivery_time_metrics_non_github_no_github_tags(self, mock_metrics: MagicMock) -> None:
responses.add(| tags = delivery_time_ms_calls[0][1].get("tags", {}) | ||
| assert tags.get("region_sent_to") == "us" | ||
| assert "github_event_type" not in tags | ||
| assert "github_action" not in tags |
There was a problem hiding this comment.
Test doesn't enable feature flag, never exercises provider filtering
Low Severity
test_delivery_time_metrics_non_github_no_github_tags is missing the @override_options({"hybridcloud.deliver_webhooks.delivery_time_include_github_tags": True}) decorator. Since the option defaults to False, _get_github_delivery_time_tags is never called, and the assertions that github_event_type and github_action are absent pass trivially. The test doesn't actually validate that non-GitHub providers are filtered out by the provider check — a bug in that check would go undetected.
We want to slice the data with more granularity.
3 participants
We want to slice the data with more granularity.