Launchpad itself

Merge lp:~stub/launchpad/page-performance-report into lp:launchpad

  1. page-performance-report
  2. Merge into devel
Proposed by Stuart Bishop
Status: Merged
Approved by: Stuart Bishop
Approved revision: no longer in the source branch.
Merge reported by: Stuart Bishop
Merged at revision: not available
Proposed branch: lp:~stub/launchpad/page-performance-report
Merge into: lp:launchpad
Diff against target: 140 lines (+50/-14)
2 files modified
lib/canonical/launchpad/webapp/publication.py (+20/-6)
lib/lp/scripts/utilities/pageperformancereport.py (+30/-8)
To merge this branch: bzr merge lp:~stub/launchpad/page-performance-report
Reviewer Review Type Date Requested Status
Curtis Hovey (community) release-critical Approve
Stuart Bishop (community) Abstain
Jeroen T. Vermeulen (community) code Approve
Review via email: mp+26017@code.launchpad.net

Commit message

Emit the pageid to the ZServer tracelogs.

Description of the change

Emit the pageid to the ZServer tracelogs.

We will use this to improve our page performance reports. The report itself will not land this cycle, but it would be good to get the information into the production logs so we have real data to test the improved report against.

To post a comment you must log in.
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Fixed some things interactively: removed a redundant comment, extracted a function so as to keep the surrounding if/elif block clear. Thanks for those, and the drive-by HTML cleanup.

Next thing to do for this is a bit of testing.

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Since this is ongoing work, not operational yet, and some parts of it need to be landed I'm okay with landing this as a whole. The two conditions are that (1) you ensure that things that used to work don't suddenly blow up, and (2) there be a bug about the missing tests.

review: Approve (code)
Revision history for this message
Stuart Bishop (stub) :
review: Abstain
Revision history for this message
Stuart Bishop (stub) wrote :

This should have landed a few hours before PQM closed, but ec2 silently ate the branch (twice!)

Revision history for this message
Curtis Hovey (sinzui) wrote :

I am sorry that I missed your original request. You have my RC to land this.

review: Approve (release-critical)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/webapp/publication.py'
2--- lib/canonical/launchpad/webapp/publication.py 2010-04-14 07:38:55 +0000
3+++ lib/canonical/launchpad/webapp/publication.py 2010-05-26 10:08:35 +0000
4@@ -413,8 +413,12 @@
5 # The view name used in the pageid usually comes from ZCML and so
6 # it will be a unicode string although it shouldn't. To avoid
7 # problems we encode it into ASCII.
8- request.setInWSGIEnvironment(
9- 'launchpad.pageid', pageid.encode('ASCII'))
10+ pageid = pageid.encode('US-ASCII')
11+
12+ request.setInWSGIEnvironment('launchpad.pageid', pageid)
13+
14+ # And spit the pageid out to our tracelog.
15+ tracelog(request, 'p', pageid)
16
17 if isinstance(removeSecurityProxy(ob), METHOD_WRAPPER_TYPE):
18 # this is a direct call on a C-defined method such as __repr__ or
19@@ -435,7 +439,6 @@
20 Because of this we cannot chain to the superclass and implement
21 the whole behaviour here.
22 """
23- orig_env = request._orig_env
24 assert hasattr(request, '_publicationticks_start'), (
25 'request._publicationticks_start, which should have been set by '
26 'callObject(), was not found.')
27@@ -497,9 +500,7 @@
28 _maybePlacefullyAuthenticate.
29 """
30 # Log the URL including vhost information to the ZServer tracelog.
31- tracelog = ITraceLog(request, None)
32- if tracelog is not None:
33- tracelog.log(request.getURL())
34+ tracelog(request, 'u', request.getURL())
35
36 assert hasattr(request, '_traversalticks_start'), (
37 'request._traversalticks_start, which should have been set by '
38@@ -788,3 +789,16 @@
39 return (
40 user_agent is not None
41 and _browser_re.search(user_agent) is not None)
42+
43+
44+def tracelog(request, prefix, msg):
45+ """Emit a message to the ITraceLog, or do nothing if there is none.
46+
47+ The message will be prefixed by ``prefix`` to make writing parsers
48+ easier. ``prefix`` should be unique and contain no spaces, and
49+ preferably a single character to save space.
50+ """
51+ tracelog = ITraceLog(request, None)
52+ if tracelog is not None:
53+ tracelog.log('%s %s' % (prefix, msg.encode('US-ASCII')))
54+
55
56=== modified file 'lib/lp/scripts/utilities/pageperformancereport.py'
57--- lib/lp/scripts/utilities/pageperformancereport.py 2010-02-03 14:52:44 +0000
58+++ lib/lp/scripts/utilities/pageperformancereport.py 2010-05-26 10:08:35 +0000
59@@ -19,7 +19,7 @@
60
61 import numpy
62 import simplejson as json
63-from zc.zservertracelog.tracereport import Request, parsedt
64+from zc.zservertracelog.tracereport import Request
65
66 from canonical.config import config
67 from canonical.launchpad.scripts.logger import log
68@@ -193,7 +193,7 @@
69 for line in smart_open(tracefile):
70 line = line.rstrip()
71 try:
72- record = line.split(' ',7)
73+ record = line.split(' ', 7)
74 try:
75 record_type, request_id, date, time_ = record[:4]
76 except ValueError:
77@@ -232,13 +232,20 @@
78 if request is None: # Just ignore partial records.
79 continue
80
81- if record_type == '-': # Extension record from Launchpad.
82- # Launchpad outputs the full URL to the tracelog,
83- # including protocol & hostname. Use this in favor of
84- # the ZServer logged path.
85- require_args(1)
86+ # Old stype extension record from Launchpad. Just
87+ # contains the URL.
88+ if record_type == '-' and len(args) == 1:
89 request.url = args[0]
90
91+ # New style extension record with a prefix.
92+ elif record_type == '-':
93+ # Launchpad outputs several things as tracelog
94+ # extension records. We include a prefix to tell
95+ # them apart.
96+ require_args(2)
97+
98+ parse_extension_record(request, args)
99+
100 elif record_type == 'I': # Got request input.
101 require_args(1)
102 request.I(dt, args[0])
103@@ -266,6 +273,18 @@
104 "Malformed line %s %s (%s)" % (repr(line), repr(args), x))
105
106
107+def parse_extension_record(self, request, args):
108+ """Decode a ZServer extension records and annotate request."""
109+ prefix = args[0]
110+ if prefix == 'u':
111+ request.url = args[1]
112+ elif prefix == 'p':
113+ request.pageid = args[1]
114+ else:
115+ raise MalformedLine(
116+ "Unknown extension prefix %s" % prefix)
117+
118+
119 def print_html_report(categories):
120
121 print dedent('''\
122@@ -318,7 +337,7 @@
123 histograms.append(histogram)
124 print dedent("""\
125 <tr class="%s">
126- <th class="category-title">%s <div class="regexp">%s</span></th>
127+ <th class="category-title">%s <span class="regexp">%s</span></th>
128 <td class="mean">%.2f s</td>
129 <td class="median">%.2f s</td>
130 <td class="standard-deviation">%.2f s</td>
131@@ -331,6 +350,9 @@
132 html_quote(category.title), html_quote(category.regexp),
133 mean, median, standard_deviation, i))
134
135+ print "</tbody></table>"
136+
137+
138 print dedent("""\
139 <script language="javascript" type="text/javascript">
140 $(function () {