Proposal: Improve logging format from QueryLogs

[modulitos]

Update: This proposal has been implemented in this PR: #45081

I’m looking for feedback on a proposal to improve the formatting of auto-injected SQL comments in Rails 7 (via QueryLogs). It seems like the current format for SQL comments isn’t correct, because:

1. It isn’t really machine-readable
2. It doesn’t adhere to existing standards, like sqlcommenter

For example, auto-commented SQL queries using Marginalia/Rails look like this:
"select id from posts; /*application:Joe's app,controller:my_controller*/"

and I'm proposing a format to use the “sqlcommenter” syntax like so:
"select id from posts; /*application='Joe\\'s app',controller='my_controller'*/"

The latter can be parsed much more easily, and will also automatically report metrics by databases that support the sqlcommenter formatting.

This also feels like a good time to propose a change because it's relatively new functionality, before too many folks develop a dependency on the format.

[tenderlove]

This also feels like a good time to propose a change because it's relatively new functionality, before too many folks develop a dependency on the format.

I think this ship has already sailed and we need to do a proper deprecation / application config change. We're not going to be backporting this to the 7-0-stable series (it's a new feature, not a bug), which gives people the lifetime of the 7.0.X series to "develop a dependency on the format". That means we really need to support both formats for the time being.

[modulitos]

That sounds good - I'll make sure we have support for both formats 👍

Note that I'm assuming the legacy format will be the default, and that we can deprecate it as a breaking change later. But if that's not the case, just let me know!

[modulitos]

Just documenting this for future reference:

If/when we merge in the attached PR to support sqlcommenter formatting, we can configure Rails 8 to use the sqlcommenter format by default with the following change in ActiveRecord::Railtie:

config.active_record.query_log_tags_format = :sqlcommenter

Assuming the attached PR gets merged, I'll keep an eye out for a Rails 8 feature branch so we can make this change!

[skipkayhil]

we can configure Rails 8 to use the sqlcommenter format by default

If the goal is to make it the default, should we not create the new_framework_default now?

[modulitos]

Ooh interesting. It doesn't seem like a blocker for the attached PR, but I'm more than happy to implement this.

I'm reading up on the configure framework defaults feature, and it seems like I'll want to create a new file like railties/lib/rails/generators/rails/app/templates/config/initializers/new_framework_defaults_8_0.rb.tt with the following contents:

# Be sure to restart your server when you modify this file.
#
# This file eases your Rails 8.0 framework defaults upgrade.
#
# Uncomment each configuration one by one to switch to the new default.
# Once your application is ready to run with all new defaults, you can remove
# this file and set the `config.load_defaults` to `8.0`.
#
# Read the Guide for Upgrading Ruby on Rails for more info on each option.
# https://guides.rubyonrails.org/upgrading_ruby_on_rails.html

# If config.active_record.query_log_tags_enabled is enabled, then comments will now use SQLCommenter formatting by default. To continue using the legacy comment format:
# Rails.application.config.active_record.query_log_tags_format = :legacy

Does that seem correct? Lmk if there's anything else I can do 👍

[skipkayhil]

It doesn't seem like a blocker for the attached PR

Agree, we can probably wait until the PR is merged before handling this. Assuming we want this in the next major version, you wouldn't need to create any new files. You would just follow this comment (4-6 really):

rails/railties/lib/rails/application/configuration.rb

Lines 90 to 96 in 9ff7c9c

	# To introduce a change in behavior, follow these steps:
	# 1. Add an accessor on the target object (e.g. the ActiveJob class for global Active Job config).
	# 2. Set a default value there preserving existing behavior for existing applications.
	# 3. Implement the behavior change based on the config value.
	# 4. In the section below corresponding to the next release of Rails, set the new value.
	# 5. Add a commented out section in the `new_framework_defaults` template, setting the new value.
	# 6. Update the guide in `configuration.md`.

[modulitos]

@skipkayhil Thanks again for the tip 💯

I configured the defaults for the 7.1 Rails release in this PR: #46180

which also outlines the plan for how we'll use SQLCommenter as the default formatter. Since this issue is closed, I created a new issue to track that work: #46179 which should be resolved by the PR linked above.