feat: New function Client.flush (getsentry#250)

untitaker · web-flow · commit 1c41dfbfcdb3 · 2019-02-13T19:49:11.000+01:00
* feat: New function Client.flush * fix: Fix tests * fix: Fix rq tests * fix: Fix AWS tests * fix: Flush behavior * fix: Remove unused import * Revert "fix: Flush behavior" This reverts commit 1ca2546. * Revert "fix: Remove unused import" This reverts commit 783592c.
diff --git a/sentry_sdk/client.py b/sentry_sdk/client.py
@@ -197,20 +197,29 @@ def capture_event(self, event, hint=None, scope=None):
         self.transport.capture_event(event)
         return rv
 
-    def close(self, timeout=None, shutdown_callback=None):
-        """Closes the client which shuts down the transport in an
-        orderly manner.
-
-        The `shutdown_callback` is invoked with two arguments: the number of
-        pending events and the configured shutdown timeout.  For instance the
-        default atexit integration will use this to render out a message on
-        stderr.
+    def close(self, timeout=None, callback=None):
+        """
+        Close the client and shut down the transport. Arguments have the same
+        semantics as `self.flush()`.
+        """
+        if self.transport is not None:
+            self.flush(timeout=timeout, callback=callback)
+            self.transport.kill()
+            self.transport = None
+
+    def flush(self, timeout=None, callback=None):
+        """
+        Wait `timeout` seconds for the current events to be sent. If no
+        `timeout` is provided, the `shutdown_timeout` option value is used.
+
+        The `callback` is invoked with two arguments: the number of pending
+        events and the configured timeout.  For instance the default atexit
+        integration will use this to render out a message on stderr.
         """
         if self.transport is not None:
             if timeout is None:
                 timeout = self.options["shutdown_timeout"]
-            self.transport.shutdown(timeout=timeout, callback=shutdown_callback)
-            self.transport = None
+            self.transport.flush(timeout=timeout, callback=callback)
 
     def __enter__(self):
         return self
diff --git a/sentry_sdk/integrations/atexit.py b/sentry_sdk/integrations/atexit.py
@@ -9,7 +9,7 @@
 from sentry_sdk.integrations import Integration
 
 
-def default_shutdown_callback(pending, timeout):
+def default_callback(pending, timeout):
     """This is the default shutdown callback that is set on the options.
     It prints out a message to stderr that informs the user that some events
     are still pending and the process is waiting for them to flush out.
@@ -29,7 +29,7 @@ class AtexitIntegration(Integration):
 
     def __init__(self, callback=None):
         if callback is None:
-            callback = default_shutdown_callback
+            callback = default_callback
         self.callback = callback
 
     @staticmethod
@@ -41,4 +41,4 @@ def _shutdown():
             integration = hub.get_integration(AtexitIntegration)
             if integration is not None:
                 logger.debug("atexit: shutting down client")
-                hub.client.close(shutdown_callback=integration.callback)
+                hub.client.close(callback=integration.callback)
diff --git a/sentry_sdk/integrations/aws_lambda.py b/sentry_sdk/integrations/aws_lambda.py
@@ -45,11 +45,8 @@ def _drain_queue():
         integration = hub.get_integration(AwsLambdaIntegration)
         if integration is not None:
             # Flush out the event queue before AWS kills the
-            # process. This is not threadsafe.
-            # make new transport with empty queue
-            new_transport = hub.client.transport.copy()
-            hub.client.close()
-            hub.client.transport = new_transport
+            # process.
+            hub.client.flush()
 
 
 class AwsLambdaIntegration(Integration):
diff --git a/sentry_sdk/integrations/rq.py b/sentry_sdk/integrations/rq.py
@@ -33,10 +33,7 @@ def sentry_patched_perform_job(self, job, *args, **kwargs):
                 # We're inside of a forked process and RQ is
                 # about to call `os._exit`. Make sure that our
                 # events get sent out.
-                #
-                # Closing the client should not affect other jobs since
-                # we're in a different process
-                hub.client.close()
+                hub.client.flush()
 
             return rv
 
diff --git a/sentry_sdk/transport.py b/sentry_sdk/transport.py
@@ -37,27 +37,14 @@ def capture_event(self, event):
         """
         raise NotImplementedError()
 
-    def shutdown(self, timeout, callback=None):
-        """Initiates a controlled shutdown that should flush out pending
-        events.  The callback must be invoked with the number of pending
-        events and the timeout if the shutting down would take some period
-        of time (eg: not instant).
-        """
-        self.kill()
+    def flush(self, timeout, callback=None):
+        """Wait `timeout` seconds for the current events to be sent out."""
+        pass
 
     def kill(self):
         """Forcefully kills the transport."""
         pass
 
-    def copy(self):
-        """Copy the transport.
-
-        The returned transport should behave completely independent from the
-        previous one.  It still may share HTTP connection pools, but not share
-        any state such as internal queues.
-        """
-        return self
-
     def __del__(self):
         try:
             self.kill()
@@ -161,22 +148,15 @@ def send_event_wrapper():
 
         self._worker.submit(send_event_wrapper)
 
-    def shutdown(self, timeout, callback=None):
-        logger.debug("Shutting down HTTP transport orderly")
-        if timeout <= 0:
-            self._worker.kill()
-        else:
-            self._worker.shutdown(timeout, callback)
+    def flush(self, timeout, callback=None):
+        logger.debug("Flushing HTTP transport")
+        if timeout > 0:
+            self._worker.flush(timeout, callback)
 
     def kill(self):
         logger.debug("Killing HTTP transport")
         self._worker.kill()
 
-    def copy(self):
-        transport = type(self)(self.options)
-        transport._pool = self._pool
-        return transport
-
 
 class _FunctionTransport(Transport):
     def __init__(self, func):
diff --git a/sentry_sdk/worker.py b/sentry_sdk/worker.py
@@ -59,22 +59,18 @@ def kill(self):
                 self._thread = None
                 self._thread_for_pid = None
 
-    def shutdown(self, timeout, callback=None):
-        logger.debug("background worker got shutdown request")
+    def flush(self, timeout, callback=None):
+        logger.debug("background worker got flush request")
         with self._lock:
-            if self.is_alive:
-                self._queue.put_nowait(_TERMINATOR)
-                if timeout > 0.0:
-                    self._wait_shutdown(timeout, callback)
-            self._thread = None
-            self._thread_for_pid = None
-        logger.debug("background worker shut down")
+            if self.is_alive and timeout > 0.0:
+                self._wait_flush(timeout, callback)
+        logger.debug("background worker flushed")
 
-    def _wait_shutdown(self, timeout, callback):
+    def _wait_flush(self, timeout, callback):
         initial_timeout = min(0.1, timeout)
         if not self._timed_queue_join(initial_timeout):
             pending = self._queue.qsize()
-            logger.debug("%d event(s) pending on shutdown", pending)
+            logger.debug("%d event(s) pending on flush", pending)
             if callback is not None:
                 callback(pending, timeout)
             self._timed_queue_join(timeout - initial_timeout)
diff --git a/tests/integrations/aws_lambda/test_aws.py b/tests/integrations/aws_lambda/test_aws.py
@@ -27,7 +27,7 @@ def __init__(self):
     def capture_event(self, event):
         self._queue.append(event)
 
-    def shutdown(self, timeout, callback=None):
+    def flush(self, timeout, callback=None):
         # Delay event output like this to test proper shutdown
         # Note that AWS Lambda trunchates the log output to 4kb, so you better
         # pray that your events are smaller than that or else tests start
diff --git a/tests/integrations/rq/test_rq.py b/tests/integrations/rq/test_rq.py
@@ -51,11 +51,11 @@ def capture_event(event):
         events_w.write(json.dumps(event).encode("utf-8"))
         events_w.write(b"\n")
 
-    def shutdown(timeout, callback=None):
-        events_w.write(b"shutdown\n")
+    def flush(timeout=None, callback=None):
+        events_w.write(b"flush\n")
 
     Hub.current.client.transport.capture_event = capture_event
-    Hub.current.client.transport.shutdown = shutdown
+    Hub.current.client.flush = flush
 
     queue = rq.Queue(connection=FakeStrictRedis())
     worker = rq.Worker([queue], connection=queue.connection)
@@ -64,9 +64,8 @@ def shutdown(timeout, callback=None):
     worker.work(burst=True)
 
     event = events_r.readline()
-    shutdown = events_r.readline()
     event = json.loads(event.decode("utf-8"))
-    assert shutdown == b"shutdown\n"
-
     exception, = event["exception"]["values"]
     assert exception["type"] == "ZeroDivisionError"
+
+    assert events_r.readline() == b"flush\n"
diff --git a/tox.ini b/tox.ini
@@ -32,7 +32,7 @@ envlist =
     {pypy,py2.7,py3.5,py3.6,py3.7,py3.8}-pyramid-{1.3,1.4,1.5,1.6,1.7,1.8,1.9}
 
     {pypy,py2.7,py3.5,py3.6}-rq-{0.6,0.7,0.8,0.9,0.10,0.11}
-    {pypy,py2.7,py3.5,py3.6,py3.7,py3.8}-rq-0.12
+    {pypy,py2.7,py3.5,py3.6,py3.7,py3.8}-rq-{0.12,0.13}
 
     py3.7-aiohttp
     {py3.7,py3.8}-tornado-{5,6}
