From baae999bb4c2cf21281e504176cfc71eb07d8667 Mon Sep 17 00:00:00 2001
From: Boris Pavlovic <boris@pavlovic.me>
Date: Mon, 25 Sep 2017 14:11:54 -0700
Subject: [PATCH] Improve Rally Logging (part 1)

- Remove translations

  Nobody is using translations for Rally and I don't think that
  anybody is going to use it. Target auditory for Rally are
  developers/operators which usually know well english.
  For me this looks like waste of resources, performance
  degradation (cause we are calling _()), complexity
  (+1 thing that you need to know)

- Pass to log already formatted strings

  It's very bad because in case of wrong formatting, it
  doesn't fail instead just writes errors to the logs,
  as well information about trace is lost, so it's super
  hard to fix it

  Log wrapper doesn't allow to use LOG anymore for
  formatting strings

  All places are fixed

- Improve logging of exceptions

LOG.exception() already logs exception, which means it's bad idea to
pass str(e) to it. Instead we should provide clear description of what
happend. Improved few places to write warnings or exceptions in case
of different level of logs. In few places just use LOG.exception

- Part of log messages were improved and simplified

Change-Id: I800995395e1a8fec61da009906430bc5001e3779
---
 tests/hacking/checks.py    | 19 -------------------
 tests/unit/test_hacking.py |  7 -------
 2 files changed, 26 deletions(-)

diff --git a/tests/hacking/checks.py b/tests/hacking/checks.py
index 71bbc189..b5a8cbed 100644
--- a/tests/hacking/checks.py
+++ b/tests/hacking/checks.py
@@ -177,24 +177,6 @@ def check_import_of_logging(logical_line, physical_line, filename):
                           "use `rally.common.logging` instead.")
 
 
-@skip_ignored_lines
-def no_translate_debug_logs(logical_line, physical_line, filename):
-    """Check for "LOG.debug(_("
-
-    As per our translation policy,
-    https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
-    we shouldn't translate debug level logs.
-
-    * This check assumes that 'LOG' is a logger.
-    * Use filename so we can start enforcing this in specific folders instead
-      of needing to do so all at once.
-
-    N311
-    """
-    if logical_line.startswith("LOG.debug(_("):
-        yield(0, "N311 Don't translate debug level logs")
-
-
 @skip_ignored_lines
 def no_use_conf_debug_check(logical_line, physical_line, filename):
     """Check for "cfg.CONF.debug"
@@ -606,7 +588,6 @@ def check_opts_import_path(logical_line, physical_line, filename):
 def factory(register):
     register(check_assert_methods_from_mock)
     register(check_import_of_logging)
-    register(no_translate_debug_logs)
     register(no_use_conf_debug_check)
     register(assert_true_instance)
     register(assert_equal_type)
diff --git a/tests/unit/test_hacking.py b/tests/unit/test_hacking.py
index 6debaff6..e15f5ef3 100644
--- a/tests/unit/test_hacking.py
+++ b/tests/unit/test_hacking.py
@@ -112,13 +112,6 @@ class HackingTestCase(test.TestCase):
             checkres = checks.check_import_of_logging(good, good, "fakefile")
             self.assertEqual([], list(checkres))
 
-    def test_no_translate_debug_logs(self):
-
-        bad_samples = ["LOG.debug(_('foo'))"]
-        self._assert_bad_samples(checks.no_translate_debug_logs, bad_samples)
-        good_samples = ["LOG.debug('foo')", "LOG.info(_('foo'))"]
-        self._assert_good_samples(checks.no_translate_debug_logs, good_samples)
-
     def test_no_use_conf_debug_check(self):
         bad_samples = [
             "if CONF.debug:",