Skip to content

Rails active support log subscribers#2690

Merged
solnic merged 21 commits intomasterfrom
2605-rails-active-support-log-subscribers
Sep 3, 2025
Merged

Rails active support log subscribers#2690
solnic merged 21 commits intomasterfrom
2605-rails-active-support-log-subscribers

Conversation

@solnic
Copy link
Copy Markdown
Collaborator

@solnic solnic commented Aug 27, 2025

Adds log subscribers for common Rails components.

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

Closes #2605

@solnic solnic force-pushed the 2605-rails-active-support-log-subscribers branch 4 times, most recently from e2de725 to 10b1acd Compare August 27, 2025 11:07
@solnic solnic force-pushed the 2605-rails-active-support-log-subscribers branch 2 times, most recently from e9a1bb4 to 6a8cea8 Compare August 27, 2025 15:42
@solnic solnic force-pushed the 2605-rails-active-support-log-subscribers branch from 6a8cea8 to be03827 Compare August 27, 2025 15:49
@solnic solnic requested a review from sl0thentr0py August 27, 2025 18:28
@solnic solnic marked this pull request as ready for review August 27, 2025 18:31
sl0thentr0py
sl0thentr0py requested changes Aug 30, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py left a comment

Choose a reason for hiding this comment

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

mainly left comments on

  • overhead
  • configuration api

some more high level feedback:
I would have much preferred this to be a simple proxy of just passing through to our logger whatever rails emits. Instead, now we have special loggers for each component. I will not block this if you are ok with maintaining all this extra code (I do not like maintaining a lot of code, my personal preference.)

Further, there is now a lot of duplication of logic between

  • breadcrumbs
  • logger
  • tracing

To reiterate from a business point of view, Logging is mainly a high volume trace connected feed of events, whereas Tracing is a much more curated instrumentation experience and they both have their place. The way this PR is makes Logging very similar to Performance.

sentry-ruby/lib/sentry/test_helper.rb

module Sentry
module TestHelper
module_function
Copy link
Copy Markdown
Member

@sl0thentr0py sl0thentr0py Aug 30, 2025

Choose a reason for hiding this comment

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

what is this for

Copy link
Copy Markdown
Collaborator Author

@solnic solnic Sep 1, 2025

Choose a reason for hiding this comment

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

@sl0thentr0py makes it possible to call methods as class methods too, or include them as RSpec example helpers. Very handy IMO.

cursor[bot]

This comment was marked as outdated.

Sign in to view

cursor[bot]

This comment was marked as outdated.

Sign in to view

@solnic solnic force-pushed the 2605-rails-active-support-log-subscribers branch 2 times, most recently from afaa8f9 to c5a451e Compare September 2, 2025 08:49
@solnic solnic force-pushed the 2605-rails-active-support-log-subscribers branch 3 times, most recently from 29ed65b to d115e0d Compare September 2, 2025 09:49
@solnic
Clean up specs organization
01ae4cb 
This avoids double-setup of test rails apps which
makes specs faster and fixes warnings
@solnic solnic force-pushed the 2605-rails-active-support-log-subscribers branch from d115e0d to 01ae4cb Compare September 2, 2025 10:55
cursor[bot]

This comment was marked as outdated.

Sign in to view

@solnic solnic requested a review from sl0thentr0py September 2, 2025 11:13
@getsentry getsentry deleted a comment from codecov bot Sep 3, 2025
@solnic solnic merged commit ec50db4 into master Sep 3, 2025
139 of 141 checks passed
@solnic solnic deleted the 2605-rails-active-support-log-subscribers branch September 3, 2025 08:02
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 3, 2025

Codecov Report

❌ Patch coverage is 93.92097% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.25%. Comparing base (337a4ca) to head (b87ad68).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...try/rails/log_subscribers/active_job_subscriber.rb 89.09% 6 Missing ⚠️
.../rails/log_subscribers/active_record_subscriber.rb 95.00% 3 Missing ⚠️
sentry-ruby/lib/sentry/configuration.rb 50.00% 3 Missing ⚠️
...ls/log_subscribers/action_controller_subscriber.rb 96.07% 2 Missing ⚠️
...entry-rails/lib/sentry/rails/structured_logging.rb 88.88% 2 Missing ⚠️
sentry-ruby/lib/sentry/test_helper.rb 77.77% 2 Missing ⚠️
.../rails/log_subscribers/action_mailer_subscriber.rb 96.77% 1 Missing ⚠️
sentry-ruby/lib/sentry-ruby.rb 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #2690      +/-   ##
==========================================
- Coverage   97.44%   97.25%   -0.19%     
==========================================
  Files         136      144       +8     
  Lines        5317     5642     +325     
==========================================
+ Hits         5181     5487     +306     
- Misses        136      155      +19
Components Coverage Δ
sentry-ruby 97.70% <90.62%> (-0.07%) ⬇️
sentry-rails 95.60% <94.71%> (-0.51%) ⬇️
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%> (ø)
sentry-rails/lib/sentry/rails/configuration.rb 100.00% <100.00%> (ø)
sentry-rails/lib/sentry/rails/log_subscriber.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%> (ø)
.../rails/log_subscribers/action_mailer_subscriber.rb 96.77% <96.77%> (ø)
sentry-ruby/lib/sentry-ruby.rb 98.73% <75.00%> (-0.41%) ⬇️
...ls/log_subscribers/action_controller_subscriber.rb 96.07% <96.07%> (ø)
...entry-rails/lib/sentry/rails/structured_logging.rb 88.88% <88.88%> (ø)
... and 4 more

... and 2 files 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 mentioned this pull request Sep 3, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@sl0thentr0py sl0thentr0py sl0thentr0py approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support for ActiveSupport instrumentation in Sentry::Logging

2 participants

@solnic @sl0thentr0py