mirror of
				https://github.com/lingble/chatwoot.git
				synced 2025-11-03 20:48:07 +00:00 
			
		
		
		
	fix: Skip HookJob for inactive or irrelevant hooks (#12093)
On Aug 2, we had a P0 because of a sudden spike in Sidekiq jobs. The queue went up to 100k jobs and workers scaled from 400 threads to 1000+. Most of the jobs were HookJobs, and a large chunk of them were for Linear but they weren’t doing anything useful. Turns out, whenever there’s an update on a contact or conversation, we were triggering all account-level hooks without checking if the event was relevant. So if someone did a bulk import or ran an update, it would enqueue a huge number of unnecessary jobs. This PR adds two checks before enqueuing: - Whether the hook is active - Whether the event is relevant for that hook
This commit is contained in:
		@@ -37,10 +37,11 @@ class HookListener < BaseListener
 | 
				
			|||||||
  private
 | 
					  private
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  def execute_hooks(event, message)
 | 
					  def execute_hooks(event, message)
 | 
				
			||||||
    message.account.hooks.each do |hook|
 | 
					    message.account.hooks.find_each do |hook|
 | 
				
			||||||
      # In case of dialogflow, we would have a hook for each inbox.
 | 
					      # In case of dialogflow, we would have a hook for each inbox.
 | 
				
			||||||
      # Which means we will execute the same hook multiple times if the below filter isn't there
 | 
					      # Which means we will execute the same hook multiple times if the below filter isn't there
 | 
				
			||||||
      next if hook.inbox.present? && hook.inbox != message.inbox
 | 
					      next if hook.inbox.present? && hook.inbox != message.inbox
 | 
				
			||||||
 | 
					      next unless supported_hook_event?(hook, event.name)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      HookJob.perform_later(hook, event.name, message: message)
 | 
					      HookJob.perform_later(hook, event.name, message: message)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
@@ -48,7 +49,24 @@ class HookListener < BaseListener
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
  def execute_account_hooks(event, account, event_data = {})
 | 
					  def execute_account_hooks(event, account, event_data = {})
 | 
				
			||||||
    account.hooks.account_hooks.find_each do |hook|
 | 
					    account.hooks.account_hooks.find_each do |hook|
 | 
				
			||||||
 | 
					      next unless supported_hook_event?(hook, event.name)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
      HookJob.perform_later(hook, event.name, event_data)
 | 
					      HookJob.perform_later(hook, event.name, event_data)
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  def supported_hook_event?(hook, event_name)
 | 
				
			||||||
 | 
					    return false if hook.disabled?
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    supported_events_map = {
 | 
				
			||||||
 | 
					      'slack' => ['message.created'],
 | 
				
			||||||
 | 
					      'dialogflow' => ['message.created', 'message.updated'],
 | 
				
			||||||
 | 
					      'google_translate' => ['message.created'],
 | 
				
			||||||
 | 
					      'leadsquared' => ['contact.updated', 'conversation.created', 'conversation.resolved']
 | 
				
			||||||
 | 
					    }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    return false unless supported_events_map.key?(hook.app_id)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    supported_events_map[hook.app_id].include?(event_name)
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
 
 | 
				
			|||||||
@@ -10,6 +10,8 @@ describe HookListener do
 | 
				
			|||||||
                     account: account, inbox: inbox, conversation: conversation)
 | 
					                     account: account, inbox: inbox, conversation: conversation)
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
  let!(:event) { Events::Base.new(event_name, Time.zone.now, message: message) }
 | 
					  let!(:event) { Events::Base.new(event_name, Time.zone.now, message: message) }
 | 
				
			||||||
 | 
					  let(:contact_event) { Events::Base.new('contact.updated', Time.zone.now, contact: conversation.contact) }
 | 
				
			||||||
 | 
					  let(:conversation_event) { Events::Base.new('conversation.created', Time.zone.now, conversation: conversation) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
  describe '#message_created' do
 | 
					  describe '#message_created' do
 | 
				
			||||||
    let(:event_name) { 'message.created' }
 | 
					    let(:event_name) { 'message.created' }
 | 
				
			||||||
@@ -42,10 +44,88 @@ describe HookListener do
 | 
				
			|||||||
 | 
					
 | 
				
			||||||
    context 'when hook is configured' do
 | 
					    context 'when hook is configured' do
 | 
				
			||||||
      it 'triggers hook job' do
 | 
					      it 'triggers hook job' do
 | 
				
			||||||
        hook = create(:integrations_hook, account: account)
 | 
					        hook = create(:integrations_hook, :dialogflow, account: account, inbox: inbox)
 | 
				
			||||||
        expect(HookJob).to receive(:perform_later).with(hook, 'message.updated', message: message).once
 | 
					        expect(HookJob).to receive(:perform_later).with(hook, 'message.updated', message: message).once
 | 
				
			||||||
        listener.message_updated(event)
 | 
					        listener.message_updated(event)
 | 
				
			||||||
      end
 | 
					      end
 | 
				
			||||||
    end
 | 
					    end
 | 
				
			||||||
  end
 | 
					  end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					  describe 'hook job enqueuing behavior' do
 | 
				
			||||||
 | 
					    let(:event_name) { 'message.created' }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'when app_id is not in the allowed list' do
 | 
				
			||||||
 | 
					      it 'does not enqueue the job' do
 | 
				
			||||||
 | 
					        create(:integrations_hook, account: account, app_id: 'unsupported_app')
 | 
				
			||||||
 | 
					        expect(HookJob).not_to receive(:perform_later)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.message_created(event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'when hook is enabled and app_id is supported' do
 | 
				
			||||||
 | 
					      it 'enqueues the job for slack' do
 | 
				
			||||||
 | 
					        hook = create(:integrations_hook, account: account)
 | 
				
			||||||
 | 
					        expect(HookJob).to receive(:perform_later).with(hook, event_name, message: message)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.message_created(event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      it 'enqueues the job for dialogflow' do
 | 
				
			||||||
 | 
					        hook = create(:integrations_hook, :dialogflow, account: account, inbox: inbox)
 | 
				
			||||||
 | 
					        expect(HookJob).to receive(:perform_later).with(hook, event_name, message: message)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.message_created(event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      it 'enqueues the job for google_translate' do
 | 
				
			||||||
 | 
					        hook = create(:integrations_hook, :google_translate, account: account)
 | 
				
			||||||
 | 
					        expect(HookJob).to receive(:perform_later).with(hook, event_name, message: message)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.message_created(event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'with disabled hook' do
 | 
				
			||||||
 | 
					      it 'does not enqueue job for disabled hooks' do
 | 
				
			||||||
 | 
					        create(:integrations_hook, account: account, status: 'disabled', app_id: 'slack')
 | 
				
			||||||
 | 
					        expect(HookJob).not_to receive(:perform_later)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.message_created(event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'with unsupported app_id and event combination' do
 | 
				
			||||||
 | 
					      it 'does not enqueue job for unsupported app_id' do
 | 
				
			||||||
 | 
					        create(:integrations_hook, account: account, app_id: 'unsupported_app')
 | 
				
			||||||
 | 
					        expect(HookJob).not_to receive(:perform_later)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.message_created(event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    context 'with leadsquared hook' do
 | 
				
			||||||
 | 
					      let(:hook) { create(:integrations_hook, :leadsquared, account: account) }
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      before do
 | 
				
			||||||
 | 
					        account.enable_features(:crm_integration)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      it 'enqueues the job for conversation.created' do
 | 
				
			||||||
 | 
					        expect(HookJob)
 | 
				
			||||||
 | 
					          .to receive(:perform_later)
 | 
				
			||||||
 | 
					          .with(hook, 'conversation.created', { conversation: conversation })
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.conversation_created(conversation_event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					      it 'enqueues the job for contact.updated' do
 | 
				
			||||||
 | 
					        expect(HookJob)
 | 
				
			||||||
 | 
					          .to receive(:perform_later)
 | 
				
			||||||
 | 
					          .with(hook, 'contact.updated', { contact: conversation.contact })
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        listener.contact_updated(contact_event)
 | 
				
			||||||
 | 
					      end
 | 
				
			||||||
 | 
					    end
 | 
				
			||||||
 | 
					  end
 | 
				
			||||||
end
 | 
					end
 | 
				
			||||||
 
 | 
				
			|||||||
		Reference in New Issue
	
	Block a user