Don't count 412 or 416 as errors in stats

The backend HTTP servers emit StatsD metrics of the form
<server>.<method>.timing and <server>.<method>.errors.timing
(e.g. object-server.GET.timing). Whether something counts as an error
or not is based on the response's HTTP status code.

Prior to this commit, "success" was 2xx, 3xx, or 404, while everything
else was considered "error". This adds 412 and 416 to the "success"
set. Like 404, these status codes indicate that we got the request and
processed it without error, but the response was "no". They shouldn't
be in the same category as statuses like 507 that indicate something
stopped the request from being processed.

Change-Id: I5582a51cf6f64aa22c890da01aaaa077f3a54202

swift/common/utils.py

@@ -64,7 +64,9 @@ utf8_encoder = codecs.getencoder('utf-8')
 
 from swift import gettext_ as _
 import swift.common.exceptions
-from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND
+from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND, \
+    HTTP_PRECONDITION_FAILED, HTTP_REQUESTED_RANGE_NOT_SATISFIABLE
+
 
 # logging doesn't import patched as cleanly as one would like
 from logging.handlers import SysLogHandler
@@ -989,6 +991,24 @@ class StatsdClient(object):
                                sample_rate)
 
 
+def server_handled_successfully(status_int):
+    """
+    True for successful responses *or* error codes that are not Swift's fault,
+    False otherwise. For example, 500 is definitely the server's fault, but
+    412 is an error code (4xx are all errors) that is due to a header the
+    client sent.
+
+    If one is tracking error rates to monitor server health, one would be
+    advised to use a function like this one, lest a client cause a flurry of
+    404s or 416s and make a spurious spike in your errors graph.
+    """
+    return (is_success(status_int) or
+            is_redirection(status_int) or
+            status_int == HTTP_NOT_FOUND or
+            status_int == HTTP_PRECONDITION_FAILED or
+            status_int == HTTP_REQUESTED_RANGE_NOT_SATISFIABLE)
+
+
 def timing_stats(**dec_kwargs):
     """
     Returns a decorator that logs timing events or errors for public methods in
@@ -1001,9 +1021,7 @@ def timing_stats(**dec_kwargs):
         def _timing_stats(ctrl, *args, **kwargs):
             start_time = time.time()
             resp = func(ctrl, *args, **kwargs)
-            if is_success(resp.status_int) or \
-                    is_redirection(resp.status_int) or \
-                    resp.status_int == HTTP_NOT_FOUND:
+            if server_handled_successfully(resp.status_int):
                 ctrl.logger.timing_since(method + '.timing',
                                          start_time, **dec_kwargs)
             else:

test/unit/common/test_utils.py

@@ -3050,6 +3050,20 @@ class TestStatsdLogging(unittest.TestCase):
         self.assertEquals(mock_controller.args[0], 'METHOD.timing')
         self.assert_(mock_controller.args[1] > 0)
 
+        mock_controller = MockController(412)
+        METHOD(mock_controller)
+        self.assertEquals(len(mock_controller.args), 2)
+        self.assertEquals(mock_controller.called, 'timing')
+        self.assertEquals(mock_controller.args[0], 'METHOD.timing')
+        self.assert_(mock_controller.args[1] > 0)
+
+        mock_controller = MockController(416)
+        METHOD(mock_controller)
+        self.assertEquals(len(mock_controller.args), 2)
+        self.assertEquals(mock_controller.called, 'timing')
+        self.assertEquals(mock_controller.args[0], 'METHOD.timing')
+        self.assert_(mock_controller.args[1] > 0)
+
         mock_controller = MockController(401)
         METHOD(mock_controller)
         self.assertEquals(len(mock_controller.args), 2)