-
Notifications
You must be signed in to change notification settings - Fork 117
Fix kombu crash when using sentry & setting parsing #1359
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
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1359 +/- ##
==========================================
+ Coverage 79.46% 79.90% +0.44%
==========================================
Files 204 204
Lines 22832 22842 +10
Branches 3618 3618
==========================================
+ Hits 18144 18253 +109
+ Misses 3500 3392 -108
- Partials 1188 1197 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
def wrap_Producer_publish(wrapped, instance, args, kwargs): | ||
transaction = current_transaction() | ||
|
||
if transaction is None: | ||
return wrapped(*args, **kwargs) | ||
|
||
bound_args = bind_args(wrapped, args, kwargs) | ||
bound_args = bind_publish(*args, **kwargs) |
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.
bound_args = bind_publish(*args, **kwargs) | |
try: | |
bound_args = bind_publish(*args, **kwargs) | |
except Exception: | |
bound_args = None | |
if not bound_args: | |
return wrapped(*args, **kwargs) |
I'm worried this will crash on other versions of Kombu since we only test the latest. Can we wrap it in a try/except clause like so?
This could use an exception message logged to debug as well.
5ca3e3f
to
164015e
Compare
Overview
Because Sentry applies a proxy wrapper around the
Producer.publish
method, when we tried to apply thebind_args
, it did not pickup the wrapped signature correctly-the instance was missing and so the value for thebody
was set as the value ofself
. Instead use a hardcoded bind to avoid this issue.Fix missing parsing of config file instrumentation settings.
This is a fix for #1347.