Merge branch 'georgekoltsov/emit-bitbucket-server-metrics' into 'master'

Emit Bitbucket Server Importer metrics

See merge request gitlab-org/gitlab!33700

changelogs/unreleased/georgekoltsov-emit-bitbucket-server-metrics.yml

+---
+title: Emit Bitbucket Server Importer metrics
+merge_request: 33700
+author:
+type: changed

lib/gitlab/bitbucket_import/importer.rb

@@ -3,8 +3,6 @@
 module Gitlab
   module BitbucketImport
     class Importer
-      include Gitlab::BitbucketImport::Metrics
-
       LABELS = [{ title: 'bug', color: '#FF0000' },
                 { title: 'enhancement', color: '#428BCA' },
                 { title: 'proposal', color: '#69D100' },
@@ -26,6 +24,7 @@ module Gitlab
         import_issues
         import_pull_requests
         handle_errors
+        metrics.track_finished_import
 
         true
       end
@@ -115,6 +114,8 @@ module Gitlab
           updated_at: issue.updated_at
         )
 
+        metrics.issues_counter.increment
+
         gitlab_issue.labels << @labels[label_name]
 
         import_issue_comments(issue, gitlab_issue) if gitlab_issue.persisted?
@@ -195,6 +196,8 @@ module Gitlab
           updated_at: pull_request.updated_at
         )
 
+        metrics.merge_requests_counter.increment
+
         import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
       rescue StandardError => e
         store_pull_request_error(pull_request, e)
@@ -288,6 +291,10 @@ module Gitlab
           project_path: project.full_path
         }
       end
+
+      def metrics
+        @metrics ||= Gitlab::Import::Metrics.new(:bitbucket_importer, @project)
+      end
     end
   end
 end

lib/gitlab/bitbucket_import/metrics.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module BitbucketImport
-    module Metrics
-      extend ActiveSupport::Concern
-
-      IMPORTER = :bitbucket_importer
-
-      included do
-        prepend Gitlab::Import::Metrics
-
-        Gitlab::Import::Metrics.measure(:execute, metrics: {
-          "#{IMPORTER}_imported_projects": {
-            type: :counter,
-            description: 'The number of imported Bitbucket projects'
-          },
-          "#{IMPORTER}_total_duration_seconds": {
-            type: :histogram,
-            labels: { importer: IMPORTER },
-            description: 'Total time spent importing Bitbucket projects, in seconds'
-          }
-        })
-
-        Gitlab::Import::Metrics.measure(:import_issue, metrics: {
-          "#{IMPORTER}_imported_issues": {
-            type: :counter,
-            description: 'The number of imported Bitbucket issues'
-          }
-        })
-
-        Gitlab::Import::Metrics.measure(:import_pull_request, metrics: {
-          "#{IMPORTER}_imported_pull_requests": {
-            type: :counter,
-            description: 'The number of imported Bitbucket pull requests'
-          }
-        })
-      end
-    end
-  end
-end

lib/gitlab/bitbucket_server_import/importer.rb

@@ -43,6 +43,7 @@ module Gitlab
         import_pull_requests
         delete_temp_branches
         handle_errors
+        metrics.track_finished_import
 
         log_info(stage: "complete")
 
@@ -219,7 +220,11 @@ module Gitlab
         creator = Gitlab::Import::MergeRequestCreator.new(project)
         merge_request = creator.execute(attributes)
 
-        import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
+        if merge_request.persisted?
+          import_pull_request_comments(pull_request, merge_request)
+
+          metrics.merge_requests_counter.increment
+        end
 
         log_info(stage: 'import_bitbucket_pull_requests', message: 'finished', iid: pull_request.iid)
       end
@@ -388,6 +393,10 @@ module Gitlab
           project_path: project.full_path
         }
       end
+
+      def metrics
+        @metrics ||= Gitlab::Import::Metrics.new(:bitbucket_server_importer, @project)
+      end
     end
   end
 end

lib/gitlab/import/metrics.rb

 # frozen_string_literal: true
 
-# Prepend `Gitlab::Import::Metrics` to a class in order
-# to measure and emit `Gitlab::Metrics` metrics of specified methods.
-#
-# @example
-#   class Importer
-#     prepend Gitlab::Import::Metrics
-#
-#     Gitlab::ImportExport::Metrics.measure :execute, metrics: {
-#       importer_counter: {
-#         type: :counter,
-#         description: 'counter'
-#       },
-#       importer_histogram: {
-#         type: :histogram,
-#         labels: { importer: 'importer' },
-#         description: 'histogram'
-#       }
-#     }
-#
-#     def execute
-#        ...
-#     end
-#   end
-#
-# Each call to `#execute` increments `importer_counter` as well as
-# measures `#execute` duration and reports histogram `importer_histogram`
 module Gitlab
   module Import
