Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and improve Hanami 2.2 support #1328

Merged
merged 5 commits into from
Nov 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changesets/fix-request-parameter-reporting-for-hanami-2-2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: fix
---

Fix request parameter reporting for Hanami 2.2.
13 changes: 13 additions & 0 deletions .changesets/ignore-hanami-errors-by-default.md
Original file line number Diff line number Diff line change
@@ -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.
6 changes: 6 additions & 0 deletions .changesets/read-hanami-action-name-without-monkeypatch.md
Original file line number Diff line number Diff line change
@@ -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.
83 changes: 82 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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':
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions build_matrix.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
7 changes: 7 additions & 0 deletions gemfiles/hanami-2.2.gemfile
Original file line number Diff line number Diff line change
@@ -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 => "../"
9 changes: 8 additions & 1 deletion lib/appsignal/loaders/hanami.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
tombruijn marked this conversation as resolved.
Show resolved Hide resolved
module HanamiIntegration
def call(env)
super
Expand Down
15 changes: 14 additions & 1 deletion lib/appsignal/rack/hanami_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
35 changes: 28 additions & 7 deletions spec/lib/appsignal/loaders/hanami_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,12 @@
:name => :hanami,
:root_path => Dir.pwd,
:env => :test,
:options => {}
:options => {
:ignore_errors => [
"Hanami::Router::NotAllowedError",
"Hanami::Router::NotFoundError"
]
}
)
end
end
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
33 changes: 31 additions & 2 deletions spec/lib/appsignal/rack/hanami_middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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, {}) }
Expand All @@ -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)

Expand All @@ -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
13 changes: 9 additions & 4 deletions spec/support/helpers/dependency_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Loading