Skip to content

Commit

Permalink
Deprecate our GenericInstrumentation middleware
Browse files Browse the repository at this point in the history
Deprecate the Appsignal::Rack::GenericInstrumentation in favor of the
Appsignal::Rack::InstrumentationMiddleware. This other middleware
doesn't set the action name to "unknown" by default.

The GenericInstrumentation is no longer loaded automatically. This helps
with printing a deprecation warning when it is used.

Update the Grape specs which I copied this from to not mention the
Minutely constant, which I copied it from there.
  • Loading branch information
tombruijn committed Jul 2, 2024
1 parent f259678 commit 2ad6fc1
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 26 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
bump: patch
type: deprecate
---

Deprecate the `Appsignal::Rack::GenericInstrumentation` middleware. Use the `Appsignal::Rack::InstrumentationMiddleware` middleware instead. See changelog entry about the `InstrumentationMiddleware`.
2 changes: 1 addition & 1 deletion lib/appsignal.rb
Original file line number Diff line number Diff line change
Expand Up @@ -324,9 +324,9 @@ def const_missing(name)
require "appsignal/probes"
require "appsignal/marker"
require "appsignal/garbage_collection"
require "appsignal/rack"
require "appsignal/rack/abstract_middleware"
require "appsignal/rack/instrumentation_middleware"
require "appsignal/rack/generic_instrumentation"
require "appsignal/rack/event_handler"
require "appsignal/integrations/railtie" if defined?(::Rails)
require "appsignal/transaction"
Expand Down
26 changes: 26 additions & 0 deletions lib/appsignal/rack.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
# frozen_string_literal: true

module Appsignal
# @api private
module Rack
# Alias constants that have moved with a warning message that points to the
# place to update the reference.
def self.const_missing(name)
case name
when :GenericInstrumentation
require "appsignal/rack/generic_instrumentation"

callers = caller
Appsignal::Utils::StdoutAndLoggerMessage.warning \
"The constant Appsignal::Rack::GenericInstrumentation has been deprecated. " \
"Please update the constant name to " \
"Appsignal::Rack::InstrumentationMiddleware in the following file to " \
"remove this message.\n#{callers.first}"
# Return the alias so it can't ever get stuck in a recursive loop
Appsignal::Rack::GenericInstrumentationAlias
else
super
end
end
end
end
5 changes: 4 additions & 1 deletion lib/appsignal/rack/generic_instrumentation.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# frozen_string_literal: true

module Appsignal
# @api private
module Rack
# @api private
class GenericInstrumentation < AbstractMiddleware
def initialize(app, options = {})
options[:instrument_span_name] ||= "process_action.generic"
Expand All @@ -14,5 +14,8 @@ def add_transaction_metadata_after(transaction, request)
transaction.set_action_if_nil("unknown")
end
end

# @api private
class GenericInstrumentationAlias < GenericInstrumentation; end
end
end
90 changes: 67 additions & 23 deletions spec/lib/appsignal/rack/generic_instrumentation_spec.rb
Original file line number Diff line number Diff line change
@@ -1,37 +1,81 @@
describe Appsignal::Rack::GenericInstrumentation do
let(:app) { double(:call => true) }
let(:env) { Rack::MockRequest.env_for("/some/path") }
let(:middleware) { Appsignal::Rack::GenericInstrumentation.new(app, {}) }
describe "Appsignal::Rack::GenericInstrumentation" do
describe "Appsignal::Rack::GenericInstrumentation constant" do
let(:err_stream) { std_stream }
let(:stderr) { err_stream.read }
before do
if Appsignal::Rack.const_defined?(:GenericInstrumentation)
hide_const "Appsignal::Rack::GenericInstrumentation"
end
end

before(:context) { start_agent }
around { |example| keep_transactions { example.run } }
it "returns the Rack::GenericInstrumentation constant" do
silence do
expect(Appsignal::Rack::GenericInstrumentation)
.to be(Appsignal::Rack::GenericInstrumentationAlias)
end
end

def make_request(env)
middleware.call(env)
end
it "prints a deprecation warning to STDERR" do
capture_std_streams(std_stream, err_stream) do
Appsignal::Rack::GenericInstrumentation
end

context "without an exception" do
it "reports a process_action.generic event" do
make_request(env)
expect(stderr).to include(
"appsignal WARNING: The constant Appsignal::Rack::GenericInstrumentation " \
"has been deprecated."
)
end

expect(last_transaction).to include_event("name" => "process_action.generic")
it "logs a warning" do
logs =
capture_logs do
silence do
Appsignal::Rack::GenericInstrumentation
end
end

expect(logs).to contains_log(
:warn,
"The constant Appsignal::Rack::GenericInstrumentation has been deprecated."
)
end
end

context "with action name env" do
it "reports the appsignal.action env as the action name" do
env["appsignal.action"] = "MyAction"
make_request(env)
describe "middleware" do
let(:app) { double(:call => true) }
let(:env) { Rack::MockRequest.env_for("/some/path") }
let(:middleware) { Appsignal::Rack::GenericInstrumentation.new(app, {}) }

before(:context) { start_agent }
around { |example| keep_transactions { example.run } }

expect(last_transaction).to have_action("MyAction")
def make_request(env)
middleware.call(env)
end

context "without an exception" do
it "reports a process_action.generic event" do
make_request(env)

expect(last_transaction).to include_event("name" => "process_action.generic")
end
end

context "with action name env" do
it "reports the appsignal.action env as the action name" do
env["appsignal.action"] = "MyAction"
make_request(env)

expect(last_transaction).to have_action("MyAction")
end
end
end

context "without action name metadata" do
it "reports 'unknown' as the action name" do
make_request(env)
context "without action name metadata" do
it "reports 'unknown' as the action name" do
make_request(env)

expect(last_transaction).to have_action("unknown")
expect(last_transaction).to have_action("unknown")
end
end
end
end
2 changes: 1 addition & 1 deletion spec/lib/appsignal/rack/grape_middleware_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
let(:err_stream) { std_stream }
let(:stderr) { err_stream.read }

it "returns the Probes constant calling the Minutely constant" do
it "returns the Rack::GrapeMiddleware constant calling the Grape::Middleware constant" do
silence { expect(Appsignal::Grape::Middleware).to be(Appsignal::Rack::GrapeMiddleware) }
end

Expand Down

0 comments on commit 2ad6fc1

Please sign in to comment.