Merge "Don't count 412 or 416 as errors in stats"
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user