Skip to content

Commit

Permalink
Merge pull request #1146 from appsignal/instrumentation-middleware-up…
Browse files Browse the repository at this point in the history
…dates

Improve instrumentation middleware option naming and documentation
  • Loading branch information
tombruijn authored Jul 8, 2024
2 parents 02f3e70 + db2ffff commit 61e97cf
Show file tree
Hide file tree
Showing 10 changed files with 74 additions and 19 deletions.
3 changes: 2 additions & 1 deletion lib/appsignal/integrations/padrino.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def self.init

Padrino.use ::Rack::Events, [Appsignal::Rack::EventHandler.new]
Padrino.use Appsignal::Rack::SinatraBaseInstrumentation,
:instrument_span_name => "process_action.padrino"
:instrument_event_name => "process_action.padrino"
end
end
end
Expand All @@ -32,6 +32,7 @@ def self.init

module Appsignal
module Integrations
# @api private
module PadrinoIntegration
def route!(base = settings, pass_block = nil)
return super if !Appsignal.active? || env["sinatra.static_file"]
Expand Down
11 changes: 8 additions & 3 deletions lib/appsignal/rack/abstract_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@

module Appsignal
module Rack
# Base instrumentation middleware.
#
# Do not use this middleware directly. Instead use
# {InstrumentationMiddleware}.
#
# @api private
class AbstractMiddleware
DEFAULT_ERROR_REPORTING = :default
Expand All @@ -14,7 +19,7 @@ def initialize(app, options = {})
@options = options
@request_class = options.fetch(:request_class, ::Rack::Request)
@params_method = options.fetch(:params_method, :params)
@instrument_span_name = options.fetch(:instrument_span_name, nil)
@instrument_event_name = options.fetch(:instrument_event_name, nil)
@report_errors = options.fetch(:report_errors, DEFAULT_ERROR_REPORTING)
end

Expand Down Expand Up @@ -77,8 +82,8 @@ def call(env)
#
# @see {#instrument_app_call_with_exception_handling}
def instrument_app_call(env)
if @instrument_span_name
Appsignal.instrument(@instrument_span_name) do
if @instrument_event_name
Appsignal.instrument(@instrument_event_name) do
@app.call(env)
end
else
Expand Down
2 changes: 1 addition & 1 deletion lib/appsignal/rack/generic_instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Rack
# @api private
class GenericInstrumentation < AbstractMiddleware
def initialize(app, options = {})
options[:instrument_span_name] ||= "process_action.generic"
options[:instrument_event_name] ||= "process_action.generic"
super
end

Expand Down
2 changes: 1 addition & 1 deletion lib/appsignal/rack/grape_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module Rack
# @api private
class GrapeMiddleware < Appsignal::Rack::AbstractMiddleware
def initialize(app, options = {})
options[:instrument_span_name] = "process_request.grape"
options[:instrument_event_name] = "process_request.grape"
options[:report_errors] = lambda { |env| !env["grape.skip_appsignal_error"] }
super
end
Expand Down
2 changes: 1 addition & 1 deletion lib/appsignal/rack/hanami_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module Rack
class HanamiMiddleware < AbstractMiddleware
def initialize(app, options = {})
options[:params_method] ||= :params
options[:instrument_span_name] ||= "process_action.hanami"
options[:instrument_event_name] ||= "process_action.hanami"
super
end

Expand Down
51 changes: 50 additions & 1 deletion lib/appsignal/rack/instrumentation_middleware.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,59 @@

