diff --git a/swift/common/utils.py b/swift/common/utils.py index 80a645b5f3..386bfcf2cc 100644 --- a/swift/common/utils.py +++ b/swift/common/utils.py @@ -75,8 +75,7 @@ from swift import gettext_ as _ import swift.common.exceptions -from swift.common.http import is_success, is_redirection, HTTP_NOT_FOUND, \ - HTTP_PRECONDITION_FAILED, HTTP_REQUESTED_RANGE_NOT_SATISFIABLE +from swift.common.http import is_server_error from swift.common.header_key_dict import HeaderKeyDict from swift.common.linkat import linkat @@ -1635,24 +1634,6 @@ def transfer_rate(self, metric, elapsed_time, byte_xfer, sample_rate=None): 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 @@ -1665,7 +1646,15 @@ def decorating_func(func): def _timing_stats(ctrl, *args, **kwargs): start_time = time.time() resp = func(ctrl, *args, **kwargs) - if server_handled_successfully(resp.status_int): + # .timing is for successful responses *or* error codes that are + # not Swift's fault. 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. + # + # .errors.timing is for failures that *are* Swift's fault. + # Examples include 507 for an unmounted drive or 500 for an + # unhandled exception. + if not is_server_error(resp.status_int): ctrl.logger.timing_since(method + '.timing', start_time, **dec_kwargs) else: diff --git a/test/unit/common/test_utils.py b/test/unit/common/test_utils.py index 2355d984c3..fffe8bef5d 100644 --- a/test/unit/common/test_utils.py +++ b/test/unit/common/test_utils.py @@ -4809,6 +4809,13 @@ def METHOD(controller): self.assertEqual(mock_controller.args[0], 'METHOD.timing') self.assertTrue(mock_controller.args[1] > 0) + mock_controller = MockController(400) + METHOD(mock_controller) + self.assertEqual(len(mock_controller.args), 2) + self.assertEqual(mock_controller.called, 'timing') + self.assertEqual(mock_controller.args[0], 'METHOD.timing') + self.assertTrue(mock_controller.args[1] > 0) + mock_controller = MockController(404) METHOD(mock_controller) self.assertEqual(len(mock_controller.args), 2) @@ -4830,7 +4837,14 @@ def METHOD(controller): self.assertEqual(mock_controller.args[0], 'METHOD.timing') self.assertTrue(mock_controller.args[1] > 0) - mock_controller = MockController(401) + mock_controller = MockController(500) + METHOD(mock_controller) + self.assertEqual(len(mock_controller.args), 2) + self.assertEqual(mock_controller.called, 'timing') + self.assertEqual(mock_controller.args[0], 'METHOD.errors.timing') + self.assertTrue(mock_controller.args[1] > 0) + + mock_controller = MockController(507) METHOD(mock_controller) self.assertEqual(len(mock_controller.args), 2) self.assertEqual(mock_controller.called, 'timing')