fix(logger): remove subclassing and move unnecessary APIs

[heitorlessa]

Issue number: #2356

Summary

Long standing tech debt to remove logging.Logger sub-classing, and move backwards compatible private functions to compat.py to keep Logger public API cleaner.

Why?

If we were to add __getattr__, mypy would then fail to detect non-existent objects like logger.foo, because it can't infer from that.

Changes

Please provide a summary of what's being changed

• Remove subclassing to prevent erroneous behaviour
• Move backwards compatibility to compat.py.
  • Double check caller filename patch
• Correct docstring for registered_handler and registered_formatter to say it picks up the original handler only for convenience.
  • This is due to niche customers wanting multiple handlers; it shall prevent confusion.
• Proxy log_level to _logger.level to make it less error-prone

Historical events that made this possible now (older to recent)

• Introduced sub-classing to address autocomplete in PyCharm
• Introduced log statements method (info/error/exception/critical/warning/debug) and addressed backward compatibility with 3.8+ stacklevel issue (e.g., location line)
  • This made it possible to get rid of the subclassing as that was the main issue
• Introduced setLevel method to correctly apply level in private logger
  • This was only needed due to the subclassing we had, where originally was to fix autocompletion and location key, eventually leading to this confusion this PR finally addresses.

User experience

Please share what the user experience looks like before and after this change

Escape hatching to standard python logging methods like addHandler, setLevel will work as expected and effect the underlying logger. However, mypy customers will receive a error at type checking - if necessary, we can port these into our Logger API on a per case basis.

Output

{
    "level": "DEBUG",
    "location": "<module>:7",
    "message": "hello from debug",
    "timestamp": "2023-05-26 16:50:51,239+0200",
    "service": "payment"
}

Checklist

If your change doesn't seem to apply, please leave them unchecked.

• Meet tenets criteria
• I have performed a self-review of this change
• Changes have been tested
• Changes are documented
• PR title follows conventional commit semantics

Is this a breaking change?

RFC issue number:

Checklist:

• Migration process documented
• Implement warnings (if it can live side by side)

Acknowledgment

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

[codecov-commenter]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (719cdc0) 97.21% compared to head (70ba63e) 97.21%.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #2334   +/-   ##
========================================
  Coverage    97.21%   97.21%           
========================================
  Files          152      153    +1     
  Lines         6998     7006    +8     
  Branches       513      513           
========================================
+ Hits          6803     6811    +8     
  Misses         155      155           
  Partials        40       40           

Impacted Files	Coverage Δ
aws_lambda_powertools/logging/compat.py	100.00% <100.00%> (ø)
aws_lambda_powertools/logging/logger.py	99.43% <100.00%> (+<0.01%)	⬆️
aws_lambda_powertools/logging/utils.py	100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[rubenfonseca]

Awesome!