Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions sentry_sdk/integrations/asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
_get_installed_modules,
capture_internal_exceptions,
event_from_exception,
has_data_collection_enabled,
logger,
nullcontext,
qualname_from_function,
Expand Down Expand Up @@ -253,10 +254,17 @@ async def _run_app(
"network.protocol.name": ty,
}

if scope.get("client") and should_send_default_pii():
sentry_scope.set_attribute(
SPANDATA.USER_IP_ADDRESS, _get_ip(scope)
)
if scope.get("client"):
client_options = sentry_sdk.get_client().options
if has_data_collection_enabled(client_options):
if client_options["data_collection"]["user_info"]:
sentry_scope.set_attribute(
SPANDATA.USER_IP_ADDRESS, _get_ip(scope)
)
elif should_send_default_pii():
sentry_scope.set_attribute(
SPANDATA.USER_IP_ADDRESS, _get_ip(scope)
)

if ty in ("http", "websocket"):
if (
Expand Down
18 changes: 17 additions & 1 deletion sentry_sdk/integrations/aws_lambda.py
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,23 @@
if "headers" in aws_event:
request["headers"] = _filter_headers(aws_event["headers"])

if should_send_default_pii():
client_options = sentry_sdk.get_client().options
if has_data_collection_enabled(client_options):
if client_options["data_collection"]["user_info"]:
user_info = sentry_event.setdefault("user", {})

identity = aws_event.get("identity")
if identity is None:
identity = {}

id = identity.get("userArn")
if id is not None:
user_info.setdefault("id", id)

ip = identity.get("sourceIp")
if ip is not None:
user_info.setdefault("ip_address", ip)
elif should_send_default_pii():

Check warning on line 456 in sentry_sdk/integrations/aws_lambda.py

View check run for this annotation

@sentry/warden / warden: code-review

AWS Lambda body handling skipped when data_collection is enabled

When `has_data_collection_enabled` is true, the `elif should_send_default_pii()` branch is skipped, so request body is neither collected nor redacted.
Comment on lines +441 to +456

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS Lambda body handling skipped when data_collection is enabled

When has_data_collection_enabled is true, the elif should_send_default_pii() branch is skipped, so request body is neither collected nor redacted.

Evidence
  • has_data_collection_enabled returns true when data_collection is configured in _experiments.
  • Entering the new if has_data_collection_enabled branch at line 441 skips the elif should_send_default_pii() branch at line 456.
  • Body collection (request["data"] = aws_event.get("body", "")) and redaction (AnnotatedValue.removed_because_raw_data()) live exclusively inside the skipped elif/else branches.
  • data_collection.py line 178 documents that bodies are collected regardless of PII settings by default.
  • All new data_collection test payloads use "body": null, so this regression is not exercised.

Identified by Warden · code-review · GGF-TPP

user_info = sentry_event.setdefault("user", {})

identity = aws_event.get("identity")
Expand Down
35 changes: 31 additions & 4 deletions sentry_sdk/integrations/quart.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,25 @@ async def _request_websocket_started(app: "Quart", **kwargs: "Any") -> None:
else parsed_url.url,
)

# TODO: Add the user properties that are seen in the branch below here once
# code is added to respect the `user_info` settings within the data collection
# configuration
if client_options["data_collection"]["user_info"]:
user_properties = {}

if len(request_websocket.access_route) >= 1:
segment.set_attribute(
"client.address", request_websocket.access_route[0]
)
user_properties["ip_address"] = request_websocket.access_route[
0
]

current_user_id = _get_current_user_id_from_quart()
if current_user_id:
user_properties["id"] = current_user_id

if user_properties:
existing_user_properties = scope._user or {}
scope.set_user({**existing_user_properties, **user_properties})

