tvalentyn commented on code in PR #22077:
URL: https://github.com/apache/beam/pull/22077#discussion_r909672769


##########
sdks/python/apache_beam/runners/worker/sdk_worker_main.py:
##########
@@ -268,6 +271,38 @@ def _get_log_level_from_options_dict(options_dict: dict) 
-> int:
   return log_level
 
 
+def _set_log_level_overrides(options_dict: dict) -> None:
+  """Set module log level overrides from options dict's entry
+  `sdk_harness_log_level_overrides`.
+  """
+  option_raw = options_dict.get('sdk_harness_log_level_overrides', None)
+
+  if option_raw is None:
+    return
+
+  parsed_overrides = {}
+
+  try:
+    # parsing and flatten the appended option
+    deserialized = [json.loads(line) for line in option_raw]
+    for line in deserialized:
+      parsed_overrides.update(line)
+  except Exception:
+    _LOGGER.error(
+        "Unable to parse sdk_harness_log_level_overrides. "

Review Comment:
   Maybe print the line that cannot be parsed here?



##########
sdks/python/apache_beam/runners/worker/sdk_worker_main.py:
##########
@@ -268,6 +271,38 @@ def _get_log_level_from_options_dict(options_dict: dict) 
-> int:
   return log_level
 
 
+def _set_log_level_overrides(options_dict: dict) -> None:
+  """Set module log level overrides from options dict's entry
+  `sdk_harness_log_level_overrides`.
+  """
+  option_raw = options_dict.get('sdk_harness_log_level_overrides', None)
+
+  if option_raw is None:
+    return
+
+  parsed_overrides = {}
+
+  try:
+    # parsing and flatten the appended option
+    deserialized = [json.loads(line) for line in option_raw]
+    for line in deserialized:
+      parsed_overrides.update(line)
+  except Exception:
+    _LOGGER.error(
+        "Unable to parse sdk_harness_log_level_overrides. "
+        "Log level overrides won't take effect.")
+    return
+
+  for module_name, log_level in parsed_overrides.items():
+    try:
+      logging.getLogger(module_name).setLevel(log_level)
+    except Exception as e:
+      # Never crash the worker when exception occurs during log level setting
+      # but logging the error.
+      _LOGGER.error(
+          "Error occurs when setting log level for %s: %s", module_name, e)

Review Comment:
   ```suggestion
             "Error occurred when setting log level for %s: %s", module_name, e)
   ```



##########
sdks/python/apache_beam/runners/worker/sdk_worker_main_test.py:
##########
@@ -121,6 +124,52 @@ def test__get_log_level_from_options_dict(self):
       self.assertEqual(
           sdk_worker_main._get_log_level_from_options_dict(case), expected)
 
+  def test__set_log_level_overrides(self):
+    test_cases = [
+
+        ([], {}), # not provided, as a smoke test
+        (
+            # single overrides
+            ['{"a.b":"DEBUG","c.d":"INFO"}'],
+            {"a.b": logging.DEBUG, "a.b.f": logging.DEBUG, "c.d": logging.INFO}
+        ),
+        (
+            # multiple overrides
+            ['{"a.b":"DEBUG"}', '{"c.d":"ERROR","c.d.e":15}'],
+            {"a.b": logging.DEBUG, "a.b.f": logging.DEBUG, "c.d": 
logging.ERROR,
+             "c.d.e": 15, "c.d.f": logging.ERROR}
+        )
+    ]
+    for case, expected in test_cases:
+      # Set log level overrides for this fake package name.
+      # This avoids pollutes real logging.

Review Comment:
   Is it to just to avoid side-effects between `single` and `multiple` 
scenarios? 



##########
sdks/python/apache_beam/runners/worker/sdk_worker_main_test.py:
##########
@@ -121,6 +124,52 @@ def test__get_log_level_from_options_dict(self):
       self.assertEqual(
           sdk_worker_main._get_log_level_from_options_dict(case), expected)
 
+  def test__set_log_level_overrides(self):
+    test_cases = [
+
+        ([], {}), # not provided, as a smoke test
+        (
+            # single overrides
+            ['{"a.b":"DEBUG","c.d":"INFO"}'],
+            {"a.b": logging.DEBUG, "a.b.f": logging.DEBUG, "c.d": logging.INFO}
+        ),
+        (
+            # multiple overrides
+            ['{"a.b":"DEBUG"}', '{"c.d":"ERROR","c.d.e":15}'],
+            {"a.b": logging.DEBUG, "a.b.f": logging.DEBUG, "c.d": 
logging.ERROR,
+             "c.d.e": 15, "c.d.f": logging.ERROR}
+        )
+    ]
+    for case, expected in test_cases:
+      # Set log level overrides for this fake package name.
+      # This avoids pollutes real logging.

Review Comment:
   ```suggestion
         # This avoids polluting real logging.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@beam.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to