Commit 43d98cd1 authored by Sean McGivern's avatar Sean McGivern

Merge branch '39958-updatemergerequestsworker-are-failing-on-metric-tagging' into 'master'

Revert "add metrics tagging to the sidekiq middleware"

Closes #39958

See merge request gitlab-org/gitlab-ce!15285
parents 618cf6c6 7fd3ce41
...@@ -2,10 +2,6 @@ class UpdateMergeRequestsWorker ...@@ -2,10 +2,6 @@ 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
...@@ -13,11 +9,6 @@ class UpdateMergeRequestsWorker ...@@ -13,11 +9,6 @@ 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: Remove update merge request worker tagging.
merge_request:
author:
type: removed
...@@ -11,8 +11,6 @@ module Gitlab ...@@ -11,8 +11,6 @@ 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,32 +4,40 @@ describe Gitlab::Metrics::SidekiqMiddleware do ...@@ -4,32 +4,40 @@ 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 } }
def run(worker, message) describe '#call' do
expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new) it 'tracks the transaction' do
.with(worker.class) worker = double(:worker, class: double(:class, name: 'TestWorker'))
.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) expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new)
.with(worker.class)
.and_call_original
middleware.call(worker, message, :test) { nil } expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:set)
end .with(:sidekiq_queue_duration, instance_of(Float))
describe '#call' do expect_any_instance_of(Gitlab::Metrics::Transaction).to receive(:finish)
let(:test_worker_class) { double(:class, name: 'TestWorker') }
let(:worker) { double(:worker, class: test_worker_class) }
it 'tracks the transaction' do 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
run(worker, {}) worker = double(:worker, class: double(:class, name: 'TestWorker'))
expect(Gitlab::Metrics::BackgroundTransaction).to receive(:new)
.with(worker.class)
.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
worker = double(:worker, class: double(:class, name: 'TestWorker'))
expect_any_instance_of(Gitlab::Metrics::Transaction) expect_any_instance_of(Gitlab::Metrics::Transaction)
.to receive(:run).and_raise(RuntimeError) .to receive(:run).and_raise(RuntimeError)
......
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