-    module Metrics
-      def self.measure(method_name, metrics:)
-        define_method "#{method_name}" do |*args|
-          start_time = Time.zone.now
+    class Metrics
+      IMPORT_DURATION_BUCKETS = [0.5, 1, 3, 5, 10, 60, 120, 240, 360, 720, 1440].freeze
 
-          result = super(*args)
+      attr_reader :importer
 
-          end_time = Time.zone.now
+      def initialize(importer, project)
+        @importer = importer
+        @project = project
+      end
 
-          report_measurement_metrics(metrics, end_time - start_time)
+      def track_finished_import
+        duration = Time.zone.now - @project.created_at
 
-          result
+        duration_histogram.observe({ importer: importer }, duration)
+        projects_counter.increment
       end
+
+      def projects_counter
+        @projects_counter ||= Gitlab::Metrics.counter(
+          :"#{importer}_imported_projects_total",
+          'The number of imported projects'
+        )
       end
 
-      def report_measurement_metrics(metrics, duration)
-        metrics.each do |metric_name, metric_value|
-          case metric_value[:type]
-          when :counter
-            Gitlab::Metrics.counter(metric_name, metric_value[:description]).increment
-          when :histogram
-            Gitlab::Metrics.histogram(metric_name, metric_value[:description]).observe(metric_value[:labels], duration)
-          else
-            nil
+      def issues_counter
+        @issues_counter ||= Gitlab::Metrics.counter(
+          :"#{importer}_imported_issues_total",
+          'The number of imported issues'
+        )
       end
+
+      def merge_requests_counter
+        @merge_requests_counter ||= Gitlab::Metrics.counter(
+          :"#{importer}_imported_merge_requests_total",
+          'The number of imported merge (pull) requests'
+        )
       end
+
+      private
+
+      def duration_histogram
+        @duration_histogram ||= Gitlab::Metrics.histogram(
+          :"#{importer}_total_duration_seconds",
+          'Total time spent importing projects, in seconds',
+          {},
+          IMPORT_DURATION_BUCKETS
+        )
       end
     end
   end

spec/lib/gitlab/bitbucket_import/importer_spec.rb

@@ -226,8 +226,8 @@ describe Gitlab::BitbucketImport::Importer do
 
       it 'counts imported pull requests' do
         expect(Gitlab::Metrics).to receive(:counter).with(
-          :bitbucket_importer_imported_pull_requests,
-          'The number of imported Bitbucket pull requests'
+          :bitbucket_importer_imported_merge_requests_total,
+          'The number of imported merge (pull) requests'
         )
 
         expect(counter).to receive(:increment)
@@ -369,8 +369,8 @@ describe Gitlab::BitbucketImport::Importer do
 
       it 'counts imported issues' do
         expect(Gitlab::Metrics).to receive(:counter).with(
-          :bitbucket_importer_imported_issues,
-          'The number of imported Bitbucket issues'
+          :bitbucket_importer_imported_issues_total,
+          'The number of imported issues'
         )
 
         expect(counter).to receive(:increment)
@@ -389,23 +389,27 @@ describe Gitlab::BitbucketImport::Importer do
         allow(subject).to receive(:import_issues)
         allow(subject).to receive(:import_pull_requests)
 
-        allow(Gitlab::Metrics).to receive(:counter) { counter }
-        allow(Gitlab::Metrics).to receive(:histogram) { histogram }
+        allow(Gitlab::Metrics).to receive(:counter).and_return(counter)
+        allow(Gitlab::Metrics).to receive(:histogram).and_return(histogram)
+        allow(histogram).to receive(:observe)
+        allow(counter).to receive(:increment)
       end
 
       it 'counts and measures duration of imported projects' do
         expect(Gitlab::Metrics).to receive(:counter).with(
-          :bitbucket_importer_imported_projects,
-          'The number of imported Bitbucket projects'
+          :bitbucket_importer_imported_projects_total,
+          'The number of imported projects'
         )
 
         expect(Gitlab::Metrics).to receive(:histogram).with(
           :bitbucket_importer_total_duration_seconds,
-          'Total time spent importing Bitbucket projects, in seconds'
+          'Total time spent importing projects, in seconds',
+          {},
+          Gitlab::Import::Metrics::IMPORT_DURATION_BUCKETS
         )
 
         expect(counter).to receive(:increment)
-        expect(histogram).to receive(:observe).with({ importer: described_class::IMPORTER }, anything)
+        expect(histogram).to receive(:observe).with({ importer: :bitbucket_importer }, anything)
 
         subject.execute
       end

spec/lib/gitlab/bitbucket_server_import/importer_spec.rb

@@ -5,7 +5,10 @@ require 'spec_helper'
 describe Gitlab::BitbucketServerImport::Importer do
   include ImportSpecHelper
 
