Skip to content

Commit 2d93987

Browse files
rustyconoverclaude
andcommitted
feat(sentry): map JWT claims to Sentry user; auto-tag attach_id/transaction_id
Two improvements to the auto-attached Sentry dispatch hook: 1. Standards-aligned ``set_user`` mapping. Previously the JWT ``sub`` claim was shoved into ``user.username``, leaving Sentry events showing opaque IdP identifiers as if they were human usernames. Now ``auth.principal`` populates ``user.id`` (Sentry's canonical opaque identifier) and the decoded JWT claims dict feeds ``user.username`` / ``user.email`` / ``user.name`` from the standard OIDC claim names. ``SentryConfig`` gains ``user_claim_map`` so non-standard IdPs (e.g. Auth0 namespaced claims) can override per-key. Static bearer tokens populate only ``user.id`` — same effective behavior as before but in the correct field. 2. Generic ``vgi.attach_id`` / ``vgi.transaction_id`` scope tags on every dispatch. ``_extract_well_known`` walks one level into kwargs to handle direct kwargs (``catalog_detach(attach_id=...)``), request dataclasses (``bind(request=BindRequest(...))``), and ``InitRequest.bind_call`` nesting — no per-method wiring required. Values are run through the new public ``short_hash`` helper (12-char SHA-256 prefix) so Sentry's tag-value distribution UI stays bounded while same-input → same-output preserves cross-event correlation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 1df0fb1 commit 2d93987

2 files changed

Lines changed: 388 additions & 5 deletions

File tree

tests/test_sentry.py

Lines changed: 288 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,13 @@
3333
)
3434
from vgi_rpc.sentry import (
3535
SentryConfig,
36+
_extract_well_known,
3637
_has_sentry_hook,
3738
_maybe_auto_instrument,
3839
_SentryDispatchHook,
3940
_strip_sentry_hook,
4041
instrument_server_sentry,
42+
short_hash,
4143
)
4244

4345
# ---------------------------------------------------------------------------
@@ -237,15 +239,96 @@ def test_scope_context_set(self, mock_sdk: MagicMock) -> None:
237239
assert ctx_data["server_id"] == "ctx-test"
238240

