-
Notifications
You must be signed in to change notification settings - Fork 532
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
Do not truncate body if request_bodies
is "always"
#2092
Conversation
f2783c5
to
a847f33
Compare
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.
lgtm! please test a bit with an app with errors and tranasctions just in case because the serializer is fairly magical and is a core code path
if is_databag and remaining_breadth is None: | ||
remaining_breadth = MAX_DATABAG_BREADTH | ||
if is_request_body is None: | ||
is_request_body = _is_request_body() |
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.
is this needed? not sure if it ever is true if it's already not true above?
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.
The idea was to distinguish between something being a request body vs. any other type of databag. Basically so that if request_bodies
is always
, we can exclude actual request bodies from being trimmed, but any other databags like e.g. breadcrumbs
will still get trimmed. Not sure if this is something we actually want though. We could just as well trim all kinds of databags.
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.
yes that I get, but I wasn't able to see where this code branch is triggered.
But now I see that is_request_body
is never passed into any of the _serialize_node
calls, so either can you remove is_request_body
as a function arg or maybe pass it in?
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.
Oh yeah, thanks for spotting that
We've received quite some feedback regarding our automatic request body trimming making it harder to debug issues. This PR changes the behavior of our serializer so that we don't apply our
MAX_DATABAG_BREADTH
andMAX_DATABAG_DEPTH
limits if the SDK has been initialized withrequest_bodies="always"
.Fixes #1041