elif should_send_default_pii():
segment.set_attribute("url.full", request_websocket.url)
segment.set_attribute(
Expand Down Expand Up @@ -284,7 +300,18 @@ def inner(event: "Event", hint: "dict[str, Any]") -> "Event":
request_info["method"] = request.method
request_info["headers"] = _filter_headers(dict(request.headers))

if should_send_default_pii():
client_options = sentry_sdk.get_client().options
if has_data_collection_enabled(client_options):
if client_options["data_collection"]["user_info"]:
if len(request.access_route) >= 1:
request_info["env"] = {"REMOTE_ADDR": request.access_route[0]}

current_user_id = _get_current_user_id_from_quart()
if current_user_id:
user_info = event.setdefault("user", {})
user_info["id"] = current_user_id

elif should_send_default_pii():
if len(request.access_route) >= 1:
request_info["env"] = {"REMOTE_ADDR": request.access_route[0]}

Expand Down
6 changes: 5 additions & 1 deletion sentry_sdk/integrations/starlette.py
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,11 @@ def _add_user_to_sentry_scope(scope: "Dict[str, Any]") -> None:
if "user" not in scope:
return

if not should_send_default_pii():
client_options = sentry_sdk.get_client().options
if has_data_collection_enabled(client_options):
if not client_options["data_collection"]["user_info"]:
return
elif not should_send_default_pii():
return

user_info: "Dict[str, Any]" = {}
Expand Down
44 changes: 36 additions & 8 deletions tests/integrations/asgi/test_asgi.py
Original file line number Diff line number Diff line change
Expand Up @@ -1446,11 +1446,41 @@ async def test_custom_transaction_name(


@pytest.mark.asyncio
@pytest.mark.parametrize("send_default_pii", [True, False])
@pytest.mark.parametrize(
"init_kwargs, expect_ip",
[
pytest.param({"send_default_pii": True}, True, id="legacy_pii_true"),
pytest.param({"send_default_pii": False}, False, id="legacy_pii_false"),
pytest.param(
{"_experiments": {"data_collection": {}}},
True,
id="dc_default_user_info",
),
pytest.param(
{"_experiments": {"data_collection": {"user_info": True}}},
True,
id="dc_user_info_true",
),
pytest.param(
{"_experiments": {"data_collection": {"user_info": False}}},
False,
id="dc_user_info_false",
),
pytest.param(
{
"send_default_pii": True,
"_experiments": {"data_collection": {"user_info": False}},
},
False,
id="dc_wins_over_pii",
),
],
)
async def test_user_ip_address_on_all_spans(
sentry_init,
capture_items,
send_default_pii,
init_kwargs,
expect_ip,
):
async def app(scope, receive, send):
if scope["type"] == "lifespan":
Expand All @@ -1474,11 +1504,9 @@ async def app(scope, receive, send):
)
await send({"type": "http.response.body", "body": b"Hello, world!"})

sentry_init(
send_default_pii=send_default_pii,
traces_sample_rate=1.0,
trace_lifecycle="stream",
)
kwargs = {k: v for k, v in init_kwargs.items() if k != "_experiments"}
experiments = {"trace_lifecycle": "stream", **init_kwargs.get("_experiments", {})}
sentry_init(traces_sample_rate=1.0, _experiments=experiments, **kwargs)
sentry_app = SentryAsgiMiddleware(app)

async def wrapped_app(scope, receive, send):
Expand All @@ -1493,7 +1521,7 @@ async def wrapped_app(scope, receive, send):

child_span, server_span = [item.payload for item in items]

if send_default_pii:
if expect_ip:
assert server_span["attributes"]["user.ip_address"] == "127.0.0.1"
assert child_span["attributes"]["user.ip_address"] == "127.0.0.1"
else:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Need to add some ignore rules in this directory, because the unit tests will add the Sentry SDK and its dependencies
# into this directory to create a Lambda function package that contains everything needed to instrument a Lambda function using Sentry.

# Ignore everything
*

# But not index.py
!index.py

# And not .gitignore itself
!.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import os

import sentry_sdk
from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration

sentry_sdk.init(
dsn=os.environ.get("SENTRY_DSN"),
traces_sample_rate=1.0,
integrations=[AwsLambdaIntegration()],
_experiments={
"data_collection": {
"user_info": False,
}
},
)


def handler(event, context):
return {"event": event}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# Need to add some ignore rules in this directory, because the unit tests will add the Sentry SDK and its dependencies
# into this directory to create a Lambda function package that contains everything needed to instrument a Lambda function using Sentry.

# Ignore everything
*

# But not index.py
!index.py

# And not .gitignore itself
!.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import os

import sentry_sdk
from sentry_sdk.integrations.aws_lambda import AwsLambdaIntegration

sentry_sdk.init(
dsn=os.environ.get("SENTRY_DSN"),
traces_sample_rate=1.0,
integrations=[AwsLambdaIntegration()],
_experiments={
"data_collection": {
"user_info": True,
}
},
)


def handler(event, context):
return {"event": event}
66 changes: 61 additions & 5 deletions tests/integrations/aws_lambda/test_aws_lambda.py
Original file line number Diff line number Diff line change
Expand Up @@ -427,11 +427,9 @@
},
"pathParameters": null,
"stageVariables": null,
"requestContext": {
"identity": {
"sourceIp": "213.47.147.207",
"userArn": "42"
}
"identity": {
"sourceIp": "213.47.147.207",
"userArn": "42"

Check warning on line 432 in tests/integrations/aws_lambda/test_aws_lambda.py

View check run for this annotation

@sentry/warden / warden: code-review

AWS Lambda user-info tests use non-standard event structure

The test payload changed from standard `requestContext.identity` to a top-level `identity` field that doesn't match real AWS API Gateway events, causing tests to verify behavior with a fictional event structure instead of production reality.
Comment on lines +430 to +432

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AWS Lambda user-info tests use non-standard event structure

The test payload changed from standard requestContext.identity to a top-level identity field that doesn't match real AWS API Gateway events, causing tests to verify behavior with a fictional event structure instead of production reality.

Evidence
  • sentry_sdk/integrations/aws_lambda.py _make_request_event_processor reads user identity via aws_event.get("identity") directly.
  • Real AWS API Gateway proxy events nest identity under requestContext (e.g., test_request_data_with_send_default_pii_false and test_request_data_with_data_collection_allowlist in the same file both use "requestContext": {"identity": {...}}).
  • The changed test and the new USER_INFO_PAYLOAD place identity at the top level, which matches the current code but does not exist in real AWS events, so the assertions pass in tests while the extraction may fail in production.

Identified by Warden · code-review · RCQ-XUC

},
"body": null,
"isBase64Encoded": false
Expand Down Expand Up @@ -464,6 +462,64 @@
"data": None,
}

