diff --git a/src/sentry/pr_metrics/webhooks.py b/src/sentry/pr_metrics/webhooks.py index 5862c676060d..5bb9a5b2d9d0 100644 --- a/src/sentry/pr_metrics/webhooks.py +++ b/src/sentry/pr_metrics/webhooks.py @@ -21,7 +21,6 @@ from datetime import datetime, timedelta from typing import Any -import sentry_sdk from django.conf import settings from django.core.cache import cache from django.db import IntegrityError, router, transaction @@ -168,9 +167,7 @@ def handle_attribution( if features.has("organizations:mcp-issue-view-attribution", organization): _write_mcp_attribution(pr) if action == "opened" and pull_request is not None: - provider_hint = _is_delegated_agent_candidate(pull_request) - if provider_hint and org_has_coding_agent_for_provider(organization, provider_hint): - _detect_delegated_agent(pr, pull_request, repo, provider_hint=provider_hint) + _attribute_delegated_agent(pr, pull_request, repo, organization, github_user) def _claim_terminal_event(pr: PullRequest, verdict: PullRequestVerdict) -> bool: @@ -982,6 +979,55 @@ def _write_author_attribution( ) +def _record_delegated_candidate(provider: str, outcome: str) -> None: + """Count where an opened PR lands in the delegated-agent attribution funnel. + + ``provider`` is the provider hint (or ``"unknown"`` when none could be + derived); ``outcome`` is the terminal stage. Every stage that previously + returned silently records here, so drop-offs before the Seer match request + are visible instead of invisible. + """ + metrics.incr( + "pr_metrics.delegated_agent.candidate", + tags={"provider": provider, "outcome": outcome}, + ) + + +def _attribute_delegated_agent( + pr: PullRequest, + webhook_pull_request: Mapping[str, Any], + repository: Repository, + organization: Organization, + github_user: Mapping[str, Any], +) -> None: + """Route an opened PR toward the Seer delegated-agent match, recording where + it drops off. + + The funnel is scoped to PRs plausibly opened by an agent — either a provider + hint (branch prefix / bot login) or authorship by the Sentry/Seer app — so + the counter isn't swamped by ordinary human PRs that legitimately carry no + hint. + """ + provider_hint = _is_delegated_agent_candidate(webhook_pull_request) + + if not provider_hint: + # Claude opens PRs as the Sentry app with no distinct bot login, so the + # ``claude/`` branch prefix is its only signal; a non-``claude/`` branch + # leaves no hint and the PR never reaches the match. Surface that only + # for app-authored PRs — the cohort that should have matched — since + # human PRs with no hint are expected and would dominate the metric. + user_id = github_user.get("id") + if user_id is not None and _detect_app_signal(user_id) is not None: + _record_delegated_candidate("unknown", "no_provider_hint") + return + + if not org_has_coding_agent_for_provider(organization, provider_hint): + _record_delegated_candidate(provider_hint, "no_org_integration") + return + + _detect_delegated_agent(pr, webhook_pull_request, repository, provider_hint=provider_hint) + + def _detect_delegated_agent( pr: PullRequest, webhook_pull_request: Mapping[str, Any], @@ -996,6 +1042,7 @@ def _detect_delegated_agent( """ group_ids = resolved_group_ids(pr) if not group_ids: + _record_delegated_candidate(provider_hint, "no_group_ids") return repo_name_sections = repository.name.split("/") @@ -1004,6 +1051,7 @@ def _detect_delegated_agent( "pr_metrics.delegated_agent.invalid_repo_name", extra={"pull_request_id": pr.id, "repo_name": repository.name}, ) + _record_delegated_candidate(provider_hint, "bad_repo") return if not repository.provider or not repository.external_id: @@ -1015,6 +1063,7 @@ def _detect_delegated_agent( "has_external_id": bool(repository.external_id), }, ) + _record_delegated_candidate(provider_hint, "bad_repo") return pr_url = webhook_pull_request.get("html_url") or "" @@ -1052,11 +1101,7 @@ def _send_seer_delegated_agent_match( response = make_match_coding_agent_pr_request(request_body, timeout=5) except Exception: logger.warning("pr_metrics.delegated_agent.seer_match.error", extra=log_extra) - sentry_sdk.metrics.count( - "pr_metrics.delegated_agent.seer_match.error", - 1, - attributes={"reason": "exception"}, - ) + _record_delegated_candidate(provider_hint, "seer_error_exception") return if response.status >= 400: @@ -1064,18 +1109,10 @@ def _send_seer_delegated_agent_match( "pr_metrics.delegated_agent.seer_match.error", extra={**log_extra, "status_code": response.status}, ) - sentry_sdk.metrics.count( - "pr_metrics.delegated_agent.seer_match.error", - 1, - attributes={"reason": "bad_status"}, - ) + _record_delegated_candidate(provider_hint, "seer_error_bad_status") return - sentry_sdk.metrics.count( - "pr_metrics.delegated_agent.seer_match.sent", - 1, - attributes={"provider": provider_hint}, - ) + _record_delegated_candidate(provider_hint, "sent") def _write_mcp_attribution(pr: PullRequest) -> None: diff --git a/tests/sentry/pr_metrics/test_webhooks.py b/tests/sentry/pr_metrics/test_webhooks.py index c05ea9b6a450..e677e3ebc176 100644 --- a/tests/sentry/pr_metrics/test_webhooks.py +++ b/tests/sentry/pr_metrics/test_webhooks.py @@ -1854,6 +1854,16 @@ def _mock_seer(self, status: int = 202) -> Any: def _mock_org_check(self) -> Any: return patch(f"{MODULE}.org_has_coding_agent_for_provider", return_value=True) + def _candidate_outcome(self, mock_incr: MagicMock) -> dict[str, str] | None: + """The tags of the single ``delegated_agent.candidate`` funnel emission.""" + calls = [ + c + for c in mock_incr.call_args_list + if c.args and c.args[0] == "pr_metrics.delegated_agent.candidate" + ] + assert len(calls) == 1 + return calls[0].kwargs.get("tags") + # --- Candidate detection calls Seer --- def test_claude_branch_prefix_sends_to_seer(self) -> None: @@ -1895,15 +1905,14 @@ def test_sent_metric_incremented_on_success(self) -> None: with ( self._mock_org_check(), self._mock_seer(status=202), - patch(f"{MODULE}.sentry_sdk.metrics.count") as mock_incr, + patch(f"{MODULE}.metrics.incr") as mock_incr, ): self._call(head_ref="claude/fix") - assert any( - call.args[0] == "pr_metrics.delegated_agent.seer_match.sent" - and call.kwargs.get("attributes", {}).get("provider") == "claude_code" - for call in mock_incr.call_args_list - ) + assert self._candidate_outcome(mock_incr) == { + "provider": "claude_code", + "outcome": "sent", + } # --- Error handling --- @@ -1911,27 +1920,27 @@ def test_seer_non_2xx_logs_warning_and_error_metric(self) -> None: with ( self._mock_org_check(), self._mock_seer(status=500), - patch(f"{MODULE}.sentry_sdk.metrics.count") as mock_incr, + patch(f"{MODULE}.metrics.incr") as mock_incr, ): self._call(head_ref="claude/fix") - assert any( - call.kwargs.get("attributes", {}).get("reason") == "bad_status" - for call in mock_incr.call_args_list - ) + assert self._candidate_outcome(mock_incr) == { + "provider": "claude_code", + "outcome": "seer_error_bad_status", + } def test_seer_exception_logs_warning_and_error_metric(self) -> None: with ( self._mock_org_check(), patch(MATCH_RPC, side_effect=Exception("network error")), - patch(f"{MODULE}.sentry_sdk.metrics.count") as mock_incr, + patch(f"{MODULE}.metrics.incr") as mock_incr, ): self._call(head_ref="claude/fix") - assert any( - call.kwargs.get("attributes", {}).get("reason") == "exception" - for call in mock_incr.call_args_list - ) + assert self._candidate_outcome(mock_incr) == { + "provider": "claude_code", + "outcome": "seer_error_exception", + } def test_seer_exception_does_not_propagate(self) -> None: with self._mock_org_check(), patch(MATCH_RPC, side_effect=Exception("network error")): @@ -2010,3 +2019,72 @@ def test_repo_name_without_slash_does_not_call_seer(self) -> None: self._call(head_ref="claude/fix") mock_rpc.assert_not_called() + + # --- Funnel drop-off metrics --- + # + # Each stage that drops a PR before the Seer match used to return silently; + # these lock in the ``delegated_agent.candidate`` outcome it now records. + + def test_app_authored_non_candidate_records_no_provider_hint(self) -> None: + # Claude opens PRs as the Sentry app with no ``claude/`` branch here, so + # there's no hint — the load-bearing blind spot. Only app-authored PRs + # are counted, keyed as provider "unknown". + with patch(f"{MODULE}.metrics.incr") as mock_incr: + self._call( + user_id=settings.SEER_AUTOFIX_GITHUB_APP_USER_ID, + head_ref="feature/x", + ) + + assert self._candidate_outcome(mock_incr) == { + "provider": "unknown", + "outcome": "no_provider_hint", + } + + def test_human_non_candidate_records_nothing(self) -> None: + # An ordinary human PR with no hint must not enter the funnel at all, + # else the metric drowns in non-agent PRs. + with patch(f"{MODULE}.metrics.incr") as mock_incr: + self._call(user_id=999, login="a-human", head_ref="feature/x") + + assert not any( + c.args and c.args[0] == "pr_metrics.delegated_agent.candidate" + for c in mock_incr.call_args_list + ) + + def test_no_org_integration_records_outcome(self) -> None: + with ( + patch(f"{MODULE}.org_has_coding_agent_for_provider", return_value=False), + patch(f"{MODULE}.metrics.incr") as mock_incr, + ): + self._call(head_ref="claude/fix") + + assert self._candidate_outcome(mock_incr) == { + "provider": "claude_code", + "outcome": "no_org_integration", + } + + def test_no_linked_groups_records_outcome(self) -> None: + GroupLink.objects.filter( + linked_type=GroupLink.LinkedType.pull_request, + linked_id=self.pr.id, + ).delete() + + with self._mock_org_check(), patch(f"{MODULE}.metrics.incr") as mock_incr: + self._call(head_ref="claude/fix") + + assert self._candidate_outcome(mock_incr) == { + "provider": "claude_code", + "outcome": "no_group_ids", + } + + def test_bad_repo_name_records_outcome(self) -> None: + self.repo.name = "repowithoutseparator" + self.repo.save() + + with self._mock_org_check(), patch(f"{MODULE}.metrics.incr") as mock_incr: + self._call(head_ref="claude/fix") + + assert self._candidate_outcome(mock_incr) == { + "provider": "claude_code", + "outcome": "bad_repo", + }