feat(logger): customizing logger output - #140

michaelbrewer · 2020-08-27T14:27:32Z

Issue #, if available: #138 and #143

Description of changes:

NOTE: sampling_rate and service can not be suppressed

Ensure that the initial key seq is: level,location,message,timestamp so by default messages in Cloudwatch logs are easier to read:

>>> logger = Logger()
>>> logger.info("Eggs")
{'level': 'INFO', 'location': 'method_name:126', 'message': 'Eggs', 'timestamp': '2020-08-27 15:32:51,285', 'service': 'app', 'sampling_rate': 0.0}

Also allow for reordering override via format_keys

>>> logger = Logger(format_keys=["message"])
>>> logger.info("Foo")
{'message': 'Message', 'level': 'INFO', 'location': 'method_name:139', 'timestamp': '2020-08-27 15:36:11,494', 'service': 'app', 'sampling_rate': 0.0}

You can also suppress timestamp or location fields by passing them in as None

>>> logger = Logger(stream=stdout, location=None, timestamp=None)
>>> logger.info("magnus")
{"level": "INFO", "message": "magnus", "service": "app", "sampling_rate": 0.0}

Finally you can already customize the format for location and timestamp. So putting it all together:

>>> logger = Logger(location="%(module)s.%(funcName)s:%(lineno)d", timestamp=None, format_keys=["location", "message"])
>>> logger.info("takeoff")
{"location": "space.fly:7", "message": "takeoff", "level": "INFO", "service": "falcon", "sampling_rate": 0.0}

Checklist

Meet tenets criteria
Update tests
Update docs
PR title follows conventional commit semantics

Ensure that the initial key seq is: timestamp,level,location,message Closes #138

codecov-commenter · 2020-08-27T14:29:34Z

Codecov Report

Merging #140 into develop will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##           develop      #140   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          706       705    -1     
  Branches        67        68    +1     
=========================================
- Hits           706       705    -1

Impacted Files	Coverage Δ
aws_lambda_powertools/logging/formatter.py	`100.00% <100.00%> (ø)`
aws_lambda_powertools/logging/logger.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f137ba7...42f6b22. Read the comment docs.

michaelbrewer · 2020-08-27T18:05:49Z

@heitorlessa maybe we can make the order configurable or get some feedback on what it should be.

Changes: * add `format_key` to allow for a custom order * remove unessarly verbose `json_formatter` with just `str` * update tests

Some keys like `location` and `timestamp` can be suppressed by setting them to None. Changes: * logger.py - Convert some methods to static functions * test_aws_lambda_logging - Add more tests and some which can be used for documentation

heitorlessa

made a few comments ;)

Additional thoughts:

I'm not 100% sure format_keys is descriptive enough for whoever unfamiliar with powertools to read and understand what's going on; I'll think through alternatives e.g. sort_keys, keys_order, log_record_order
You had a comment about suppressing sampling_rate and possibly others in the future, has that changed? This doesn't address that
- In hindsight (6:22am), ignore this. I think we should continue to be opinionated about what can be changed or else this will become harder to maintain

heitorlessa · 2020-08-28T04:12:54Z

                handler.setFormatter(JsonFormatter(**self._default_log_keys, **kwargs))


+def _get_log_level(level: Union[str, int]) -> Union[str, int]:


plz make this a classmethod, not a separate function - it's still part of the encapsulated logger class logic

heitorlessa · 2020-08-28T04:13:03Z

+    return log_level
+
+
+def _get_caller_filename():


plz make this a classmethod, not a separate function - it's still part of the encapsulated logger class logic

@heitorlessa should I use @staticmethod then ?

sorry, yes, that's what I meant - staticmethod.

heitorlessa · 2020-08-28T04:15:46Z

-            "level": "%(levelname)s",
-            "location": "%(funcName)s:%(lineno)d",
-        }
+        self.format_dict = dict.fromkeys(kwargs.pop("format_keys", ["level", "location", "message", "timestamp"]))


could you please add a comment about insertion order here?

michaelbrewer · 2020-08-28T04:26:06Z

made a few comments ;)

Additional thoughts:

I'm not 100% sure format_keys is descriptive enough for whoever unfamiliar with powertools to read and understand what's going on

Maybe ordered_message_keys? Kind of unsure of the name. :)

You had a comment about suppressing sampling_rate and possibly others in the future, has that changed? This doesn't address that

For service and sample_rate I would have to add a check for if it was set to None, vs just being left out. I will have to think about how.

