Skip to content

feat: reorder log format to level-context-message#11

Merged
janmatzek merged 1 commit into
gooddata:masterfrom
janmatzek:jmat/reorder-logs-context
Jul 1, 2026
Merged

feat: reorder log format to level-context-message#11
janmatzek merged 1 commit into
gooddata:masterfrom
janmatzek:jmat/reorder-logs-context

Conversation

@janmatzek

@janmatzek janmatzek commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • Bug Fixes
    • Improved log message formatting so object context appears consistently in the right place.
    • Preserved colored log levels while making context display cleaner and more reliable.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The ColoredLevelFormatter's format method was changed to attach object context as a context_str attribute on the log record rather than appending it to the formatted message string. The logger's format template was updated to include %(context_str)s.

Changes

Log formatting update

Layer / File(s) Summary
Context injection via log record attribute
src/gooddata_legacy2cloud/logging/config.py
format() now computes context via get_object_context() and sets it as record.context_str before delegating to super().format(record); the format template now includes %(context_str)s between %(levelname)s and %(message)s.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Poem

A hop, a tweak, a tiny log's delight,
Context now rides the record, snug and tight.
No more stitching strings by paw,
Just clean formats, without a flaw. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: reordering log output to level-context-message.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Comment @coderabbitai help to get the list of available commands.

@janmatzek janmatzek marked this pull request as ready for review July 1, 2026 12:19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/gooddata_legacy2cloud/logging/config.py (1)

43-43: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Format string tightly coupled to ColoredLevelFormatter.

The literal %(context_str)s token only resolves correctly because ColoredLevelFormatter.format() always sets record.context_str first. If this template string is ever reused with a plain logging.Formatter (or any formatter that doesn't set context_str), it will raise KeyError at format time. Not an issue today since both live together in configure_logger, but worth a short comment noting this coupling for future maintainers.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/gooddata_legacy2cloud/logging/config.py` at line 43, The formatter string
in `configure_logger` is tightly coupled to `ColoredLevelFormatter`, since
`%(context_str)s` only works after `ColoredLevelFormatter.format()` injects that
attribute. Add a short inline comment near the `formatter =
ColoredLevelFormatter(...)` assignment to note that this template must be used
only with `ColoredLevelFormatter` (or any formatter that sets `context_str`) so
future reuse with a plain `logging.Formatter` is avoided.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/gooddata_legacy2cloud/logging/config.py`:
- Line 43: The formatter string in `configure_logger` is tightly coupled to
`ColoredLevelFormatter`, since `%(context_str)s` only works after
`ColoredLevelFormatter.format()` injects that attribute. Add a short inline
comment near the `formatter = ColoredLevelFormatter(...)` assignment to note
that this template must be used only with `ColoredLevelFormatter` (or any
formatter that sets `context_str`) so future reuse with a plain
`logging.Formatter` is avoided.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 1926a6dc-e815-4be9-bcef-dd935ca96eba

📥 Commits

Reviewing files that changed from the base of the PR and between d88b7c4 and e4066b9.

📒 Files selected for processing (1)
  • src/gooddata_legacy2cloud/logging/config.py

@janmatzek janmatzek enabled auto-merge (rebase) July 1, 2026 12:23
@janmatzek janmatzek merged commit ac75c8a into gooddata:master Jul 1, 2026
2 checks passed
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.

2 participants