Deprecate GenericRequest usage

Prefer people use our `Appsignal.set_*` helpers defined in the `Helpers::Instrumentation` module. Update the usage of GenericRequest to InternalGenericRequest so this isn't triggered from our own internal instrumentation.
appsignal · Jul 11, 2024 · 1c69d3f · 1c69d3f
1 parent 7cc3c0e
commit 1c69d3f
Show file tree

Hide file tree

Showing 19 changed files with 66 additions and 34 deletions.
diff --git a/.changesets/deprecate-genericrequest-usage-.md b/.changesets/deprecate-genericrequest-usage-.md
@@ -0,0 +1,7 @@
+---
+bump: patch
+type: deprecate
+---
+
+Deprecate the `Appsignal::Transaction::GenericRequest` class. Use the `Appsignal.set_*` helpers to set metadata on the Transaction instead. Read our [sample data guide](https://docs.appsignal.com/guides/custom-data/sample-data.html) for more information.
+
diff --git a/benchmark.rake b/benchmark.rake
@@ -67,7 +67,7 @@ def start_agent
 end
 
 def monitor_transaction(transaction_id)
-  request = Appsignal::Transaction::GenericRequest.new({})
+  request = Appsignal::Transaction::InternalGenericRequest.new({})
   transaction = Appsignal::Transaction.create(
     transaction_id,
     Appsignal::Transaction::HTTP_REQUEST,

diff --git a/lib/appsignal/demo.rb b/lib/appsignal/demo.rb
@@ -42,7 +42,7 @@ def create_example_error_request
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::HTTP_REQUEST,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
         begin
           raise TestError,
@@ -63,7 +63,7 @@ def create_example_performance_request
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::HTTP_REQUEST,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
         Appsignal.instrument "action_view.render", "Render hello.html.erb",
           "<h1>Hello world!</h1>" do

diff --git a/lib/appsignal/helpers/instrumentation.rb b/lib/appsignal/helpers/instrumentation.rb
@@ -63,7 +63,7 @@ def monitor_transaction(name, env = {}, &block)
         # environments.
         if name.start_with?("perform_job")
           namespace = Appsignal::Transaction::BACKGROUND_JOB
-          request   = Appsignal::Transaction::GenericRequest.new(env)
+          request   = Appsignal::Transaction::InternalGenericRequest.new(env)
         elsif name.start_with?("process_action")
           namespace = Appsignal::Transaction::HTTP_REQUEST
           request   = ::Rack::Request.new(env)
@@ -238,7 +238,7 @@ def send_error(
         transaction = Appsignal::Transaction.new(
           SecureRandom.uuid,
           namespace || Appsignal::Transaction::HTTP_REQUEST,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
         transaction.set_tags(tags) if tags
         transaction.set_error(error)
@@ -393,7 +393,7 @@ def report_error(exception)
             Appsignal::Transaction.new(
               SecureRandom.uuid,
               Appsignal::Transaction::HTTP_REQUEST,
-              Appsignal::Transaction::GenericRequest.new({})
+              Appsignal::Transaction::InternalGenericRequest.new({})
             )
           end
 

diff --git a/lib/appsignal/hooks/action_cable.rb b/lib/appsignal/hooks/action_cable.rb
@@ -39,7 +39,7 @@ def install_subscribe_callback
           transaction = Appsignal::Transaction.create(
             env[Appsignal::Hooks::ActionCableHook::REQUEST_ID],
             Appsignal::Transaction::ACTION_CABLE,
-            Appsignal::Transaction::GenericRequest.new({})
+            Appsignal::Transaction::InternalGenericRequest.new({})
           )
 
           begin
@@ -78,7 +78,7 @@ def install_unsubscribe_callback
           transaction = Appsignal::Transaction.create(
             env[Appsignal::Hooks::ActionCableHook::REQUEST_ID],
             Appsignal::Transaction::ACTION_CABLE,
-            Appsignal::Transaction::GenericRequest.new({})
+            Appsignal::Transaction::InternalGenericRequest.new({})
           )
 
           begin

diff --git a/lib/appsignal/hooks/active_job.rb b/lib/appsignal/hooks/active_job.rb
@@ -57,7 +57,7 @@ def execute(job)
               Appsignal::Transaction.create(
                 SecureRandom.uuid,
                 Appsignal::Transaction::BACKGROUND_JOB,
-                Appsignal::Transaction::GenericRequest.new({})
+                Appsignal::Transaction::InternalGenericRequest.new({})
               )
             end
 

diff --git a/lib/appsignal/integrations/action_cable.rb b/lib/appsignal/integrations/action_cable.rb
@@ -14,7 +14,7 @@ def perform_action(*args, &block)
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::ACTION_CABLE,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
 
         begin

diff --git a/lib/appsignal/integrations/delayed_job_plugin.rb b/lib/appsignal/integrations/delayed_job_plugin.rb
@@ -20,7 +20,7 @@ def self.invoke_with_instrumentation(job, block)
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::BACKGROUND_JOB,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
 
         Appsignal.instrument("perform_job.delayed_job") do

diff --git a/lib/appsignal/integrations/que.rb b/lib/appsignal/integrations/que.rb
@@ -8,7 +8,7 @@ def _run(*)
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::BACKGROUND_JOB,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
 
         begin

diff --git a/lib/appsignal/integrations/rake.rb b/lib/appsignal/integrations/rake.rb
@@ -36,7 +36,7 @@ def _appsignal_create_transaction
         Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::BACKGROUND_JOB,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
       end
     end

diff --git a/lib/appsignal/integrations/resque.rb b/lib/appsignal/integrations/resque.rb
@@ -8,7 +8,7 @@ def perform
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::BACKGROUND_JOB,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
 
         Appsignal.instrument "perform.resque" do

diff --git a/lib/appsignal/integrations/shoryuken.rb b/lib/appsignal/integrations/shoryuken.rb
@@ -8,7 +8,7 @@ def call(worker_instance, queue, sqs_msg, body, &block)
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::BACKGROUND_JOB,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
 
         Appsignal.instrument("perform_job.shoryuken", &block)

diff --git a/lib/appsignal/integrations/sidekiq.rb b/lib/appsignal/integrations/sidekiq.rb
@@ -42,7 +42,7 @@ def call(exception, sidekiq_context, _sidekiq_config = nil)
             Appsignal::Transaction.create(
               SecureRandom.uuid, # Newly generated job id
               Appsignal::Transaction::BACKGROUND_JOB,
-              Appsignal::Transaction::GenericRequest.new({})
+              Appsignal::Transaction::InternalGenericRequest.new({})
             )
           transaction.set_action_if_nil("SidekiqInternal")
           transaction.set_metadata("sidekiq_error", sidekiq_context[:context])
@@ -68,7 +68,7 @@ def call(_worker, item, _queue, &block)
         transaction = Appsignal::Transaction.create(
           SecureRandom.uuid,
           Appsignal::Transaction::BACKGROUND_JOB,
-          Appsignal::Transaction::GenericRequest.new({})
+          Appsignal::Transaction::InternalGenericRequest.new({})
         )
         transaction.set_action_if_nil(formatted_action_name(item))
 

diff --git a/lib/appsignal/integrations/webmachine.rb b/lib/appsignal/integrations/webmachine.rb
@@ -13,7 +13,7 @@ def run
             Appsignal::Transaction.create(
               SecureRandom.uuid,
               Appsignal::Transaction::HTTP_REQUEST,
-              Appsignal::Transaction::GenericRequest.new({})
+              Appsignal::Transaction::InternalGenericRequest.new({})
             )
           end
 

diff --git a/lib/appsignal/rack/abstract_middleware.rb b/lib/appsignal/rack/abstract_middleware.rb
@@ -37,7 +37,7 @@ def call(env)
               Appsignal::Transaction.create(
                 SecureRandom.uuid,
                 Appsignal::Transaction::HTTP_REQUEST,
-                Appsignal::Transaction::GenericRequest.new({})
+                Appsignal::Transaction::InternalGenericRequest.new({})
               )
             end
 

diff --git a/lib/appsignal/rack/event_handler.rb b/lib/appsignal/rack/event_handler.rb
@@ -64,7 +64,7 @@ def on_start(request, _response)
           transaction = Appsignal::Transaction.create(
             SecureRandom.uuid,
             Appsignal::Transaction::HTTP_REQUEST,
-            Appsignal::Transaction::GenericRequest.new({})
+            Appsignal::Transaction::InternalGenericRequest.new({})
           )
           request.env[APPSIGNAL_TRANSACTION] = transaction
 

diff --git a/lib/appsignal/transaction.rb b/lib/appsignal/transaction.rb
@@ -609,7 +609,7 @@ def to_h
     alias_method :to_hash, :to_h
 
     # @api private
-    class GenericRequest
+    class InternalGenericRequest
       attr_reader :env
 
       def initialize(env)
@@ -621,6 +621,21 @@ def params
       end
     end
 
+    # @deprecated Use the instrumentation helpers to set metadata on the
+    #   transaction, rather than rely on the GenericRequest automation. See the
+    #   {Helpers::Instrumentation} module for a list of helpers.
+    # @api private
+    class GenericRequest < InternalGenericRequest
+      def initialize(_env)
+        Appsignal::Utils::StdoutAndLoggerMessage.warning(
+          "The use of Appsignal::Transaction::GenericRequest is deprecated. " \
+            "Use the `Appsignal.set_*` helpers instead. " \
+            "https://docs.appsignal.com/guides/custom-data/sample-data.html"
+        )
+        super
+      end
+    end
+
     private
 
     # @api private

diff --git a/spec/lib/appsignal/transaction_spec.rb b/spec/lib/appsignal/transaction_spec.rb
@@ -1625,10 +1625,28 @@ def to_s
       end
     end
 
-    context "generic request" do
+    context "GenericRequest" do
       let(:env) { {} }
       subject { Appsignal::Transaction::GenericRequest.new(env) }
 
+      it "prints a deprecation warning on use" do
+        err_stream = std_stream
+        capture_std_streams(std_stream, err_stream) { subject }
+
+        expect(err_stream.read).to include(
+          "appsignal WARNING: The use of Appsignal::Transaction::GenericRequest is deprecated."
+        )
+      end
+
+      it "logs a deprecation warning on use" do
+        logs = capture_logs { subject }
+
+        expect(logs).to contains_log(
+          :warn,
+          "The use of Appsignal::Transaction::GenericRequest is deprecated."
+        )
+      end
+
       it "initializes with an empty env" do
         expect(subject.env).to be_empty
       end

diff --git a/spec/support/helpers/transaction_helpers.rb b/spec/support/helpers/transaction_helpers.rb
@@ -7,25 +7,17 @@ def uploaded_file
     end
   end
 
-  def background_job_transaction(args = {})
+  def background_job_transaction(_args = {})
     Appsignal::Transaction.new(
       "1",
-      Appsignal::Transaction::BACKGROUND_JOB,
-      Appsignal::Transaction::GenericRequest.new({
-        "SERVER_NAME" => "localhost",
-        "action_dispatch.routes" => "not_available"
-      }.merge(args))
+      Appsignal::Transaction::BACKGROUND_JOB
     )
   end
 
-  def http_request_transaction(args = {})
+  def http_request_transaction(_args = {})
     Appsignal::Transaction.new(
       "1",
-      Appsignal::Transaction::HTTP_REQUEST,
-      Appsignal::Transaction::GenericRequest.new({
-        "SERVER_NAME" => "localhost",
-        "action_dispatch.routes" => "not_available"
-      }.merge(args))
+      Appsignal::Transaction::HTTP_REQUEST
     )
   end