module Appsignal
module Rack
# Rack instrumentation middleware.
#
# This Ruby gem automatically instruments several Rack based libraries,
# like Rails and Sinatra. This middleware does not need to be added
# manually to these frameworks.
#
# This instrumentation middleware will wrap an app and report how long the
# request and response took, report errors that occurred in the app, and
# report metadata about the request method and path.
#
# The action name for the endpoint is not set by default, which is required
# for performance monitoring. Set the action name in each endpoint using
# the {Appsignal::Helpers::Instrumentation#set_action} helper.
#
# If multiple of these middlewares, or
# {AbstractMiddleware} subclasses are present in an app, only the top
# middleware will report errors from apps and other middleware.
#
# This middleware is best used in combination with the {EventHandler}.
#
# @example
# # config.ru
# require "appsignal"
# # Configure and start AppSignal
#
# # Add the EventHandler first
# use ::Rack::Events, [Appsignal::Rack::EventHandler.new]
# # Add the instrumentation middleware second
# use Appsignal::Rack::InstrumentationMiddleware
#
# # Other middleware
#
# # Start app
#
# @example Customize instrumentation event category
# use Appsignal::Rack::InstrumentationMiddleware,
# :instrument_event_name => "custom.goup"
#
# @example Disable error reporting for this middleware
# use Appsignal::Rack::InstrumentationMiddleware, :report_errors => false
#
# @example Always report errors, even when wrapped by other instrumentation middleware
# use Appsignal::Rack::InstrumentationMiddleware, :report_errors => true
#
# @example Disable error reporting for this middleware based on the request env
# use Appsignal::Rack::InstrumentationMiddleware,
# :report_errors => lambda { |env| env["some_key"] == "some value" }
#
# @see https://docs.appsignal.com/ruby/integrations/rack.html
# @api public
class InstrumentationMiddleware < AbstractMiddleware
def initialize(app, options = {})
options[:instrument_span_name] ||= "process_request_middleware.rack"
options[:instrument_event_name] ||= "process_request_middleware.rack"
super
end
end
Expand Down
2 changes: 1 addition & 1 deletion lib/appsignal/rack/rails_instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class RailsInstrumentation < Appsignal::Rack::AbstractMiddleware
def initialize(app, options = {})
options[:request_class] ||= ActionDispatch::Request
options[:params_method] ||= :filtered_parameters
options[:instrument_span_name] = nil
options[:instrument_event_name] = nil
options[:report_errors] = true
super
end
Expand Down
2 changes: 1 addition & 1 deletion lib/appsignal/rack/sinatra_instrumentation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ class SinatraBaseInstrumentation < AbstractMiddleware
def initialize(app, options = {})
options[:request_class] ||= Sinatra::Request
options[:params_method] ||= :params
options[:instrument_span_name] ||= "process_action.sinatra"
options[:instrument_event_name] ||= "process_action.sinatra"
super
@raise_errors_on = raise_errors?(app)
end
Expand Down
6 changes: 3 additions & 3 deletions spec/lib/appsignal/integrations/padrino_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def uninstall_padrino_integration
[
Appsignal::Rack::SinatraBaseInstrumentation,
[
:instrument_span_name => "process_action.padrino"
:instrument_event_name => "process_action.padrino"
],
nil
]
Expand Down Expand Up @@ -77,7 +77,7 @@ def uninstall_padrino_integration
[
Appsignal::Rack::SinatraBaseInstrumentation,
[
:instrument_span_name => "process_action.padrino"
:instrument_event_name => "process_action.padrino"
],
nil
]
Expand Down Expand Up @@ -123,7 +123,7 @@ def uninstall_padrino_integration
Appsignal::Rack::SinatraBaseInstrumentation,
[
:request_class => ::Sinatra::Request,
:instrument_span_name => "process_action.padrino"
:instrument_event_name => "process_action.padrino"
],
nil
]
Expand Down
12 changes: 6 additions & 6 deletions spec/lib/appsignal/rack/abstract_middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,19 @@ def make_request_with_error(error_class, error_message)
expect(last_transaction).to_not have_error
end

context "without :instrument_span_name option set" do
context "without :instrument_event_name option set" do
let(:options) { {} }

it "does not record an instrumentation event" do
expect(last_transaction).to_not include_event
end
end

context "with :instrument_span_name option set" do
let(:options) { { :instrument_span_name => "span_name.category" } }
context "with :instrument_event_name option set" do
let(:options) { { :instrument_event_name => "event_name.category" } }

it "records an instrumentation event" do
expect(last_transaction).to include_event(:name => "span_name.category")
expect(last_transaction).to include_event(:name => "event_name.category")
end
end

Expand All @@ -78,8 +78,8 @@ def make_request_with_error(error_class, error_message)
.to be_kind_of(Appsignal::Transaction::NilTransaction)
end

context "when instrument_span_name option is nil" do
let(:options) { { :instrument_span_name => nil } }
context "when instrument_event_name option is nil" do
let(:options) { { :instrument_event_name => nil } }

it "does not record an instrumentation event" do
expect(last_transaction).to_not include_events
Expand Down

0 comments on commit 61e97cf

Please sign in to comment.