heitorlessa · 2020-08-28T04:38:03Z

made a few comments ;)
Additional thoughts:

I'm not 100% sure format_keys is descriptive enough for whoever unfamiliar with powertools to read and understand what's going on

Maybe ordered_message_keys? Kind of unsure of the name. :)

You had a comment about suppressing sampling_rate and possibly others in the future, has that changed? This doesn't address that

For service and sample_rate I would have to add a check for if it was set to None, vs just being left out. I will have to think about how.

What about log_record_order? Seems explicit enough to me even as sleepy as I am now.

I'd suggest keeping as is in terms of suppression, there could be side effects on these two:

If the customer wants to use the dynamic sampling feat (I have a list of customers already using in prod)
service is integral to Powertools terminologies, and this will degrade ops capabilities when creating metrics/visualization from logs etc

michaelbrewer · 2020-08-28T04:52:52Z

I will keep the log suppression as it then. We should make have a better overview of Powertools in the main website which highlights this. I still don't quite understand log sampling :-)

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

heitorlessa · 2020-08-28T14:57:04Z

+
+
+def test_log_dict_key_custom_seq(stdout):
+    # GIVEN a logger configuration with format_keys set to ["message"]


Suggested change

# GIVEN a logger configuration with format_keys set to ["message"]

# GIVEN a logger configuration with log_record_order set to ["message"]

heitorlessa · 2020-08-28T15:04:09Z

Thank you @michaelbrewer - I've just noticed I don't have push permissions to your branch to update a tiny typo in the test comments, so I'm merging as-is and will fix-later.

michaelbrewer · 2020-08-28T15:45:06Z

Thank you @michaelbrewer - I've just noticed I don't have push permissions to your branch to update a tiny typo in the test comments, so I'm merging as-is and will fix-later.

Oops. Thanks.

feat(logger): readable log_dict seq

133f260

Ensure that the initial key seq is: timestamp,level,location,message Closes #138

refactor: use None instead of

f9f4bde

Michael Brewer added 3 commits August 27, 2020 11:12

Merge branch 'develop' into fix-log-key-order

a9cb740

feat(logging): allow for custom json order

e148ffc

Changes: * add `format_key` to allow for a custom order * remove unessarly verbose `json_formatter` with just `str` * update tests

feat(logging): suppress some log keys

3e0116b

Some keys like `location` and `timestamp` can be suppressed by setting them to None. Changes: * logger.py - Convert some methods to static functions * test_aws_lambda_logging - Add more tests and some which can be used for documentation

heitorlessa added area/logger labels Aug 28, 2020

heitorlessa suggested changes Aug 28, 2020

View reviewed changes

Michael Brewer added 2 commits August 27, 2020 22:47

refactor: make requested changes

e4c1e8e

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

chore: doc typo

42f6b22

heitorlessa self-assigned this Aug 28, 2020

heitorlessa reviewed Aug 28, 2020

View reviewed changes

heitorlessa merged commit a7d7c86 into aws-powertools:develop Aug 28, 2020

		handler.setFormatter(JsonFormatter(self._default_log_keys, kwargs))


		def _get_log_level(level: Union[str, int]) -> Union[str, int]:



		def test_log_dict_key_custom_seq(stdout):
		# GIVEN a logger configuration with format_keys set to ["message"]

	# GIVEN a logger configuration with format_keys set to ["message"]
	# GIVEN a logger configuration with log_record_order set to ["message"]

Conversation

michaelbrewer commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes:

Uh oh!

codecov-commenter commented Aug 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

michaelbrewer commented Aug 27, 2020

Uh oh!

heitorlessa left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

heitorlessa Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heitorlessa Aug 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

michaelbrewer Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

heitorlessa Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

heitorlessa Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

michaelbrewer commented Aug 28, 2020

Uh oh!

heitorlessa commented Aug 28, 2020

Uh oh!

michaelbrewer commented Aug 28, 2020

Uh oh!

heitorlessa Aug 28, 2020

Choose a reason for hiding this comment

Uh oh!

heitorlessa commented Aug 28, 2020

Uh oh!

michaelbrewer commented Aug 28, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

michaelbrewer commented Aug 27, 2020 •

edited

Loading

codecov-commenter commented Aug 27, 2020 •

edited

Loading

heitorlessa left a comment •

edited

Loading

heitorlessa Aug 28, 2020 •

edited

Loading

heitorlessa Aug 28, 2020 •

edited

Loading