launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #25352
[Merge] ~twom/launchpad:stats-fix-pageid-doctests into launchpad:master
Tom Wardill has proposed merging ~twom/launchpad:stats-fix-pageid-doctests into launchpad:master.
Commit message:
Fix hotpath, fix pageids
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~twom/launchpad/+git/launchpad/+merge/391202
Stop always generating the pageid in callObject.
Also fix the pageids so they are valid in the statsd line protocol.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~twom/launchpad:stats-fix-pageid-doctests into launchpad:master.
diff --git a/configs/development/launchpad-lazr.conf b/configs/development/launchpad-lazr.conf
index a4956f3..7a75c9e 100644
--- a/configs/development/launchpad-lazr.conf
+++ b/configs/development/launchpad-lazr.conf
@@ -233,3 +233,8 @@ hostname: feeds.launchpad.test
# so disable that here. note that the testrunner config inherits
# this setting from us.
send_email: false
+
+[statsd]
+host: 127.0.0.1
+port: 8125
+prefix: lp.development
diff --git a/lib/lp/services/webapp/publication.py b/lib/lp/services/webapp/publication.py
index 5c3e473..eb65fc8 100644
--- a/lib/lp/services/webapp/publication.py
+++ b/lib/lp/services/webapp/publication.py
@@ -430,9 +430,9 @@ class LaunchpadBrowserPublication(
# pageid is calculated at `afterTraversal`, but can be missing
# if callObject is used directly, so either use the one we've got
# or work it out.
- pageid = request._orig_env.get(
- 'launchpad.pageid',
- self._setPageIDInEnvironment(request, ob))
+ pageid = request._orig_env.get('launchpad.pageid')
+ if not pageid:
+ self._setPageIDInEnvironment(request, ob)
# And spit the pageid out to our tracelog.
tracelog(request, 'p', pageid)
@@ -487,7 +487,8 @@ class LaunchpadBrowserPublication(
# Update statsd, timing is in milliseconds
getUtility(IStatsdClient).timing(
'publication_duration,success=True,pageid={}'.format(
- request._orig_env.get('launchpad.pageid')),
+ self._prepPageIDForMetrics(
+ request._orig_env.get('launchpad.pageid'))),
publication_duration * 1000)
# Calculate SQL statement statistics.
@@ -557,6 +558,7 @@ class LaunchpadBrowserPublication(
notify(BeforeTraverseEvent(ob, request))
def _setPageIDInEnvironment(self, request, view):
+ """Set the page ID in the WSGI environment."""
# The view may be security proxied
view = removeSecurityProxy(view)
# It's possible that the view is a bound method.
@@ -566,6 +568,10 @@ class LaunchpadBrowserPublication(
request.setInWSGIEnvironment('launchpad.pageid', pageid)
return pageid
+ def _prepPageIDForMetrics(self, pageid):
+ """Remove characters from PageID that can't be used in statsd."""
+ return re.sub('[^0-9a-zA-Z]+', '-', pageid)
+
def afterTraversal(self, request, ob):
"""See zope.publisher.interfaces.IPublication.
@@ -586,7 +592,8 @@ class LaunchpadBrowserPublication(
'launchpad.traversalduration', traversal_duration)
# Update statsd, timing is in milliseconds
getUtility(IStatsdClient).timing(
- 'traversal_duration,success=True,pageid={}'.format(pageid),
+ 'traversal_duration,success=True,pageid={}'.format(
+ self._prepPageIDForMetrics(pageid)),
traversal_duration * 1000)
if request._traversal_thread_start is not None:
traversal_thread_duration = (
@@ -623,7 +630,8 @@ class LaunchpadBrowserPublication(
# Update statsd, timing is in milliseconds
getUtility(IStatsdClient).timing(
'publication_duration,success=False,pageid={}'.format(
- request._orig_env.get('launchpad.pageid')),
+ self._prepPageIDForMetrics(
+ request._orig_env.get('launchpad.pageid'))),
publication_duration * 1000)
if thread_now is not None:
publication_thread_duration = (
@@ -640,7 +648,8 @@ class LaunchpadBrowserPublication(
# Update statsd, timing is in milliseconds
getUtility(IStatsdClient).timing(
'traversal_duration,success=False,pageid={}'.format(
- request._orig_env.get('launchpad.pageid')
+ self._prepPageIDForMetrics(
+ request._orig_env.get('launchpad.pageid'))
), traversal_duration * 1000)
if thread_now is not None:
traversal_thread_duration = (