Name Sidekiq delayed enqueues after their target

The server middleware decodes the Sidekiq delayed extensions' YAML
payload to name the perform transaction after the real target and
method, rather than the internal wrapper class. The client middleware
titled the enqueue event with the raw wrapper class instead, so the
enqueue and perform of the same delayed job read different names.

Extract that name resolution into a `SidekiqActionName` module shared by
both middlewares, and title the enqueue event with it. The two sides now
agree on the name.

Decoding the payload means deserializing YAML on the enqueue path, but
that is the same YAML the server middleware already deserializes, and it
runs only for delayed extension jobs. At enqueue the payload is the one
we just built locally, so it carries no trust or missing-constant risk
the perform side isn't already taking.

Author: unflxw

.changesets/title-sidekiq-delayed-enqueues-by-target.md

@@ -0,0 +1,9 @@
+---
+bump: patch
+type: change
+---
+
+Title `enqueue.sidekiq` events for Sidekiq delayed extension jobs after the
+delayed target and method, such as `enqueue MyClass.my_method job`, instead of
+the internal wrapper class. This matches the name the job is given when it
+later performs, so the enqueue and perform of the same job read the same name.

lib/appsignal/integrations/sidekiq.rb

@@ -49,6 +49,55 @@ def call(exception, sidekiq_context, _sidekiq_config = nil)
       end
     end
 
+    # Resolves the name of a Sidekiq job. That's normally the job class, but
+    # the Sidekiq delayed extensions encode the real target and method as YAML
+    # in the job's first argument, so those are decoded into a `Target.method`
+    # (class) or `Target#method` (instance) name.
+    #
+    # Shared between the client middleware, which titles the enqueue event with
+    # it, and the server middleware, which names the perform transaction with
+    # it. The same job then reads the same name on both sides.
+    #
+    # @!visibility private
+    module SidekiqActionName
+      module_function
+
+      # Based on: https://github.com/mperham/sidekiq/blob/63ee43353bd3b753beb0233f64865e658abeb1c3/lib/sidekiq/api.rb#L316-L334
+      def parse_action_name(job)
+        args = job.fetch("args", [])
+        job_class = job["class"]
+        case job_class
+        when "Sidekiq::Extensions::DelayedModel"
+          safe_load(args[0], job_class) do |target, method, _|
+            "#{target.class}##{method}"
+          end
+        when /\ASidekiq::Extensions::Delayed/
+          safe_load(args[0], job_class) do |target, method, _|
+            "#{target}.#{method}"
+          end
+        else
+          job_class
+        end
+      end
+
+      # Based on: https://github.com/mperham/sidekiq/blob/63ee43353bd3b753beb0233f64865e658abeb1c3/lib/sidekiq/api.rb#L403-L412
+      def safe_load(content, default)
+        if Gem::Version.new(YAML::VERSION) >= Gem::Version.new("4.0.0")
+          yield(*YAML.unsafe_load(content))
+        else
+          yield(*YAML.load(content)) # rubocop:disable Security/YAMLLoad
+        end
+      rescue => error
+        # Sidekiq issue #1761: in dev mode, it's possible to have jobs enqueued
+        # which haven't been loaded into memory yet so the YAML can't be
+        # loaded.
+        Appsignal.internal_logger.warn(
+          "Unable to load YAML from Sidekiq delayed extension job: #{error.message}"
+        )
+        default
+      end
+    end
+
     # Sidekiq client middleware that runs on enqueue. Records an
     # `enqueue.sidekiq` event so the enqueue shows up under the active
     # transaction.
@@ -65,7 +114,8 @@ def call(_worker_class, job, _queue, _redis_pool, &block)
         return yield if Appsignal::Transaction.current? &&
           Appsignal::Transaction.current.job_enqueue_events_suppressed?
 
-        Appsignal.instrument("enqueue.sidekiq", "enqueue #{job["class"]} job", &block)
+        title = "enqueue #{SidekiqActionName.parse_action_name(job)} job"
+        Appsignal.instrument("enqueue.sidekiq", title, &block)
       end
     end
 
@@ -144,7 +194,7 @@ def increment_counter(key, value, tags = {})
       end
 
       def formatted_action_name(job)
-        sidekiq_action_name = parse_action_name(job)
+        sidekiq_action_name = SidekiqActionName.parse_action_name(job)
         return unless sidekiq_action_name
 
         complete_action = sidekiq_action_name =~ /\.|#/
@@ -163,30 +213,12 @@ def formatted_metadata(item)
         end
       end
 
