From f093fa20ab1360660a0c1ab07f5219dd9c6daa1e Mon Sep 17 00:00:00 2001 From: Giovanni M Guidini Date: Fri, 3 Jul 2026 20:13:20 +0200 Subject: [PATCH] feat(pr-metrics): Instrument the delegated-agent attribution funnel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The webhook path that matches a freshly-opened PR to a Seer-delegated coding-agent launch dropped PRs at several silent early returns: no provider hint (the load-bearing case for Claude, which opens PRs as the Sentry app so the ``claude/`` branch prefix is its only signal), no org integration, no resolved groups, and bad repo metadata. None of these emitted anything, so the biggest failure modes were invisible. Record every terminal stage under a single ``pr_metrics.delegated_agent .candidate`` counter tagged with provider + outcome, scoped to PRs plausibly opened by an agent (a provider hint or app authorship) so ordinary human PRs don't swamp it. This makes the drop-off funnel queryable end to end, from candidate detection through the Seer match request. Also migrate the existing seer_match metrics off the deprecated ``sentry_sdk.metrics.count`` onto ``sentry.utils.metrics.incr`` — the convention used everywhere else in pr_metrics — folding success and error into the same funnel so the whole path reports through one metric. Co-Authored-By: Claude Opus 4.8 --- src/sentry/pr_metrics/webhooks.py | 75 ++++++++++++---- tests/sentry/pr_metrics/test_webhooks.py | 110 +++++++++++++++++++---- 2 files changed, 150 insertions(+), 35 deletions(-) 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", + }