diff --git a/.changesets/fix-request-parameter-reporting-for-hanami-2-2.md b/.changesets/fix-request-parameter-reporting-for-hanami-2-2.md new file mode 100644 index 000000000..dcec61a79 --- /dev/null +++ b/.changesets/fix-request-parameter-reporting-for-hanami-2-2.md @@ -0,0 +1,6 @@ +--- +bump: patch +type: fix +--- + +Fix request parameter reporting for Hanami 2.2. diff --git a/.changesets/ignore-hanami-errors-by-default.md b/.changesets/ignore-hanami-errors-by-default.md new file mode 100644 index 000000000..6df189707 --- /dev/null +++ b/.changesets/ignore-hanami-errors-by-default.md @@ -0,0 +1,13 @@ +--- +bump: patch +type: change +--- + +Ignore these Hanami errors by default: + +- Hanami::Router::NotAllowedError (for example: sending a GET request to POST endpoint) +- Hanami::Router::NotFoundError + +They are usually errors you don't want to be notified about, so we ignore them by default now. + +Customize the `ignore_errors` config option to continue receiving these errors. diff --git a/.changesets/read-hanami-action-name-without-monkeypatch.md b/.changesets/read-hanami-action-name-without-monkeypatch.md new file mode 100644 index 000000000..4db8350f9 --- /dev/null +++ b/.changesets/read-hanami-action-name-without-monkeypatch.md @@ -0,0 +1,6 @@ +--- +bump: patch +type: change +--- + +Read the Hanami Action name without metaprogramming in Hanami 2.2 and newer. This makes our instrumentation more stable whenever something changes in future Hanami releases. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c7dbf0ac4..a7c084bc9 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: 124 +# Generated job count: 127 --- name: Ruby gem CI 'on': @@ -264,6 +264,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/hanami-2.1.gemfile + ruby_3-3-4__hanami-2-2_ubuntu-latest: + name: Ruby 3.3.4 - hanami-2.2 + 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/hanami-2.2.gemfile ruby_3-3-4__http5_ubuntu-latest: name: Ruby 3.3.4 - http5 needs: ruby_3-3-4_ubuntu-latest @@ -916,6 +943,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/hanami-2.1.gemfile + ruby_3-2-5__hanami-2-2_ubuntu-latest: + name: Ruby 3.2.5 - hanami-2.2 + 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/hanami-2.2.gemfile ruby_3-2-5__http5_ubuntu-latest: name: Ruby 3.2.5 - http5 needs: ruby_3-2-5_ubuntu-latest @@ -1568,6 +1622,33 @@ jobs: JRUBY_OPTS: '' COV: '1' BUNDLE_GEMFILE: gemfiles/hanami-2.1.gemfile + ruby_3-1-6__hanami-2-2_ubuntu-latest: + name: Ruby 3.1.6 - hanami-2.2 + needs: ruby_3-1-6_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.1.6 + 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/hanami-2.2.gemfile ruby_3-1-6__http5_ubuntu-latest: name: Ruby 3.1.6 - http5 needs: ruby_3-1-6_ubuntu-latest diff --git a/build_matrix.yml b/build_matrix.yml index 8b984c042..ac02c1297 100644 --- a/build_matrix.yml +++ b/build_matrix.yml @@ -121,6 +121,12 @@ matrix: - "3.2.5" - "3.1.6" - "3.0.7" + - gem: "hanami-2.2" + only: + ruby: + - "3.3.4" + - "3.2.5" + - "3.1.6" - gem: "http5" - gem: "padrino" - gem: "psych-3" diff --git a/gemfiles/hanami-2.2.gemfile b/gemfiles/hanami-2.2.gemfile new file mode 100644 index 000000000..ce08093d4 --- /dev/null +++ b/gemfiles/hanami-2.2.gemfile @@ -0,0 +1,7 @@ +source "https://rubygems.org" + +gem "hanami", "~> 2.2.0" +gem "hanami-controller", "~> 2.2.0" +gem "hanami-router", "~> 2.2.0" + +gemspec :path => "../" diff --git a/lib/appsignal/loaders/hanami.rb b/lib/appsignal/loaders/hanami.rb index e0862ad3c..976b642fe 100644 --- a/lib/appsignal/loaders/hanami.rb +++ b/lib/appsignal/loaders/hanami.rb @@ -9,7 +9,11 @@ def on_load hanami_app_config = ::Hanami.app.config register_config_defaults( :root_path => hanami_app_config.root.to_s, - :env => hanami_app_config.env + :env => hanami_app_config.env, + :ignore_errors => [ + "Hanami::Router::NotAllowedError", + "Hanami::Router::NotFoundError" + ] ) end @@ -23,9 +27,12 @@ def on_start ) hanami_app_config.middleware.use(Appsignal::Rack::HanamiMiddleware) + return unless Gem::Version.new(Hanami::VERSION) < Gem::Version.new("2.2.0") + ::Hanami::Action.prepend Appsignal::Loaders::HanamiLoader::HanamiIntegration end + # Legacy instrumentation to set the action name in Hanami apps older than Hanami 2.2 module HanamiIntegration def call(env) super diff --git a/lib/appsignal/rack/hanami_middleware.rb b/lib/appsignal/rack/hanami_middleware.rb index 1cad0abd2..f3ab8c5cd 100644 --- a/lib/appsignal/rack/hanami_middleware.rb +++ b/lib/appsignal/rack/hanami_middleware.rb @@ -12,12 +12,25 @@ def initialize(app, options = {}) private + HANAMI_ACTION_INSTANCE = "hanami.action_instance" + ROUTER_PARAMS = "router.params" + def add_transaction_metadata_after(transaction, request) + action_name = fetch_hanami_action(request.env) + transaction.set_action_if_nil(action_name) if action_name transaction.add_params { params_for(request) } end def params_for(request) - ::Hanami::Action.params_class.new(request.env).to_h + request.env.fetch(ROUTER_PARAMS, nil) + end + + def fetch_hanami_action(env) + # This env key is available in Hanami 2.2+ + action_instance = env.fetch(HANAMI_ACTION_INSTANCE, nil) + return unless action_instance + + action_instance.class.name end end end diff --git a/spec/lib/appsignal/loaders/hanami_spec.rb b/spec/lib/appsignal/loaders/hanami_spec.rb index fc7942147..4a09dc728 100644 --- a/spec/lib/appsignal/loaders/hanami_spec.rb +++ b/spec/lib/appsignal/loaders/hanami_spec.rb @@ -8,7 +8,12 @@ :name => :hanami, :root_path => Dir.pwd, :env => :test, - :options => {} + :options => { + :ignore_errors => [ + "Hanami::Router::NotAllowedError", + "Hanami::Router::NotFoundError" + ] + } ) end end @@ -37,9 +42,16 @@ def uninstall_hanami_middleware ) end - it "prepends the integration to Hanami::Action" do - expect(::Hanami::Action) - .to have_received(:prepend).with(Appsignal::Loaders::HanamiLoader::HanamiIntegration) + if DependencyHelper.hanami2_2_present? + it "does not prepend a monkeypatch integration to Hanami::Action" do + expect(::Hanami::Action).to_not have_received(:prepend) + .with(Appsignal::Loaders::HanamiLoader::HanamiIntegration) + end + else + it "prepends the integration to Hanami::Action" do + expect(::Hanami::Action).to have_received(:prepend) + .with(Appsignal::Loaders::HanamiLoader::HanamiIntegration) + end end def hanami_middleware_options @@ -80,10 +92,19 @@ def make_request(env) context "with an active transaction" do let(:env) { { Appsignal::Rack::APPSIGNAL_TRANSACTION => transaction } } - it "sets action name on the transaction" do - make_request(env) + if DependencyHelper.hanami2_2_present? + it "does not set an action name on the transaction" do + # This is done by the middleware instead + make_request(env) + + expect(transaction).to_not have_action + end + else + it "sets action name on the transaction" do + make_request(env) - expect(transaction).to have_action("HanamiApp::Actions::Books::Index") + expect(transaction).to have_action("HanamiApp::Actions::Books::Index") + end end end end diff --git a/spec/lib/appsignal/rack/hanami_middleware_spec.rb b/spec/lib/appsignal/rack/hanami_middleware_spec.rb index 2a9687ee9..eccd441fa 100644 --- a/spec/lib/appsignal/rack/hanami_middleware_spec.rb +++ b/spec/lib/appsignal/rack/hanami_middleware_spec.rb @@ -3,11 +3,13 @@ if DependencyHelper.hanami2_present? describe Appsignal::Rack::HanamiMiddleware do let(:app) { double(:call => true) } - let(:router_params) { { "param1" => "value1", "param2" => "value2" } } + let(:router_params) { nil } let(:env) do + options = {} + options["router.params"] = router_params if router_params Rack::MockRequest.env_for( "/some/path", - "router.params" => router_params + options ) end let(:middleware) { Appsignal::Rack::HanamiMiddleware.new(app, {}) } @@ -16,10 +18,29 @@ around { |example| keep_transactions { example.run } } def make_request(env) + if DependencyHelper.hanami2_2_present? + instance = + Class.new do + def self.name + "HanamiApp::Actions::Books::Index" + end + end.new + env["hanami.action_instance"] = instance + end middleware.call(env) end + context "without params" do + it "sets no request parameters on the transaction" do + make_request(env) + + expect(last_transaction).to_not include_params + end + end + context "with params" do + let(:router_params) { { "param1" => "value1", "param2" => "value2" } } + it "sets request parameters on the transaction" do make_request(env) @@ -32,5 +53,13 @@ def make_request(env) expect(last_transaction).to include_event("name" => "process_action.hanami") end + + if DependencyHelper.hanami2_2_present? + it "sets action name on the transaction" do + make_request(env) + + expect(last_transaction).to have_action("HanamiApp::Actions::Books::Index") + end + end end end diff --git a/spec/support/helpers/dependency_helper.rb b/spec/support/helpers/dependency_helper.rb index 429b83a09..ed2bd821b 100644 --- a/spec/support/helpers/dependency_helper.rb +++ b/spec/support/helpers/dependency_helper.rb @@ -136,17 +136,22 @@ def hanami_present? dependency_present? "hanami" end + def hanami2_present? + hanami_present? && Gem.loaded_specs["hanami"].version >= Gem::Version.new("2.0") + end + def hanami2_1_present? hanami_present? && Gem::Version.new(::Hanami::VERSION) >= Gem::Version.new("2.1.0") end - def dry_monitor_present? - dependency_present? "dry-monitor" + def hanami2_2_present? + hanami_present? && + Gem::Version.new(::Hanami::VERSION) >= Gem::Version.new("2.2.0") end - def hanami2_present? - hanami_present? && Gem.loaded_specs["hanami"].version >= Gem::Version.new("2.0") + def dry_monitor_present? + dependency_present? "dry-monitor" end def dependency_present?(dependency_file)