# Legacy send_default_pii=True attaches the user identity.
assert transaction_event["user"] == {
"id": "42",
"ip_address": "213.47.147.207",
}


USER_INFO_PAYLOAD = b"""
{
"resource": "/asd",
"path": "/asd",
"httpMethod": "GET",
"headers": {
"Host": "iwsz2c7uwi.execute-api.us-east-1.amazonaws.com",
"User-Agent": "custom",
"X-Forwarded-Proto": "https"
},
"queryStringParameters": {
"bonkers": "true"
},
"pathParameters": null,
"stageVariables": null,
"identity": {
"sourceIp": "213.47.147.207",
"userArn": "42"
},
"body": null,
"isBase64Encoded": false
}
"""


def test_user_info_with_data_collection_user_info_on(lambda_client, test_environment):
lambda_client.invoke(
FunctionName="BasicOkDataCollectionUserInfoOn",
Payload=USER_INFO_PAYLOAD,
)
envelopes = test_environment["server"].envelopes

(transaction_event,) = envelopes

assert transaction_event["user"] == {
"id": "42",
"ip_address": "213.47.147.207",
}


def test_user_info_with_data_collection_user_info_off(lambda_client, test_environment):
lambda_client.invoke(
FunctionName="BasicOkDataCollectionUserInfoOff",
Payload=USER_INFO_PAYLOAD,
)
envelopes = test_environment["server"].envelopes

(transaction_event,) = envelopes

assert "user" not in transaction_event


def test_request_data_with_data_collection_allowlist(lambda_client, test_environment):
payload = b"""
Expand Down
Loading
Loading