feat(asgi): Migrate away from event processor in span first #5920
3 issues
code-review: Found 3 issues (1 high, 2 medium)
High
Inverted filter logic causes all items to be skipped when types=None - `tests/conftest.py:341-342`
The condition if types is None or item.type not in types: continue has inverted logic. When types is None (intended to mean 'capture all types'), the first condition types is None evaluates to True, causing continue to execute and skip ALL items. This defeats the purpose of having a default behavior that captures everything.
Medium
AnnotatedValue objects from sensitive headers not properly handled in attributes - `sentry_sdk/integrations/_asgi_common.py:123-124`
When should_send_default_pii() returns False, _filter_headers() returns AnnotatedValue objects for sensitive headers (like authorization, cookie, etc.). The new _get_request_attributes() function directly assigns these values to attributes. When set_attribute() processes these, format_attribute() will fall back to safe_repr(), producing unhelpful string representations like <AnnotatedValue object at 0x...> instead of properly filtered values. While no actual PII leaks (the original sensitive data is already stripped), the resulting attribute values are not meaningful.
KeyError when span has no attributes - `tests/conftest.py:1272-1274`
The envelopes_to_spans helper accesses span_json["attributes"] directly, but in _span_batcher.py:116-119, the attributes key is only added to the span JSON when item._attributes is truthy. If a span has no attributes, this will raise a KeyError. This would cause tests to fail when processing spans without attributes.
Duration: 2m 26s · Tokens: 1.3M in / 16.9k out · Cost: $2.00 (+extraction: $0.01, +merge: $0.00, +fix_gate: $0.00)
Annotations
Check failure on line 342 in tests/conftest.py
sentry-warden / warden: code-review
Inverted filter logic causes all items to be skipped when types=None
The condition `if types is None or item.type not in types: continue` has inverted logic. When `types` is `None` (intended to mean 'capture all types'), the first condition `types is None` evaluates to `True`, causing `continue` to execute and skip ALL items. This defeats the purpose of having a default behavior that captures everything.
Check warning on line 124 in sentry_sdk/integrations/_asgi_common.py
sentry-warden / warden: code-review
AnnotatedValue objects from sensitive headers not properly handled in attributes
When `should_send_default_pii()` returns False, `_filter_headers()` returns `AnnotatedValue` objects for sensitive headers (like `authorization`, `cookie`, etc.). The new `_get_request_attributes()` function directly assigns these values to attributes. When `set_attribute()` processes these, `format_attribute()` will fall back to `safe_repr()`, producing unhelpful string representations like `<AnnotatedValue object at 0x...>` instead of properly filtered values. While no actual PII leaks (the original sensitive data is already stripped), the resulting attribute values are not meaningful.
Check warning on line 1274 in tests/conftest.py
sentry-warden / warden: code-review
KeyError when span has no attributes
The `envelopes_to_spans` helper accesses `span_json["attributes"]` directly, but in `_span_batcher.py:116-119`, the `attributes` key is only added to the span JSON when `item._attributes` is truthy. If a span has no attributes, this will raise a `KeyError`. This would cause tests to fail when processing spans without attributes.