Skip to content

Commit ec10e7b

Browse files
committed
Refactor Active Job enqueues
Record the enqueue.active_job event from a prepend on ActiveJob::Base#enqueue instead of Rails' native enqueue.active_job notification, which the notifications path now suppresses so the enqueue is recorded once. This gives AppSignal a single enqueue event it owns, matching the other job integrations. Behaviour is otherwise unchanged: the event still records only when a transaction is active at enqueue time.
1 parent 109c54a commit ec10e7b

4 files changed

Lines changed: 74 additions & 6 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
bump: patch
3+
type: change
4+
---
5+
6+
Record the `enqueue.active_job` event from AppSignal's own Active Job
7+
instrumentation rather than from Rails' native `enqueue.active_job`
8+
notification, which is now suppressed so the enqueue is recorded once. The
9+
event still shows up on the active transaction when enqueuing from within a web
10+
request or another job; there is no change to what you see.

lib/appsignal/hooks/active_job.rb

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ def install
2929
ActiveSupport.on_load(:active_job) do
3030
::ActiveJob::Base
3131
.extend ::Appsignal::Hooks::ActiveJobHook::ActiveJobClassInstrumentation
32+
::ActiveJob::Base
33+
.prepend ::Appsignal::Hooks::ActiveJobHook::ActiveJobEnqueueInstrumentation
3234

3335
next unless Appsignal::Hooks::ActiveJobHook.version_7_1_or_higher?
3436

@@ -41,6 +43,23 @@ def install
4143
end
4244
end
4345

46+
# Records an `enqueue.active_job` event when a job is enqueued, so the
47+
# enqueue shows up on the active transaction's timeline (e.g. when
48+
# enqueuing from within a web request or another job).
49+
#
50+
# Wrapping `enqueue` ourselves -- rather than relying on Rails' native
51+
# `enqueue.active_job` notification, which the AppSignal notifications
52+
# path now suppresses -- gives us a single event we own. Like all
53+
# AppSignal events, this only records when there's an active transaction;
54+
# an enqueue with no transaction is a transparent pass-through.
55+
#
56+
# @!visibility private
57+
module ActiveJobEnqueueInstrumentation
58+
def enqueue(*)
59+
Appsignal.instrument("enqueue.active_job") { super }
60+
end
61+
end
62+
4463
module ActiveJobClassInstrumentation
4564
def execute(job) # rubocop:disable Metrics/CyclomaticComplexity
4665
enqueued_at = job["enqueued_at"]

lib/appsignal/integrations/active_support_notifications.rb

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,16 +7,20 @@ module ActiveSupportNotificationsIntegration
77
class << self
88
BANG = "!"
99

10+
# Events a dedicated AppSignal integration already records, so the
11+
# generic notifications path must not record them a second time. The
12+
# ActiveJob hook owns `enqueue.active_job`: it wraps the enqueue in its
13+
# own event, and Rails' native notification fires nested inside it.
14+
SUPPRESSED_EVENT_NAMES = ["enqueue.active_job"].freeze
15+
1016
def start_event(name)
11-
# Events that start with a bang are internal to Rails
12-
instrument_this = name[0] != BANG
13-
Appsignal::Transaction.current.start_event if instrument_this
17+
return unless record_event?(name)
18+
19+
Appsignal::Transaction.current.start_event
1420
end
1521

1622
def finish_event(name, payload = {})
17-
# Events that start with a bang are internal to Rails
18-
instrument_this = name[0] != BANG
19-
return unless instrument_this
23+
return unless record_event?(name)
2024

2125
title, body, body_format = Appsignal::EventFormatter.format(name, payload)
2226
Appsignal::Transaction.current.finish_event(
@@ -26,6 +30,13 @@ def finish_event(name, payload = {})
2630
body_format
2731
)
2832
end
33+
34+
# Events starting with a bang are internal to Rails; suppressed events
35+
# are recorded by a dedicated integration instead. Both `start_event`
36+
# and `finish_event` gate on this so the event stack stays balanced.
37+
def record_event?(name)
38+
name[0] != BANG && !SUPPRESSED_EVENT_NAMES.include?(name.to_s)
39+
end
2940
end
3041

3142
module InstrumentIntegration

spec/lib/appsignal/hooks/activejob_spec.rb

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,34 @@ def perform(*_args)
333333
end
334334
end
335335

336+
context "when enqueuing a job" do
337+
before { ActiveJob::Base.queue_adapter = :test }
338+
339+
context "with an active transaction" do
340+
it "records a single enqueue.active_job event on the transaction" do
341+
transaction = http_request_transaction
342+
set_current_transaction(transaction)
343+
344+
ActiveJobTestJob.perform_later
345+
346+
# Exactly one enqueue event: ours. Rails' native `enqueue.active_job`
347+
# notification is suppressed so it isn't recorded a second time.
348+
event_names = transaction.to_h["events"].map { |event| event["name"] }
349+
expect(event_names.count("enqueue.active_job")).to eq(1)
350+
end
351+
end
352+
353+
context "without an active transaction" do
354+
it "is a transparent pass-through that still enqueues the job" do
355+
expect do
356+
ActiveJobTestJob.perform_later
357+
end.to_not(change { created_transactions.count })
358+
359+
expect(ActiveJob::Base.queue_adapter.enqueued_jobs.count).to eq(1)
360+
end
361+
end
362+
end
363+
336364
context "with params" do
337365
let(:options) { { :filter_parameters => ["foo"] } }
338366

0 commit comments

Comments
 (0)