Merge branch 'georgekoltsov/add-metrics-to-importers' into 'master'

Add Bitbucket Importer metrics

See merge request gitlab-org/gitlab!27524

changelogs/unreleased/georgekoltsov-add-metrics-to-importers.yml

+---
+title: Add Bitbucket Importer metrics
+merge_request: 27524
+author:
+type: other

lib/gitlab/bitbucket_import/importer.rb

@@ -3,6 +3,8 @@
 module Gitlab
   module BitbucketImport
     class Importer
+      include Gitlab::BitbucketImport::Metrics
+
       LABELS = [{ title: 'bug', color: '#FF0000' },
                 { title: 'enhancement', color: '#428BCA' },
                 { title: 'proposal', color: '#69D100' },
@@ -83,38 +85,42 @@ module Gitlab
         errors << { type: :wiki, errors: e.message }
       end
 
-      # rubocop: disable CodeReuse/ActiveRecord
       def import_issues
         return unless repo.issues_enabled?
 
         create_labels
 
         client.issues(repo).each do |issue|
-          description = ''
-          description += @formatter.author_line(issue.author) unless find_user_id(issue.author)
-          description += issue.description
-
-          label_name = issue.kind
-          milestone = issue.milestone ? project.milestones.find_or_create_by(title: issue.milestone) : nil
-
-          gitlab_issue = project.issues.create!(
-            iid: issue.iid,
-            title: issue.title,
-            description: description,
-            state_id: Issue.available_states[issue.state],
-            author_id: gitlab_user_id(project, issue.author),
-            milestone: milestone,
-            created_at: issue.created_at,
-            updated_at: issue.updated_at
-          )
-
-          gitlab_issue.labels << @labels[label_name]
-
-          import_issue_comments(issue, gitlab_issue) if gitlab_issue.persisted?
-        rescue StandardError => e
-          errors << { type: :issue, iid: issue.iid, errors: e.message }
+          import_issue(issue)
         end
       end
+
+      # rubocop: disable CodeReuse/ActiveRecord
+      def import_issue(issue)
+        description = ''
+        description += @formatter.author_line(issue.author) unless find_user_id(issue.author)
+        description += issue.description
+
+        label_name = issue.kind
+        milestone = issue.milestone ? project.milestones.find_or_create_by(title: issue.milestone) : nil
+
+        gitlab_issue = project.issues.create!(
+          iid: issue.iid,
+          title: issue.title,
+          description: description,
+          state_id: Issue.available_states[issue.state],
+          author_id: gitlab_user_id(project, issue.author),
+          milestone: milestone,
+          created_at: issue.created_at,
+          updated_at: issue.updated_at
+        )
+
+        gitlab_issue.labels << @labels[label_name]
+
+        import_issue_comments(issue, gitlab_issue) if gitlab_issue.persisted?
+      rescue StandardError => e
+        errors << { type: :issue, iid: issue.iid, errors: e.message }
+      end
       # rubocop: enable CodeReuse/ActiveRecord
 
       def import_issue_comments(issue, gitlab_issue)
@@ -159,37 +165,41 @@ module Gitlab
         pull_requests = client.pull_requests(repo)
 
         pull_requests.each do |pull_request|
-          description = ''
-          description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author)
-          description += pull_request.description
-
-          source_branch_sha = pull_request.source_branch_sha
-          target_branch_sha = pull_request.target_branch_sha
-          source_branch_sha = project.repository.commit(source_branch_sha)&.sha || source_branch_sha
-          target_branch_sha = project.repository.commit(target_branch_sha)&.sha || target_branch_sha
-
-          merge_request = project.merge_requests.create!(
-            iid: pull_request.iid,
-            title: pull_request.title,
-            description: description,
-            source_project: project,
-            source_branch: pull_request.source_branch_name,
-            source_branch_sha: source_branch_sha,
-            target_project: project,
-            target_branch: pull_request.target_branch_name,
-            target_branch_sha: target_branch_sha,
-            state: pull_request.state,
-            author_id: gitlab_user_id(project, pull_request.author),
-            created_at: pull_request.created_at,
-            updated_at: pull_request.updated_at
-          )
-
-          import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
-        rescue StandardError => e
-          store_pull_request_error(pull_request, e)
+          import_pull_request(pull_request)
         end
       end
 
