-
Notifications
You must be signed in to change notification settings - Fork 116
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Deprecate our GenericInstrumentation middleware
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
Showing
6 changed files
with
106 additions
and
26 deletions.
There are no files selected for viewing
6 changes: 6 additions & 0 deletions
6
.changesets/deprecate-appsignal--rack--genericinstrumentation-middleware.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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`. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,82 @@ | ||
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) | ||
# Appsignal::Rack.send(:remove_const, :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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters