diff --git a/src/sentry/api/client_kind.py b/src/sentry/api/client_kind.py index 4e9d63e95656..1e3fd6ffd10b 100644 --- a/src/sentry/api/client_kind.py +++ b/src/sentry/api/client_kind.py @@ -194,6 +194,10 @@ def set_client_kind_attributes(request: Request) -> None: client_host = get_client_host(request) user_agent = get_user_agent(request) + # Stored to be available for the access log. + request._request.client_kind = client_kind + request._request.client_host = client_host + _record_attribution_span(request, client_kind, client_host, user_agent) # `_test` suffix while this is a POC, to keep it out of the way of a diff --git a/src/sentry/middleware/access_log.py b/src/sentry/middleware/access_log.py index 983916d73e1d..4a48f81f6723 100644 --- a/src/sentry/middleware/access_log.py +++ b/src/sentry/middleware/access_log.py @@ -119,6 +119,8 @@ def _create_api_access_log( org_id = getattr(getattr(request, "organization", None), "id", None) entity_id = getattr(request_auth, "entity_id", None) status_code = getattr(response, "status_code", 500) + # Set during dispatch, for organizations opted into `client_kind`. + client_kind = getattr(request, "client_kind", None) log_metrics = dict( method=request.method, view=view, @@ -132,6 +134,8 @@ def _create_api_access_log( path=request.path, caller_ip=request.META.get("REMOTE_ADDR"), user_agent=request.META.get("HTTP_USER_AGENT"), + client_kind=client_kind.value if client_kind is not None else None, + client_host=getattr(request, "client_host", None), rate_limited=getattr(request, "will_be_rate_limited", False), rate_limit_category=getattr(request, "rate_limit_category", None), request_duration_seconds=access_log_metadata.get_request_duration(), diff --git a/tests/sentry/api/test_client_kind.py b/tests/sentry/api/test_client_kind.py index de84417a5229..ae40c208e966 100644 --- a/tests/sentry/api/test_client_kind.py +++ b/tests/sentry/api/test_client_kind.py @@ -304,6 +304,38 @@ def test_records_for_the_internal_api_client_too(self) -> None: assert sdk.set_tag.call_args_list == [mock.call("client_kind_test", "script")] +class AccessLogAttributesTest(TestCase): + """Asserted on the underlying Django request, which is all `access_log` sees.""" + + def stored(self, request: Request) -> tuple[Any, Any]: + django_request: Any = request._request + return ( + getattr(django_request, "client_kind", None), + getattr(django_request, "client_host", None), + ) + + def test_stores_the_derived_kind(self) -> None: + request = make_request(auth=api_token(), user_agent="curl/8.7.1") + set_client_kind_attributes(request) + assert self.stored(request) == (ClientKind.SCRIPT, None) + + def test_stores_the_client_host_for_mcp(self) -> None: + request = make_request( + auth=api_token(), + user_agent="sentry-mcp/1.0", + headers={ + "X-Sentry-MCP-Version": "1.0", + "X-Sentry-MCP-Client-Family": "Claude-Code", + }, + ) + set_client_kind_attributes(request) + assert self.stored(request) == (ClientKind.MCP, "claude-code") + + def test_absent_until_dispatch_runs(self) -> None: + # An un-attributed request leaves the attributes absent, not empty. + assert self.stored(make_request(auth=api_token())) == (None, None) + + class AttributionSpanTest(TestCase): def record(self, request: Request) -> tuple[Any, list[tuple[str, Any]]]: with ( diff --git a/tests/sentry/middleware/test_access_log_middleware.py b/tests/sentry/middleware/test_access_log_middleware.py index 01048de169a1..b3b1e9d0d42d 100644 --- a/tests/sentry/middleware/test_access_log_middleware.py +++ b/tests/sentry/middleware/test_access_log_middleware.py @@ -10,6 +10,7 @@ from sentry.api.base import Endpoint from sentry.api.bases.organization import ControlSiloOrganizationEndpoint, OrganizationEndpoint +from sentry.api.client_kind import FEATURE_FLAG as CLIENT_KIND_FEATURE_FLAG from sentry.api.endpoints.internal.rpc import InternalRpcServiceEndpoint from sentry.api.permissions import SentryIsAuthenticated from sentry.models.apitoken import ApiToken @@ -206,6 +207,9 @@ def get(self, request, organization_context, organization): "snuba_throttle_threshold", "token_last_characters", "gateway_proxy", + # Only present for organizations opted into `client_kind`. + "client_kind", + "client_host", ) @@ -481,6 +485,50 @@ def test_org_id_populated(self) -> None: assert tested_log.organization_id == str(self.organization.id) +class TestClientKindLogged(LogCaptureAPITestCase): + """`client_kind` has to reach the access log, not just the span. + + Span attributes stop at Snuba -- the analytics warehouse promotes a fixed + allow-list of span columns -- so the access log is the only path by which caller + attribution becomes queryable downstream. + """ + + endpoint = "sentry-api-0-organization-stats-v2" + + def setUp(self) -> None: + self.login_as(user=self.user) + + def request_stats(self) -> None: + self._caplog.set_level(logging.INFO, logger="sentry") + self.get_success_response( + self.organization.slug, + qs_params={ + "project": [-1], + "category": ["error"], + "statsPeriod": "1d", + "interval": "1d", + "field": ["sum(quantity)"], + }, + ) + + def test_client_kind_logged_for_an_opted_in_org(self) -> None: + with self.feature(CLIENT_KIND_FEATURE_FLAG): + self.request_stats() + + tested_log = self.get_tested_log(args=[self.organization.slug]) + # A session cookie and no token is the web UI. + assert tested_log.client_kind == "frontend" + + def test_absent_for_an_org_that_has_not_opted_in(self) -> None: + with self.feature({CLIENT_KIND_FEATURE_FLAG: False}): + self.request_stats() + + tested_log = self.get_tested_log(args=[self.organization.slug]) + # Absent rather than "unknown": a disabled org has to stay distinguishable + # from one whose traffic genuinely classifies as unknown. + assert not hasattr(tested_log, "client_kind") + + @control_silo_test class TestOrganizationIdPresentForControl(LogCaptureAPITestCase): endpoint = "sentry-api-0-organization-members" diff --git a/tools/mypy_helpers/plugin.py b/tools/mypy_helpers/plugin.py index 86a05c4b0add..649dda00651c 100644 --- a/tools/mypy_helpers/plugin.py +++ b/tools/mypy_helpers/plugin.py @@ -139,6 +139,9 @@ def _adjust_http_request_members(ctx: ClassDefContext) -> None: add_attribute_to_class(ctx.api, ctx.cls, "superuser", AnyType(TypeOfAny.explicit)) # added by OrganizationEndpoint.convert_args and similar add_attribute_to_class(ctx.api, ctx.cls, "organization", AnyType(TypeOfAny.explicit)) + # added by sentry.api.client_kind.set_client_kind_attributes + add_attribute_to_class(ctx.api, ctx.cls, "client_kind", AnyType(TypeOfAny.explicit)) + add_attribute_to_class(ctx.api, ctx.cls, "client_host", AnyType(TypeOfAny.explicit)) # added by sentry.api.authentication.RelayAuthentication add_attribute_to_class(ctx.api, ctx.cls, "relay", AnyType(TypeOfAny.explicit)) add_attribute_to_class(ctx.api, ctx.cls, "relay_request_data", AnyType(TypeOfAny.explicit))