+      def import_pull_request(pull_request)
+        description = ''
+        description += @formatter.author_line(pull_request.author) unless find_user_id(pull_request.author)
+        description += pull_request.description
+
+        source_branch_sha = pull_request.source_branch_sha
+        target_branch_sha = pull_request.target_branch_sha
+        source_branch_sha = project.repository.commit(source_branch_sha)&.sha || source_branch_sha
+        target_branch_sha = project.repository.commit(target_branch_sha)&.sha || target_branch_sha
+
+        merge_request = project.merge_requests.create!(
+          iid: pull_request.iid,
+          title: pull_request.title,
+          description: description,
+          source_project: project,
+          source_branch: pull_request.source_branch_name,
+          source_branch_sha: source_branch_sha,
+          target_project: project,
+          target_branch: pull_request.target_branch_name,
+          target_branch_sha: target_branch_sha,
+          state: pull_request.state,
+          author_id: gitlab_user_id(project, pull_request.author),
+          created_at: pull_request.created_at,
+          updated_at: pull_request.updated_at
+        )
+
+        import_pull_request_comments(pull_request, merge_request) if merge_request.persisted?
+      rescue StandardError => e
+        store_pull_request_error(pull_request, e)
+      end
+
       def import_pull_request_comments(pull_request, merge_request)
         comments = client.pull_request_comments(repo, pull_request.iid)
 

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/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
+
+          result = super(*args)
+
+          end_time = Time.zone.now
+
+          report_measurement_metrics(metrics, end_time - start_time)
+
+          result
+        end
+      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
+          end
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/bitbucket_import/importer_spec.rb

@@ -87,6 +87,7 @@ describe Gitlab::BitbucketImport::Importer do
       values: sample_issues_statuses
     }
   end
+  let(:counter) { double('counter', increment: true) }
 
   subject { described_class.new(project) }
 
@@ -213,6 +214,24 @@ describe Gitlab::BitbucketImport::Importer do
         expect(merge_request_diff.start_commit_sha).to eq target_branch_sha
       end
     end
+
+    context 'metrics' do
+      before do
+        allow(Gitlab::Metrics).to receive(:counter) { counter }
+        allow(pull_request).to receive(:raw).and_return('hello world')
+      end
+
+      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'
+        )
+
+        expect(counter).to receive(:increment)
+
+        subject.execute
+      end
+    end
   end
 
   context 'issues statuses' do
@@ -339,5 +358,54 @@ describe Gitlab::BitbucketImport::Importer do
         expect(importer.errors).to be_empty
       end
     end
+
+    context 'metrics' do
+      before do
+        allow(Gitlab::Metrics).to receive(:counter) { counter }
+      end
+
+      it 'counts imported issues' do
+        expect(Gitlab::Metrics).to receive(:counter).with(
+          :bitbucket_importer_imported_issues,
+          'The number of imported Bitbucket issues'
+        )
+
+        expect(counter).to receive(:increment)
+
+        subject.execute
+      end
+    end
+  end
+
+  describe '#execute' do
+    context 'metrics' do
+      let(:histogram) { double(:histogram) }
+
+      before do
+        allow(subject).to receive(:import_wiki)
+        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 }
+      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'
+        )
+
+        expect(Gitlab::Metrics).to receive(:histogram).with(
+          :bitbucket_importer_total_duration_seconds,
+          'Total time spent importing Bitbucket projects, in seconds'
+        )
+
+        expect(counter).to receive(:increment)
+        expect(histogram).to receive(:observe).with({ importer: described_class::IMPORTER }, anything)
+
+        subject.execute
+      end
+    end
   end
 end

spec/lib/gitlab/import/metrics_spec.rb

+# frozen_string_literal: true
+
+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(:histogram) { double(:histogram) }
+
+    it 'increments counter metric' do
+      expect(Gitlab::Metrics)
+        .to receive(:counter)
+              .with(:importer_counter, 'description')
+              .and_return(counter)
+
+      expect(counter).to receive(:increment)
+
+      subject
+    end
+
+    it 'measures method duration and reports histogram metric' do
+      expect(Gitlab::Metrics)
+        .to receive(:histogram)
+              .with(:importer_histogram, 'description')
+              .and_return(histogram)
+
+      expect(histogram).to receive(:observe).with({ importer: 'importer' }, anything)
+
+      subject
+    end
+  end
+end

spec/services/projects/import_service_spec.rb

@@ -123,8 +123,13 @@ describe Projects::ImportService do
 
         it 'succeeds if repository import is successful' do
           expect(project.repository).to receive(:import_repository).and_return(true)
-          expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true)
-          expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :success)
+          expect_next_instance_of(Gitlab::BitbucketImport::Importer) do |importer|
+            expect(importer).to receive(:execute).and_return(true)
+          end
+
+          expect_next_instance_of(Projects::LfsPointers::LfsImportService) do |service|
+            expect(service).to receive(:execute).and_return(status: :success)
+          end
 
           result = subject.execute
 
@@ -147,8 +152,15 @@ describe Projects::ImportService do
             error_message = 'error message'
 
             expect(project.repository).to receive(:import_repository).and_return(true)
-            expect_any_instance_of(Gitlab::BitbucketImport::Importer).to receive(:execute).and_return(true)
-            expect_any_instance_of(Projects::LfsPointers::LfsImportService).to receive(:execute).and_return(status: :error, message: error_message)
+
+            expect_next_instance_of(Gitlab::BitbucketImport::Importer) do |importer|
+              expect(importer).to receive(:execute).and_return(true)
+            end
+
+            expect_next_instance_of(Projects::LfsPointers::LfsImportService) do |service|
+              expect(service).to receive(:execute).and_return(status: :error, message: error_message)
+            end
+
             expect(Gitlab::AppLogger).to receive(:error).with("The Lfs import process failed. #{error_message}")
 
             subject.execute