ref(api): Report client_kind attributes from every endpoint

src/sentry/api/base.py

@@ -29,10 +29,12 @@
 audit_logger = logging.getLogger("sentry.audit.api")
 api_access_logger = logging.getLogger("sentry.access.api")
 
-from sentry import analytics, tsdb
+from sentry import analytics, features, tsdb
 from sentry.analytics.events.release_set_commits import ReleaseSetCommitsLocalEvent
 from sentry.api.api_owners import ApiOwner
 from sentry.api.api_publish_status import ApiPublishStatus
+from sentry.api.client_kind import FEATURE_FLAG as CLIENT_KIND_FEATURE_FLAG
+from sentry.api.client_kind import set_client_kind_attributes
 from sentry.api.exceptions import (
     INSUFFICIENT_SCOPE_ATTR,
     InsufficientScope,
@@ -45,7 +47,9 @@
 from sentry.auth.staff import has_staff_option
 from sentry.hybridcloud.apigateway.cell_request_resolvers import CellRequestResolver
 from sentry.middleware import is_frontend_request
+from sentry.models.organization import Organization
 from sentry.organizations.absolute_url import generate_organization_url
+from sentry.organizations.services.organization import RpcOrganization
 from sentry.ratelimits.config import DEFAULT_RATE_LIMIT_CONFIG, RateLimitConfig
 from sentry.seer import agent_token
 from sentry.silo.base import SiloLimit, SiloMode
@@ -493,6 +497,17 @@ def dispatch(self, request: Request, *args, **kwargs) -> Response:
                     (args, kwargs) = self.convert_args(request, *args, **kwargs)
                     self.args = args
                     self.kwargs = kwargs
+
+                    # Resolved solely to check the opt-in; everything else is
+                    # derived from the request. Both sources are conventions rather
+                    # than contracts, so the result is type-checked before use.
+                    organization = kwargs.get("organization") or getattr(
+                        request, "organization", None
+                    )
+                    if isinstance(organization, (Organization, RpcOrganization)) and features.has(
+                        CLIENT_KIND_FEATURE_FLAG, organization, actor=request.user
+                    ):
+                        set_client_kind_attributes(request)
                 else:
                     handler = self.http_method_not_allowed
 

src/sentry/api/bases/organization_events.py

@@ -21,7 +21,6 @@
 from sentry.api.base import CURSOR_LINK_HEADER
 from sentry.api.bases import NoProjects
 from sentry.api.bases.organization import FilterParamsDateNotNull, OrganizationEndpoint
-from sentry.api.client_kind import set_client_kind_attributes
 from sentry.api.helpers.error_upsampling import (
     are_any_projects_error_upsampled,
     convert_fields_for_upsampling,
@@ -113,19 +112,6 @@ def resolve_axis_column(
 class OrganizationEventsEndpointBase(OrganizationEndpoint):
     owner = ApiOwner.DATA_BROWSING
 
-    def convert_args(
-        self,
-        request: Request,
-        *args: Any,
-        **kwargs: Any,
-    ) -> tuple[tuple[Any, ...], dict[str, Any]]:
-        (args, kwargs) = super().convert_args(request, *args, **kwargs)
-        # Runs after authentication, so the credential-based checks in
-        # `get_client_kind` see the resolved auth. Done here rather than in each
-        # handler so every events endpoint reports the caller the same way.
-        set_client_kind_attributes(request, kwargs["organization"])
-        return (args, kwargs)
-
     def has_feature(self, organization: Organization, request: Request) -> bool:
         return (
             features.has("organizations:discover-basic", organization, actor=request.user)

src/sentry/api/client_kind.py

@@ -20,11 +20,9 @@
 from rest_framework.request import Request
 from sentry_conventions.attributes import ATTRIBUTE_NAMES
 
-from sentry import features
 from sentry.auth.services.auth import AuthenticatedToken
 from sentry.auth.system import is_system_auth
 from sentry.middleware import is_frontend_request
-from sentry.models.organization import Organization
 from sentry.seer.agent_token import is_agent_auth
 from sentry.utils.http import SEER_REFERRER_HEADER, get_mcp_client_family, is_mcp_request
 from sentry.utils.sdk import get_transaction_name_from_request
@@ -101,16 +99,16 @@ def client_kind_scope(kind: ClientKind) -> Generator[None]:
 )
 
 
-def get_client_kind(request: Request, organization: Organization) -> ClientKind | None:
+def get_client_kind(request: Request) -> ClientKind:
     """Classify the caller of an API request.
 
-    Returns ``None`` when the org has not opted in, so that a disabled org is
-    distinguishable from one whose traffic genuinely classifies as ``UNKNOWN``.
-    Otherwise never raises; unrecognized callers fall back to ``UNKNOWN``.
-    """
-    if not features.has(FEATURE_FLAG, organization, actor=request.user):
-        return None
+    Never raises; unrecognized callers fall back to ``UNKNOWN``.
 
+    Says nothing about whether the caller's organization opted in -- ``FEATURE_FLAG``
+    is checked by the caller, which is what holds the organization. Callers must
+    check it before reaching here, or a ``client_kind_scope`` declaration becomes a
+    way around the opt-in.
+    """
     declared = _client_kind_override.get()
     if declared is not None:
         return declared
@@ -184,16 +182,14 @@ def get_client_kind(request: Request, organization: Organization) -> ClientKind
     return ClientKind.UNKNOWN
 
 
-def set_client_kind_attributes(request: Request, organization: Organization) -> None:
+def set_client_kind_attributes(request: Request) -> None:
     """Record who called the endpoint, on a span and on the enclosing transaction.
 
-    A no-op when the org has not opted into ``client_kind``. Wired into
-    ``OrganizationEventsEndpointBase.convert_args`` so every events endpoint
-    reports the same set of attributes without hand-wiring them per handler.
+    Called once from ``Endpoint.dispatch``, behind the opt-in check it makes for
+    whichever organization ``Endpoint.client_kind_organization`` resolves, so every
+    endpoint reports the same set of attributes without hand-wiring them per handler.
     """
-    client_kind = get_client_kind(request, organization)
-    if client_kind is None:
-        return
+    client_kind = get_client_kind(request)
 
     client_host = get_client_host(request)
     user_agent = get_user_agent(request)

tests/sentry/api/test_client_kind.py

@@ -23,7 +23,7 @@
 from sentry.auth.system import SystemToken
 from sentry.seer.agent_token import AGENT_TOKEN_KIND
 from sentry.seer.endpoints.seer_rpc import SeerRpcSignatureAuthentication
-from sentry.testutils.cases import TestCase
+from sentry.testutils.cases import APITestCase, TestCase
 from sentry.utils.sdk import get_transaction_name_from_request
 
 EVENTS_PATH = "/api/0/organizations/my-org/events/"
@@ -80,14 +80,8 @@ def api_token(*, application_id: int | None = None) -> AuthenticatedToken:
 
 
 class GetClientKindTest(TestCase):
-    def classify(self, request: Request) -> ClientKind | None:
-        with self.feature(FEATURE_FLAG):
-            return get_client_kind(request, self.organization)
-
-    def test_returns_none_when_feature_is_disabled(self) -> None:
-        # A disabled org has to stay distinguishable from one that classifies as UNKNOWN.
-        with self.feature({FEATURE_FLAG: False}):
-            assert get_client_kind(make_request(), self.organization) is None
+    def classify(self, request: Request) -> ClientKind:
+        return get_client_kind(request)
 
     def test_session_auth_is_frontend(self) -> None:
         assert self.classify(make_request(user=session_user())) == ClientKind.FRONTEND
@@ -259,25 +253,12 @@ def test_absent_header_is_none(self) -> None:
 
 
 class SetClientKindAttributesTest(TestCase):
-    def test_noop_when_feature_is_disabled(self) -> None:
-        request = make_request(auth=api_token(), user_agent="curl/8.7.1")
-        with (
-            self.feature({FEATURE_FLAG: False}),
-            mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
-            mock.patch("sentry.api.client_kind.start_span") as start_span,
-        ):
-            set_client_kind_attributes(request, self.organization)
-        sdk.set_tag.assert_not_called()
-        sdk.set_attribute.assert_not_called()
-        start_span.assert_not_called()
-
     def test_records_kind_and_user_agent(self) -> None:
         request = make_request(auth=api_token(), user_agent="curl/8.7.1")
         with (
-            self.feature(FEATURE_FLAG),
             mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
         ):
-            set_client_kind_attributes(request, self.organization)
+            set_client_kind_attributes(request)
         assert sdk.set_tag.call_args_list == [mock.call("client_kind_test", "script")]
         assert sdk.set_attribute.call_args_list == [
             mock.call("client_kind_test", "script"),
@@ -294,20 +275,18 @@ def test_records_client_host_for_mcp(self) -> None:
             },
         )
         with (
-            self.feature(FEATURE_FLAG),
             mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
         ):
-            set_client_kind_attributes(request, self.organization)
+            set_client_kind_attributes(request)
         assert mock.call("client_host_test", "claude-code") in sdk.set_tag.call_args_list
         assert mock.call("client_host_test", "claude-code") in sdk.set_attribute.call_args_list
 
     def test_omits_user_agent_when_absent(self) -> None:
         request = make_request(auth=api_token())
         with (
-            self.feature(FEATURE_FLAG),
             mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
         ):
-            set_client_kind_attributes(request, self.organization)
+            set_client_kind_attributes(request)
         for call in sdk.set_attribute.call_args_list:
             assert call.args[0] != ATTRIBUTE_NAMES.USER_AGENT_ORIGINAL
 
@@ -318,22 +297,20 @@ def test_records_for_the_internal_api_client_too(self) -> None:
         request = make_request(auth=api_token(), user_agent="curl/8.7.1")
         mark_from_api_client(request)
         with (
-            self.feature(FEATURE_FLAG),
             mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
             mock.patch("sentry.api.client_kind.start_span"),
         ):
-            set_client_kind_attributes(request, self.organization)
+            set_client_kind_attributes(request)
         assert sdk.set_tag.call_args_list == [mock.call("client_kind_test", "script")]
 
 
 class AttributionSpanTest(TestCase):
     def record(self, request: Request) -> tuple[Any, list[tuple[str, Any]]]:
         with (
-            self.feature(FEATURE_FLAG),
             mock.patch("sentry.api.client_kind.start_span") as start_span,
             mock.patch("sentry.api.client_kind.set_span_data") as set_span_data,
         ):
-            set_client_kind_attributes(request, self.organization)
+            set_client_kind_attributes(request)
         span = start_span.return_value.__enter__.return_value
         return start_span, [
             call.args[1:] for call in set_span_data.call_args_list if call.args[0] is span
@@ -396,9 +373,8 @@ def test_an_unmatched_path_collapses_onto_a_catch_all(self) -> None:
 
 
 class ClientKindScopeTest(TestCase):
-    def classify(self, request: Request) -> ClientKind | None:
-        with self.feature(FEATURE_FLAG):
-            return get_client_kind(request, self.organization)
+    def classify(self, request: Request) -> ClientKind:
+        return get_client_kind(request)
 
     def test_declared_kind_wins_over_a_signal_less_request(self) -> None:
         request = make_request(cookies=False)
@@ -432,18 +408,68 @@ def test_nested_scopes_restore_the_outer_kind(self) -> None:
                 assert self.classify(request) == ClientKind.MCP
             assert self.classify(request) == ClientKind.SEER
 
-    def test_org_opt_in_still_governs(self) -> None:
-        with client_kind_scope(ClientKind.SEER):
-            with self.feature({FEATURE_FLAG: False}):
-                assert get_client_kind(make_request(), self.organization) is None
-
     def test_declared_kind_is_recorded(self) -> None:
         request = make_request(cookies=False)
         with (
-            self.feature(FEATURE_FLAG),
             client_kind_scope(ClientKind.SEER),
             mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
         ):
-            set_client_kind_attributes(request, self.organization)
+            set_client_kind_attributes(request)
         assert sdk.set_tag.call_args_list == [mock.call("client_kind_test", "seer")]
         assert mock.call("client_kind_test", "seer") in sdk.set_attribute.call_args_list
+
+
+class DispatchWiringTest(APITestCase):
+    """Coverage reaches endpoints beyond the events base this started on.
+
+    Driven through real requests rather than a resolver seam: `Endpoint.dispatch`
+    reads the organization straight off `kwargs`/`request`, so the only honest way
+    to pin which endpoint families are covered is to call them.
+    """
+
+    def setUp(self) -> None:
+        super().setUp()
+        self.login_as(self.user)
+
+    def tags_for(self, url: str, *, enabled: bool = True) -> list[Any]:
+        with (
+            self.feature(FEATURE_FLAG if enabled else {FEATURE_FLAG: False}),
+            mock.patch("sentry.api.client_kind.sentry_sdk") as sdk,
+        ):
+            assert self.client.get(url).status_code == 200
+        return sdk.set_tag.call_args_list
+
+    def test_an_organization_endpoint_records_the_caller(self) -> None:
+        url = f"/api/0/organizations/{self.organization.slug}/"
+        assert mock.call("client_kind_test", "frontend") in self.tags_for(url)
+
+    def test_a_project_endpoint_records_the_caller(self) -> None:
+        url = f"/api/0/projects/{self.organization.slug}/{self.project.slug}/"
+        assert mock.call("client_kind_test", "frontend") in self.tags_for(url)
+
+    def test_a_team_endpoint_records_the_caller(self) -> None:
+        url = f"/api/0/teams/{self.organization.slug}/{self.team.slug}/"
+        assert mock.call("client_kind_test", "frontend") in self.tags_for(url)
+
+    def test_an_issue_endpoint_records_the_caller(self) -> None:
+        # Team and issue endpoints resolve their organization off the related object
+        # rather than into an `organization` kwarg, so they are the families most
+        # likely to silently fall out of coverage.
+        url = f"/api/0/organizations/{self.organization.slug}/issues/{self.group.id}/"
+        assert mock.call("client_kind_test", "frontend") in self.tags_for(url)
+
+    def test_records_nothing_when_the_organization_has_not_opted_in(self) -> None:
+        url = f"/api/0/projects/{self.organization.slug}/{self.project.slug}/"
+        assert self.tags_for(url, enabled=False) == []
+
+    def test_a_declared_kind_does_not_bypass_the_opt_in(self) -> None:
+        """A declared caller must not also grant the organization's opt-in.
+
+        The opt-in check moved out of `get_client_kind` and up to the dispatch call
+        site, so it is the ordering there -- not the function -- that now keeps a
+        `client_kind_scope` declaration from reporting for an org that never enabled
+        the feature.
+        """
+        url = f"/api/0/projects/{self.organization.slug}/{self.project.slug}/"
+        with client_kind_scope(ClientKind.SEER):
+            assert self.tags_for(url, enabled=False) == []

tests/sentry/seer/endpoints/test_seer_rpc.py

@@ -116,13 +116,13 @@ def test_get_organization_projects_registered_on_internal_rpc(self) -> None:
 
     def test_dispatch_declares_seer_as_the_client_kind(self) -> None:
         org = self.create_organization()
-        captured: list[ClientKind | None] = []
+        captured: list[ClientKind] = []
 
         def fake_method(**kwargs: Any) -> dict[str, Any]:
             nested = Request(APIRequestFactory().get("/"))
             nested.user = AnonymousUser()
             nested.auth = None
-            captured.append(get_client_kind(nested, org))
+            captured.append(get_client_kind(nested))
             return {"features": []}
 
         path = self._get_path("get_organization_features")
@@ -151,7 +151,7 @@ def test_client_kind_scope_does_not_outlive_the_dispatch(self) -> None:
             nested = Request(APIRequestFactory().get("/"))
             nested.user = AnonymousUser()
             nested.auth = None
-            assert get_client_kind(nested, org) == ClientKind.UNKNOWN
+            assert get_client_kind(nested) == ClientKind.UNKNOWN
 
     def test_snuba_rate_limit_returns_429(self) -> None:
         """Test that SnubaRPCRateLimitExceeded returns 429 to Seer for retry."""