launchpad-reviewers team mailing list archive

[Martin Pool <mbp@xxxxxxxxxxxxx>]

You have been requested to review the proposed merge of lp:~mbp/launchpad/show-timeline into lp:launchpad.

For more details, see:
https://code.launchpad.net/~mbp/launchpad/show-timeline/+merge/80166

With this change, you can click the "%d queries/external actions" show to developers to actually see what the queries were.  An HTML form of the request timeline is revealed at the bottom of the page.  I think I would find this interesting or useful on pages that are slow but not failing, or that are intermittently oops.

I realize you can force it to oops, wait for the oops to sync, then look at it elsewhere but having the data right there seems easier and perhaps more likely to actually be inspected.  Also, because performance depends on cache hits, retrying the request to get an oops may give different behaviour.

This just renders data already collected in the timeline so it should not affect performance very much.

It's not automatically tested yet.  Pointers to where the tests ought to be added would be welcome.

I thought about highlighting the slow actions but I haven't done that either.

Clicking the link reveals the list but doesn't scroll to it; I have mixed feelings about whether it should, and in any case making the anchor the target and the javascript show it from onclick didn't seem to work on the first pageload.  Advice welcome.

screenshots:
  http://people.canonical.com/~mbp/launchpad-screenshots/20111024_006.png
  http://people.canonical.com/~mbp/launchpad-screenshots/20111024_007.png


-- 
https://code.launchpad.net/~mbp/launchpad/show-timeline/+merge/80166
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~mbp/launchpad/show-timeline into lp:launchpad.

=== modified file 'lib/canonical/launchpad/icing/style-3-0.css'
--- lib/canonical/launchpad/icing/style-3-0.css	2011-08-22 17:01:19 +0000
+++ lib/canonical/launchpad/icing/style-3-0.css	2011-10-24 00:37:40 +0000
@@ -2064,6 +2064,12 @@
     padding: 0;
     }
 
+/* ====================
+   In-page timeline
+*/
+table.debug-timeline td.amount {
+    font-weight: bold;
+}
 
 /*
  * YOU HAVE REACHED THE END OF THIS FILE. IF YOU SEE ANYTHING BELOW SPRITES

=== modified file 'lib/canonical/launchpad/webapp/adapter.py'
--- lib/canonical/launchpad/webapp/adapter.py	2011-10-11 07:09:47 +0000
+++ lib/canonical/launchpad/webapp/adapter.py	2011-10-24 00:37:40 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 # We use global in this module.
@@ -233,6 +233,11 @@
     return log
 
 
+def get_timeline_actions():
+    """Return an iterable of timeline actions"""
+    return get_request_timeline(get_current_browser_request()).actions
+
+
 def store_sql_statements_and_request_duration(event):
     actions = get_request_timeline(get_current_browser_request()).actions
     event.request.setInWSGIEnvironment(

=== modified file 'lib/lp/app/browser/tales.py'
--- lib/lp/app/browser/tales.py	2011-10-20 00:53:01 +0000
+++ lib/lp/app/browser/tales.py	2011-10-24 00:37:40 +0000
@@ -2175,6 +2175,8 @@
             return self.exactduration()
         elif name == 'approximateduration':
             return self.approximateduration()
+        elif name == 'millisecondduration':
+            return self.millisecondduration()
         else:
             raise TraversalError(name)
 
@@ -2320,6 +2322,12 @@
         weeks = int(round(seconds / (7 * 24 * 3600.0)))
         return "%d weeks" % weeks
 
+    def millisecondduration(self):
+        return str(
+            (self._duration.days * 24 * 3600
+             + self._duration.seconds * 1000
+             + self._duration.microseconds // 1000)) + 'ms'
+
 
 class LinkFormatterAPI(ObjectFormatterAPI):
     """Adapter from Link objects to a formatted anchor."""

=== modified file 'lib/lp/app/templates/base-layout.pt'
--- lib/lp/app/templates/base-layout.pt	2011-09-06 15:31:28 +0000
+++ lib/lp/app/templates/base-layout.pt	2011-10-24 00:37:40 +0000
@@ -180,9 +180,28 @@
     in scopes ${request/features/usedScopes}
 
     r${revno}
-  -->" />
+
+    -->" />
 </tal:template>
 
+<table 
+  class="debug-timeline listing"
+  tal:define="timeline_actions modules/canonical.launchpad.webapp.adapter/get_timeline_actions">
+  <thead>
+    <th>Duration</th>
+    <th>Action</th>
+  </thead>
+  <tr tal:repeat="action timeline_actions">
+    <td class="amount" tal:content="action/duration/fmt:millisecondduration"/>	  
+    <td style="font-family: monospace; text-align: left;">
+      <pre class="wrap">
+	<span class="action-category" tal:content="action/category"/>:
+	<span class="action-details" tal:content="action/detail"/>	  
+      </pre>
+    </td>
+  </tr>
+</table>
+
 <tal:comment
     tal:condition="request/features/visible_render_time"
     define="render_time modules/canonical.launchpad.webapp.adapter/summarize_requests;"
@@ -196,6 +215,7 @@
     });
   });
 &lt;/script&gt;' />
+get_request_timeline(get_current_browser_request()).start(
 
 </html>
 </metal:page>