Merge lp:~stub/launchpad/opstats into lp:launchpad

[Stuart Bishop]

[Stuart Bishop (stub)]

Add a new metrics to the Launchpad operational statistics, as requested by the Losas in Bug #426416.

The new metric counts the number of 5xx response codes sent to known web browsers. This will allow Losas to monitor user experience without the noise generated by robots.

[Michael Hudson-Doyle (mwhudson)]

The testing purist in me wants you to split test_is_browser into three test cases, but basically this looks fine.

review: Approve

[Francis J. Lacoste (flacoste)]

On September 15, 2009, Stuart Bishop wrote:
> +
> +
> +_browser_re = re.compile(r"""(?x)^(
> + Mozilla |
> + Opera |
> + Lynx |
> + Links |
> + w3m
> + )""")
>

Will this cover Webkit?

--
Francis J. Lacoste
<email address hidden>

[Stuart Bishop (stub)]

On Wed, Sep 16, 2009 at 8:54 PM, Francis J. Lacoste
<email address hidden> wrote:
> On September 15, 2009, Stuart Bishop wrote:
>> +
>> +
>> +_browser_re = re.compile(r"""(?x)^(
>> +    Mozilla |
>> +    Opera |
>> +    Lynx |
>> +    Links |
>> +    w3m
>> +    )""")
>>
>
> Will this cover Webkit?

I believe so. I think it is one of the vast majority that starts their
user-agent string with 'Mozilla'.

--
Stuart Bishop <email address hidden>
http://www.stuartbishop.net/

=== modified file 'lib/canonical/launchpad/pagetests/standalone/xx-opstats.txt'
--- lib/canonical/launchpad/pagetests/standalone/xx-opstats.txt	2009-02-05 20:46:59 +0000
+++ lib/canonical/launchpad/pagetests/standalone/xx-opstats.txt	2009-09-15 10:55:13 +0000
@@ -46,6 +46,7 @@
     500s: 0
     503s: 0
     5XXs: 0
+    5XXs_b: 0
     6XXs: 0
     http requests: 0
     requests: 0
@@ -108,6 +109,26 @@
     http requests: 1
     requests: 1
 
+We also have a special metric counting server errors returned to known
+web browsers (5XXs_b) - in the production environment we care more
+about errors returned to people than robots crawling obscure parts of
+the site.
+
+    >>> from textwrap import dedent
+    >>> output = http(dedent("""\
+    ...     GET /error-test HTTP/1.1
+    ...     Host: launchpad.dev
+    ...     User-Agent: Mozilla/42.0
+    ...     """))
+    >>> output.getStatus()
+    500
+    >>> report()
+    500s: 1
+    5XXs: 1
+    5XXs_b: 1
+    http requests: 1
+    requests: 1
+
 == Number of XML-RPC Faults ==
 
     >>> try:
@@ -123,7 +144,6 @@
 
 == Number of soft timeouts ==
 