-      # Based on: https://github.com/mperham/sidekiq/blob/63ee43353bd3b753beb0233f64865e658abeb1c3/lib/sidekiq/api.rb#L316-L334
-      def parse_action_name(job)
-        args = job.fetch("args", [])
-        job_class = job["class"]
-        case job_class
-        when "Sidekiq::Extensions::DelayedModel"
-          safe_load(args[0], job_class) do |target, method, _|
-            "#{target.class}##{method}"
-          end
-        when /\ASidekiq::Extensions::Delayed/
-          safe_load(args[0], job_class) do |target, method, _|
-            "#{target}.#{method}"
-          end
-        else
-          job_class
-        end
-      end
-
       # Based on: https://github.com/mperham/sidekiq/blob/63ee43353bd3b753beb0233f64865e658abeb1c3/lib/sidekiq/api.rb#L336-L358
       def parse_arguments(job)
         args = job.fetch("args", [])
         case job["class"]
         when /\ASidekiq::Extensions::Delayed/
-          safe_load(args[0], args) do |_, _, arg|
+          SidekiqActionName.safe_load(args[0], args) do |_, _, arg|
             arg
           end
         else
@@ -199,21 +231,6 @@ def parse_arguments(job)
           args
         end
       end
-
-      # Based on: https://github.com/mperham/sidekiq/blob/63ee43353bd3b753beb0233f64865e658abeb1c3/lib/sidekiq/api.rb#L403-L412
-      def safe_load(content, default)
-        if YAML::VERSION >= "4.0.0"
-          yield(*YAML.unsafe_load(content))
-        else
-          yield(*YAML.load(content)) # rubocop:disable Security/YAMLLoad
-        end
-      rescue => error
-        # Sidekiq issue #1761: in dev mode, it's possible to have jobs enqueued
-        # which haven't been loaded into memory yet so the YAML can't be
-        # loaded.
-        Appsignal.internal_logger.warn "Unable to load YAML: #{error.message}"
-        default
-      end
     end
   end
 end

spec/lib/appsignal/integrations/sidekiq_spec.rb

@@ -198,6 +198,66 @@ def enqueue
       end
     end
 
+    context "when using the Sidekiq delayed extension" do
+      let(:job) do
+        {
+          "class" => "Sidekiq::Extensions::DelayedClass",
+          "args" => [
+            "---\n- !ruby/class 'DelayedTestClass'\n- :foo_method\n- - :bar: baz\n"
+          ]
+        }
+      end
+
+      it "titles the enqueue event with the delayed class and method name" do
+        transaction = http_request_transaction
+        set_current_transaction(transaction)
+
+        expect(enqueue).to eq(:enqueued)
+
+        event = transaction.to_h["events"].find { |e| e["name"] == "enqueue.sidekiq" }
+        expect(event["title"]).to eq("enqueue DelayedTestClass.foo_method job")
+      end
+
+      context "when the delayed job arguments are malformed YAML" do
+        before { job["args"] = [] }
+
+        it "logs a warning and titles the enqueue event with the job class" do
+          transaction = http_request_transaction
+          set_current_transaction(transaction)
+
+          logs = capture_logs { expect(enqueue).to eq(:enqueued) }
+
+          event = transaction.to_h["events"].find { |e| e["name"] == "enqueue.sidekiq" }
+          expect(event["title"]).to eq("enqueue Sidekiq::Extensions::DelayedClass job")
+          expect(logs).to contains_log(
+            :warn,
+            "Unable to load YAML from Sidekiq delayed extension job"
+          )
+        end
+      end
+    end
+
+    context "when using the Sidekiq ActiveRecord instance delayed extension" do
+      let(:job) do
+        {
+          "class" => "Sidekiq::Extensions::DelayedModel",
+          "args" => [
+            "---\n- !ruby/object:DelayedTestClass {}\n- :foo_method\n- - :bar: :baz\n"
+          ]
+        }
+      end
+
+      it "titles the enqueue event with the delayed class and method name" do
+        transaction = http_request_transaction
+        set_current_transaction(transaction)
+
+        expect(enqueue).to eq(:enqueued)
+
+        event = transaction.to_h["events"].find { |e| e["name"] == "enqueue.sidekiq" }
+        expect(event["title"]).to eq("enqueue DelayedTestClass#foo_method job")
+      end
+    end
+
     context "without an active transaction" do
       it "passes through without recording" do
         expect { |block| plugin.call("TestClass", job, "default", nil, &block) }