239241
@patch("vgi_rpc.sentry.sentry_sdk")
240-
def test_auth_user_set(self, mock_sdk: MagicMock) -> None:
241-
"""Auth principal is set as Sentry user."""
242+
def test_auth_user_id_only(self, mock_sdk: MagicMock) -> None:
243+
"""Auth principal becomes ``user.id`` even when no claims are present."""
242244
mock_scope = MagicMock()
243245
mock_sdk.get_current_scope.return_value = mock_scope
244246
server = RpcServer(SentryTestService, SentryTestServiceImpl())
245247
instrument_server_sentry(server)
246-
auth = AuthContext(domain="test", authenticated=True, principal="bob")
248+
auth = AuthContext(domain="bearer", authenticated=True, principal="bob")
247249
_run_pipe_call(server, "add", kwargs={"a": 1, "b": 2}, auth=auth)
248-
mock_scope.set_user.assert_called_with({"username": "bob"})
250+
mock_scope.set_user.assert_called_with({"id": "bob"})
251+
252+
@patch("vgi_rpc.sentry.sentry_sdk")
253+
def test_auth_user_oidc_claims(self, mock_sdk: MagicMock) -> None:
254+
"""Standard OIDC claims populate username/email/name; sub stays in id."""
255+
mock_scope = MagicMock()
256+
mock_sdk.get_current_scope.return_value = mock_scope
257+
server = RpcServer(SentryTestService, SentryTestServiceImpl())
258+
instrument_server_sentry(server)
259+
auth = AuthContext(
260+
domain="jwt",
261+
authenticated=True,
262+
principal="4FySzCeE4zIYuvph49iD9tcJL0_zDWfpMqarUUPc1uA",
263+
claims={
264+
"sub": "4FySzCeE4zIYuvph49iD9tcJL0_zDWfpMqarUUPc1uA",
265+
"preferred_username": "rusty",
266+
"email": "rusty@luckydinosaur.com",
267+
"name": "Rusty Conover",
268+
},
269+
)
270+
_run_pipe_call(server, "add", kwargs={"a": 1, "b": 2}, auth=auth)
271+
mock_scope.set_user.assert_called_with(
272+
{
273+
"id": "4FySzCeE4zIYuvph49iD9tcJL0_zDWfpMqarUUPc1uA",
274+
"username": "rusty",
275+
"email": "rusty@luckydinosaur.com",
276+
"name": "Rusty Conover",
277+
}
278+
)
279+
280+
@patch("vgi_rpc.sentry.sentry_sdk")
281+
def test_auth_user_claim_map_override(self, mock_sdk: MagicMock) -> None:
282+
"""user_claim_map overrides default claim names (e.g. Auth0 namespaced claims)."""
283+
mock_scope = MagicMock()
284+
mock_sdk.get_current_scope.return_value = mock_scope
285+
config = SentryConfig(
286+
user_claim_map={
287+
"username": "https://example.com/handle",
288+
"email": "https://example.com/email",
289+
}
290+
)
291+
server = RpcServer(SentryTestService, SentryTestServiceImpl())
292+
instrument_server_sentry(server, config)
293+
auth = AuthContext(
294+
domain="jwt",
295+
authenticated=True,
296+
principal="auth0|abc",
297+
claims={
298+
"sub": "auth0|abc",
299+
"https://example.com/handle": "rusty",
300+
"https://example.com/email": "rusty@example.com",
301+
"preferred_username": "ignored", # default key, but map overrides
302+
},
303+
)
304+
_run_pipe_call(server, "add", kwargs={"a": 1, "b": 2}, auth=auth)
305+
mock_scope.set_user.assert_called_with(
306+
{
307+
"id": "auth0|abc",
308+
"username": "rusty",
309+
"email": "rusty@example.com",
310+
}
311+
)
312+
313+
@patch("vgi_rpc.sentry.sentry_sdk")
314+
def test_auth_user_skips_non_string_claims(self, mock_sdk: MagicMock) -> None:
315+
"""Non-string or empty claim values are ignored — they don't poison user fields."""
316+
mock_scope = MagicMock()
317+
mock_sdk.get_current_scope.return_value = mock_scope
318+
server = RpcServer(SentryTestService, SentryTestServiceImpl())
319+
instrument_server_sentry(server)
320+
auth = AuthContext(
321+
domain="jwt",
322+
authenticated=True,
323+
principal="bob",
324+
claims={
325+
"preferred_username": "", # empty
326+
"email": ["a@b.com"], # wrong type
327+
"name": 42, # wrong type
328+
},
329+
)
330+
_run_pipe_call(server, "add", kwargs={"a": 1, "b": 2}, auth=auth)
331+
mock_scope.set_user.assert_called_with({"id": "bob"})
249332

