fix(scope): Drop None user attribute values in set_user#6692
Conversation
When set_user is called with a dict, any keys with None values are now filtered out before storing on the scope. This prevents None user attributes from being sent to Sentry.
Codecov Results 📊✅ 89952 passed | ⏭️ 6288 skipped | Total: 96240 | Pass Rate: 93.47% | Execution Time: 315m 26s 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 2436 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 89.77% 89.77% —%
==========================================
Files 192 192 —
Lines 23809 23809 —
Branches 8218 8218 —
==========================================
+ Hits 21374 21373 -1
- Misses 2435 2436 +1
- Partials 1347 1349 +2Generated by Codecov Action |
There was a problem hiding this comment.
Since this is only a problem for attributes-based telemetry, I'd make the fix at the source in _apply_user_attributes_to_telemetry: basically, we're just missing a None check on this line: https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/scope.py#L1756
My concern with changing it directly in the _user object on the scope is that this might be changing the existing behavior for (non-attributes-based) events, which is probably not what we want.
|
Sounds good - I thought that removing Have made the necessary changes in 37ba0fd |
| @@ -891,6 +891,7 @@ def user(self, value: "Optional[Dict[str, Any]]") -> None: | |||
| def set_user(self, value: "Optional[Dict[str, Any]]") -> None: | |||
| """Sets a user for the scope.""" | |||
| self._user = value | |||
There was a problem hiding this comment.
Bug: The set_user method stores the user dictionary without filtering None values, causing regular events to include null attributes, which contradicts the PR's goal.
Severity: MEDIUM
Suggested Fix
Filter None values from the user dictionary within the set_user method before storing it in self._user. This ensures all subsequent uses of self._user, including in _apply_user_to_event, will have the correctly filtered data. Alternatively, modify _apply_user_to_event to filter the dictionary before adding it to the event.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: sentry_sdk/scope.py#L893
Potential issue: The `set_user` method stores the user dictionary in `self._user`
without filtering out keys that have `None` values. Subsequently, the
`_apply_user_to_event` method copies this dictionary directly to the event payload. As a
result, if a user is set with a dictionary containing `None` values (e.g.,
`scope.set_user({"username": None})`), the resulting Sentry event will still contain
`null` for that attribute. This behavior is inconsistent, as the filtering logic was
only added for the telemetry/span path in `_apply_user_attributes_to_telemetry`, but not
for the regular event path.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
This is the existing behaviour. Leaving as-is because the string-ifying of 'None' happens on attributes-based telemetry (i.e. streamed spans)
set_userpreviously stored None values verbatim in the user dict, which would send null attributes to Sentry. Now any key with a None value is filtered out before the dict is stored on the scope, so only meaningful user attributes are captured in events.Fixes PY-2567
Fixes #6689