Skip to content

Conversation

@asnare
Copy link
Contributor

@asnare asnare commented May 15, 2025

This PR fixes an issue with the colorised log formatter whereby it wasn't properly formatting log entries if they contained %-style placeholders (with arguments). Although we don't generally use this style in our own projects, 3rd-party code does and if we don't support this then logging from those components is incorrect.

This is stacked on top of #232 and resolves #230.

@asnare asnare requested review from a team and sundarshankar89 May 15, 2025 13:14
@asnare asnare self-assigned this May 15, 2025
@asnare asnare requested a review from nfx as a code owner May 15, 2025 13:14
@asnare asnare added the bug Something isn't working label May 15, 2025
@asnare asnare added this to UCX May 15, 2025
@asnare asnare moved this to Ready for Review in UCX May 15, 2025
@asnare asnare removed the request for review from nfx May 15, 2025 13:15
@github-actions
Copy link

github-actions bot commented May 15, 2025

✅ 40/40 passed, 2 skipped, 1m59s total

Running from acceptance #276

Copy link
Collaborator

@sundarshankar89 sundarshankar89 left a comment

Choose a reason for hiding this comment

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

LGTM

Base automatically changed from logger-tests to main May 15, 2025 17:05
@asnare asnare merged commit cc9876f into main May 15, 2025
11 checks passed
@asnare asnare deleted the fix/logger-argument-interpolation branch May 15, 2025 17:36
@github-project-automation github-project-automation bot moved this from Ready for Review to Done in UCX May 15, 2025
ericvergnaud added a commit that referenced this pull request May 15, 2025
* main:
  Fix argument interpolation in colorised logs (#233)
  Ensure the non-colorized logs also include timestamps with second-lev… (#235)
  Test coverage for the customised logger setup (#232)
  Type hints for existing installation tests (#231)

# Conflicts:
#	tests/unit/test_installation.py
sundarshankar89 added a commit that referenced this pull request May 16, 2025
* Consistent exception formatting in logs ([#237](#237)). The logger's exception formatting has been enhanced to provide a consistent and readable log format, adhering to standard Python norms. When an exception occurs, the log message now ensures a newline character separates the error message from the exception details, regardless of whether logs are colorized or not. This update applies to both exception text and stack information, which are now prepended with a newline character if necessary, resulting in a uniform format for all log types. This change resolves previous inconsistencies between colorized and non-colorized logs, aligning the logging functionality with standard Python practices for exception logging, and improving overall log readability.
* Ensure that App logger emits `DEBUG` events if the CLI is invoked with `--debug` ([#238](#238)). The `get_logger` function has been enhanced to provide more flexibility and consistency with standard logging practices. It now accepts an optional `manager` parameter, allowing for customization of the logging manager, and returns a `logging.Logger` object. The logger level is automatically set to `DEBUG` when the application is running in debug mode, as detected by the `is_in_debug` function, and the level is set using the `logging.DEBUG` constant for consistency. This change simplifies the code and ensures that the logger emits `DEBUG` events when the application is run with the debug flag, which is verified through an updated test suite that covers various scenarios, including logger name setting, debug mode behavior, and logger propagation.
* Ensure the names of logger levels are consistent ([#234](#234)). The logger has been updated to use consistent naming conventions for logging levels, aligning with the Python ecosystem's norms. Previously, colorized logs used compact names `WARN` and `FATAL` for warning and critical levels, while non-colorized logs used the conventional `WARNING` and `CRITICAL` names. To address this inconsistency, two new dictionaries have been introduced to store colorized level names and color codes, and the `format` method has been modified to utilize these dictionaries, ensuring consistent logging level names and colorized message text. As a result, logging level names have been updated to use the conventional `WARNING` and `CRITICAL` instead of `WARN` and "FATAL", and color codes for message text have been added for each logging level, promoting consistency and adherence to Python logging conventions.
* Ensure the non-colorized logs also include timestamps with second-lev… ([#235](#235)). The log formatter has been updated to include second-granular timestamps in non-colorized logs, providing more precise logging information and ensuring consistency with colorized output. Previously, only minute-granular timestamps were logged, which was insufficient for logging purposes. The update changes the timestamp format from `%H:%M` to `%H:%M:%S` to include seconds, resulting in more detailed timestamp information. This change resolves the inconsistency between colorized and non-colorized logs, and is verified by updated tests that validate the formatter's behavior with and without colors, confirming that the formatter now correctly starts with a timestamp in both cases.
* Fixed Blueprint Install ([#225](#225)). The `__version__` variable import statement has been updated to utilize a fully qualified module name, providing a more explicit and absolute reference to the module containing version information. This change ensures that the correct version is imported and used to set the user agent extra in relevant function calls, enhancing the reliability and accuracy of version tracking within the library.
* Fixed argument interpolation in colorised logs ([#233](#233)). The colorised log formatter has been enhanced to correctly handle log entries containing `%`-style placeholders with arguments, a common pattern in third-party code, by retrieving the log message using `record.getMessage()` instead of directly accessing `record.msg`. This update resolves an issue with improper formatting of logging from third-party components and builds upon previous changes to address the underlying logging problem. Additionally, the corresponding test case has been updated to verify that the formatter correctly handles messages with arguments that require interpolation, both with and without colors enabled, and is no longer expected to fail, indicating that the issue with argument interpolation in the colorized log formatter has been resolved.
* Fixed logger name abbreviation fails if the logger name contains `..` ([#236](#236)). The logger's format method has been enhanced to correctly abbreviate logger names containing multiple consecutive dots, which previously led to exceptions. The new logic splits the logger name into components, abbreviating all but the last two, and then reassembles them, ensuring correct abbreviation and formatting even when consecutive dots are present. This improvement also fixes the colorized logger output to handle logger names with consecutive dots without throwing an exception, and the corresponding test case has been updated to reflect this change, now directly testing the logging functionality by formatting the log record and stripping ANSI escape sequences, providing a more straightforward verification of the logging functionality.
@sundarshankar89 sundarshankar89 mentioned this pull request May 16, 2025
sundarshankar89 added a commit that referenced this pull request May 16, 2025
* Consistent exception formatting in logs
([#237](#237)). The
logger's exception formatting has been enhanced to provide a consistent
and readable log format, adhering to standard Python norms. When an
exception occurs, the log message now ensures a newline character
separates the error message from the exception details, regardless of
whether logs are colorized or not. This update applies to both exception
text and stack information, which are now prepended with a newline
character if necessary, resulting in a uniform format for all log types.
This change resolves previous inconsistencies between colorized and
non-colorized logs, aligning the logging functionality with standard
Python practices for exception logging, and improving overall log
readability.
* Ensure that App logger emits `DEBUG` events if the CLI is invoked with
`--debug`
([#238](#238)). The
`get_logger` function has been enhanced to provide more flexibility and
consistency with standard logging practices. It now accepts an optional
`manager` parameter, allowing for customization of the logging manager,
and returns a `logging.Logger` object. The logger level is automatically
set to `DEBUG` when the application is running in debug mode, as
detected by the `is_in_debug` function, and the level is set using the
`logging.DEBUG` constant for consistency. This change simplifies the
code and ensures that the logger emits `DEBUG` events when the
application is run with the debug flag, which is verified through an
updated test suite that covers various scenarios, including logger name
setting, debug mode behavior, and logger propagation.
* Ensure the names of logger levels are consistent
([#234](#234)). The
logger has been updated to use consistent naming conventions for logging
levels, aligning with the Python ecosystem's norms. Previously,
colorized logs used compact names `WARN` and `FATAL` for warning and
critical levels, while non-colorized logs used the conventional
`WARNING` and `CRITICAL` names. To address this inconsistency, two new
dictionaries have been introduced to store colorized level names and
color codes, and the `format` method has been modified to utilize these
dictionaries, ensuring consistent logging level names and colorized
message text. As a result, logging level names have been updated to use
the conventional `WARNING` and `CRITICAL` instead of `WARN` and "FATAL",
and color codes for message text have been added for each logging level,
promoting consistency and adherence to Python logging conventions.
* Ensure the non-colorized logs also include timestamps with second-lev…
([#235](#235)). The
log formatter has been updated to include second-granular timestamps in
non-colorized logs, providing more precise logging information and
ensuring consistency with colorized output. Previously, only
minute-granular timestamps were logged, which was insufficient for
logging purposes. The update changes the timestamp format from `%H:%M`
to `%H:%M:%S` to include seconds, resulting in more detailed timestamp
information. This change resolves the inconsistency between colorized
and non-colorized logs, and is verified by updated tests that validate
the formatter's behavior with and without colors, confirming that the
formatter now correctly starts with a timestamp in both cases.
* Fixed Blueprint Install
([#225](#225)). The
`__version__` variable import statement has been updated to utilize a
fully qualified module name, providing a more explicit and absolute
reference to the module containing version information. This change
ensures that the correct version is imported and used to set the user
agent extra in relevant function calls, enhancing the reliability and
accuracy of version tracking within the library.
* Fixed argument interpolation in colorised logs
([#233](#233)). The
colorised log formatter has been enhanced to correctly handle log
entries containing `%`-style placeholders with arguments, a common
pattern in third-party code, by retrieving the log message using
`record.getMessage()` instead of directly accessing `record.msg`. This
update resolves an issue with improper formatting of logging from
third-party components and builds upon previous changes to address the
underlying logging problem. Additionally, the corresponding test case
has been updated to verify that the formatter correctly handles messages
with arguments that require interpolation, both with and without colors
enabled, and is no longer expected to fail, indicating that the issue
with argument interpolation in the colorized log formatter has been
resolved.
* Fixed logger name abbreviation fails if the logger name contains `..`
([#236](#236)). The
logger's format method has been enhanced to correctly abbreviate logger
names containing multiple consecutive dots, which previously led to
exceptions. The new logic splits the logger name into components,
abbreviating all but the last two, and then reassembles them, ensuring
correct abbreviation and formatting even when consecutive dots are
present. This improvement also fixes the colorized logger output to
handle logger names with consecutive dots without throwing an exception,
and the corresponding test case has been updated to reflect this change,
now directly testing the logging functionality by formatting the log
record and stripping ANSI escape sequences, providing a more
straightforward verification of the logging functionality.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[BUG]: Colorised logs don't interpolate %-arguments

3 participants