Add latest changes from gitlab-org/gitlab@master

3 files changed, 45 insertions, 36 deletions

diff --git a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
index 8c64019a0a1..9d5c4bdf9e2 100644
--- a/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
+++ b/spec/lib/gitlab/metrics/requests_rack_middleware_spec.rb
@@ -79,6 +79,17 @@ RSpec.describe Gitlab::Metrics::RequestsRackMiddleware, :aggregate_failures do
       end
     end
 
+    context '@app.call returns an error code' do
+      let(:status) { '500' }
+
+      it 'tracks count but not duration' do
+        expect(described_class).to receive_message_chain(:http_requests_total, :increment).with(method: 'get', status: '500', feature_category: 'unknown')
+        expect(described_class).not_to receive(:http_request_duration_seconds)
+
+        subject.call(env)
+      end
+    end
+
     context '@app.call throws exception' do
       let(:http_request_duration_seconds) { double('http_request_duration_seconds') }
       let(:http_requests_total) { double('http_requests_total') }
diff --git a/spec/lib/gitlab/metrics/transaction_spec.rb b/spec/lib/gitlab/metrics/transaction_spec.rb
index d4e5a1a94f2..2ff8efcd7cb 100644
--- a/spec/lib/gitlab/metrics/transaction_spec.rb
+++ b/spec/lib/gitlab/metrics/transaction_spec.rb
@@ -12,32 +12,6 @@ RSpec.describe Gitlab::Metrics::Transaction do
     }
   end
 
-  describe '#duration' do
-    it 'returns the duration of a transaction in seconds' do
-      transaction.run { }
-
-      expect(transaction.duration).to be > 0
-    end
-  end
-
-  describe '#run' do
-    it 'yields the supplied block' do
-      expect { |b| transaction.run(&b) }.to yield_control
-    end
-
-    it 'stores the transaction in the current thread' do
-      transaction.run do
-        expect(described_class.current).to eq(transaction)
-      end
-    end
-
-    it 'removes the transaction from the current thread upon completion' do
-      transaction.run { }
-
-      expect(described_class.current).to be_nil
-    end
-  end
-
   describe '#method_call_for' do
     it 'returns a MethodCall' do
       method = transaction.method_call_for('Foo#bar', :Foo, '#bar')
@@ -46,6 +20,10 @@ RSpec.describe Gitlab::Metrics::Transaction do
     end
   end
 
+  describe '#run' do
+    specify { expect { transaction.run }.to raise_error(NotImplementedError) }
+  end
+
   describe '#add_event' do
     let(:prometheus_metric) { instance_double(Prometheus::Client::Counter, increment: nil, base_labels: {}) }
 
diff --git a/spec/lib/gitlab/metrics/web_transaction_spec.rb b/spec/lib/gitlab/metrics/web_transaction_spec.rb
index 6ee9564ef75..5261d04c879 100644
--- a/spec/lib/gitlab/metrics/web_transaction_spec.rb
+++ b/spec/lib/gitlab/metrics/web_transaction_spec.rb
@@ -38,16 +38,6 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
     end
   end
 
-  describe '#duration' do
-    include_context 'transaction observe metrics'
-
-    it 'returns the duration of a transaction in seconds' do
-      transaction.run { sleep(0.5) }
-
-      expect(transaction.duration).to be >= 0.5
-    end
-  end
-
   describe '#run' do
     include_context 'transaction observe metrics'
 
@@ -58,6 +48,9 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
     it 'stores the transaction in the current thread' do
       transaction.run do
         expect(Thread.current[described_class::THREAD_KEY]).to eq(transaction)
+        expect(described_class.current).to eq(transaction)
+
+        ['200', {}, '']
       end
     end
 
@@ -65,6 +58,33 @@ RSpec.describe Gitlab::Metrics::WebTransaction do
       transaction.run { }
 
       expect(Thread.current[described_class::THREAD_KEY]).to be_nil
+      expect(described_class.current).to be_nil
+    end
+
+    it 'records the duration of the transaction if the request was successful' do
+      expect(transaction).to receive(:observe).with(:gitlab_transaction_duration_seconds, instance_of(Float))
+
+      transaction.run { ['200', {}, ''] }
+    end
+
+    it 'does not record the duration of the transaction if the request failed' do
+      expect(transaction).not_to receive(:observe).with(:gitlab_transaction_duration_seconds, instance_of(Float))
+
+      transaction.run { ['500', {}, ''] }
+    end
+
+    it 'does not record the duration of the transaction if it raised' do
+      expect(transaction).not_to receive(:observe).with(:gitlab_transaction_duration_seconds, instance_of(Float))
+
+      expect do
+        transaction.run { raise 'broken' }
+      end.to raise_error('broken')
+    end
+
+    it 'returns the rack response' do
+      response = ['500', {}, '']
+
+      expect(transaction.run { response }).to eq(response)
     end
   end