Deprecate the request env keys to set the action (#1155)

Configuring the action name with the request env keys `appsignal.action`
and `appsignal.route` don't give a reasonable guarantee when the action
name is set. The `Appsignal.set_action` helper sets the action name
immediately.

The method via the request env sets the action name using
`set_action_if_nil`. If the action name is set by one of our
instrumentations beforehand or another `Appsiganl.set_action` call,
there's no guarantee this value is used as the action name.

We also never documented either of these keys, so I don't expect a lot
of people to be using them.

Closes #1116

.changesets/deprecate-appsignal-route-and-appsignal-action-request-env.md

@@ -0,0 +1,15 @@
+---
+bump: patch
+type: deprecate
+---
+
+Deprecate the `appsignal.action` and `appsignal.route` request env methods to set the transaction action name. Use the `Appsignal.set_action` helper instead.
+
+```ruby
+# Before
+env["appsignal.action"] = "POST /my-action"
+env["appsignal.route"] = "POST /my-action"
+
+# After
+Appsignal.set_action("POST /my-action")
+```

lib/appsignal/rack/abstract_middleware.rb

@@ -127,7 +127,7 @@ def add_transaction_metadata_before(transaction, request)
       # Call `super` to also include the default set metadata.
       def add_transaction_metadata_after(transaction, request)
         default_action =
-          request.env["appsignal.route"] || request.env["appsignal.action"]
+          appsignal_route_env_value(request) || appsignal_action_env_value(request)
         transaction.set_action_if_nil(default_action)
         transaction.set_metadata("path", request.path)
 
@@ -160,6 +160,26 @@ def request_method_for(request)
       def request_for(env)
         @request_class.new(env)
       end
+
+      def appsignal_route_env_value(request)
+        request.env["appsignal.route"].tap do |value|
+          next unless value
+
+          Appsignal::Utils::StdoutAndLoggerMessage.warning \
+            "Setting the action name with the request env 'appsignal.route' is deprecated. " \
+              "Please use `Appsignal.set_action` instead. "
+        end
+      end
+
+      def appsignal_action_env_value(request)
+        request.env["appsignal.action"].tap do |value|
+          next unless value
+
+          Appsignal::Utils::StdoutAndLoggerMessage.warning \
+            "Setting the action name with the request env 'appsignal.action' is deprecated. " \
+              "Please use `Appsignal.set_action` instead. "
+        end
+      end
     end
   end
 end

spec/lib/appsignal/rack/abstract_middleware_spec.rb

@@ -160,21 +160,61 @@ def make_request_with_error(error_class, error_message)
       end
 
       context "with appsignal.route env" do
+        before { env["appsignal.route"] = "POST /my-route" }
+
         it "reports the appsignal.route value as the action name" do
-          env["appsignal.route"] = "POST /my-route"
           make_request
 
           expect(last_transaction).to have_action("POST /my-route")
         end
+
+        it "prints a deprecation warning" do
+          err_stream = std_stream
+          capture_std_streams(std_stream, err_stream) do
+            make_request
+          end
+
+          expect(err_stream.read).to include(
+            "Setting the action name with the request env 'appsignal.route' is deprecated."
+          )
+        end
+
+        it "logs a deprecation warning" do
+          logs = capture_logs { make_request }
+          expect(logs).to contains_log(
+            :warn,
+            "Setting the action name with the request env 'appsignal.route' is deprecated."
+          )
+        end
       end
 
       context "with appsignal.action env" do
-        it "reports the appsignal.route value as the action name" do
-          env["appsignal.action"] = "POST /my-action"
+        before { env["appsignal.action"] = "POST /my-action" }
+
+        it "reports the appsignal.action value as the action name" do
           make_request
 
           expect(last_transaction).to have_action("POST /my-action")
         end
+
+        it "prints a deprecation warning" do
+          err_stream = std_stream
+          capture_std_streams(std_stream, err_stream) do
+            make_request
+          end
+
+          expect(err_stream.read).to include(
+            "Setting the action name with the request env 'appsignal.action' is deprecated."
+          )
+        end
+
+        it "logs a deprecation warning" do
+          logs = capture_logs { make_request }
+          expect(logs).to contains_log(
+            :warn,
+            "Setting the action name with the request env 'appsignal.action' is deprecated."
+          )
+        end
       end
 
       describe "request metadata" do