From 6149e6273f841e6b13292ce3eddc19391688603e Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Wed, 9 Sep 2026 12:18:52 -0400 Subject: [PATCH 1/2] ref(api): Log client_kind on the API access log The client_kind attributes ride on spans today, and spans do not reach the analytics warehouse: it promotes a fixed, hand-curated allow-list of columns off `spans_gcs` and drops every other attribute, so nothing set through `sentry_sdk.set_attribute` is queryable downstream. The `api.access` log line is warehouse-visible -- it lands in `internal-sentry.getsentry_api_access_logs.stdout` as `jsonPayload` and is rolled up into `api_logs_us.api_log_stdout`, the table behind the API Trends dashboard, which today separates callers by regex-matching `sentry-mcp` in the user agent. Stash the kind and host `set_client_kind_attributes` already derives onto the underlying Django request -- the DRF wrapper is gone by the time the middleware runs -- and log them. The span is untouched: developers keep reading attribution off a trace, this only adds the path that also reaches analysis. --- src/sentry/api/client_kind.py | 26 ++++++++++ src/sentry/middleware/access_log.py | 8 ++++ tests/sentry/api/test_client_kind.py | 38 +++++++++++++++ .../middleware/test_access_log_middleware.py | 48 +++++++++++++++++++ 4 files changed, 120 insertions(+) diff --git a/src/sentry/api/client_kind.py b/src/sentry/api/client_kind.py index 4e9d63e95656..5c5afb2a34dc 100644 --- a/src/sentry/api/client_kind.py +++ b/src/sentry/api/client_kind.py @@ -15,6 +15,7 @@ import re from collections.abc import Generator from enum import StrEnum +from typing import Any import sentry_sdk from rest_framework.request import Request @@ -194,6 +195,7 @@ def set_client_kind_attributes(request: Request) -> None: client_host = get_client_host(request) user_agent = get_user_agent(request) + _stash_for_access_log(request, client_kind, 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 @@ -209,6 +211,30 @@ def set_client_kind_attributes(request: Request) -> None: sentry_sdk.set_attribute(ATTRIBUTE_NAMES.USER_AGENT_ORIGINAL, user_agent) +def _stash_for_access_log( + request: Request, client_kind: ClientKind, client_host: str | None +) -> None: + """Hand the derived caller to the access log, the only path that reaches the warehouse. + + The span attributes above are for developers reading a trace, and they stop at + Snuba: the analytics warehouse promotes a fixed, hand-curated allow-list of span + columns and drops every other attribute, so nothing set on a span is queryable in + BigQuery. The ``api.access`` log line is warehouse-visible -- it lands in + ``internal-sentry.getsentry_api_access_logs.stdout`` as ``jsonPayload`` and is + rolled up into ``api_logs_us.api_log_stdout`` -- so attribution reaches analysis + through this stash rather than through the span. + + Stashed on the *underlying Django* request because ``access_log`` middleware runs + outside DRF and never sees the ``rest_framework`` wrapper; that is the same reason + ``convert_args`` assigns ``request._request.organization``. Re-deriving the kind in + the middleware instead is not an option: the organization whose opt-in gates all of + this is resolved during dispatch and out of scope by the time the middleware runs. + """ + django_request: Any = request._request + django_request.client_kind = client_kind + django_request.client_host = client_host + + def _record_attribution_span( request: Request, client_kind: ClientKind, diff --git a/src/sentry/middleware/access_log.py b/src/sentry/middleware/access_log.py index 983916d73e1d..d7307873944e 100644 --- a/src/sentry/middleware/access_log.py +++ b/src/sentry/middleware/access_log.py @@ -119,6 +119,12 @@ 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) + # Derived and stashed during `Endpoint.dispatch`, for organizations that + # opted into `client_kind`; absent for every other request, and absent for + # anything that never reached an `Endpoint`. Note that `internal_service` + # cannot appear here at all -- system-auth requests return above -- so this + # field describes the mix of *external* callers, by construction. + client_kind = getattr(request, "client_kind", None) log_metrics = dict( method=request.method, view=view, @@ -132,6 +138,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..0360b2ddde2d 100644 --- a/tests/sentry/api/test_client_kind.py +++ b/tests/sentry/api/test_client_kind.py @@ -304,6 +304,44 @@ 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 AccessLogStashTest(TestCase): + """The stash is how attribution reaches the warehouse -- the span does not. + + Asserted on the underlying Django request specifically: the DRF wrapper is gone + by the time `access_log` middleware runs, so stashing on it would silently log + nothing. + """ + + def stashed(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_stashes_the_derived_kind(self) -> None: + request = make_request(auth=api_token(), user_agent="curl/8.7.1") + set_client_kind_attributes(request) + assert self.stashed(request) == (ClientKind.SCRIPT, None) + + def test_stashes_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.stashed(request) == (ClientKind.MCP, "claude-code") + + def test_nothing_is_stashed_until_dispatch_runs(self) -> None: + # `Endpoint.dispatch` checks the organization opt-in before calling in, so an + # un-attributed request has to leave the attributes absent rather than empty. + assert self.stashed(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" From c55ed322eeb25e136e1618d0a4f3bc94a548370b Mon Sep 17 00:00:00 2001 From: Dominik Buszowiecki Date: Wed, 9 Sep 2026 13:10:01 -0400 Subject: [PATCH 2/2] ref(api): Set client_kind inline, drop the helper Set the attributes the way `convert_args` sets `organization`, with one comment instead of a docstring. Registers both on the mypy plugin's `HttpRequest`, which is what lets the `organization` assignment type-check today. --- src/sentry/api/client_kind.py | 30 ++++------------------------ src/sentry/middleware/access_log.py | 6 +----- tests/sentry/api/test_client_kind.py | 26 ++++++++++-------------- tools/mypy_helpers/plugin.py | 3 +++ 4 files changed, 18 insertions(+), 47 deletions(-) diff --git a/src/sentry/api/client_kind.py b/src/sentry/api/client_kind.py index 5c5afb2a34dc..1e3fd6ffd10b 100644 --- a/src/sentry/api/client_kind.py +++ b/src/sentry/api/client_kind.py @@ -15,7 +15,6 @@ import re from collections.abc import Generator from enum import StrEnum -from typing import Any import sentry_sdk from rest_framework.request import Request @@ -195,7 +194,10 @@ def set_client_kind_attributes(request: Request) -> None: client_host = get_client_host(request) user_agent = get_user_agent(request) - _stash_for_access_log(request, client_kind, client_host) + # 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 @@ -211,30 +213,6 @@ def set_client_kind_attributes(request: Request) -> None: sentry_sdk.set_attribute(ATTRIBUTE_NAMES.USER_AGENT_ORIGINAL, user_agent) -def _stash_for_access_log( - request: Request, client_kind: ClientKind, client_host: str | None -) -> None: - """Hand the derived caller to the access log, the only path that reaches the warehouse. - - The span attributes above are for developers reading a trace, and they stop at - Snuba: the analytics warehouse promotes a fixed, hand-curated allow-list of span - columns and drops every other attribute, so nothing set on a span is queryable in - BigQuery. The ``api.access`` log line is warehouse-visible -- it lands in - ``internal-sentry.getsentry_api_access_logs.stdout`` as ``jsonPayload`` and is - rolled up into ``api_logs_us.api_log_stdout`` -- so attribution reaches analysis - through this stash rather than through the span. - - Stashed on the *underlying Django* request because ``access_log`` middleware runs - outside DRF and never sees the ``rest_framework`` wrapper; that is the same reason - ``convert_args`` assigns ``request._request.organization``. Re-deriving the kind in - the middleware instead is not an option: the organization whose opt-in gates all of - this is resolved during dispatch and out of scope by the time the middleware runs. - """ - django_request: Any = request._request - django_request.client_kind = client_kind - django_request.client_host = client_host - - def _record_attribution_span( request: Request, client_kind: ClientKind, diff --git a/src/sentry/middleware/access_log.py b/src/sentry/middleware/access_log.py index d7307873944e..4a48f81f6723 100644 --- a/src/sentry/middleware/access_log.py +++ b/src/sentry/middleware/access_log.py @@ -119,11 +119,7 @@ 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) - # Derived and stashed during `Endpoint.dispatch`, for organizations that - # opted into `client_kind`; absent for every other request, and absent for - # anything that never reached an `Endpoint`. Note that `internal_service` - # cannot appear here at all -- system-auth requests return above -- so this - # field describes the mix of *external* callers, by construction. + # Set during dispatch, for organizations opted into `client_kind`. client_kind = getattr(request, "client_kind", None) log_metrics = dict( method=request.method, diff --git a/tests/sentry/api/test_client_kind.py b/tests/sentry/api/test_client_kind.py index 0360b2ddde2d..ae40c208e966 100644 --- a/tests/sentry/api/test_client_kind.py +++ b/tests/sentry/api/test_client_kind.py @@ -304,27 +304,22 @@ 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 AccessLogStashTest(TestCase): - """The stash is how attribution reaches the warehouse -- the span does not. +class AccessLogAttributesTest(TestCase): + """Asserted on the underlying Django request, which is all `access_log` sees.""" - Asserted on the underlying Django request specifically: the DRF wrapper is gone - by the time `access_log` middleware runs, so stashing on it would silently log - nothing. - """ - - def stashed(self, request: Request) -> tuple[Any, Any]: + 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_stashes_the_derived_kind(self) -> 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.stashed(request) == (ClientKind.SCRIPT, None) + assert self.stored(request) == (ClientKind.SCRIPT, None) - def test_stashes_the_client_host_for_mcp(self) -> None: + def test_stores_the_client_host_for_mcp(self) -> None: request = make_request( auth=api_token(), user_agent="sentry-mcp/1.0", @@ -334,12 +329,11 @@ def test_stashes_the_client_host_for_mcp(self) -> None: }, ) set_client_kind_attributes(request) - assert self.stashed(request) == (ClientKind.MCP, "claude-code") + assert self.stored(request) == (ClientKind.MCP, "claude-code") - def test_nothing_is_stashed_until_dispatch_runs(self) -> None: - # `Endpoint.dispatch` checks the organization opt-in before calling in, so an - # un-attributed request has to leave the attributes absent rather than empty. - assert self.stashed(make_request(auth=api_token())) == (None, None) + 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): 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))