-
Notifications
You must be signed in to change notification settings - Fork 572
feat: Support array types for logs and metrics attributes #5314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Semver Impact of This PR🟡 Minor (new features) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧Release
Other
🤖 This preview updates automatically when you update the PR. |
| Hint = Dict[str, Any] | ||
|
|
||
| AttributeValue = ( | ||
| str | bool | float | int | ||
| # TODO: relay support coming soon for | ||
| # | list[str] | list[bool] | list[float] | list[int] | ||
| str | bool | float | int | list[str] | list[bool] | list[float] | list[int] | ||
| ) | ||
| Attributes = dict[str, AttributeValue] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The format_attribute and serialize_attribute functions don't handle list types, causing array attributes to be incorrectly serialized as strings instead of the intended array types.
Severity: CRITICAL
Suggested Fix
Update format_attribute and serialize_attribute in sentry_sdk/utils.py to correctly handle list types. In format_attribute, ensure lists are passed through without modification. In serialize_attribute, add logic to detect lists of primitives (str, bool, int, float) and serialize them to the corresponding array types (string[], boolean[], integer[], double[]). Add unit tests to cover these new array attribute types.
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/_types.py#L217-L223
Potential issue: The PR introduced support for array types (`list[str]`, `list[int]`,
etc.) in the `AttributeValue` type definition. However, the functions responsible for
processing these attributes, `format_attribute` and `serialize_attribute`, were not
updated. When an array is passed as an attribute, `format_attribute` converts it to its
string representation (e.g., `"['a', 'b']"`) via `safe_repr`. Subsequently,
`serialize_attribute` treats this string as a simple string value, sending `{"type":
"string"}` to the backend instead of the expected array type like `{"type":
"string[]"}`. This leads to silent data corruption for metrics and logs using array
attributes.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, will address tomorrow
| Hint = Dict[str, Any] | ||
|
|
||
| AttributeValue = ( | ||
| str | bool | float | int | ||
| # TODO: relay support coming soon for | ||
| # | list[str] | list[bool] | list[float] | list[int] | ||
| str | bool | float | int | list[str] | list[bool] | list[float] | list[int] | ||
| ) | ||
| Attributes = dict[str, AttributeValue] | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: The functions format_attribute and serialize_attribute do not handle list types, causing array attributes to be incorrectly serialized as strings via safe_repr.
Severity: CRITICAL
Suggested Fix
Update format_attribute and serialize_attribute to properly handle list types. In serialize_attribute, add logic to detect if the value is a list, determine the type of its elements (e.g., int, str), and serialize it to the corresponding array type (e.g., {"value": [1, 2], "type": "integer[]"}). Ensure format_attribute preserves the list type instead of converting it to a string.
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/_types.py#L217-L223
Potential issue: The pull request updates the `AttributeValue` type to support lists
(e.g., `list[int]`), but the implementation in `format_attribute` and
`serialize_attribute` was not updated to handle them. When an array is passed as an
attribute, `format_attribute` converts it to its string representation using
`safe_repr`. Consequently, `serialize_attribute` treats this value as a string and
serializes it with `{"type": "string"}`. This silently breaks the new array attribute
feature, as the backend receives a string instead of an array, preventing correct
processing and querying.
Did we get this right? 👍 / 👎 to inform future reviews.
Description
Allow array attribute types as Relay now supports them.
Note: the monolith may still need to make changes to query and expose the attributes.
Issues
Reminders
tox -e linters.feat:,fix:,ref:,meta:)