Repository navigation
Audit the docs, README and homepage against the code, and fix what they promised - #27
Merged
Merged
Conversation
Their comments (shown in the YARD docs) promised log rotation, compression, hot reloading, "10-100x" speed and zero-copy serialization, said the color formatter is on by default in development (it needs prefer_json_in_development off) and showed a Log::Plain example that doesn't work.
Tags (logger.tagged("billing"), Rails' config.log_tags, ActiveJob's job tags) never reached the JSON line: the
formatter read only the request ID and Sidekiq job context named tags. Tags are now written as tags, and other
named tags (logger.tagged(user_id: 1)) in ctx with the job's context, filtered and scrubbed like the rest of the
line. The tagged logging test asserted nothing about tags.
- Keys in filter_keys_with_hashes are filtered, with a hash of their value. They only changed the summary of
keys that were already filtered, so adding :username logged usernames in full. The email and email_address
keys (the defaults) become a filtered summary with the email's hash, as they already did where Rails'
filter_parameters include :email.
- An email address has one hash however it's written (User@Example.com and user@example.com), and the same
hash under a filtered key as in a scrubbed string.
- Email addresses with letters of any script are scrubbed (an accented name). Strings are made valid UTF-8
before scrubbing.
- The default key pattern filters apikey, passcode, credit_card, card_number and social_security_number.
- New authorization_credentials setting, on by default: Bearer tokens and Basic credentials in text are
scrubbed ("Bearer [FILTERED]").
- Filtered Hashes and Arrays are summarized without their size, like Strings: _bytes said how long the secrets
in them were.
- Lines written with << reach broadcast loggers scrubbed.
When LogStruct was enabled at boot (LOGSTRUCT_ENABLED, production servers), it added its appenders before config/initializers ran and never again, so prefer_json_in_development, enable_color_output, color_map, filter_noisy_loggers and local_environments set in an initializer did nothing. The logstruct.setup initializer now adds the appenders again after the initializers.
A mailer with rescue_from ..., with: :log_and_ignore_error (as the Postmark guide shows) raised NameError in development, consoles, Rake tasks and other processes where LogStruct doesn't set up ActionMailer, instead of handling the error. There, plain versions of log_and_ignore_error, log_and_report_error and log_and_reraise_error log the error with the mailer's logger, then ignore it, report it to Rails.error or raise it again.
The proc's return value was thrown away, so a proc returning a Hash of fields (as Lograge's custom_options do, and as the configuration example showed) added nothing. A returned Hash is now merged into the request line; a proc can still change options instead.
…est what each handler does
…gers - Setting up LogStruct's appenders again (after config/initializers) replaces only the ones LogStruct added, so appenders an app adds in an initializer are kept. SemanticLogger's clear_appenders! did nothing while its thread was stopped. - Loggers added with broadcast_to got each `logger << "..."` line twice. - `rails server` in development broadcast to a STDOUT logger, which printed every message again as text beside LogStruct's line. broadcast_to ignores a logger that writes to STDOUT or STDERR when SemanticLogger already does. - A color_map changes only the colors it names. - Tests no longer stop SemanticLogger's thread, which left later tests' lines waiting in its queue.
A subscriber that raised stopped the line from being written and raised from the app's log call. Subscribers now get the entry after its line is written, and a subscriber's error is handled like the app's other errors (standard_errors: logged in production, raised in tests). Lines written while handling it aren't sent to the subscribers again.
- retry_on writes a retry line (the exception, and the wait before the retry), retry_stopped a line when it runs out of attempts, and discard_on a discard line. The exception was in no line. - A job the queue couldn't take (an EnqueueError that perform_later keeps on the job, or an exception from the adapter) is an error line instead of an enqueue line. A job a before_enqueue callback stopped writes no line, and a job a before_perform callback stopped no finish line. - perform_all_later writes an enqueue or schedule line for each job it enqueued, and an error line for each it couldn't. - With the inline adapter, a failed perform_later writes one error line: the error that raised out of the enqueue was logged again. - A finish line's ts is when the job finished. - Enqueue lines lose `retries`, which nothing set.
- Deleting files by prefix (a blob's variants, when an image is purged) is a delete line with `prefix`. It raised a type error inside LogStruct and wrote no line, since such a delete has no key. - Remove Log::ActiveStorage::Stream and the stream event: Rails sends no service_stream event, so no line ever had it. Streamed downloads are download lines.
- Upload lines have the file's filename, mime_type and size, from the metadata Shrine's upload event carries. - Metadata lines have how long extracting the metadata took. They had a `metadata` field and exists lines an `exist` field that Shrine's events don't carry, so they were never written. - The integration no longer passes Shrine's IO and name payload on to the line.
- An upload line is written when store! stores a cached file. A record's save calls store! on every uploader, which wrote an upload line with no file. - download! (a remote URL fetched into the cache) writes a download line, and remove! a delete line. - retrieve_from_store! isn't logged: it only points the uploader at a stored file, and ran each time a record's mounted file was read. - A problem building the line is a LogStruct error (logstruct_errors: logged in production, raised in development and test), so it can't fail an upload in production.
- Mail error lines say "Email delivery failed" with the mail's recipients (to, cc and bcc, scrubbed like any string), not "Email delivery error, will retry": LogStruct doesn't retry anything, and the error's own recipients were often missing. - The entry a mailer's own rescue_from handler logs has level error, like its line. It said info, which log.logstruct subscribers saw. - The fallback handlers (where LogStruct is off) share one set of methods between a mailer and the mailer class.
Blocked host lines have blocked_hosts: every host Rails blocked (the Host header, and X-Forwarded-Host). They had allowed_hosts and allow_ip_hosts fields that Rails doesn't pass to the response app, so they were never written.
An error the app didn't handle (Rails' 500 page, and the errors Rails turns into a 404 or 422) is an error line with src rails, the error's class and message, the trace Rails logs, the request's path, method and IP, and the response's status, instead of Rails' block of text. It's written where DebugExceptions logs, at config.action_dispatch.debug_exception_log_level (fatal by default), and log_rescued_responses = false still leaves out the 404s and 422s. A CSRF or IP spoofing error has only its security line.
The Sidekiq logger reads Sidekiq::Context once, and writes no ctx when there's no job context, instead of an empty Hash.
- Every time in a struct line (scheduled_at, started_at, finished_at, ...) is written with milliseconds, like ts. - Error lines' backtraces were cut to 5 lines before the formatter's limits applied. They're cut at limits.max_backtrace_lines, whose default is now 20.
Type error lines and reports have a backtrace that starts at the code that failed, below sorbet-runtime's frames. Method signature errors weren't raised, so they had none, and the others started inside sorbet-runtime.
…inds by key only The phone scrubber now matches US numbers written with a leading 1 (1-555-123-4567), and the IPv6 scrubber matches addresses with a zone index (fe80::1%en0). The SSN scrubber leaves SSN-shaped parts of longer numbers alone (a part number like 123-45-6789-0). SQL bind values were filtered when a word in the value looked sensitive, which filtered ordinary names and left real secrets under ordinary column names. Binds are now filtered by their column's key, like every other value, plus long values with no spaces (tokens and digests). ActiveRecordTest restores the notifier it replaces, so later tests keep the ActiveJob subscriber.
…e, and spellcheck TypeScript CI's Ruby matrix runs without the development group, so the gemspec's development dependencies (the gems the integrations are tested with) weren't installed and those tests were skipped. The gemspec's dependencies now go in the test group. The docs job runs the Playwright specs against the built site. The spellcheck covers .ts and .tsx files, and drops the docs lockfile dictionary that nothing generated. Contractions written as ' and ANSI color codes in strings are ignored, so pages don't need their own cspell:ignore lines. Playwright's results folder is no longer committed.
…ptions An upload's content_disposition option (which Shrine's upload_options plugin is often set up to build from the file's name) was logged as given, with the file's original name in it. Its filename and filename* parameters are replaced.
…eported one A custom error_reporting_handler that raised (a timeout calling its service) made log_and_report_error raise, and replaced the original error in log_and_reraise_error and in handle_exception's Report modes. The reporter's error is now a logstruct error line, and the reported error is handled as before.
The cut-end cleanup ran on the kept part, so a value the scrubbers had seen whole
("Basic plan" ending at the cut, or a URL that fit in what they saw) lost its end
too. It now runs on the end of what the scrubbers saw, where a value they saw only
part of ends. A string the scrubbers saw whole, which their replacements made
short enough, is kept whole.
…e job that raised When Rails' own enqueue loop raised, the error line went to the first job that wasn't enqueued, which could be one an earlier ActiveJob::EnqueueError refused, and that job's own error was lost. A refused job keeps its error line, the error goes to the job the loop stopped at, and an adapter that marked every job enqueued before raising gets an error line for the batch.
On Rails main, ActiveStorage's, ActionMailer's and ActiveRecord's log subscribers are ActiveSupport::EventReporter::LogSubscribers that read Rails.event, and have no detach_from. Detaching ActiveStorage's raised NoMethodError at boot. One helper now detaches a subscriber either way, as the ActiveJob integration already did.
Lograge detaches Rails' ActionController and ActionView log subscribers from ActiveSupport::Notifications. On Rails main they read Rails.event instead, so every request also logged "Processing by" and "Completed 200 OK" text lines beside the request line. LogStruct now unsubscribes them from Rails.event after initialization, unless lograge's keep_original_rails_log is set.
checkout, setup-node, upload-artifact, download-artifact, the Pages actions, setup-go, setup-terraform and setup-tflint move to their Node 24 releases, pinned by SHA. GitHub warned that it forces Node 20 actions to run on Node 24.
When Rails can't parse a request's body (malformed JSON), log_parse_error_once writes the whole body at debug level, where nothing filters it, so a password in it was logged. With the rack error handler on, LogStruct logs the body's size and media type instead. The request line and the ParseError's error line are unchanged.
When Rails can't parse a request's params, the parser's message quotes the input
where it stopped: JSON's "unexpected character: '<body>'", Rack's "invalid
%-encoding (<value>)". The error line copied it into cause_message, and Rails'
BadRequest wraps it in its own message ("Invalid request parameters: ..."), so a
password there was logged unfiltered, at any log level.
Error lines for Rails' ParseError (and its ParamErrors), Rack's params errors and
errors raised because of them keep the error and cause classes and Rails' words
before the input, without the parser's message.
…self An app that rescues a request's params error and passes it to LogStruct.handle_exception (or logs it with Rails.logger.error) got a line built by Log.from_exception, which copied the error's message: for Rack's errors, the value it couldn't decode. ErrorLines' rule moves to ParamsErrors, which Log.from_exception uses too.
Rails.logger.error(error) goes through LogStruct's SemanticLogger formatters, not Log.from_exception. The JSON formatter's error line copied the exception's message, and the color formatter wrote SemanticLogger's text for the exception and each cause, so a params error logged the input its parser quoted. Both use ParamsErrors now; for the color text, ParamsErrors.replacements gives each quoting message in the chain, longest first.
…ored output Rack's and Rails' depth limits raise without a message, so the message is the class name. The colored output replaced it everywhere, including the class label: "Cause: (left out: ...): (left out: ...)". A message that is the class name quotes no input, so it stays.
|
Warning Review limit reached
This review includes 190 billable files and costs up to $47.50.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 39 minutes for your next included review. View limit details
Comment |
This was referenced Oct 17, 2026
Merged
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
1 participant
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.
Last of twelve stacked PRs for 0.2.0. Every page of the docs, the README and the homepage was checked against what the
code does. Where the code didn't do what a page promised, or did it wrong, the code is fixed here; otherwise the page
is corrected.
Fixes found by the audit
Logger setup
config/initializers(prefer_json_in_development,enable_color_output,color_map,filter_noisy_loggers,local_environments) did nothing when LogStruct was enabled at boot: its appenders wereadded before the initializers ran. They're added again after, and only LogStruct's own appenders are replaced, so
appenders an app adds are kept.
rails serverin development printed every message twice: once as LogStruct's line and again as text from theSTDOUT logger Rails broadcasts to.
broadcast_tonow ignores a logger that writes to STDOUT or STDERR whenSemanticLogger already does. Loggers added with
broadcast_togot eachlogger << "..."line twice.logger.tagged("billing"),config.log_tagsand ActiveJob's job tags are written astags, and named tags (logger.tagged(user_id: 1)) inctx, filtered and scrubbed.log.logstructsubscriber that raised stopped the line from being written. Subscribers now get the entry afterits line is queued to be written, and a subscriber's error is handled like the app's other errors
(
standard_errors, which raises it from the log call by default).lograge_custom_optionsproc's returned Hash was thrown away. It's now merged into the request line.source_ipandhost, as Rails' own request line does.ts. Error backtraces are cut atlimits.max_backtrace_lines, now 20 bydefault (they were cut to 5 before the limits applied).
srcrails, the error, the request, the response'sstatus) instead of Rails' block of text, at Rails'
debug_exception_log_level(fatalby default).then replaced is removed.
Filtering
filter_keys_with_hashes(email,email_address) are filtered, with a hash of their value.They only changed the summary of keys that were already filtered, so adding
:usernamelogged usernames in full.scrubbed.
apikey,passcode,credit_card,card_numberandsocial_security_number.authorization_credentialssetting, on by default:BearerandBasiccredentials in text are scrubbed._bytes, like Strings: it said how long the secrets in them were.(555) 123-4567,555.123.4567, a leading1or+1). SSN-shaped parts of longer numbers are left alone. The opt-in scrubbers match dashed MAC addresses and IPv6addresses, zones included.
keyorauth: binds are filtered by their column names,and values shaped like tokens still are.
Integrations
retry_on,retry_stoppedanddiscard_onlines; a job the queue couldn't take is an error line, notan enqueue line;
perform_all_laterwrites a line per job; the inline adapter logs a failed job once.prefix. Breaking:the stream line is removed (Rails sends no such event).
and deletes, not every
retrieve_from_store!and every record save.level
error. Mailers keep their logger, so an app's ownlogger.infoin a mailer is written; Rails' mail lines aredetached instead. Where LogStruct is off, mailers still have
log_and_ignore_errorand the other handlers, so arescue_fromthat names one doesn't raiseNameError.blocked_hosts).ctx.config.integrations.enable_semantic_logger, whichdid nothing, is removed.
Fixes from the second review
A second review of the last four PRs found these. The commits at the end of this branch fix them, and what turned up
while fixing them:
limits.max_string_bytescould keep the start of a credential longer than the 256 bytes thescrubbers see past the cut: a URL password whose closing
@was beyond what they saw (or that has an@of itsown), and
Basiccredentials cut short, which don't decode. Both are replaced at the end of what the scrubberssaw, so values they saw whole aren't touched.
filename,mime_type,sizeormetadata(Rails' events never carry them; metadata lines have the
mime_typeinstead), and Shrine's upload lines leave outthe file's original name, which can name a person. Shrine's upload options are logged with the file names taken
out of a
content_disposition.perform_all_lateron Sidekiq's adapter wrote no lines: the adapter says only how many jobs it enqueued. Each jobhas its line when it enqueued them all, there's one warn line with the counts when it didn't, and one error line
when the adapter raised. With Rails' own enqueue loop, a refused job keeps its error line and the error that
stopped the loop goes to the job that raised it.
version.rbbefore any job runs, so the provider sync can't publishfor a mismatched tag.
status,cause_classandcause_messagefields instead ofadditional_data, and writes its trace as strings when the app has no backtrace cleaner (it raised a type error).filter_noisy_loggerswrote only the noisy loggers' lines and left out every other line (SemanticLogger keeps thenames a Regexp filter matches). It's a Proc that leaves them out.
logger.tagged(password: ...)wrote the password. Named tags are filtered as a JSON line's
ctxis, and colored Hash and LogStruct lines are theJSON line, pretty-printed, with their message and tags.
"card.number"), every line and top-level value was copied toindifferent access before the size limits cut it. The parameters are built only when a three-argument filter
block reads them (
PathFilterMatchergainsuses_root:).log_and_report_erroralways reports the error and never raises it, as its name says (inRaisemode it raised without reporting).
log_and_reraise_erroralways raises, and reports only in theReportmodes.error_reporting_handlerthat raised (a timeout calling its service) replaced the error it wasreporting, and made
log_and_report_errorraise. Its error is logged as alogstructerror line instead.T.mustin production code, and the CarrierWave tests are type-checked.Rails 8.2 (Rails main)
CI's pilot job runs the Rails app tests on Rails main, where the framework log subscribers read
Rails.eventandhave no
detach_from:NoMethodError. One helper detachesActiveStorage's, ActionMailer's, ActiveRecord's and ActiveJob's subscribers on every version.
lines only on earlier versions, so LogStruct unsubscribes them from
Rails.eventthere (unlesskeep_original_rails_logis set).Params Rails can't parse
When Rails can't parse a request's params, nothing can filter them, and two lines wrote them anyway (on every Rails
version):
type instead.
(...)"), and the error line copied that into
cause_message, or intomsgthrough Rails' "Invalid requestparameters: ...". Those lines keep the error and cause classes and Rails' words before the input.
Docs, README and homepage
ActiveJob, GoodJob, Sidekiq, SQL and Security. Filtering Sensitive Data, SOC 2 and Your Logs, Logging to STDOUT, the
Postmark guide and the comparison with other gems are rewritten.
test/code_examples, and the JSON shown under it is what the codewrites.
the gem writes for the same values. The integrations page rendered differently on the server and in the browser
(hydration error 418, from timestamps); its lines now use one fixed time.
each integrated gem the tests use.
Tooling
group), and runs the site's Playwright specs.
SHA.
task gem:releasetags the version on main and pushes the tag. Bundler'srake releaserefuses, and thepost-commit hook no longer tags commits that change
version.rb.Release
The last commit sets the version to 0.2.0 and dates the changelog. Merging this stack and tagging
v0.2.0onmain releases it.
Testing
New tests for each fix, each checked to fail without it. At this branch's tip: 490 unit tests pass, and 75 Rails app
tests on Rails 7.1.6 and on Rails main, as do Sorbet, RuboCop, cspell, Biome, tsc, Jest (184 tests), the site build
with its link check, Playwright (14 specs), actionlint and the generated-files check.
Stack for 0.2.0, merged in order: #16 → #17 → #18 → #19 → #20 → #21 → #22 → #23 → #24 → #25 → #26 → #27