-
Notifications
You must be signed in to change notification settings - Fork 20
[core] fix field detection for escaped % and { format styles #75
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file. | |
| The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), | ||
| and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). | ||
|
|
||
| ## [4.2.1.dev1](https://github.com/nhairs/python-json-logger/compare/v4.2.0...main) - unreleased | ||
|
|
||
| ### Fixed | ||
| - `%` style formats no longer treat the escaped literal `%%` as the start of a field, so | ||
| `"%%(notafield)s"` is correctly read as literal text. | ||
| - `{` style formats now use `string.Formatter` (as `logging.StrFormatStyle.validate` does) to find | ||
| fields, so escaped literal braces (`{{`/`}}`) are skipped and a conversion (`{message!r}`) or | ||
| format spec (`{levelname:>8}`) is no longer treated as part of the field name. | ||
|
Comment on lines
+10
to
+14
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These should have a reference to the issue |
||
|
|
||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should add your own
|
||
| ## [4.2.0](https://github.com/nhairs/python-json-logger/compare/v4.1.0...v4.2.0) - 2026-08-15 | ||
|
|
||
| ### Changed | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
| from datetime import datetime, timezone | ||
| import logging | ||
| import re | ||
| import string | ||
| import sys | ||
| from typing import TypeAlias, Any | ||
| from collections.abc import Container, Sequence | ||
|
|
@@ -65,7 +66,8 @@ | |
| r"\$(?:\$|\{(?P<braced>.+?)\}|(?P<named>[_a-z][_a-z0-9]*))", re.IGNORECASE | ||
| ) # $ style | ||
| STYLE_STRING_FORMAT_REGEX = re.compile(r"\{(.+?)\}", re.IGNORECASE) # { style | ||
| STYLE_PERCENT_REGEX = re.compile(r"%\((.+?)\)", re.IGNORECASE) # % style | ||
| # Deprecated: no longer used by `parse`, which uses `string.Formatter` instead. | ||
|
Comment on lines
68
to
+69
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should remove this rather than marking it as deprecated. |
||
| STYLE_PERCENT_REGEX = re.compile(r"%(?:%|\((?P<named>.+?)\))", re.IGNORECASE) # % style | ||
|
|
||
| ## Type Aliases | ||
| ## ----------------------------------------------------------------------------- | ||
|
|
@@ -311,12 +313,24 @@ def parse(self) -> list[str]: | |
| ] | ||
|
|
||
| if isinstance(self._style, logging.StrFormatStyle): | ||
| return STYLE_STRING_FORMAT_REGEX.findall(self._fmt) | ||
| # str.format escapes literal braces as {{ and }}, and a replacement field may | ||
| # carry a conversion (!r) or a format spec (:>10) that is not part of its name. | ||
| # string.Formatter is what logging.StrFormatStyle.validate itself parses with. | ||
|
Comment on lines
+316
to
+318
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need these comments |
||
| return [ | ||
| field_name | ||
| for _, field_name, _, _ in string.Formatter().parse(self._fmt) | ||
| if field_name | ||
| ] | ||
|
|
||
| if isinstance(self._style, logging.PercentStyle): | ||
| # PercentStyle is parent class of StringTemplateStyle and StrFormatStyle | ||
| # so it must be checked last. | ||
| return STYLE_PERCENT_REGEX.findall(self._fmt) | ||
| # %% is an escaped literal percent, so %%(name)s is not a field. | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need this comment |
||
| return [ | ||
| match.group("named") | ||
| for match in STYLE_PERCENT_REGEX.finditer(self._fmt) | ||
| if match.group("named") | ||
| ] | ||
|
|
||
| raise ValueError(f"Style {self._style!r} is not supported") | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -183,6 +183,41 @@ def test_string_template_format(env: LoggingEnvironment, class_: type[BaseJsonFo | |
| return | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("class_", ALL_FORMATTERS) | ||
| def test_percentage_format_escaped_percent( | ||
| env: LoggingEnvironment, class_: type[BaseJsonFormatter] | ||
| ): | ||
| # Note: %% is an escaped literal percent, so %%(notafield)s is not a field | ||
| env.set_formatter(class_("%(levelname)s %(message)s 100%% %%(notafield)s")) | ||
|
|
||
| msg = "testing logging format" | ||
| env.logger.info(msg) | ||
| log_json = env.load_json() | ||
|
|
||
| assert log_json["message"] == msg | ||
| assert log_json.keys() == {"levelname", "message"} | ||
| return | ||
|
|
||
|
|
||
|
Comment on lines
+186
to
+201
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be merged with |
||
| @pytest.mark.parametrize("class_", ALL_FORMATTERS) | ||
| def test_str_format_format(env: LoggingEnvironment, class_: type[BaseJsonFormatter]): | ||
| # Note: {{ }} is an escaped literal brace, and !r / :>{width} are not part of a field name | ||
| env.set_formatter( | ||
| class_( | ||
| "{{literal}} {levelname:>{width}} {message!r} {filename} {lineno} {asctime}", | ||
| style="{", | ||
| ) | ||
| ) | ||
|
|
||
| msg = "testing logging format" | ||
| env.logger.info(msg) | ||
| log_json = env.load_json() | ||
|
|
||
| assert log_json["message"] == msg | ||
| assert log_json.keys() == {"levelname", "message", "filename", "lineno", "asctime"} | ||
| return | ||
|
|
||
|
|
||
| @pytest.mark.parametrize("class_", ALL_FORMATTERS) | ||
| def test_comma_format(env: LoggingEnvironment, class_: type[BaseJsonFormatter]): | ||
| # Note: we have double comma `,,` to test handling "empty" names | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: