fix(celery): Fix RecursionError (getsentry#381)

untitaker · web-flow · commit cdad688cb75b · 2019-05-31T09:19:44.000+02:00
* fix(celery): Fix RecursionError

* fix: Add doc comment
diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py
@@ -33,11 +33,18 @@ def setup_once():
         old_build_tracer = trace.build_tracer
 
         def sentry_build_tracer(name, task, *args, **kwargs):
-            # Need to patch both methods because older celery sometimes
-            # short-circuits to task.run if it thinks it's safe.
-            task.__call__ = _wrap_task_call(task, task.__call__)
-            task.run = _wrap_task_call(task, task.run)
-            task.apply_async = _wrap_apply_async(task, task.apply_async)
+            if not getattr(task, "_sentry_is_patched", False):
+                # Need to patch both methods because older celery sometimes
+                # short-circuits to task.run if it thinks it's safe.
+                task.__call__ = _wrap_task_call(task, task.__call__)
+                task.run = _wrap_task_call(task, task.run)
+                task.apply_async = _wrap_apply_async(task, task.apply_async)
+
+                # `build_tracer` is apparently called for every task
+                # invocation. Can't wrap every celery task for every invocation
+                # or we will get infinitely nested wrapper functions.
+                task._sentry_is_patched = True
+
             return _wrap_tracer(task, old_build_tracer(name, task, *args, **kwargs))
 
         trace.build_tracer = sentry_build_tracer
diff --git a/tests/integrations/celery/test_celery.py b/tests/integrations/celery/test_celery.py
@@ -71,6 +71,7 @@ def dummy_task(x, y):
     invocation(dummy_task, 1, 0)
 
     event, = events
+
     assert event["contexts"]["trace"]["trace_id"] == span_context.trace_id
     assert event["contexts"]["trace"]["span_id"] != span_context.span_id
     assert event["transaction"] == "dummy_task"
@@ -84,6 +85,31 @@ def dummy_task(x, y):
     assert exception["stacktrace"]["frames"][0]["vars"]["foo"] == "42"
 
 
+def test_no_stackoverflows(celery):
+    """We used to have a bug in the Celery integration where its monkeypatching
+    was repeated for every task invocation, leading to stackoverflows.
+
+    See https://github.com/getsentry/sentry-python/issues/265
+    """
+
+    results = []
+
+    @celery.task(name="dummy_task")
+    def dummy_task():
+        with configure_scope() as scope:
+            scope.set_tag("foo", "bar")
+
+        results.append(42)
+
+    for _ in range(10000):
+        dummy_task.delay()
+
+    assert results == [42] * 10000
+
+    with configure_scope() as scope:
+        assert not scope._tags
+
+
 def test_simple_no_propagation(capture_events, init_celery):
     celery = init_celery(propagate_traces=False)
     events = capture_events()
