Task: Release v2.3.0 - Logging improvements q1-2026#69
Open
jonrandahl wants to merge 29 commits intomainfrom
Open
Task: Release v2.3.0 - Logging improvements q1-2026#69jonrandahl wants to merge 29 commits intomainfrom
jonrandahl wants to merge 29 commits intomainfrom
Conversation
- Adopt a broad ignore template for Ruby and major OSes - Exclude OS, editor and test artefacts from version control - Prevent accidental commits of tokens and local configs - Replace outdated minimal ignores to reduce repo noise
- coerce request_time to integer before splitting - prevent errors when value arrives as string or float - keep millisecond formatting stable in output
- expose request_params under accurate key name - use blank? to safely check for empty query string - include only when params present and query is empty
- include request path prominently in the message - state controller and action in natural order - improve readability of action/controller logs
- add level to the message schema and payload flow - seed level from incoming severity for a default - remove level from base payload to avoid duplication - normalise level by HTTP status (info/warn/error)
- Expand task set: assets, help, checks, bundles, coverage, update, vars - Standardise dependency install and test/lint via Bundler - Auto-correct RuboCop offences safely to reduce noise - Improve Ruby path resolution for better portability - Add clearer output and AWS profile guidance for developers - Add interactive dependency updates for JS and Ruby - Keep publish default while making clean/build explicit
- Introduce maintenance group to track runtime deps with bundler - Mirror runtime deps in Gemfile for tooling validation - Enable 'bundle outdated --only-explicit' in update workflow - Remove broken dev dependency on meta_request fork - Add SimpleCov in dev/test to measure code coverage - Bump json and refresh lockfile for consistency
- Adopt Rails 8.1.2 for bug fixes and stability - Refresh transitive deps (Rack 3.2.5, Nokogiri 1.19) - Pull in security patches and performance optimisations
- Update debugging tool to latest patch for bug fixes - Support smoother debugging during logging work
- Update linter and AST parser to latest versions - Improve support for current Ruby syntax and nodes - Reduce false positives with refreshed lint rules - Refresh Unicode width/emoji data for accuracy - Improve editor LSP via language server updates - Affect dev-only tooling; no runtime impact
- Use SimpleCov to collect code coverage during tests - Exclude test files to keep coverage metrics accurate - Make untested areas visible to guide upcoming work
- Adopt industry-standard format for clearer release notes - Structure entries by Added/Changed to cut ambiguity - Normalise headings and dates for quicker scanning - Aid readability for devs and tooling that parses notes - Replace ad hoc history with a structured, durable format
- Document automatic log level normalisation by status code - Note inclusion of log level in required fields for consistency - Update request completion message details for clarity - Simplify payload build order to improve reliability - Fix incorrect params key, time formatting, and query handling - Add test coverage reporting via SimpleCov to track gaps
- Indicate a minor, backwards-compatible release
- Update dependency snapshot to match 2.3.0 release - Ensure local development uses the latest logging changes
- Default to original level if status is missing or invalid - Avoid downgrading to debug for non-HTTP or missing status - Improve severity accuracy while retaining 5xx/4xx/1xx-3xx mappings
- Improve log readability by prioritising timestamp and level - Ensure consistent key order across log entries - Keep payload content unchanged; only field order resolved
- Collapse extra spaces in level for cleaner JSON output - Prevent errors when severity is missing during format
- Set request id via thread-local storage to match runtime behaviour - Assert against plain string id instead of a wrapped hash - Ensure thread cleanup to avoid cross-test leakage
- Reduce verbosity by removing path and boilerplate text - Standardise wording for easier scanning and parsing - Capitalise controller name for consistent style - Flag potential impact to parsers expecting old format - Adjustments based on internal discussions
- Add configuration flag to include optional fields - Merge optional data into structured logs when enabled - Default to disabled to preserve existing behaviour - Update tests to exercise the opt-in path - Enable richer logs without breaking current consumers
- Assert optional fields are excluded by default for clean logs - Verify inclusion of optional fields when explicitly enabled - Ensure nil severity is handled safely without truncation errors - Check `ts` and `level` appear first to keep output readable - Confirm required fields stay consistent across configurations - Add regression coverage using header-like messages
- Document optional fields toggle via initialiser (default off) - Clarify payload order to prioritise timestamp and level - Simplify completion message format with controller/action - Note log level normalisation by HTTP status and required field - Add test coverage notes incl. optional modes and SimpleCov - Record fixes for nil severity, whitespace, params key, time, query
- Introduce shared utilities for branch, skipping, and colour - Enforce RuboCop on staged Ruby files with auto-fix - Re-add modified files and block commit on lint errors - Run Rails tests post-commit; reset on failures to keep green - Build gem on push and block if packaging fails - Allow skipping via special branches or --no-verify - Respect amend flows via reflog and parent process flags - Document setup and usage for local hook path and permissions
- Ensure base class setup runs when constructing formatter - Prevent missing setup that could break base behaviour - Tidy stray whitespace for minor style consistency
jonrandahl
changed the title
jonrandahl
requested review from
joescottdave
and removed request for
ajtucker
joescottdave
left a comment
joescottdave
left a comment
There was a problem hiding this comment.
You might need to convince me because I'm unsure about normalising the level here. I'm not sure that we should deprive individual applications of the ability to choose their own log levels.
The logger should be an honest broker of messages from the application and if we start down the road of side-effecting messages inside the logger it could eventually resemble a game of chinese whispers.
If this needs enforcing I think it should be at the application or developer process layer.
| REQUEST_METHODS = %w[GET POST PUT DELETE PATCH].freeze | ||
|
|
||
| def initialize(include_optional: false) | ||
| super() # dont pass any arguments to the parent class as it does not expect any |
There was a problem hiding this comment.
I don't think this needs parens without arguments?
Contributor
Author
There was a problem hiding this comment.
I thought the same but it failed without
Comment on lines
+329
to
+344
| def normalise_level(msg) # rubocop:disable Metrics/MethodLength | ||
| status = msg[:status] || msg['status'] | ||
| msg.to_h do |k, v| | ||
| if k.to_s == 'level' | ||
| level = case status.to_i | ||
| when 500..599 then process_severity(Logger::ERROR) | ||
| when 400..499 then process_severity(Logger::WARN) | ||
| when 100..399 then process_severity(Logger::INFO) | ||
| # If the status code is not present or does not match any of the above ranges, keep the original level | ||
| else v | ||
| end | ||
| [:level, level] | ||
| else | ||
| [k, v] | ||
| end | ||
| end |
There was a problem hiding this comment.
I'm unsure about this. You could avoid it of course by not providing the status at all, but you're taking away a lot of room to choose what level certain statuses should be logged at on a per application basis, and it wouldn't be clear to the user of the interface that, that was going to happen.
Contributor
Author
There was a problem hiding this comment.
This is specific to logs we were receiving that were status: 5.. but the logs were set to INFO/DEBUG level, to ensure the two match.
You could avoid it of course by not providing the status at all, but you're taking away a lot of room to choose what level certain statuses should be logged at on a per application basis, and it wouldn't be clear to the user of the interface that, that was going to happen.
I'm not sure I agree with this though as the status code should denote the level of message?
Happy to discuss further though.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improve log clarity and configurability, and strengthen local quality checks with unified hooks and workflow updates. Modernise dependencies and documentation while increasing test coverage and visibility.
What's changed: