Hacking: do not translate log messages

Community has decided to do not translate log messages anymore.
The motivation behind removing translation jobs for log messages
is that when operators encounters any unfamiliar situations(e.g.
nova error, keystone error etc.), searching on the Internet
based on actual log content would have more results than searching
translated log messages.

In this patch I have added hacking rule to check that logs are
not translated.

Change-Id: I90d53d617aac6839c2f2ab119847cbf24a1300e0

HACKING.rst

@@ -19,11 +19,10 @@ Masakari Specific Commandments
   assertIsInstance(A, B).
 - [M306] Change assertEqual(type(A), B) by optimal assert like
   assertIsInstance(A, B)
-- [M308] Validate that debug level logs are not translated.
+- [M308] Validate that log messages are not translated.
 - [M309] Don't import translation in tests
 - [M310] Setting CONF.* attributes directly in tests is forbidden. Use
   self.flags(option=value) instead.
-- [M314] Log messages require translations!
 - [M315] Method's default argument shouldn't be mutable
 - [M316] Ensure that the _() function is explicitly imported to ensure proper translations.
 - [M317] Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s

masakari/hacking/checks.py

@@ -85,6 +85,15 @@ spawn_re = re.compile(
 contextlib_nested = re.compile(r"^with (contextlib\.)?nested\(")
 doubled_words_re = re.compile(
     r"\b(then?|[iao]n|i[fst]|but|f?or|at|and|[dt]o)\s+\1\b")
+_all_log_levels = {'critical', 'error', 'exception', 'info',
+                   'warning', 'debug'}
+_all_hints = {'_', '_LE', '_LI', '_LW', '_LC'}
+
+log_translation_re = re.compile(
+    r".*LOG\.(%(levels)s)\(\s*(%(hints)s)\(" % {
+        'levels': '|'.join(_all_log_levels),
+        'hints': '|'.join(_all_hints),
+    })
 
 
 def no_db_session_in_public_api(logical_line, filename):
@@ -139,21 +148,18 @@ def assert_equal_type(logical_line):
         yield (0, "M306: assertEqual(type(A), B) sentences not allowed")
 
 
-def no_translate_debug_logs(logical_line, filename):
+def no_translate_logs(logical_line):
-    """Check for 'LOG.debug(_('
+    """Check for 'LOG.*(_*("'
 
-    As per our translation policy,
+    OpenStack no longer supports log translation, so we shouldn't
-    https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
+    translate logs.
-    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.
 
     M308
     """
-    if logical_line.startswith("LOG.debug(_("):
+    if log_translation_re.match(logical_line):
-        yield(0, "M308 Don't translate debug level logs")
+        yield(0, "M308: Log messages should not be translated")
 
 
 def no_import_translation_in_tests(logical_line, filename):
@@ -384,7 +390,7 @@ def factory(register):
     register(assert_true_instance)
     register(assert_equal_type)
     register(assert_raises_regexp)
-    register(no_translate_debug_logs)
+    register(no_translate_logs)
     register(no_setting_conf_directly_in_tests)
     register(no_mutable_default_args)
     register(check_explicit_underscore_import)

masakari/tests/unit/test_hacking.py

@@ -14,6 +14,7 @@
 
 import textwrap
 
+import ddt
 import mock
 import pep8
 
@@ -21,6 +22,7 @@ from masakari.hacking import checks
 from masakari import test
 
 
+@ddt.ddt
 class HackingTestCase(test.NoDBTestCase):
     """This class tests the hacking checks in masakari.hacking.checks by
     passing strings to the check methods like the pep8/flake8 parser would.
@@ -151,16 +153,6 @@ class HackingTestCase(test.NoDBTestCase):
         self.assertEqual(len(list(checks.assert_true_or_false_with_in(
             "self.assertFalse(some in list1 and some2 in list2)"))), 0)
 
-    def test_no_translate_debug_logs(self):
-        self.assertEqual(len(list(checks.no_translate_debug_logs(
-            "LOG.debug(_('foo'))", "masakari/foo.py"))), 1)
-
-        self.assertEqual(len(list(checks.no_translate_debug_logs(
-            "LOG.debug('foo')", "masakari/foo.py"))), 0)
-
-        self.assertEqual(len(list(checks.no_translate_debug_logs(
-            "LOG.info(_('foo'))", "masakari/foo.py"))), 0)
-
     def test_no_setting_conf_directly_in_tests(self):
         self.assertEqual(len(list(checks.no_setting_conf_directly_in_tests(
             "CONF.option = 1", "masakari/tests/test_foo.py"))), 1)
@@ -431,3 +423,15 @@ class HackingTestCase(test.NoDBTestCase):
                   LOG.warning("LOG.warn is deprecated")
                """
         self._assert_has_no_errors(code, checks.no_log_warn)
+
+    @ddt.data('LOG.info(_LI("Bad"))',
+              'LOG.warning(_LW("Bad"))',
+              'LOG.error(_LE("Bad"))',
+              'LOG.exception(_("Bad"))',
+              'LOG.debug(_("Bad"))',
+              'LOG.critical(_LC("Bad"))')
+    def test_no_translate_logs(self, log_statement):
+        self.assertEqual(1, len(list(checks.no_translate_logs(log_statement))))
+        errors = [(1, 0, 'M308')]
+        self._assert_has_errors(log_statement, checks.no_translate_logs,
+                                expected_errors=errors)