Skip to content

ref(pr-metrics): Add webhook name to webhook-triggered PR logs #118597

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.

Sign up for GitHub

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

Open
vaind wants to merge 1 commit into
base: master
Choose a base branch
from
+59 −10
Open

ref(pr-metrics): Add webhook name to webhook-triggered PR logs #118597

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 53 additions & 10 deletions src/sentry/pr_metrics/webhooks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,13 @@ def handle_attribution(
if not features.has("organizations:pr-metrics-attribution", organization):
return

pr = _get_pull_request(organization, repo, pull_request, kwargs.get("github_delivery_id"))
pr = _get_pull_request(
organization,
repo,
pull_request,
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return

Expand Down Expand Up @@ -320,7 +326,11 @@ def handle_emission(
return

pr = _get_pull_request(
organization, repo, event.get("pull_request"), kwargs.get("github_delivery_id")
organization,
repo,
event.get("pull_request"),
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return
Expand Down Expand Up @@ -370,7 +380,13 @@ def handle_metrics(
if not features.has("organizations:pr-metrics-emit", organization):
return

pr = _get_pull_request(organization, repo, pull_request, kwargs.get("github_delivery_id"))
pr = _get_pull_request(
organization,
repo,
pull_request,
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return

Expand All @@ -395,7 +411,13 @@ def handle_activity(
if not action or action not in _ACTIVITY_ACTIONS:
return

pr = _get_pull_request(organization, repo, pull_request_data, kwargs.get("github_delivery_id"))
pr = _get_pull_request(
organization,
repo,
pull_request_data,
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return

Expand Down Expand Up @@ -440,6 +462,7 @@ def handle_comment(
opened_at=parse_datetime(issue_created_at) if issue_created_at else None,
title=issue.get("title"),
github_delivery_id=webhook_id,
github_event=github_event,
)
if pr is None:
return
Expand Down Expand Up @@ -491,7 +514,11 @@ def handle_review(
return

pr = _get_pull_request(
organization, repo, event.get("pull_request"), kwargs.get("github_delivery_id")
organization,
repo,
event.get("pull_request"),
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return
Expand Down Expand Up @@ -543,7 +570,11 @@ def handle_review_comment(
return

pr = _get_pull_request(
organization, repo, event.get("pull_request"), kwargs.get("github_delivery_id")
organization,
repo,
event.get("pull_request"),
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return
Expand Down Expand Up @@ -594,7 +625,11 @@ def handle_review_thread(
return

pr = _get_pull_request(
organization, repo, event.get("pull_request"), kwargs.get("github_delivery_id")
organization,
repo,
event.get("pull_request"),
kwargs.get("github_delivery_id"),
github_event=github_event,
)
if pr is None:
return
Expand Down Expand Up @@ -663,7 +698,7 @@ def handle_check_suite(
)
)

for pr in _prs_from_check_payload(organization, repo, check_suite, webhook_id):
for pr in _prs_from_check_payload(organization, repo, check_suite, webhook_id, github_event):
_write_activity_row(pr, webhook_id, PullRequestActivityType.CHECK_SUITE_COMPLETED, payload)


Expand Down Expand Up @@ -706,7 +741,7 @@ def handle_check_run(
)
)

for pr in _prs_from_check_payload(organization, repo, check_run, webhook_id):
for pr in _prs_from_check_payload(organization, repo, check_run, webhook_id, github_event):
_write_activity_row(pr, webhook_id, PullRequestActivityType.CHECK_RUN_COMPLETED, payload)


Expand All @@ -715,6 +750,7 @@ def _prs_from_check_payload(
repo: Repository,
container: Mapping[str, Any],
webhook_id: str,
github_event: GithubWebhookType,
) -> list[PullRequest]:
"""Resolve the tracked PRs a check_suite/check_run payload references.

Expand Down Expand Up @@ -746,7 +782,9 @@ def _prs_from_check_payload(
metrics.incr("pr_metrics.check.foreign_pull_request")
continue
seen.add(str(number))
pr = _get_pull_request(organization, repo, {"number": number}, webhook_id)
pr = _get_pull_request(
organization, repo, {"number": number}, webhook_id, github_event=github_event
)
if pr is not None:
prs.append(pr)
return prs
Expand All @@ -770,6 +808,7 @@ def _resolve_or_stub_pull_request(
opened_at: datetime | None,
title: str | None,
github_delivery_id: str | None,
github_event: GithubWebhookType,
) -> PullRequest | None:
"""Return the PullRequest row, creating a minimal stub for a recent miss.

Expand All @@ -791,6 +830,7 @@ def _resolve_or_stub_pull_request(
pass

log_extra = {
"github_event": github_event,
"organization_id": organization.id,
"repository_id": repo.id,
"repo_name": repo.name,
Expand Down Expand Up @@ -822,6 +862,8 @@ def _get_pull_request(
repo: Repository,
pull_request: dict[str, Any] | None,
github_delivery_id: str | None = None,
*,
github_event: GithubWebhookType,
) -> PullRequest | None:
"""Resolve the PullRequest row for a ``pull_request``-shaped payload.

Expand All @@ -840,6 +882,7 @@ def _get_pull_request(
opened_at=parse_datetime(created_at) if created_at else None,
title=pull_request.get("title"),
github_delivery_id=github_delivery_id,
github_event=github_event,
)


Expand Down
6 changes: 6 additions & 0 deletions tests/sentry/pr_metrics/test_webhooks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ def test_missing_pr_logs_unresolved_and_does_not_raise(self) -> None:
mock_logger.info.assert_called_once_with(
"pr_metrics.pull_request.unresolved",
extra={
"github_event": GithubWebhookType.PULL_REQUEST,
"organization_id": self.organization.id,
"repository_id": self.repo.id,
"repo_name": self.repo.name,
Expand Down Expand Up @@ -453,6 +454,7 @@ def test_missing_pr_logs_unresolved_and_does_not_emit(
mock_logger.info.assert_called_once_with(
"pr_metrics.pull_request.unresolved",
extra={
"github_event": GithubWebhookType.PULL_REQUEST,
"organization_id": self.organization.id,
"repository_id": self.repo.id,
"repo_name": self.repo.name,
Expand Down Expand Up @@ -1042,6 +1044,7 @@ def test_unknown_pr_number_logs_unresolved_and_does_not_raise(self) -> None:
mock_logger.info.assert_called_once_with(
"pr_metrics.pull_request.unresolved",
extra={
"github_event": GithubWebhookType.ISSUE_COMMENT,
"organization_id": self.organization.id,
"repository_id": self.repo.id,
"repo_name": self.repo.name,
Expand Down Expand Up @@ -1226,6 +1229,7 @@ def test_unknown_pr_number_logs_unresolved_and_does_not_raise(self) -> None:
mock_logger.info.assert_called_once_with(
"pr_metrics.pull_request.unresolved",
extra={
"github_event": GithubWebhookType.PULL_REQUEST_REVIEW,
"organization_id": self.organization.id,
"repository_id": self.repo.id,
"repo_name": self.repo.name,
Expand Down Expand Up @@ -1331,6 +1335,7 @@ def test_unknown_pr_number_logs_unresolved_and_does_not_raise(self) -> None:
mock_logger.info.assert_called_once_with(
"pr_metrics.pull_request.unresolved",
extra={
"github_event": GithubWebhookType.PULL_REQUEST_REVIEW_COMMENT,
"organization_id": self.organization.id,
"repository_id": self.repo.id,
"repo_name": self.repo.name,
Expand Down Expand Up @@ -1429,6 +1434,7 @@ def test_unknown_pr_number_logs_unresolved_and_does_not_raise(self) -> None:
mock_logger.info.assert_called_once_with(
"pr_metrics.pull_request.unresolved",
extra={
"github_event": GithubWebhookType.PULL_REQUEST_REVIEW_THREAD,
"organization_id": self.organization.id,
"repository_id": self.repo.id,
"repo_name": self.repo.name,
Expand Down
Loading