-    >>> from textwrap import dedent
     >>> from canonical.config import config
     >>> test_data = dedent("""
     ...     [database]

=== modified file 'lib/canonical/launchpad/webapp/opstats.py'
--- lib/canonical/launchpad/webapp/opstats.py	2009-06-25 05:30:52 +0000
+++ lib/canonical/launchpad/webapp/opstats.py	2009-09-15 09:56:29 +0000
@@ -43,6 +43,7 @@
             '4XXs': 0, # Client Errors
             '5XXs': 0, # Server Errors
             '6XXs': 0, # Internal Errors
+            '5XXs_b': 0, # Server Errors returned to browsers (not robots).
             })
 
     def opstats(self):
@@ -51,13 +52,14 @@
         Keys currently are:
             requests -- # requests served by this appserver.
             xml-rpc requests -- # xml-rpc requests served.
-            404s -- # 404 status responses served (Not Found)
-            500s -- # 500 status responses served (Unhandled exceptions)
-            503s -- # 503 status responses served (Timeouts)
-            3XXs -- # 300-399 status responses served (Redirection)
-            4XXs -- # 400-499 status responses served (Client Errors)
-            5XXs -- # 500-599 status responses served (Server Errors)
-            6XXs -- # 600-600 status responses served (Internal Errors)
+            404s   -- 404 status responses served (Not Found)
+            500s   -- 500 status responses served (Unhandled exceptions)
+            503s   -- 503 status responses served (Timeouts)
+            3XXs   -- 300-399 status responses served (Redirection)
+            4XXs   -- 400-499 status responses served (Client Errors)
+            5XXs   -- 500-599 status responses served (Server Errors)
+            6XXs   -- 600-600 status responses served (Internal Errors)
+            5XXs_b -- As 5XXs, but returned to browsers (not robots).
         """
         return OpStats.stats
 

=== modified file 'lib/canonical/launchpad/webapp/publication.py'
--- lib/canonical/launchpad/webapp/publication.py	2009-08-06 12:23:39 +0000
+++ lib/canonical/launchpad/webapp/publication.py	2009-09-15 10:28:23 +0000
@@ -10,6 +10,7 @@
 
 import gc
 import os
+import re
 import thread
 import threading
 import traceback
@@ -596,7 +597,12 @@
                     OpStats.stats['503s'] += 1
 
                 # Increment counters for status code groups.
-                OpStats.stats[str(status)[0] + 'XXs'] += 1
+                status_group = str(status)[0] + 'XXs'
+                OpStats.stats[status_group] += 1
+
+                # Increment counter for 5XXs_b.
+                if is_browser(request) and status_group == '5XXs':
+                    OpStats.stats['5XXs_b'] += 1
 
         # Reset all Storm stores when not running the test suite. We could
         # reset them when running the test suite but that'd make writing tests
@@ -765,3 +771,30 @@
 
     def __init__(self, context):
         self.context = context
+
+
+_browser_re = re.compile(r"""(?x)^(
+    Mozilla |
+    Opera |
+    Lynx |
+    Links |
+    w3m
+    )""")
+
+def is_browser(request):
+    """Return True if we believe the request was from a browser.
+
+    There will be false positives and false negatives, as we can
+    only tell this from the User-Agent: header and this cannot be
+    trusted.
+
+    Almost all web browsers provide a User-Agent: header starting
+    with 'Mozilla'. This is good enough for our uses. We also
+    add a few other common matches as well for good measure.
+    We could massage one of the user-agent databases that are
+    available into a usable, but we would gain little.
+    """
+    user_agent = request.getHeader('User-Agent')
+    return (
+        user_agent is not None
+        and _browser_re.search(user_agent) is not None)

=== modified file 'lib/canonical/launchpad/webapp/tests/test_publication.py'
--- lib/canonical/launchpad/webapp/tests/test_publication.py	2009-08-18 08:32:15 +0000
+++ lib/canonical/launchpad/webapp/tests/test_publication.py	2009-09-15 10:28:23 +0000
@@ -23,6 +23,7 @@
 from lp.testing import TestCase, TestCaseWithFactory
 import canonical.launchpad.webapp.adapter as da
 from canonical.launchpad.webapp.interfaces import OAuthPermission
+from canonical.launchpad.webapp.publication import is_browser
 from canonical.launchpad.webapp.servers import (
     LaunchpadTestRequest, WebServicePublication)
 
@@ -93,6 +94,20 @@
         # Ensure that it is different to the last logged OOPS.
         self.assertNotEqual(repr(last_oops), repr(next_oops))
 
+    def test_is_browser(self):
+        # No User-Agent: header.
+        request = LaunchpadTestRequest()
+        self.assertFalse(is_browser(request))
+
+        # Browser User-Agent: header.
+        request = LaunchpadTestRequest(environ={
+            'USER_AGENT': 'Mozilla/42 Extreme Edition'})
+        self.assertTrue(is_browser(request))
+
+        # Robot User-Agent: header.
+        request = LaunchpadTestRequest(environ={'USER_AGENT': 'BottyBot'})
+        self.assertFalse(is_browser(request))
+
 
 def test_suite():
     suite = unittest.TestLoader().loadTestsFromName(__name__)