Merge branch 'metrics-measure-block' into 'master'

Support for measuring Ruby blocks using GitLab performance monitoring

This adds support for measuring timings of arbitrary Ruby blocks.

Fixes #14710 

See merge request !3515

4 files changed, 108 insertions, 3 deletions

diff --git a/doc/development/README.md b/doc/development/README.md
index 1b281809afc..8940b558fb6 100644
--- a/doc/development/README.md
+++ b/doc/development/README.md
@@ -4,6 +4,7 @@
 - [CI setup](ci_setup.md) for testing GitLab
 - [Gotchas](gotchas.md) to avoid
 - [How to dump production data to staging](db_dump.md)
+- [Instrumentation](instrumentation.md)
 - [Migration Style Guide](migration_style_guide.md) for creating safe migrations
 - [Rake tasks](rake_tasks.md) for development
 - [Shell commands](shell_commands.md) in the GitLab codebase
diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md
new file mode 100644
index 00000000000..c0192bd6709
--- /dev/null
+++ b/doc/development/instrumentation.md
@@ -0,0 +1,37 @@
+# Instrumenting Ruby Code
+
+GitLab Performance Monitoring allows instrumenting of custom blocks of Ruby
+code. This can be used to measure the time spent in a specific part of a larger
+chunk of code. The resulting data is written to a separate series.
+
+To start measuring a block of Ruby code you should use
+`Gitlab::Metrics.measure` and give it a name for the series to store the data
+in:
+
+```ruby
+Gitlab::Metrics.measure(:user_logins) do
+  ...
+end
+```
+
+The first argument of this method is the series name and should be plural. This
+name will be prefixed with `rails_` or `sidekiq_` depending on whether the code
+was run in the Rails application or one of the Sidekiq workers. In the
+above example the final series names would be as follows:
+
+- rails_user_logins
+- sidekiq_user_logins
+
+Series names should be plural as this keeps the naming style in line with the
+other series names.
+
+By default metrics measured using a block contain a single value, "duration",
+which contains the number of milliseconds it took to execute the block. Custom
+values can be added by passing a Hash as the 2nd argument. Custom tags can be
+added by passing a Hash as the 3rd argument. A simple example is as follows:
+
+```ruby
+Gitlab::Metrics.measure(:example_series, { number: 10 }, { class: self.class.to_s }) do
+  ...
+end
+```
diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb
index 88a265c6af2..4a3f47b5a95 100644
--- a/lib/gitlab/metrics.rb
+++ b/lib/gitlab/metrics.rb
@@ -70,6 +70,32 @@ module Gitlab
       value.to_s.gsub('=', '\\=')
     end
 
+    # Measures the execution time of a block.
+    #
+    # Example:
+    #
+    #     Gitlab::Metrics.measure(:find_by_username_timings) do
+    #       User.find_by_username(some_username)
+    #     end
+    #
+    # series - The name of the series to store the data in.
+    # values - A Hash containing extra values to add to the metric.
+    # tags - A Hash containing extra tags to add to the metric.
+    #
+    # Returns the value yielded by the supplied block.
+    def self.measure(series, values = {}, tags = {})
+      return yield unless Transaction.current
+
+      start = Time.now.to_f
+      retval = yield
+      duration = (Time.now.to_f - start) * 1000.0
+      values = values.merge(duration: duration)
+
+      Transaction.current.add_metric(series, values, tags)
+
+      retval
+    end
+
     # When enabled this should be set before being used as the usual pattern
     # "@foo ||= bar" is _not_ thread-safe.
     if enabled?
diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb
index 0ec8a6dc5cb..8f63a5f2043 100644
--- a/spec/lib/gitlab/metrics_spec.rb
+++ b/spec/lib/gitlab/metrics_spec.rb
@@ -13,7 +13,7 @@ describe Gitlab::Metrics do
     end
   end
 
-  describe '#submit_metrics' do
+  describe '.submit_metrics' do
     it 'prepares and writes the metrics to InfluxDB' do
       connection = double(:connection)
       pool       = double(:pool)
@@ -26,7 +26,7 @@ describe Gitlab::Metrics do
     end
   end
 
-  describe '#prepare_metrics' do
+  describe '.prepare_metrics' do
     it 'returns a Hash with the keys as Symbols' do
       metrics = described_class.
         prepare_metrics([{ 'values' => {}, 'tags' => {} }])
@@ -51,7 +51,7 @@ describe Gitlab::Metrics do
     end
   end
 
-  describe '#escape_value' do
+  describe '.escape_value' do
     it 'escapes an equals sign' do
       expect(described_class.escape_value('foo=')).to eq('foo\\=')
     end
@@ -60,4 +60,45 @@ describe Gitlab::Metrics do
       expect(described_class.escape_value(10)).to eq('10')
     end
   end
+
+  describe '.measure' do
+    context 'without a transaction' do
+      it 'returns the return value of the block' do
+        val = Gitlab::Metrics.measure(:foo) { 10 }
+
+        expect(val).to eq(10)
+      end
+    end
+
+    context 'with a transaction' do
+      let(:transaction) { Gitlab::Metrics::Transaction.new }
+
+      before do
+        allow(Gitlab::Metrics::Transaction).to receive(:current).
+          and_return(transaction)
+      end
+
+      it 'adds a metric to the current transaction' do
+        expect(transaction).to receive(:add_metric).
+          with(:foo, { duration: a_kind_of(Numeric) }, { tag: 'value' })
+
+        Gitlab::Metrics.measure(:foo, {}, tag: 'value') { 10 }
+      end
+
+      it 'supports adding of custom values' do
+        values = { duration: a_kind_of(Numeric), number: 10 }
+
+        expect(transaction).to receive(:add_metric).
+          with(:foo, values, { tag: 'value' })
+
+        Gitlab::Metrics.measure(:foo, { number: 10 }, tag: 'value') { 10 }
+      end
+
+      it 'returns the return value of the block' do
+        val = Gitlab::Metrics.measure(:foo) { 10 }
+
+        expect(val).to eq(10)
+      end
+    end
+  end
 end