feat(integrations): Gate user info behind data_collection config#6876
feat(integrations): Gate user info behind data_collection config#6876ericapisani wants to merge 23 commits into
Conversation
Apply the data_collection.user_info setting to Django user identity (id, email, username) collection, giving it precedence over the legacy send_default_pii boolean when explicitly configured. This mirrors the existing behavior for client IP/user info already shipped for Flask, WSGI, and ASGI. The user extraction logic in _after_get_response is pulled into a shared helper used by both the WSGI and ASGI event processors, and the parametrized send_default_pii/data_collection test matrix is extracted into tests/integrations/django/utils.py for reuse across the WSGI and ASGI test suites. Refs PY-2583
The new test was the only DB-touching test without @pytest.mark.forked. Running in the parent process leaked the created user into the shared in-memory sqlite DB (UNIQUE constraint failures in test_basic) and caused the parent to hold the session-scoped django_db_setup fixture, so forked children skipped postgres test DB creation but still destroyed it on teardown (database does not exist errors).
Move the identical USER_INFO_INIT_KWARGS parametrization from the flask, wsgi, and django test suites into tests/integrations/utils.py as DATA_COLLECTION_USER_INFO_CASES, documenting that the second element of each case is whether user info is expected to be collected.
Apply the data_collection.user_info setting to the Sanic integration's user IP collection, giving it precedence over the legacy send_default_pii boolean when explicitly configured. This covers both the user.ip_address scope attribute applied to all spans and the client.address span attribute, mirroring the behavior already shipped for Flask, WSGI, ASGI, and Django. The user IP span test is reparametrized with the shared DATA_COLLECTION_USER_INFO_CASES matrix from tests/integrations/utils.py, and a new test covers the previously untested client.address span attribute. Refs PY-2583
Apply the data_collection.user_info setting to the Tornado integration's user info collection, giving it precedence over the legacy send_default_pii boolean when explicitly configured. This covers the user.ip_address scope attribute applied to all spans, the client.address span attribute, and the is_authenticated user flag on error events, mirroring the behavior already shipped for Flask, WSGI, ASGI, Django, and Sanic. The user IP span test is reparametrized with the shared DATA_COLLECTION_USER_INFO_CASES matrix from tests/integrations/utils.py, and new tests cover the previously untested client.address span attribute and the data_collection gating of the is_authenticated flag. Refs PY-2583
Apply the data_collection.user_info setting to the Pyramid integration's user id collection, giving it precedence over the legacy send_default_pii boolean when explicitly configured. This covers both the user.id segment attribute under span streaming and the user id attached to error events, mirroring the behavior already shipped for Flask, WSGI, ASGI, Django, Sanic, and Tornado. The user id segment test is reparametrized with the shared DATA_COLLECTION_USER_INFO_CASES matrix from tests/integrations/utils.py, and a new test adds the first coverage of the user id on error events. Refs PY-2583
Apply the data_collection.user_info setting to the aiohttp integration's user IP collection under span streaming, giving it precedence over the legacy send_default_pii boolean when explicitly configured. This covers both the client.address span attribute and the user.ip_address scope attribute, mirroring the behavior already shipped for Flask, WSGI, ASGI, Django, Sanic, Tornado, and Pyramid. Add test_user_address_with_data_collection_and_span_streaming, parametrized with the shared DATA_COLLECTION_USER_INFO_CASES matrix from tests/integrations/utils.py, and remove the now-redundant IP assertion block from test_sensitive_header_passthrough_with_pii_span_streaming that its TODO reserved for this change. Refs PY-2583
Apply the data_collection.user_info setting to the Starlite and Litestar integrations' user info collection on exception events, giving it precedence over the legacy send_default_pii boolean when explicitly configured. This gates the retrieve_user_from_scope call in each integration's exception handler, mirroring the behavior already shipped for Flask, WSGI, ASGI, Django, Sanic, Tornado, Pyramid, and aiohttp. The scope user exception event tests in both suites are reparametrized with the shared DATA_COLLECTION_USER_INFO_CASES matrix from tests/integrations/utils.py. Refs PY-2583
The request event processor reads aws_event["identity"] at the top level, but the payloads used by the user info tests nested it under requestContext, so no user was attached and the assertions failed in CI.
Codecov Results 📊✅ 97631 passed | ❌ 1 failed | ⏭️ 6410 skipped | Total: 104042 | Pass Rate: 93.84% | Execution Time: 351m 56s 📊 Comparison with Base Branch
➕ New Tests (1)View new tests
➖ Removed Tests (1)View removed tests
❌ Failed Tests
|
| File | Patch % | Lines |
|---|---|---|
| sentry_sdk/integrations/aws_lambda.py | 20.00% | |
| sentry_sdk/integrations/quart.py | 100.00% |
Coverage diff
@@ Coverage Diff @@
## main #PR +/-##
==========================================
- Coverage 89.71% 89.65% -0.06%
==========================================
Files 193 193 —
Lines 24392 24434 +42
Branches 8642 8678 +36
==========================================
+ Hits 21882 21906 +24
- Misses 2510 2528 +18
- Partials 1399 1403 +4Generated by Codecov Action
The JSON payloads in test_request_data_with_send_default_pii_true and USER_INFO_PAYLOAD contained trailing commas inside requestContext, which the Lambda runtime rejects with InvalidRequestContent. This caused three test failures in CI.
| user_info = sentry_event.setdefault("user", {}) | ||
|
|
||
| identity = aws_event.get("identity") | ||
| identity = aws_event.get("requestContext", {}).get("identity") |
There was a problem hiding this comment.
This was a bug that was found in adding test coverage for this. Docs for the expected payload can be found here but the tl;dr image from the page:
This is a clean up pass from areas that I missed initially.
Refs #6743
Fixes #6746