-  let(:project) { create(:project, :repository, import_url: 'http://my-bitbucket') }
+  let(:import_url) { 'http://my-bitbucket' }
+  let(:user) { 'bitbucket' }
+  let(:password) { 'test' }
+  let(:project) { create(:project, :repository, import_url: import_url) }
   let(:now) { Time.now.utc.change(usec: 0) }
   let(:project_key) { 'TEST' }
   let(:repo_slug) { 'rouge' }
@@ -16,7 +19,7 @@ describe Gitlab::BitbucketServerImport::Importer do
   before do
     data = project.create_or_update_import_data(
       data: { project_key: project_key, repo_slug: repo_slug },
-      credentials: { base_uri: 'http://my-bitbucket', user: 'bitbucket', password: 'test' }
+      credentials: { base_uri: import_url, user: user, password: password }
     )
     data.save
     project.save
@@ -125,6 +128,48 @@ describe Gitlab::BitbucketServerImport::Importer do
       expect(note.updated_at).to eq(@pr_note.created_at)
     end
 
+    context 'metrics' do
+      let(:histogram) { double(:histogram) }
+      let(:counter) { double('counter', increment: true) }
+
+      before do
+        allow(Gitlab::Metrics).to receive(:counter) { counter }
+        allow(Gitlab::Metrics).to receive(:histogram) { histogram }
+        allow(subject.client).to receive(:activities).and_return([@merge_event])
+      end
+
+      it 'counts and measures duration of imported projects' do
+        expect(Gitlab::Metrics).to receive(:counter).with(
+          :bitbucket_server_importer_imported_projects_total,
+          'The number of imported projects'
+        )
+
+        expect(Gitlab::Metrics).to receive(:histogram).with(
+          :bitbucket_server_importer_total_duration_seconds,
+          'Total time spent importing projects, in seconds',
+          {},
+          Gitlab::Import::Metrics::IMPORT_DURATION_BUCKETS
+        )
+
+        expect(counter).to receive(:increment)
+        expect(histogram).to receive(:observe).with({ importer: :bitbucket_server_importer }, anything)
+
+        subject.execute
+      end
+
+      it 'counts imported pull requests' do
+        expect(Gitlab::Metrics).to receive(:counter).with(
+          :bitbucket_server_importer_imported_merge_requests_total,
+          'The number of imported merge (pull) requests'
+        )
+
+        expect(counter).to receive(:increment)
+        allow(histogram).to receive(:observe).with({ importer: :bitbucket_server_importer }, anything)
+
+        subject.execute
+      end
+    end
+
     it 'imports threaded discussions' do
       reply = instance_double(
         BitbucketServer::Representation::PullRequestComment,

spec/lib/gitlab/import/metrics_spec.rb

@@ -3,54 +3,38 @@
 require 'spec_helper'
 
 describe Gitlab::Import::Metrics do
-  let(:importer_stub) do
-    Class.new do
-      prepend Gitlab::Import::Metrics
-
-      Gitlab::Import::Metrics.measure :execute, metrics: {
-        importer_counter:   {
-          type: :counter,
-          description: 'description'
-        },
-        importer_histogram: {
-          type: :histogram,
-          labels: { importer: 'importer' },
-          description: 'description'
-        }
-      }
-
-      def execute
-        true
-      end
-    end
-  end
-
-  subject { importer_stub.new.execute }
-
-  describe '#execute' do
-    let(:counter) { double(:counter) }
+  let(:importer) { :test_importer }
+  let(:project) { create(:project) }
   let(:histogram) { double(:histogram) }
+  let(:counter) { double(:counter) }
 
-    it 'increments counter metric' do
-      expect(Gitlab::Metrics)
-        .to receive(:counter)
-              .with(:importer_counter, 'description')
-              .and_return(counter)
-
-      expect(counter).to receive(:increment)
+  subject { described_class.new(importer, project) }
 
-      subject
+  describe '#report_import_time' do
+    before do
+      allow(Gitlab::Metrics).to receive(:counter) { counter }
+      allow(Gitlab::Metrics).to receive(:histogram) { histogram }
+      allow(counter).to receive(:increment)
+      allow(counter).to receive(:observe)
     end
 
-    it 'measures method duration and reports histogram metric' do
-      expect(Gitlab::Metrics)
-        .to receive(:histogram)
-              .with(:importer_histogram, 'description')
-              .and_return(histogram)
+    it 'emits importer metrics' do
+      expect(Gitlab::Metrics).to receive(:counter).with(
+        :test_importer_imported_projects_total,
+        'The number of imported projects'
+      )
 
-      expect(histogram).to receive(:observe).with({ importer: 'importer' }, anything)
+      expect(Gitlab::Metrics).to receive(:histogram).with(
+        :test_importer_total_duration_seconds,
+        'Total time spent importing projects, in seconds',
+        {},
+        described_class::IMPORT_DURATION_BUCKETS
+      )
+
+      expect(counter).to receive(:increment)
+      expect(histogram).to receive(:observe).with({ importer: :test_importer }, anything)
 
-      subject
+      subject.track_finished_import
     end
   end
 end