launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:py3-publicationthreadduration into launchpad:master.

Commit message:
Fix setting of launchpad.publicationthreadduration on Python 3

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/396860

We forgot to set it in LaunchpadBrowserPublication.afterCall.

While we're here, rearrange the other settings of launchpad.{traversal,publication}threadduration so that they immediately follow the corresponding settings of launchpad.{traversal,publication}duration rather than having a statsd call in between, to make it (at least in my mind) a little easier to follow.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:py3-publicationthreadduration into launchpad:master.

diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 7325d9f..c7f8d7c 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -489,6 +489,10 @@ class LaunchpadBrowserPublication(
             publication_thread_duration = None
         request.setInWSGIEnvironment(
             'launchpad.publicationduration', publication_duration)
+        if publication_thread_duration is not None:
+            request.setInWSGIEnvironment(
+                'launchpad.publicationthreadduration',
+                publication_thread_duration)
         # Update statsd, timing is in milliseconds
         getUtility(IStatsdClient).timing(
             'publication_duration,success=True,pageid={}'.format(
@@ -600,16 +604,16 @@ class LaunchpadBrowserPublication(
         traversal_duration = time.time() - request._traversal_start
         request.setInWSGIEnvironment(
             'launchpad.traversalduration', traversal_duration)
-        # Update statsd, timing is in milliseconds
-        getUtility(IStatsdClient).timing(
-            'traversal_duration,success=True,pageid={}'.format(
-                self._prepPageIDForMetrics(pageid)),
-            traversal_duration * 1000)
         if request._traversal_thread_start is not None:
             traversal_thread_duration = (
                 _get_thread_time() - request._traversal_thread_start)
             request.setInWSGIEnvironment(
                 'launchpad.traversalthreadduration', traversal_thread_duration)
+        # Update statsd, timing is in milliseconds
+        getUtility(IStatsdClient).timing(
+            'traversal_duration,success=True,pageid={}'.format(
+                self._prepPageIDForMetrics(pageid)),
+            traversal_duration * 1000)
 
     def _maybePlacefullyAuthenticate(self, request, ob):
         """ This should never be called because we've excised it in
@@ -637,36 +641,36 @@ class LaunchpadBrowserPublication(
             publication_duration = now - request._publication_start
             request.setInWSGIEnvironment(
                 'launchpad.publicationduration', publication_duration)
-            # Update statsd, timing is in milliseconds
-            getUtility(IStatsdClient).timing(
-                'publication_duration,success=False,pageid={}'.format(
-                    self._prepPageIDForMetrics(
-                        request._orig_env.get('launchpad.pageid'))),
-                publication_duration * 1000)
             if thread_now is not None:
                 publication_thread_duration = (
                     thread_now - request._publication_thread_start)
                 request.setInWSGIEnvironment(
                     'launchpad.publicationthreadduration',
                     publication_thread_duration)
+            # Update statsd, timing is in milliseconds
+            getUtility(IStatsdClient).timing(
+                'publication_duration,success=False,pageid={}'.format(
+                    self._prepPageIDForMetrics(
+                        request._orig_env.get('launchpad.pageid'))),
+                publication_duration * 1000)
         elif (hasattr(request, '_traversal_start') and
               ('launchpad.traversalduration' not in orig_env)):
             # The traversal process has been started but hasn't completed.
             traversal_duration = now - request._traversal_start
             request.setInWSGIEnvironment(
                 'launchpad.traversalduration', traversal_duration)
-            # Update statsd, timing is in milliseconds
-            getUtility(IStatsdClient).timing(
-                'traversal_duration,success=False,pageid={}'.format(
-                    self._prepPageIDForMetrics(
-                        request._orig_env.get('launchpad.pageid'))
-                ), traversal_duration * 1000)
             if thread_now is not None:
                 traversal_thread_duration = (
                     thread_now - request._traversal_thread_start)
                 request.setInWSGIEnvironment(
                     'launchpad.traversalthreadduration',
                     traversal_thread_duration)
+            # Update statsd, timing is in milliseconds
+            getUtility(IStatsdClient).timing(
+                'traversal_duration,success=False,pageid={}'.format(
+                    self._prepPageIDForMetrics(
+                        request._orig_env.get('launchpad.pageid'))
+                ), traversal_duration * 1000)
         else:
             # The exception wasn't raised in the middle of the traversal nor
             # the publication, so there's nothing we need to do here.