Skip to content

Commit

Permalink
Instrument entire Sinatra request (#1112)
Browse files Browse the repository at this point in the history
Add the EventHandler to the default Sinatra instrumentation. This will
have it report more of the request runtime.

It will also report the `response_status` tag and metric, as reported by
the EventHandler.

This is compatible with other instrumentations using the EventHandler or
an AbstractMiddleware subclass.

Part of #329
  • Loading branch information
tombruijn authored Jun 25, 2024
1 parent b65d667 commit 15b3390
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 3 deletions.
6 changes: 6 additions & 0 deletions .changesets/instrument-the-entire-sinatra-request.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: change
---

Instrument the entire Sinatra request. Instrumenting Sinatra apps using `require "appsignal/integrations/sinatra"` will now report more of the request, if previously other middleware were not instrumented. It will also report the response status with the `response_status` tag and metric.
8 changes: 7 additions & 1 deletion lib/appsignal/integrations/sinatra.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,10 @@
Appsignal.start
end

::Sinatra::Base.use(Appsignal::Rack::SinatraBaseInstrumentation) if Appsignal.active?
if Appsignal.active?
::Sinatra::Base.use(
::Rack::Events,
[Appsignal::Rack::EventHandler.new]
)
::Sinatra::Base.use(Appsignal::Rack::SinatraBaseInstrumentation)
end
10 changes: 8 additions & 2 deletions spec/lib/appsignal/integrations/sinatra_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,13 @@ def uninstall_sinatra_integration
context "when AppSignal is not active" do
it "does not add the instrumentation middleware to Sinatra::Base" do
install_sinatra_integration
expect(Sinatra::Base.middleware.to_a).to_not include(
middlewares = Sinatra::Base.middleware.to_a
expect(middlewares).to_not include(
[Appsignal::Rack::SinatraBaseInstrumentation, [], nil]
)
expect(middlewares).to_not include(
[Rack::Events, [Appsignal::Rack::EventHandler], nil]
)
end
end

Expand All @@ -63,7 +67,9 @@ def uninstall_sinatra_integration
ENV["APPSIGNAL_PUSH_API_KEY"] = "my-key"

install_sinatra_integration
expect(Sinatra::Base.middleware.to_a).to include(
middlewares = Sinatra::Base.middleware.to_a
expect(middlewares).to include(
[Rack::Events, [[Appsignal::Rack::EventHandler]], nil],
[Appsignal::Rack::SinatraBaseInstrumentation, [], nil]
)
end
Expand Down

0 comments on commit 15b3390

Please sign in to comment.