Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(logging): Don't include json_default in logs #132

Merged
merged 1 commit into from
Aug 25, 2020

Conversation

michaelbrewer
Copy link
Contributor

@michaelbrewer michaelbrewer commented Aug 25, 2020

Issue #, if available:

Currently given a custom json_default with the Logger When we log out message Then we include the json_default in the log message:

>>> logger = Logger(level="DEBUG", stream=stdout, json_default=lambda o: f"<non-serializable: {type(o).__name__}>")
>>> logger.debug({"x": Unserializable()})
{'timestamp': '2020-08-25 11:55:27,114', 'level': 'DEBUG', 'location': 'test:112', 'service': 'service_undefined', 'sampling_rate': 0.0, 'json_default': '<non-serializable: function>', 'message': {'x': '<non-serializable: Unserializable>'}}

The output should exclude json_default:

>>> logger = Logger(level="DEBUG", stream=stdout, json_default=lambda o: f"<non-serializable: {type(o).__name__}>")
>>> logger.debug({"x": Unserializable()})
{'timestamp': '2020-08-25 11:59:27,962', 'level': 'DEBUG', 'location': 'test:112', 'service': 'service_undefined', 'sampling_rate': 0.0, 'message': {'x': '<non-serializable: Unserializable>'}}

Description of changes:

  • formatter.py - pop json_default from the **kwargs before updating self.format_dict
  • formatter.py - clean up and remove uneeded pragma: no cover
  • formatter.py - remove unused decode case
  • test_aws_lambda_logging.py - update unit tests to include a check for json_default
  • general - clean up to please sonarlint

Checklist

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Changes:
* formatter.py - pop `json_default` from the **kwargs before updating `self.format_dict`
* formatter.py - clean up and remove uneeded `pragma: no cover`
* formatter.py - remove unused `decode` case
* test_aws_lambda_logging.py - update unit tests to include a check for `json_default`
@codecov-commenter
Copy link

codecov-commenter commented Aug 25, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##           develop      #132   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           24        24           
  Lines          706       706           
  Branches        66        67    +1     
=========================================
  Hits           706       706           
Impacted Files Coverage Δ
aws_lambda_powertools/logging/formatter.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 814062e...718e185. Read the comment docs.

@heitorlessa
Copy link
Contributor

Thanks a lot Mike!!

Copy link
Contributor

@heitorlessa heitorlessa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@heitorlessa heitorlessa merged commit 026faaf into aws-powertools:develop Aug 25, 2020
@michaelbrewer michaelbrewer deleted the refactor-logging branch August 25, 2020 20:25
heitorlessa referenced this pull request in ran-isenberg/aws-lambda-powertools-python Aug 26, 2020
* develop:
  docs: Fix doc for log sampling (#135)
  fix(logging): Don't include `json_default` in logs (#132)
  chore: bump to 1.4.0
  docs: add Lambda Layer SAR App url and ARN
  fix: upgrade dot-prop, serialize-javascript
  fix heading error due to merge
  formatting for bash script
  add layer to docs and how to use it from SAR
  moved publish step to publish workflow after pypi push
  fix(ssm): Make decrypt an explicit option and refactoring (#123)
  change to eu-west-1 default region
  remove tmp release flag and set trigger to release published
  add overwrite flag for ssm
  add relase tag simulation
  more typos
  fix typo in branch trigger
  fix indent, yaml ...
  line endings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants