-
-
Notifications
You must be signed in to change notification settings - Fork 463
Try to represent any logs attribute as string #1950
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?
Try to represent any logs attribute as string #1950
Conversation
| if ($value instanceof SerializableInterface) { | ||
| try { | ||
| return new self(JSON::encode($value->toSentry()), 'string'); | ||
| } catch (\Throwable $e) { | ||
| // Ignore the exception and continue trying other methods | ||
| } | ||
| } |
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.
Because we have it, I prefered this over the __toString or just raw JSON encode.
For safety we do ignore any errors to prevent crashing because serializing fails.
| // Since we support almost any type, we use a resource to trigger the exception | ||
| Attribute::fromValue(fopen(__FILE__, 'r')); |
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.
It's now a challenge to find something we can't represent as an attribute.
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.
nice! ![]()
We should try (harder) to represent values the user provides us as log attributes. So we now as last resort JSON encode the value which results in almost any value to be serialized and correctly logged. Also
nullvalues are now serialized as string instead of discarded.