Skip to content

Performance, issue in doRollover, bug in exception, inconsistency in default formatting, static analysis issues, documentation/API mismatch #68

@jbanaszczyk

Description

@jbanaszczyk

Fixes in https://github.com/adafruit/Adafruit_CircuitPython_Logging/compare/main...co-rc:Adafruit_CircuitPython_Logging:main

1. Performance Optimization in _log()

  • Issue: The library was performing string formatting (msg % args) and creating a LogRecord object (including a time.monotonic() call) before checking if the message level met the logger's or handlers' thresholds. In resource-constrained environments like CircuitPython, this causes unnecessary CPU and RAM overhead for logs that are ultimately suppressed.
  • Fix: Added early exit checks in Logger._log(). Now, formatting and record creation only occur if the level passes the logger's threshold AND at least one handler (or the default handler) is configured to accept that level.

2. Fixed RotatingFileHandler.doRollover() Logic

  • Issue:
    • Broken Rename Sequence: The previous implementation attempted to rename files from oldest to newest (e.g., .1 -> .2, then .2 -> .3). This fails if the target filename already exists.
    • Off-by-one Error: The rollover logic incorrectly handled the backupCount when deleting the oldest file.
    • Typos: Misspelling exsist instead of exists in OSError comments and handling.
  • Fix: Rewrote doRollover to rename files from newest to oldest (e.g., .4 -> .5, then .3 -> .4), which is the standard way to avoid collisions. Corrected the backupCount logic and fixed the typos.

3. Bug in Logger.exception()

  • Issue: The exception() method was duplicating the exception message and incorrectly joining traceback lines, leading to redundant and poorly formatted output.
  • Fix: Simplified the method to use traceback.format_exception(err) properly, which inherently includes the exception description and correctly formatted traceback lines.

4. Consistency in Default Formatting

  • Issue: Handler.format() used a hardcoded default format that was inconsistent with the default behavior of the Formatter class (which defaults to just the message).
  • Fix: Unified the behavior. If no formatter is set, the handler now returns record.msg, ensuring consistency across the API.

5. IDE Support for Logging Constants

  • Issue: Logging levels (DEBUG, INFO, etc.) were being injected into globals() dynamically. This prevents IDEs and static analyzers from recognizing these constants, leading to "Unresolved reference" warnings.
  • Fix: Explicitly defined NOTSET, DEBUG, INFO, WARNING, ERROR, and CRITICAL as module-level constants.

6. Documentation/API Mismatch

  • Issue: The Formatter docstring stated the default style is {, whereas the __init__ method actually defaulted to %.
  • Fix: Updated the docstring to correctly reflect the default % style.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions