refactor: make requested changes

Changes:
* Add missing comments and docs
* formatter - Group the kwargs code
* logger - convert back to class static methods

Author: michaelbrewer

aws_lambda_powertools/logging/formatter.py

@@ -20,18 +20,23 @@ def __init__(self, **kwargs):
         unserializable values.  It must not throw.  Defaults to a function that
         coerces the value to a string.
 
+        The `log_record_order` kwarg is used to specify the order of the keys used in
+        the structured json logs. By default the order is: "level", "location", "message", "timestamp",
+        "service" and "sampling_rate".
+
         Other kwargs are used to specify log field format strings.
         """
+        # Set the default unserializable function, by default values will be cast as str.
         self.default_json_formatter = kwargs.pop("json_default", str)
-        datefmt = kwargs.pop("datefmt", None)
+        # Set the insertion order for the log messages
+        self.format_dict = dict.fromkeys(kwargs.pop("log_record_order", ["level", "location", "message", "timestamp"]))
+        # Set timestamp the date format
+        super(JsonFormatter, self).__init__(datefmt=kwargs.pop("datefmt", None))
 
-        super(JsonFormatter, self).__init__(datefmt=datefmt)
         self.reserved_keys = ["timestamp", "level", "location"]
-        self.format_dict = dict.fromkeys(kwargs.pop("format_keys", ["level", "location", "message", "timestamp"]))
         self.format_dict.update(
-            {"level": "%(levelname)s", "location": "%(funcName)s:%(lineno)d", "timestamp": "%(asctime)s"}
+            {"level": "%(levelname)s", "location": "%(funcName)s:%(lineno)d", "timestamp": "%(asctime)s", **kwargs}
         )
-        self.format_dict.update(kwargs)
 
     def update_formatter(self, **kwargs):
         self.format_dict.update(kwargs)

aws_lambda_powertools/logging/logger.py

@@ -123,7 +123,7 @@ def __init__(
     ):
         self.service = service or os.getenv("POWERTOOLS_SERVICE_NAME") or "service_undefined"
         self.sampling_rate = sampling_rate or os.getenv("POWERTOOLS_LOGGER_SAMPLE_RATE") or 0.0
-        self.log_level = _get_log_level(level)
+        self.log_level = self._get_log_level(level)
         self.child = child
         self._handler = logging.StreamHandler(stream) if stream is not None else logging.StreamHandler(sys.stdout)
         self._default_log_keys = {"service": self.service, "sampling_rate": self.sampling_rate}
@@ -140,7 +140,7 @@ def _get_logger(self):
         """ Returns a Logger named {self.service}, or {self.service.filename} for child loggers"""
         logger_name = self.service
         if self.child:
-            logger_name = f"{self.service}.{_get_caller_filename()}"
+            logger_name = f"{self.service}.{self._get_caller_filename()}"
 
         return logging.getLogger(logger_name)
 
@@ -272,28 +272,28 @@ def structure_logs(self, append: bool = False, **kwargs):
                 # Set a new formatter for a logger handler
                 handler.setFormatter(JsonFormatter(**self._default_log_keys, **kwargs))
 
+    @staticmethod
+    def _get_log_level(level: Union[str, int]) -> Union[str, int]:
+        """ Returns preferred log level set by the customer in upper case """
+        if isinstance(level, int):
+            return level
 
-def _get_log_level(level: Union[str, int]) -> Union[str, int]:
-    """ Returns preferred log level set by the customer in upper case """
-    if isinstance(level, int):
-        return level
+        log_level: str = level or os.getenv("LOG_LEVEL")
+        log_level = log_level.upper() if log_level is not None else logging.INFO
 
-    log_level: str = level or os.getenv("LOG_LEVEL")
-    log_level = log_level.upper() if log_level is not None else logging.INFO
+        return log_level
 
-    return log_level
+    @staticmethod
+    def _get_caller_filename():
+        """ Return caller filename by finding the caller frame """
+        # Current frame         => _get_logger()
+        # Previous frame        => logger.py
+        # Before previous frame => Caller
+        frame = inspect.currentframe()
+        caller_frame = frame.f_back.f_back.f_back
+        filename = caller_frame.f_globals["__name__"]
 
-
-def _get_caller_filename():
-    """ Return caller filename by finding the caller frame """
-    # Current frame         => _get_logger()
-    # Previous frame        => logger.py
-    # Before previous frame => Caller
-    frame = inspect.currentframe()
-    caller_frame = frame.f_back.f_back.f_back
-    filename = caller_frame.f_globals["__name__"]
-
-    return filename
+        return filename
 
 
 def set_package_logger(

tests/functional/test_aws_lambda_logging.py

@@ -119,8 +119,8 @@ class Unserializable:
 
 
 def test_log_dict_key_seq(stdout):
-    # GIVEN any logger configuration
-    logger = Logger(level="INFO", stream=stdout, another="xxx")
+    # GIVEN the default logger configuration
+    logger = Logger(stream=stdout)
 
     # WHEN logging a message
     logger.info("Message")
@@ -133,7 +133,7 @@ def test_log_dict_key_seq(stdout):
 
 def test_log_dict_key_custom_seq(stdout):
     # GIVEN a logger configuration with format_keys set to ["message"]
-    logger = Logger(stream=stdout, format_keys=["message"])
+    logger = Logger(stream=stdout, log_record_order=["message"])
 
     # WHEN logging a message
     logger.info("Message")
@@ -145,21 +145,23 @@ def test_log_dict_key_custom_seq(stdout):
 
 
 def test_log_custom_formatting(stdout):
-    # GIVEN a logger where we have a custom location format
-    logger = Logger(stream=stdout, location="[%(funcName)s] %(module)s")
+    # GIVEN a logger where we have a custom `location`, 'datefmt' format
+    logger = Logger(stream=stdout, location="[%(funcName)s] %(module)s", datefmt="fake-datefmt")
 
     # WHEN logging a message
     logger.info("foo")
 
     log_dict: dict = json.loads(stdout.getvalue())
 
-    # THEN the `location` match the formatting
+    # THEN the `location` and "timestamp" should match the formatting
     assert log_dict["location"] == "[test_log_custom_formatting] test_aws_lambda_logging"
+    assert log_dict["timestamp"] == "fake-datefmt"
 
 
 def test_log_dict_key_strip_nones(stdout):
     # GIVEN a logger confirmation where we set `location` and `timestamp` to None
-    logger = Logger(stream=stdout, location=None, timestamp=None)
+    # Note: level, sampling_rate and service can not be suppressed
+    logger = Logger(stream=stdout, level=None, location=None, timestamp=None, sampling_rate=None, service=None)
 
     # WHEN logging a message
     logger.info("foo")