Commit 5c1459ef authored by Sean McGivern's avatar Sean McGivern

Merge branch '35914-merge-request-update-worker-is-slow' into 'master'

Add metrics tagging to the sidekiq middleware

See merge request gitlab-org/gitlab-ce!15111
parents e1f11899 d5859bb9
...@@ -2,6 +2,10 @@ class UpdateMergeRequestsWorker ...@@ -2,6 +2,10 @@ class UpdateMergeRequestsWorker
include Sidekiq::Worker include Sidekiq::Worker
include DedicatedSidekiqQueue include DedicatedSidekiqQueue
def metrics_tags
@metrics_tags || {}
end
def perform(project_id, user_id, oldrev, newrev, ref) def perform(project_id, user_id, oldrev, newrev, ref)
project = Project.find_by(id: project_id) project = Project.find_by(id: project_id)
return unless project return unless project
...@@ -9,6 +13,11 @@ class UpdateMergeRequestsWorker ...@@ -9,6 +13,11 @@ class UpdateMergeRequestsWorker
user = User.find_by(id: user_id) user = User.find_by(id: user_id)
return unless user return unless user
@metrics_tags = {
project_id: project_id,
user_id: user_id
}
MergeRequests::RefreshService.new(project, user).execute(oldrev, newrev, ref) MergeRequests::RefreshService.new(project, user).execute(oldrev, newrev, ref)
end end
end end
---
title: Add metric tagging for sidekiq workers
merge_request: 15111
author:
type: added
...@@ -11,6 +11,8 @@ module Gitlab ...@@ -11,6 +11,8 @@ module Gitlab
# Old gitlad-shell messages don't provide enqueued_at/created_at attributes # Old gitlad-shell messages don't provide enqueued_at/created_at attributes
trans.set(:sidekiq_queue_duration, Time.now.to_f - (message['enqueued_at'] || message['created_at'] || 0)) trans.set(:sidekiq_queue_duration, Time.now.to_f - (message['enqueued_at'] || message['created_at'] || 0))
trans.run { yield } trans.run { yield }
worker.metrics_tags.each { |tag, value| trans.add_tag(tag, value) } if worker.respond_to?(:metrics_tags)
rescue Exception => error # rubocop: disable Lint/RescueException rescue Exception => error # rubocop: disable Lint/RescueException
trans.add_event(:sidekiq_exception) trans.add_event(:sidekiq_exception)
......
...@@ -4,35 +4,30 @@ describe Gitlab::Metrics::SidekiqMiddleware do ...@@ -4,35 +4,30 @@ describe Gitlab::Metrics::SidekiqMiddleware do
let(:middleware) { described_class.new } let(:middleware) { described_class.new }
let(:message) { { 'args' => ['test'], 'enqueued_at' => Time.new(2016, 6, 23, 6, 59).to_f } } let(:message) { { 'args' => ['test'], 'enqueued_at' => Time.new(2016, 6, 23, 6, 59).to_f } }
describe '#call' do def run(worker, message)
it 'tracks the transaction' do expect(Gitlab::Metrics::Transaction).to receive(:new)
worker = double(:worker, class: double(:class, name: 'TestWorker')) .with('TestWorker#perform')
.and_call_original
expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set)
.with(:sidekiq_queue_duration, instance_of(Float))
expect(Gitlab::Metrics::Transaction).to receive(:new) expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish)
.with('TestWorker#perform')
.and_call_original
expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set) middleware.call(worker, message, :test) { nil }
.with(:sidekiq_queue_duration, instance_of(Float)) end
expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish) describe '#call' do
it 'tracks the transaction' do
worker = double(:worker, class: double(:class, name: 'TestWorker'))
middleware.call(worker, message, :test) { nil } run(worker, message)
end end
it 'tracks the transaction (for messages without `enqueued_at`)' do it 'tracks the transaction (for messages without `enqueued_at`)' do
worker = double(:worker, class: double(:class, name: 'TestWorker')) worker = double(:worker, class: double(:class, name: 'TestWorker'))
expect(Gitlab::Metrics::Transaction).to receive(:new) run(worker, {})
.with('TestWorker#perform')
.and_call_original
expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set)
.with(:sidekiq_queue_duration, instance_of(Float))
expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish)
middleware.call(worker, {}, :test) { nil }
end end
it 'tracks any raised exceptions' do it 'tracks any raised exceptions' do
...@@ -50,5 +45,18 @@ describe Gitlab::Metrics::SidekiqMiddleware do ...@@ -50,5 +45,18 @@ describe Gitlab::Metrics::SidekiqMiddleware do
expect { middleware.call(worker, message, :test) } expect { middleware.call(worker, message, :test) }
.to raise_error(RuntimeError) .to raise_error(RuntimeError)
end end
it 'tags the metrics accordingly' do
tags = { one: 1, two: 2 }
worker = double(:worker, class: double(:class, name: 'TestWorker'))
allow(worker).to receive(:metrics_tags).and_return(tags)
tags.each do |tag, value|
expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:add_tag)
.with(tag, value)
end
run(worker, message)
end
end end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment