From 330de255b75fa7b9897844582b8fd7e020f4efa7 Mon Sep 17 00:00:00 2001
From: Paco Guzman <pacoguzmanp@gmail.com>
Date: Wed, 29 Jun 2016 17:03:20 +0200
Subject: [PATCH] RailsCache metrics now includes fetch_hit/fetch_miss and
read_hit/read_miss info.
---
CHANGELOG | 1 +
lib/gitlab/metrics/subscribers/rails_cache.rb | 14 ++-
.../metrics/subscribers/rails_cache_spec.rb | 103 ++++++++++++++++++
3 files changed, 116 insertions(+), 2 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index bda8c3d50bcf7..174422ef95ca0 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -30,6 +30,7 @@ v 8.10.0 (unreleased)
- Add API endpoint for a group issues !4520 (mahcsig)
- Add Bugzilla integration !4930 (iamtjg)
- Metrics for Rouge::Plugins::Redcarpet and Rouge::Formatters::HTMLGitlab
+ - RailsCache metris now includes fetch_hit/fetch_miss and read_hit/read_miss info.
- Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w)
- Add basic system information like memory and disk usage to the admin panel
- Don't garbage collect commits that have related DB records like comments
diff --git a/lib/gitlab/metrics/subscribers/rails_cache.rb b/lib/gitlab/metrics/subscribers/rails_cache.rb
index 277c860fbef0c..aaed2184f44ae 100644
--- a/lib/gitlab/metrics/subscribers/rails_cache.rb
+++ b/lib/gitlab/metrics/subscribers/rails_cache.rb
@@ -2,11 +2,21 @@ module Gitlab
module Metrics
module Subscribers
# Class for tracking the total time spent in Rails cache calls
+ # http://guides.rubyonrails.org/active_support_instrumentation.html
class RailsCache < ActiveSupport::Subscriber
attach_to :active_support
def cache_read(event)
increment(:cache_read, event.duration)
+
+ return unless current_transaction
+ return if event.payload[:super_operation] == :fetch
+
+ if event.payload[:hit]
+ current_transaction.increment(:cache_read_hit_count, 1)
+ else
+ current_transaction.increment(:cache_read_miss_count, 1)
+ end
end
def cache_write(event)
@@ -24,13 +34,13 @@ def cache_exist?(event)
def cache_fetch_hit(event)
return unless current_transaction
- current_transaction.increment(:cache_fetch_hit, 1)
+ current_transaction.increment(:cache_read_hit_count, 1)
end
def cache_generate(event)
return unless current_transaction
- current_transaction.increment(:cache_fetch_miss, 1)
+ current_transaction.increment(:cache_read_miss_count, 1)
end
def increment(key, duration)
diff --git a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb
index d824dc54438bb..d986c6fac4380 100644
--- a/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb
+++ b/spec/lib/gitlab/metrics/subscribers/rails_cache_spec.rb
@@ -13,6 +13,61 @@
subscriber.cache_read(event)
end
+
+ context 'with a transaction' do
+ before do
+ allow(subscriber).to receive(:current_transaction).
+ and_return(transaction)
+ end
+
+ context 'with hit event' do
+ let(:event) { double(:event, duration: 15.2, payload: { hit: true }) }
+
+ it 'increments the cache_read_hit count' do
+ expect(transaction).to receive(:increment).
+ with(:cache_read_hit_count, 1)
+ expect(transaction).to receive(:increment).
+ with(any_args).at_least(1) # Other calls
+
+ subscriber.cache_read(event)
+ end
+
+ context 'when super operation is fetch' do
+ let(:event) { double(:event, duration: 15.2, payload: { hit: true, super_operation: :fetch }) }
+
+ it 'does not increment cache read miss' do
+ expect(transaction).not_to receive(:increment).
+ with(:cache_read_hit_count, 1)
+
+ subscriber.cache_read(event)
+ end
+ end
+ end
+
+ context 'with miss event' do
+ let(:event) { double(:event, duration: 15.2, payload: { hit: false }) }
+
+ it 'increments the cache_read_miss count' do
+ expect(transaction).to receive(:increment).
+ with(:cache_read_miss_count, 1)
+ expect(transaction).to receive(:increment).
+ with(any_args).at_least(1) # Other calls
+
+ subscriber.cache_read(event)
+ end
+
+ context 'when super operation is fetch' do
+ let(:event) { double(:event, duration: 15.2, payload: { hit: false, super_operation: :fetch }) }
+
+ it 'does not increment cache read miss' do
+ expect(transaction).not_to receive(:increment).
+ with(:cache_read_miss_count, 1)
+
+ subscriber.cache_read(event)
+ end
+ end
+ end
+ end
end
describe '#cache_write' do
@@ -42,6 +97,54 @@
end
end
+ describe '#cache_fetch_hit' do
+ context 'without a transaction' do
+ it 'returns' do
+ expect(transaction).not_to receive(:increment)
+
+ subscriber.cache_fetch_hit(event)
+ end
+ end
+
+ context 'with a transaction' do
+ before do
+ allow(subscriber).to receive(:current_transaction).
+ and_return(transaction)
+ end
+
+ it 'increments the cache_read_hit count' do
+ expect(transaction).to receive(:increment).
+ with(:cache_read_hit_count, 1)
+
+ subscriber.cache_fetch_hit(event)
+ end
+ end
+ end
+
+ describe '#cache_generate' do
+ context 'without a transaction' do
+ it 'returns' do
+ expect(transaction).not_to receive(:increment)
+
+ subscriber.cache_generate(event)
+ end
+ end
+
+ context 'with a transaction' do
+ before do
+ allow(subscriber).to receive(:current_transaction).
+ and_return(transaction)
+ end
+
+ it 'increments the cache_fetch_miss count' do
+ expect(transaction).to receive(:increment).
+ with(:cache_read_miss_count, 1)
+
+ subscriber.cache_generate(event)
+ end
+ end
+ end
+
describe '#increment' do
context 'without a transaction' do
it 'returns' do
--
GitLab