Skip to content

Conversation

solnic
Copy link
Collaborator

@solnic solnic commented Aug 14, 2025

This adds config.rails.structured_logging setup that can be used to enable structured logging for common rails components:

  • action_controller
  • active_record
  • action_mailer
  • active_job

Closes #2605

Screenshots

Screenshot 2025-08-25 at 10 57 03 Screenshot 2025-08-25 at 10 56 58 Screenshot 2025-08-25 at 10 56 51

@solnic solnic linked an issue Aug 14, 2025 that may be closed by this pull request
4 tasks
@solnic solnic changed the title 2605 support for activesupport instrumentation in sentrylogging Support for ActiveSupport log subscribers Aug 14, 2025
@solnic solnic force-pushed the 2605-support-for-activesupport-instrumentation-in-sentrylogging branch 6 times, most recently from 016b4ad to b1bae5d Compare August 21, 2025 07:55
@solnic solnic force-pushed the 2605-support-for-activesupport-instrumentation-in-sentrylogging branch 17 times, most recently from 0523e66 to 7814451 Compare August 25, 2025 08:29
Copy link

codecov bot commented Aug 25, 2025

Codecov Report

❌ Patch coverage is 91.66667% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.12%. Comparing base (a3d87a6) to head (93903de).

Files with missing lines Patch % Lines
...try/rails/log_subscribers/active_job_subscriber.rb 87.27% 7 Missing ⚠️
...ls/log_subscribers/action_controller_subscriber.rb 91.83% 4 Missing ⚠️
.../rails/log_subscribers/active_record_subscriber.rb 93.84% 4 Missing ⚠️
...entry-rails/lib/sentry/rails/structured_logging.rb 80.95% 4 Missing ⚠️
sentry-rails/lib/sentry/rails/configuration.rb 80.00% 3 Missing ⚠️
sentry-ruby/lib/sentry/configuration.rb 50.00% 3 Missing ⚠️
sentry-rails/lib/sentry/rails/log_subscriber.rb 95.00% 1 Missing ⚠️
.../rails/log_subscribers/action_mailer_subscriber.rb 96.87% 1 Missing ⚠️
sentry-ruby/lib/sentry-ruby.rb 75.00% 1 Missing ⚠️
sentry-ruby/lib/sentry/test_helper.rb 85.71% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2676      +/-   ##
==========================================
- Coverage   97.44%   97.12%   -0.33%     
==========================================
  Files         136      144       +8     
  Lines        5317     5660     +343     
==========================================
+ Hits         5181     5497     +316     
- Misses        136      163      +27     
Components Coverage Δ
sentry-ruby 97.72% <91.93%> (-0.05%) ⬇️
sentry-rails 94.75% <91.60%> (-1.36%) ⬇️
sentry-sidekiq 96.57% <ø> (ø)
sentry-resque 94.44% <ø> (ø)
sentry-delayed_job 94.68% <ø> (ø)
sentry-opentelemetry 99.31% <ø> (ø)
Files with missing lines Coverage Δ
sentry-rails/lib/sentry/rails.rb 100.00% <100.00%> (ø)
...b/sentry/rails/log_subscribers/parameter_filter.rb 100.00% <100.00%> (ø)
sentry-rails/lib/sentry/rails/railtie.rb 98.83% <100.00%> (+0.05%) ⬆️
sentry-ruby/lib/sentry/debug_structured_logger.rb 100.00% <100.00%> (ø)
sentry-rails/lib/sentry/rails/log_subscriber.rb 95.00% <95.00%> (ø)
.../rails/log_subscribers/action_mailer_subscriber.rb 96.87% <96.87%> (ø)
sentry-ruby/lib/sentry-ruby.rb 98.73% <75.00%> (-0.41%) ⬇️
sentry-ruby/lib/sentry/test_helper.rb 97.95% <85.71%> (-2.05%) ⬇️
sentry-rails/lib/sentry/rails/configuration.rb 94.82% <80.00%> (-5.18%) ⬇️
sentry-ruby/lib/sentry/configuration.rb 97.42% <50.00%> (-0.94%) ⬇️
... and 4 more

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@solnic solnic force-pushed the 2605-support-for-activesupport-instrumentation-in-sentrylogging branch 2 times, most recently from c8f3822 to cc39c9c Compare August 25, 2025 08:58
@solnic solnic marked this pull request as ready for review August 25, 2025 09:16
@solnic solnic requested a review from sl0thentr0py August 25, 2025 09:16
Comment on lines 173 to 174
@structured_logging.enabled = false
when StructuredLoggingConfiguration

Choose a reason for hiding this comment

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

Potential bug: A typo in the structured_logging= setter (.enable instead of .enabled) causes a NoMethodError, crashing the application when enabling the feature.
  • Description: The structured_logging= setter method contains a typo. When true is passed as the value, the code attempts to call @structured_logging.enable = true. However, the StructuredLoggingConfiguration class does not have an enable= method; it only defines enabled=. This will raise a NoMethodError when a user attempts to enable the feature with config.rails.structured_logging = true, causing the application to crash during its initialization phase. The configuration works as expected when set to false.

  • Suggested fix: In the structured_logging= method, change the line @structured_logging.enable = true to @structured_logging.enabled = true. This aligns the setter with the enabled attribute defined in the StructuredLoggingConfiguration class, resolving the NoMethodError.
    severity: 0.95, confidence: 1.0

Did we get this right? 👍 / 👎 to inform future reviews.

cursor[bot]

This comment was marked as outdated.

@solnic solnic force-pushed the 2605-support-for-activesupport-instrumentation-in-sentrylogging branch from 93903de to 98226ec Compare August 26, 2025 11:42
@solnic solnic marked this pull request as draft August 26, 2025 11:42
@solnic solnic removed the request for review from sl0thentr0py August 26, 2025 11:42
Copy link

Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Support for ActiveSupport log subscribers ([#2676](https://github.com/getsentry/sentry-ruby/pull/2676))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description or adding a skip-changelog label.

Generated by 🚫 dangerJS against 98226ec

end

if RUBY_VERSION >= "3.5"
if ruby_version >= Gem::Version.new("2.5")
Copy link

Choose a reason for hiding this comment

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

Bug: Inconsistent CGI Gem Requirement Across Ruby Versions

The cgi gem is now required for Ruby 2.5+, but it was only extracted from the standard library in Ruby 3.0. This leads to unnecessary installation and potential conflicts for Ruby 2.5-2.7, where it's already built-in.

Fix in Cursor Fix in Web

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.

Support for ActiveSupport instrumentation in Sentry::Logging
1 participant