From f6d6c927fe31f2af24bf824eb2aedffc0714e412 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Fri, 17 Oct 2025 09:07:41 +0200 Subject: [PATCH 1/3] Revert "Disable Rails 8.1 CI builds (#1470)" This reverts commit c0eee2ea16db1086e4c3346f77780e11af683a6b. --- .github/workflows/ci.yml | 110 ++++++++++++++++++++++++++++++++++++++- build_matrix.yml | 16 +++--- 2 files changed, 117 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0d3caa11..afbb2711 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2,7 +2,7 @@ # This is a generated file by the `rake build_matrix:github:generate` task. # See `build_matrix.yml` for the build matrix. # Generate this file with `rake build_matrix:github:generate`. -# Generated job count: 194 +# Generated job count: 198 --- name: Ruby gem CI 'on': @@ -643,6 +643,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/rails-8.0.gemfile + ruby_3-5-0-preview1__rails-8-1_ubuntu-latest: + name: Ruby 3.5.0-preview1 - rails-8.1 + needs: ruby_3-5-0-preview1_ubuntu-latest + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + - name: Install Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.5.0-preview1 + bundler-cache: true + - name: Install gem extension + run: "./script/bundler_wrapper exec rake extension:install" + - name: Print extension install report + run: "[ -e ext/install.report ] && cat ext/install.report || echo 'No ext/install.report + file found'" + - name: Print Makefile log file + run: "[ -f ext/mkmf.log ] && cat ext/mkmf.log || echo 'No ext/mkmf.log file + found'" + - name: Run tests + run: "./script/bundler_wrapper exec rake test" + env: + RAILS_ENV: test + JRUBY_OPTS: '' + COV: '1' + BUNDLE_GEMFILE: gemfiles/rails-8.1.gemfile ruby_3-5-0-preview1__sequel_ubuntu-latest: name: Ruby 3.5.0-preview1 - sequel needs: ruby_3-5-0-preview1_ubuntu-latest @@ -1403,6 +1430,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/rails-8.0.gemfile + ruby_3-4-1__rails-8-1_ubuntu-latest: + name: Ruby 3.4.1 - rails-8.1 + needs: ruby_3-4-1_ubuntu-latest + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + - name: Install Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.4.1 + bundler-cache: true + - name: Install gem extension + run: "./script/bundler_wrapper exec rake extension:install" + - name: Print extension install report + run: "[ -e ext/install.report ] && cat ext/install.report || echo 'No ext/install.report + file found'" + - name: Print Makefile log file + run: "[ -f ext/mkmf.log ] && cat ext/mkmf.log || echo 'No ext/mkmf.log file + found'" + - name: Run tests + run: "./script/bundler_wrapper exec rake test" + env: + RAILS_ENV: test + JRUBY_OPTS: '' + COV: '1' + BUNDLE_GEMFILE: gemfiles/rails-8.1.gemfile ruby_3-4-1__sequel_ubuntu-latest: name: Ruby 3.4.1 - sequel needs: ruby_3-4-1_ubuntu-latest @@ -2190,6 +2244,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/rails-8.0.gemfile + ruby_3-3-4__rails-8-1_ubuntu-latest: + name: Ruby 3.3.4 - rails-8.1 + needs: ruby_3-3-4_ubuntu-latest + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + - name: Install Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.3.4 + bundler-cache: true + - name: Install gem extension + run: "./script/bundler_wrapper exec rake extension:install" + - name: Print extension install report + run: "[ -e ext/install.report ] && cat ext/install.report || echo 'No ext/install.report + file found'" + - name: Print Makefile log file + run: "[ -f ext/mkmf.log ] && cat ext/mkmf.log || echo 'No ext/mkmf.log file + found'" + - name: Run tests + run: "./script/bundler_wrapper exec rake test" + env: + RAILS_ENV: test + JRUBY_OPTS: '' + COV: '1' + BUNDLE_GEMFILE: gemfiles/rails-8.1.gemfile ruby_3-3-4__sequel_ubuntu-latest: name: Ruby 3.3.4 - sequel needs: ruby_3-3-4_ubuntu-latest @@ -2923,6 +3004,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/rails-8.0.gemfile + ruby_3-2-5__rails-8-1_ubuntu-latest: + name: Ruby 3.2.5 - rails-8.1 + needs: ruby_3-2-5_ubuntu-latest + runs-on: ubuntu-latest + steps: + - name: Check out repository + uses: actions/checkout@v4 + - name: Install Ruby + uses: ruby/setup-ruby@v1 + with: + ruby-version: 3.2.5 + bundler-cache: true + - name: Install gem extension + run: "./script/bundler_wrapper exec rake extension:install" + - name: Print extension install report + run: "[ -e ext/install.report ] && cat ext/install.report || echo 'No ext/install.report + file found'" + - name: Print Makefile log file + run: "[ -f ext/mkmf.log ] && cat ext/mkmf.log || echo 'No ext/mkmf.log file + found'" + - name: Run tests + run: "./script/bundler_wrapper exec rake test" + env: + RAILS_ENV: test + JRUBY_OPTS: '' + COV: '1' + BUNDLE_GEMFILE: gemfiles/rails-8.1.gemfile ruby_3-2-5__sequel_ubuntu-latest: name: Ruby 3.2.5 - sequel needs: ruby_3-2-5_ubuntu-latest diff --git a/build_matrix.yml b/build_matrix.yml index 15846ce8..df49232e 100644 --- a/build_matrix.yml +++ b/build_matrix.yml @@ -115,7 +115,7 @@ matrix: - "rails-7.1" - "rails-7.2" - "rails-8.0" - # - "rails-8.1" + - "rails-8.1" ruby: - ruby: "3.5.0-preview1" @@ -245,13 +245,13 @@ matrix: - "3.4.1" - "3.3.4" - "3.2.5" - # - gem: "rails-8.1" - # only: - # ruby: - # - "3.5.0-preview1" - # - "3.4.1" - # - "3.3.4" - # - "3.2.5" + - gem: "rails-8.1" + only: + ruby: + - "3.5.0-preview1" + - "3.4.1" + - "3.3.4" + - "3.2.5" - gem: "sequel" - gem: "sinatra" - gem: "webmachine2" From 79e5d5b264411c837db264f84207f124babae9bf Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Thu, 16 Oct 2025 17:13:18 +0200 Subject: [PATCH 2/3] Monkey patch `NullHandle` for Rails 8.1 Monkey patch the `ActiveSupport::Notifications` `NullHandle` so that we get events reported again. The `NullHandle` is used when no subscribers are registered. That means we get no events reported when there are no subscribers. We're not a subscriber because of a performance problem in PR #150 we decided instead to monkey patch it. See issue #1471 ## To do - This requires unit tests. - This requires testing in a real world Rails 8.1 app. --- .../hooks/active_support_notifications.rb | 11 ++++++ .../active_support_notifications.rb | 35 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/lib/appsignal/hooks/active_support_notifications.rb b/lib/appsignal/hooks/active_support_notifications.rb index a238d6f9..e8032f6f 100644 --- a/lib/appsignal/hooks/active_support_notifications.rb +++ b/lib/appsignal/hooks/active_support_notifications.rb @@ -29,6 +29,17 @@ def self.instrument(name, payload = {}) parent_integration_module::StartFinishHandlerIntegration, ::ActiveSupport::Notifications::Fanout::Handle ) + + # Rails 8.1+ optimization: when there are no subscribers, build_handle returns + # NullHandle instead of Handle. We need to also hook into Instrumenter to + # catch these cases. + if defined?(::ActiveSupport::Notifications::Fanout::NullHandle) + instrumenter = ::ActiveSupport::Notifications::Instrumenter + install_module( + parent_integration_module::NullHandleAwareInstrumentIntegration, + instrumenter + ) + end else instrumenter = ::ActiveSupport::Notifications::Instrumenter diff --git a/lib/appsignal/integrations/active_support_notifications.rb b/lib/appsignal/integrations/active_support_notifications.rb index 3d20daab..e65340d1 100644 --- a/lib/appsignal/integrations/active_support_notifications.rb +++ b/lib/appsignal/integrations/active_support_notifications.rb @@ -99,6 +99,41 @@ def finish_with_state(listeners_state, name, payload = {}) super end end + + # Rails 8.1+ introduced NullHandle as a performance optimization. + # This integration only instruments when NullHandle is being used + # (i.e., when there are no other subscribers). + module NullHandleAwareInstrumentIntegration + def instrument(name, payload = {}, &block) + handle = build_handle(name, payload) + + # Only instrument if NullHandle is being used. + # If Handle is being used, StartFinishHandlerIntegration will handle it. + if defined?(::ActiveSupport::Notifications::Fanout::NullHandle) && + handle == ::ActiveSupport::Notifications::Fanout::NullHandle + instrument_this = name[0] != ActiveSupportNotificationsIntegration::BANG + + Appsignal::Transaction.current.start_event if instrument_this + + begin + super + ensure + if instrument_this + title, body, body_format = Appsignal::EventFormatter.format(name, payload) + Appsignal::Transaction.current.finish_event( + name.to_s, + title, + body, + body_format + ) + end + end + else + # Regular Handle case: let StartFinishHandlerIntegration handle it + super + end + end + end end end end From b14f7c3ea4973508d227bad32d0754ee7aa3e8d0 Mon Sep 17 00:00:00 2001 From: Jeff Kreeftmeijer Date: Thu, 23 Oct 2025 19:30:52 +0200 Subject: [PATCH 3/3] Don't run start & finish examples when Fanout::NullHandle exists With ActiveSupport::Notifications::Fanout::NullHandle now existing, we have a state where ActiveSupport::Notifications::Fanout::Handle exists, but the start_finish_shared_examples will fail because the NullHandleAwareInstrumentIntegration uses the #instrument integration we use when Fanout::Handle doesn't exist. Therefor, when Fanout::NullHandle exists, we can rely on the main tests and don't have to include the start_finish ones. --- .../active_support_notifications_spec.rb | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/spec/lib/appsignal/hooks/active_support_notifications_spec.rb b/spec/lib/appsignal/hooks/active_support_notifications_spec.rb index 201a2ab0..df7095a7 100644 --- a/spec/lib/appsignal/hooks/active_support_notifications_spec.rb +++ b/spec/lib/appsignal/hooks/active_support_notifications_spec.rb @@ -20,22 +20,24 @@ it_behaves_like "activesupport instrument override" - if defined?(::ActiveSupport::Notifications::Fanout::Handle) - require_relative "active_support_notifications/start_finish_shared_examples" + unless defined?(::ActiveSupport::Notifications::Fanout::NullHandle) + if defined?(::ActiveSupport::Notifications::Fanout::Handle) + require_relative "active_support_notifications/start_finish_shared_examples" - it_behaves_like "activesupport start finish override" - end + it_behaves_like "activesupport start finish override" + end - if ::ActiveSupport::Notifications::Instrumenter.method_defined?(:start) - require_relative "active_support_notifications/start_finish_shared_examples" + if ::ActiveSupport::Notifications::Instrumenter.method_defined?(:start) + require_relative "active_support_notifications/start_finish_shared_examples" - it_behaves_like "activesupport start finish override" - end + it_behaves_like "activesupport start finish override" + end - if ::ActiveSupport::Notifications::Instrumenter.method_defined?(:finish_with_state) - require_relative "active_support_notifications/finish_with_state_shared_examples" + if ::ActiveSupport::Notifications::Instrumenter.method_defined?(:finish_with_state) + require_relative "active_support_notifications/finish_with_state_shared_examples" - it_behaves_like "activesupport finish_with_state override" + it_behaves_like "activesupport finish_with_state override" + end end else describe "#dependencies_present?" do