250333
@patch("vgi_rpc.sentry.sentry_sdk")
251334
def test_custom_tags_applied(self, mock_sdk: MagicMock) -> None:
@@ -1104,3 +1187,204 @@ def test_strip_keeps_other_hooks(self, mock_sdk: MagicMock) -> None:
11041187
def test_strip_handles_none(self) -> None:
11051188
"""Stripping None returns None."""
11061189
assert _strip_sentry_hook(None) is None
1190+
1191+
1192+
# ---------------------------------------------------------------------------
1193+
# short_hash + well-known kwarg tagging
1194+
# ---------------------------------------------------------------------------
1195+
1196+
1197+
class TestShortHash:
1198+
"""short_hash helper: stable hex prefix of sha256."""
1199+
1200+
def test_none_returns_none(self) -> None:
1201+
"""None passes through to None."""
1202+
assert short_hash(None) is None
1203+
1204+
def test_deterministic_length(self) -> None:
1205+
"""Default output is 12 lowercase hex characters."""
1206+
h = short_hash(b"\x01\x02\x03")
1207+
assert h is not None
1208+
assert len(h) == 12
1209+
assert all(c in "0123456789abcdef" for c in h)
1210+
1211+
def test_bytes_and_hex_string_match(self) -> None:
1212+
"""``short_hash(b)`` and ``short_hash(b.hex())`` produce the same value."""
1213+
raw = b"\x4f\x3c\x2a\x1b\x9d\x8e\xff\x01"
1214+
assert short_hash(raw) == short_hash(raw.hex())
1215+
1216+
def test_different_inputs_different_hashes(self) -> None:
1217+
"""Distinct inputs yield distinct hashes."""
1218+
assert short_hash(b"a") != short_hash(b"b")
1219+
1220+
def test_custom_length(self) -> None:
1221+
"""``length`` parameter controls prefix size."""
1222+
h = short_hash(b"x", length=8)
1223+
assert h is not None
1224+
assert len(h) == 8
1225+
1226+
1227+
@dataclass
1228+
class _FakeAttachId:
1229+
"""Stand-in for a request dataclass with an ``attach_id`` attribute."""
1230+
1231+
attach_id: bytes
1232+
transaction_id: bytes | None = None
1233+
1234+
1235+
@dataclass
1236+
class _FakeBindRequest:
1237+
"""Stand-in for ``BindRequest``."""
1238+
1239+
attach_id: bytes
1240+
transaction_id: bytes | None = None
1241+
1242+
1243+
@dataclass
1244+
class _FakeInitRequest:
1245+
"""Stand-in for ``InitRequest`` (wraps a ``bind_call`` carrying the IDs)."""
1246+
1247+
bind_call: _FakeBindRequest
1248+
1249+
1250+
class TestExtractWellKnown:
1251+
"""_extract_well_known walks the three protocol shapes."""
1252+
1253+
def test_top_level_kwarg(self) -> None:
1254+
"""Direct kwarg lookup."""
1255+
kwargs = {"attach_id": b"\x01\x02"}
1256+
assert _extract_well_known(kwargs, "attach_id") == b"\x01\x02"
1257+
1258+
def test_nested_attribute_on_request_dataclass(self) -> None:
1259+
"""One level of attribute descent into a request dataclass."""
1260+
kwargs = {"request": _FakeAttachId(attach_id=b"\x0a\x0b")}
1261+
assert _extract_well_known(kwargs, "attach_id") == b"\x0a\x0b"
1262+
1263+
def test_init_request_descends_bind_call(self) -> None:
1264+
"""Two-level descent: ``request.bind_call.attach_id``."""
1265+
kwargs = {"request": _FakeInitRequest(bind_call=_FakeBindRequest(attach_id=b"\xff"))}
1266+
assert _extract_well_known(kwargs, "attach_id") == b"\xff"
1267+
1268+
def test_missing_returns_none(self) -> None:
1269+
"""No matching field anywhere returns None."""
1270+
assert _extract_well_known({"unrelated": 5}, "attach_id") is None
1271+
1272+
def test_none_value_returns_none(self) -> None:
1273+
"""An explicit None on a request dataclass returns None, not the attr."""
1274+
kwargs = {"request": _FakeAttachId(attach_id=b"\x01", transaction_id=None)}
1275+
assert _extract_well_known(kwargs, "transaction_id") is None
1276+
1277+
def test_skips_non_bytes_str(self) -> None:
1278+
"""Integer or list values are not extracted (defends against accidental shadowing)."""
1279+
kwargs = {"attach_id": 42}
1280+
assert _extract_well_known(kwargs, "attach_id") is None
1281+
1282+
1283+
class TestWellKnownAutoTagging:
1284+
"""_SentryDispatchHook auto-tags vgi.attach_id / vgi.transaction_id."""
1285+
1286+
def _make_info(self, name: str = "test_method") -> MagicMock:
1287+
"""Build a minimal RpcMethodInfo stand-in for hook invocation."""
1288+
info = MagicMock()
1289+
info.name = name
1290+
info.method_type.value = "unary"
1291+
return info
1292+
1293+
@patch("vgi_rpc.sentry.sentry_sdk")
1294+
def test_top_level_attach_id_kwarg_tagged(self, mock_sdk: MagicMock) -> None:
1295+
"""A direct ``attach_id: bytes`` kwarg becomes ``vgi.attach_id`` tag."""
1296+
mock_scope = MagicMock()
1297+
mock_sdk.get_current_scope.return_value = mock_scope
1298+
mock_sdk.get_current_span.return_value = None
1299+
hook = _SentryDispatchHook(SentryConfig(), "TestProto", "srv-1")
1300+
attach_id = b"\x4f\x3c\x2a\x1b\x9d\x8e\xff\x01\x02\x03\x04\x05\x06\x07\x08\x09"
1301+
hook.on_dispatch_start(
1302+
self._make_info("catalog_detach"),
1303+
AuthContext(domain=None, authenticated=False),
1304+
{},
1305+
{"attach_id": attach_id},
1306+
)
1307+
tag_calls = {call[0][0]: call[0][1] for call in mock_scope.set_tag.call_args_list}
1308+
assert tag_calls["vgi.attach_id"] == short_hash(attach_id)
1309+
1310+
@patch("vgi_rpc.sentry.sentry_sdk")
1311+
def test_nested_request_dataclass_tagged(self, mock_sdk: MagicMock) -> None:
1312+
"""A request dataclass carrying ``attach_id``/``transaction_id`` is unwrapped."""
1313+
mock_scope = MagicMock()
1314+
mock_sdk.get_current_scope.return_value = mock_scope
1315+
mock_sdk.get_current_span.return_value = None
1316+
hook = _SentryDispatchHook(SentryConfig(), "TestProto", "srv-1")
1317+
attach_id = b"\xab\xcd\xef\x01"
1318+
tx_id = b"\x99\x88\x77\x66"
1319+
hook.on_dispatch_start(
1320+
self._make_info("bind"),
1321+
AuthContext(domain=None, authenticated=False),
1322+
{},
1323+
{"request": _FakeBindRequest(attach_id=attach_id, transaction_id=tx_id)},
1324+
)
1325+
tag_calls = {call[0][0]: call[0][1] for call in mock_scope.set_tag.call_args_list}
1326+
assert tag_calls["vgi.attach_id"] == short_hash(attach_id)
1327+
assert tag_calls["vgi.transaction_id"] == short_hash(tx_id)
1328+
1329+
@patch("vgi_rpc.sentry.sentry_sdk")
1330+
def test_init_request_descends_bind_call(self, mock_sdk: MagicMock) -> None:
1331+
"""An ``InitRequest``-shaped kwarg is descended via ``bind_call``."""
1332+
mock_scope = MagicMock()
1333+
mock_sdk.get_current_scope.return_value = mock_scope
1334+
mock_sdk.get_current_span.return_value = None
1335+
hook = _SentryDispatchHook(SentryConfig(), "TestProto", "srv-1")
1336+
attach_id = b"\xde\xad\xbe\xef"
1337+
hook.on_dispatch_start(
1338+
self._make_info("init"),
1339+
AuthContext(domain=None, authenticated=False),
1340+
{},
1341+
{"request": _FakeInitRequest(bind_call=_FakeBindRequest(attach_id=attach_id))},
1342+
)
1343+
tag_calls = {call[0][0]: call[0][1] for call in mock_scope.set_tag.call_args_list}
1344+
assert tag_calls["vgi.attach_id"] == short_hash(attach_id)
1345+
1346+
@patch("vgi_rpc.sentry.sentry_sdk")
1347+
def test_no_tag_when_absent(self, mock_sdk: MagicMock) -> None:
1348+
"""Methods without these IDs do not get the tags set."""
1349+
mock_scope = MagicMock()
1350+
mock_sdk.get_current_scope.return_value = mock_scope
1351+
mock_sdk.get_current_span.return_value = None
1352+
hook = _SentryDispatchHook(SentryConfig(), "TestProto", "srv-1")
1353+
hook.on_dispatch_start(
1354+
self._make_info("add"),
1355+
AuthContext(domain=None, authenticated=False),
1356+
{},
1357+
{"a": 1, "b": 2},
1358+
)
1359+
tag_calls = {call[0][0]: call[0][1] for call in mock_scope.set_tag.call_args_list}
1360+
assert "vgi.attach_id" not in tag_calls
1361+
assert "vgi.transaction_id" not in tag_calls
1362+
1363+
@patch("vgi_rpc.sentry.sentry_sdk")
1364+
def test_tag_value_is_stable_across_bytes_and_hex(self, mock_sdk: MagicMock) -> None:
1365+
"""Same logical ID hashed the same whether it arrived as bytes or as a hex string."""
1366+
mock_scope_bytes = MagicMock()
1367+
mock_scope_hex = MagicMock()
1368+
mock_sdk.get_current_span.return_value = None
1369+
hook = _SentryDispatchHook(SentryConfig(), "TestProto", "srv-1")
1370+
attach_id = b"\x12\x34\x56\x78\x9a\xbc"
1371+
1372+
mock_sdk.get_current_scope.return_value = mock_scope_bytes
1373+
hook.on_dispatch_start(
1374+
self._make_info(),
1375+
AuthContext(domain=None, authenticated=False),
1376+
{},
1377+
{"attach_id": attach_id},
1378+
)
1379+
bytes_tag = {c[0][0]: c[0][1] for c in mock_scope_bytes.set_tag.call_args_list}["vgi.attach_id"]
1380+
1381+
mock_sdk.get_current_scope.return_value = mock_scope_hex
1382+
hook.on_dispatch_start(
1383+
self._make_info(),
1384+
AuthContext(domain=None, authenticated=False),
1385+
{},
1386+
{"attach_id": attach_id.hex()},
1387+
)
1388+
hex_tag = {c[0][0]: c[0][1] for c in mock_scope_hex.set_tag.call_args_list}["vgi.attach_id"]
1389+
1390+
assert bytes_tag == hex_tag

0 commit comments

Comments
 (0)