From 9cb6f8910e85d134558d0edbe3a4eb50579ae698 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Thu, 4 Jul 2024 09:18:15 +0200 Subject: [PATCH 1/2] Use Transaction API naming in middleware option Use "event" rather than "span" for the Rack AbstractMiddleware config option naming to be consistent with the rest of the Ruby gem. This is a (still) private API change so no backwards compatibility has been added. [skip changeset] as it's a private change. --- lib/appsignal/integrations/padrino.rb | 2 +- lib/appsignal/rack/abstract_middleware.rb | 6 +++--- lib/appsignal/rack/generic_instrumentation.rb | 2 +- lib/appsignal/rack/grape_middleware.rb | 2 +- lib/appsignal/rack/hanami_middleware.rb | 2 +- lib/appsignal/rack/instrumentation_middleware.rb | 2 +- lib/appsignal/rack/rails_instrumentation.rb | 2 +- lib/appsignal/rack/sinatra_instrumentation.rb | 2 +- spec/lib/appsignal/integrations/padrino_spec.rb | 6 +++--- spec/lib/appsignal/rack/abstract_middleware_spec.rb | 12 ++++++------ 10 files changed, 19 insertions(+), 19 deletions(-) diff --git a/lib/appsignal/integrations/padrino.rb b/lib/appsignal/integrations/padrino.rb index 748989ded..e88037fdd 100644 --- a/lib/appsignal/integrations/padrino.rb +++ b/lib/appsignal/integrations/padrino.rb @@ -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 diff --git a/lib/appsignal/rack/abstract_middleware.rb b/lib/appsignal/rack/abstract_middleware.rb index 3f381bfc5..9137eef10 100644 --- a/lib/appsignal/rack/abstract_middleware.rb +++ b/lib/appsignal/rack/abstract_middleware.rb @@ -14,7 +14,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 @@ -77,8 +77,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 diff --git a/lib/appsignal/rack/generic_instrumentation.rb b/lib/appsignal/rack/generic_instrumentation.rb index 8f9982a5a..8cdc56ab5 100644 --- a/lib/appsignal/rack/generic_instrumentation.rb +++ b/lib/appsignal/rack/generic_instrumentation.rb @@ -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 diff --git a/lib/appsignal/rack/grape_middleware.rb b/lib/appsignal/rack/grape_middleware.rb index 190e4e3e7..f1f86aed3 100644 --- a/lib/appsignal/rack/grape_middleware.rb +++ b/lib/appsignal/rack/grape_middleware.rb @@ -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 diff --git a/lib/appsignal/rack/hanami_middleware.rb b/lib/appsignal/rack/hanami_middleware.rb index 4fd479823..25ec3d6ff 100644 --- a/lib/appsignal/rack/hanami_middleware.rb +++ b/lib/appsignal/rack/hanami_middleware.rb @@ -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 diff --git a/lib/appsignal/rack/instrumentation_middleware.rb b/lib/appsignal/rack/instrumentation_middleware.rb index e6a44f8fb..93e9c8ed0 100644 --- a/lib/appsignal/rack/instrumentation_middleware.rb +++ b/lib/appsignal/rack/instrumentation_middleware.rb @@ -5,7 +5,7 @@ module Rack # @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 diff --git a/lib/appsignal/rack/rails_instrumentation.rb b/lib/appsignal/rack/rails_instrumentation.rb index 3004b9aa1..ebaa22de6 100644 --- a/lib/appsignal/rack/rails_instrumentation.rb +++ b/lib/appsignal/rack/rails_instrumentation.rb @@ -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 diff --git a/lib/appsignal/rack/sinatra_instrumentation.rb b/lib/appsignal/rack/sinatra_instrumentation.rb index 514d7fed1..ce0e66606 100644 --- a/lib/appsignal/rack/sinatra_instrumentation.rb +++ b/lib/appsignal/rack/sinatra_instrumentation.rb @@ -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 diff --git a/spec/lib/appsignal/integrations/padrino_spec.rb b/spec/lib/appsignal/integrations/padrino_spec.rb index 8c7c7a017..c18a493ce 100644 --- a/spec/lib/appsignal/integrations/padrino_spec.rb +++ b/spec/lib/appsignal/integrations/padrino_spec.rb @@ -47,7 +47,7 @@ def uninstall_padrino_integration [ Appsignal::Rack::SinatraBaseInstrumentation, [ - :instrument_span_name => "process_action.padrino" + :instrument_event_name => "process_action.padrino" ], nil ] @@ -77,7 +77,7 @@ def uninstall_padrino_integration [ Appsignal::Rack::SinatraBaseInstrumentation, [ - :instrument_span_name => "process_action.padrino" + :instrument_event_name => "process_action.padrino" ], nil ] @@ -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 ] diff --git a/spec/lib/appsignal/rack/abstract_middleware_spec.rb b/spec/lib/appsignal/rack/abstract_middleware_spec.rb index 69cd9aa57..69bfbf5f8 100644 --- a/spec/lib/appsignal/rack/abstract_middleware_spec.rb +++ b/spec/lib/appsignal/rack/abstract_middleware_spec.rb @@ -56,7 +56,7 @@ 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 @@ -64,11 +64,11 @@ def make_request_with_error(error_class, error_message) 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 @@ -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 From db2ffff64f5bea309bc375b79ce127f77d3f9e43 Mon Sep 17 00:00:00 2001 From: Tom de Bruijn Date: Thu, 4 Jul 2024 09:28:57 +0200 Subject: [PATCH 2/2] Document Rack InstrumentationMiddleware Add documentation for the InstrumentationMiddleware for the API documentation. This InstrumentationMiddleware is a public API so let's document it a little. I've also linked to our Rack instrumentation documentation page for more information. --- lib/appsignal/integrations/padrino.rb | 1 + lib/appsignal/rack/abstract_middleware.rb | 5 ++ .../rack/instrumentation_middleware.rb | 49 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/lib/appsignal/integrations/padrino.rb b/lib/appsignal/integrations/padrino.rb index e88037fdd..c84cd2d9e 100644 --- a/lib/appsignal/integrations/padrino.rb +++ b/lib/appsignal/integrations/padrino.rb @@ -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"] diff --git a/lib/appsignal/rack/abstract_middleware.rb b/lib/appsignal/rack/abstract_middleware.rb index 9137eef10..69271efcc 100644 --- a/lib/appsignal/rack/abstract_middleware.rb +++ b/lib/appsignal/rack/abstract_middleware.rb @@ -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 diff --git a/lib/appsignal/rack/instrumentation_middleware.rb b/lib/appsignal/rack/instrumentation_middleware.rb index 93e9c8ed0..6ac23d6cc 100644 --- a/lib/appsignal/rack/instrumentation_middleware.rb +++ b/lib/appsignal/rack/instrumentation_middleware.rb @@ -2,6 +2,55 @@ 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 = {})