fix(code_review): Fix and more debugging #105996
Conversation
We're debugging why Seer is not triggering the check runs. Changes included: * More logging * sample_rate of 1 for all metrics * Minor refactor * Test to catch check_run regression
github-actions
bot
added
the
Scope: Backend
armenzg
changed the title
| *, | ||
| github_event: GithubWebhookType, | ||
| event: Mapping[str, Any], | ||
| organization: Organization, |
There was a problem hiding this comment.
We don't really use organization.
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| METRICS_PREFIX = "seer.code_review.webhook" |
There was a problem hiding this comment.
We don't have anymore metrics in this module.
| from .config import get_direct_to_seer_gh_orgs | ||
|
|
||
| gh_orgs_to_only_send_to_seer = get_direct_to_seer_gh_orgs() | ||
| logger.info("gh_orgs_to_only_send_to_seer: %s", gh_orgs_to_only_send_to_seer) |
There was a problem hiding this comment.
Debugging purposes. We will remove it.
| metrics.incr( | ||
| Metrics.ERROR.value, | ||
| tags={"error_status": ErrorStatus.MISSING_INTEGRATION.value}, | ||
| sample_rate=1.0, |
There was a problem hiding this comment.
We want to have certainty of events happening while debugging.
There was a problem hiding this comment.
Nice, I missed this. I'll add it to my other metrics pr
| return | ||
|
|
||
| if pull_request.get("draft") is True: | ||
| _warn_and_increment_metric(ErrorStatus.DRAFT_PR, action=action_value, extra=extra) |
There was a problem hiding this comment.
This unnecessarily shows up in the logs.
| if option_key and options.get(option_key): | ||
| return | ||
| # Skip this check for CHECK_RUN events (always go to Seer) | ||
| if github_event != GithubWebhookType.CHECK_RUN: |
There was a problem hiding this comment.
The bypassing system is only for issue_comment and pull_request. This is a regression.
There was a problem hiding this comment.
Good catch, though I'm confused when the regression occurred because the last spot I updated it here would have retained the logic for everyone except our 2 test orgs if I did that correctly.
I feel like this additional spot checking the feature flag may just be able to remove completely since we're doing the checks upstream already? Anyway we can just leave it in since it will be temporary
| # Check if repo owner is in the whitelist (always send to Seer for these orgs) | ||
| # Otherwise, check option key to see if Overwatch should handle this | ||
| repo_owner = event_payload.get("data", {}).get("repo", {}).get("owner") | ||
| if repo_owner not in get_direct_to_seer_gh_orgs(): |
There was a problem hiding this comment.
Note that once we're migrated we will remove this bypassing system.
| This verifies the bug fix: CHECK_RUN events should bypass the org whitelist | ||
| check because they are user-initiated reruns from GitHub UI. | ||
| """ | ||
| mock_get_orgs.return_value = [] |
There was a problem hiding this comment.
Even if the re-run comes from a non-whitelisted org it will work.
| from ..preflight import CodeReviewPreflightService | ||
|
|
||
| if TYPE_CHECKING: | ||
| from sentry.integrations.github.webhook import WebhookProcessor |
|
|
||
|
|
||
| EVENT_TYPE_TO_HANDLER: dict[GithubWebhookType, WebhookProcessor] = { | ||
| EVENT_TYPE_TO_HANDLER: dict[GithubWebhookType, Callable[..., None]] = { |
There was a problem hiding this comment.
This is a more sensible signature.
The only function that needs to follow WebhookProcessor is handle_webhook_event since it's the one called by the GitHub processors system.
| github_org=github_org, | ||
| repo=repo, | ||
| integration=integration, | ||
| **kwargs, |
We have regressed the re-run case and we're debugging why Seer is not triggering the check runs on issue_comment and pull_request via Seer (new approach).
Changes included: