Merge branch '297401-usage-tracking-for-jira-integration' into 'master'

Track usage of the Jira project service [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!52110

app/models/project_services/jira_service.rb

 # frozen_string_literal: true
 
+# Accessible as Project#external_issue_tracker
 class JiraService < IssueTrackerService
   extend ::Gitlab::Utils::Override
   include Gitlab::Routing
@@ -162,7 +163,7 @@ class JiraService < IssueTrackerService
     jira_request { client.Issue.find(issue_key) }
   end
 
-  def close_issue(entity, external_issue)
+  def close_issue(entity, external_issue, current_user)
     issue = find_issue(external_issue.iid)
 
     return if issue.nil? || has_resolution?(issue) || !jira_issue_transition_id.present?
@@ -179,6 +180,7 @@ class JiraService < IssueTrackerService
     # if it is closed, so we don't have one comment for every commit.
     issue = find_issue(issue.key) if transition_issue(issue)
     add_issue_solved_comment(issue, commit_id, commit_url) if has_resolution?(issue)
+    log_usage(:close_issue, current_user)
   end
 
   def create_cross_reference_note(mentioned, noteable, author)
@@ -214,7 +216,7 @@ class JiraService < IssueTrackerService
       }
     }
 
-    add_comment(data, jira_issue)
+    add_comment(data, jira_issue).tap { log_usage(:cross_reference, author) }
   end
 
   def valid_connection?
@@ -275,6 +277,12 @@ class JiraService < IssueTrackerService
     end
   end
 
+  def log_usage(action, user)
+    key = "i_ecosystem_jira_service_#{action}"
+
+    Gitlab::UsageDataCounters::HLLRedisCounter.track_event(key, values: user.id)
+  end
+
   def add_issue_solved_comment(issue, commit_id, commit_url)
     link_title   = "Solved by commit #{commit_id}."
     comment      = "Issue solved with [#{commit_id}|#{commit_url}]."

app/services/issues/close_service.rb

@@ -55,7 +55,7 @@ module Issues
     def close_external_issue(issue, closed_via)
       return unless project.external_issue_tracker&.support_close_issue?
 
-      project.external_issue_tracker.close_issue(closed_via, issue)
+      project.external_issue_tracker.close_issue(closed_via, issue, current_user)
       todo_service.close_issue(issue, current_user)
     end
 

config/feature_flags/development/usage_data_track_ecosystem_jira_service.yml

+---
+name: usage_data_track_ecosystem_jira_service
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/52110
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/300447
+milestone: '13.9'
+type: development
+group: group::ecosystem
+default_enabled: false

doc/development/redis.md

@@ -185,9 +185,8 @@ The Redis [`PFCOUNT`](https://redis.io/commands/pfcount),
 [`PFMERGE`](https://redis.io/commands/pfmergge) commands operate on
 HyperLogLogs, a data structure that allows estimating the number of unique
 elements with low memory usage. (In addition to the `PFCOUNT` documentation,
-Thoughtbot's article on [HyperLogLogs in
-Redis](https://thoughtbot.com/blog/hyperloglogs-in-redis) provides a good
-background here.)
+Thoughtbot's article on [HyperLogLogs in Redis](https://thoughtbot.com/blog/hyperloglogs-in-redis)
+provides a good background here.)
 
 [`Gitlab::Redis::HLL`](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/gitlab/redis/hll.rb)
 provides a convenient interface for adding and counting values in HyperLogLogs.

ee/app/controllers/projects/integrations/jira/issues_controller.rb

@@ -7,6 +7,11 @@ module Projects
         include RecordUserLastActivity
         include SortingHelper
         include SortingPreference
+        include RedisTracking
+
+        track_redis_hll_event :index,
+          name: 'i_ecosystem_jira_service_list_issues',
+          feature: :usage_data_track_ecosystem_jira_service
 
         before_action :check_feature_enabled!
         before_action :check_issues_show_enabled!, only: :show
@@ -43,6 +48,10 @@ module Projects
 
         private
 
+        def visitor_id
+          current_user&.id
+        end
+
         def issues_json
           jira_issues = Kaminari.paginate_array(
             finder.execute,

ee/app/models/project_services/ee/jira_service.rb

@@ -53,7 +53,7 @@ module EE
       "#{url}/secure/CreateIssueDetails!init.jspa?pid=#{jira_project_id}&issuetype=#{vulnerabilities_issuetype}&summary=#{escaped_summary}&description=#{escaped_description}"[0..MAX_URL_LENGTH]
     end
 
-    def create_issue(summary, description)
+    def create_issue(summary, description, current_user)
       return if client_url.blank?
 
       jira_request do
@@ -66,6 +66,7 @@ module EE
             description: description
           }
         )
+        log_usage(:create_issue, current_user)
         issue
       end
     end

ee/app/services/vulnerability_external_issue_links/create_service.rb

@@ -37,7 +37,7 @@ module VulnerabilityExternalIssueLinks
       summary = _('Investigate vulnerability: %{title}') % { title: vulnerability.title }
       description = vulnerability.present.jira_issue_description
 
-      external_provider_service.create_issue(summary, description)
+      external_provider_service.create_issue(summary, description, user)
     end
 
     def prepare_external_provider_service

ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb

@@ -38,6 +38,14 @@ RSpec.describe Projects::Integrations::Jira::IssuesController do
       expect(response).to render_template(:index)
     end
 
+    it 'tracks usage' do
+      expect(Gitlab::UsageDataCounters::HLLRedisCounter)
+        .to receive(:track_event)
+        .with('i_ecosystem_jira_service_list_issues', values: user.id)
+
+      get :index, params: { namespace_id: project.namespace, project_id: project }
+    end
+
     context 'when project has moved' do
       let(:new_project) { create(:project) }
 

ee/spec/models/project_services/jira_service_spec.rb

@@ -150,13 +150,23 @@ RSpec.describe JiraService do
       end
 
       it 'creates issue in Jira API' do
-        issue = jira_service.create_issue("Special Summary!?", "*ID*: 2\n_Issue_: !")
+        issue = jira_service.create_issue("Special Summary!?", "*ID*: 2\n_Issue_: !", build(:user))
 
         expect(WebMock).to have_requested(:post, 'http://jira.example.com/rest/api/2/issue').with(
           body: { fields: { project: { id: '11223' }, issuetype: { id: '10001' }, summary: 'Special Summary!?', description: "*ID*: 2\n_Issue_: !" } }.to_json
         ).once
         expect(issue.id).to eq('10000')
       end
+
+      it 'tracks usage' do
+        user = build_stubbed(:user)
+
+        expect(Gitlab::UsageDataCounters::HLLRedisCounter)
+          .to receive(:track_event)
+          .with('i_ecosystem_jira_service_create_issue', values: user.id)
+
+        jira_service.create_issue('x', 'y', user)
+      end
     end
 
     context 'when there is an error in Jira' do
@@ -167,7 +177,7 @@ RSpec.describe JiraService do
       end
 
       it 'returns issue with errors' do
-        issue = jira_service.create_issue('', "*ID*: 2\n_Issue_: !")
+        issue = jira_service.create_issue('', "*ID*: 2\n_Issue_: !", build(:user))
 
         expect(WebMock).to have_requested(:post, 'http://jira.example.com/rest/api/2/issue').with(
           body: { fields: { project: { id: '11223' }, issuetype: { id: '10001' }, summary: '', description: "*ID*: 2\n_Issue_: !" } }.to_json

ee/spec/services/vulnerability_external_issue_links/create_service_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe VulnerabilityExternalIssueLinks::CreateService do
         end
 
         it 'creates issue using jira service' do
-          expect(jira_service).to receive(:create_issue).with("Investigate vulnerability: #{vulnerability.title}", kind_of(String))
+          expect(jira_service).to receive(:create_issue).with("Investigate vulnerability: #{vulnerability.title}", kind_of(String), user)
 
           subject
         end

lib/gitlab/usage_data_counters/known_events/ecosystem.yml

+---
+# Ecosystem category
+- name: i_ecosystem_jira_service_close_issue
+  category: ecosystem
+  redis_slot: ecosystem
+  aggregation: weekly
+  feature_flag: usage_data_track_ecosystem_jira_service
+- name: i_ecosystem_jira_service_cross_reference
+  category: ecosystem
+  redis_slot: ecosystem
+  aggregation: weekly
+  feature_flag: usage_data_track_ecosystem_jira_service
+- name: i_ecosystem_jira_service_list_issues
+  category: ecosystem
+  redis_slot: ecosystem
+  aggregation: weekly
+  feature_flag: usage_data_track_ecosystem_jira_service
+- name: i_ecosystem_jira_service_create_issue
+  category: ecosystem
+  redis_slot: ecosystem
+  aggregation: weekly
+  feature_flag: usage_data_track_ecosystem_jira_service

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -27,6 +27,7 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
         'deploy_token_packages',
         'user_packages',
         'compliance',
+        'ecosystem',
         'analytics',
         'ide_edit',
         'search',

spec/models/project_services/jira_service_spec.rb

@@ -6,6 +6,8 @@ RSpec.describe JiraService do
   include AssetsHelpers
 
   let_it_be(:project) { create(:project, :repository) }
+
+  let(:current_user) { build_stubbed(:user) }
   let(:url) { 'http://jira.example.com' }
   let(:api_url) { 'http://api-jira.example.com' }
   let(:username) { 'jira-username' }
@@ -498,25 +500,38 @@ RSpec.describe JiraService do
         WebMock.stub_request(:post, @remote_link_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password))
       end
 
+      let(:external_issue) { ExternalIssue.new('JIRA-123', project) }
+
+      def close_issue
+        @jira_service.close_issue(resource, external_issue, current_user)
+      end
+
       it 'calls Jira API' do
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(WebMock).to have_requested(:post, @comment_url).with(
           body: /Issue solved with/
         ).once
       end
 
+      it 'tracks usage' do
+        expect(Gitlab::UsageDataCounters::HLLRedisCounter)
+          .to receive(:track_event)
+          .with('i_ecosystem_jira_service_close_issue', values: current_user.id)
+
+        close_issue
+      end
+
       it 'does not fail if remote_link.all on issue returns nil' do
         allow(JIRA::Resource::Remotelink).to receive(:all).and_return(nil)
 
-        expect { @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project)) }
-            .not_to raise_error
+        expect { close_issue }.not_to raise_error
       end
 
       # Check https://developer.atlassian.com/jiradev/jira-platform/guides/other/guide-jira-remote-issue-links/fields-in-remote-issue-links
       # for more information
       it 'creates Remote Link reference in Jira for comment' do
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         favicon_path = "http://localhost/assets/#{find_asset('favicon.png').digest_path}"
 
@@ -540,7 +555,7 @@ RSpec.describe JiraService do
       context 'when "comment_on_event_enabled" is set to false' do
         it 'creates Remote Link reference but does not create comment' do
           allow(@jira_service).to receive_messages(comment_on_event_enabled: false)
-          @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+          close_issue
 
           expect(WebMock).not_to have_requested(:post, @comment_url)
           expect(WebMock).to have_requested(:post, @remote_link_url)
@@ -562,7 +577,7 @@ RSpec.describe JiraService do
 
           expect(remote_link).to receive(:save!)
 
-          @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+          close_issue
 
           expect(WebMock).not_to have_requested(:post, @comment_url)
         end
@@ -571,7 +586,7 @@ RSpec.describe JiraService do
       it 'does not send comment or remote links to issues already closed' do
         allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(true)
 
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(WebMock).not_to have_requested(:post, @comment_url)
         expect(WebMock).not_to have_requested(:post, @remote_link_url)
@@ -580,7 +595,7 @@ RSpec.describe JiraService do
       it 'does not send comment or remote links to issues with unknown resolution' do
         allow_any_instance_of(JIRA::Resource::Issue).to receive(:respond_to?).with(:resolution).and_return(false)
 
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(WebMock).not_to have_requested(:post, @comment_url)
         expect(WebMock).not_to have_requested(:post, @remote_link_url)
@@ -589,7 +604,7 @@ RSpec.describe JiraService do
       it 'references the GitLab commit' do
         stub_config_setting(base_url: custom_base_url)
 
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(WebMock).to have_requested(:post, @comment_url).with(
           body: %r{#{custom_base_url}/#{project.full_path}/-/commit/#{commit_id}}
@@ -604,7 +619,7 @@ RSpec.describe JiraService do
           { script_name: '/gitlab' }
         end
 
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(WebMock).to have_requested(:post, @comment_url).with(
           body: %r{#{Gitlab.config.gitlab.url}/#{project.full_path}/-/commit/#{commit_id}}
@@ -615,7 +630,7 @@ RSpec.describe JiraService do
         allow(@jira_service).to receive(:log_error)
         WebMock.stub_request(:post, @transitions_url).with(basic_auth: %w(gitlab_jira_username gitlab_jira_password)).and_raise("Bad Request")
 
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(@jira_service).to have_received(:log_error).with(
           "Issue transition failed",
@@ -628,7 +643,7 @@ RSpec.describe JiraService do
       end
 
       it 'calls the api with jira_issue_transition_id' do
-        @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+        close_issue
 
         expect(WebMock).to have_requested(:post, @transitions_url).with(
           body: /999/
@@ -639,7 +654,7 @@ RSpec.describe JiraService do
         it 'calls the api with transition ids separated by comma' do
           allow(@jira_service).to receive_messages(jira_issue_transition_id: '1,2,3')
 
-          @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+          close_issue
 
           1.upto(3) do |transition_id|
             expect(WebMock).to have_requested(:post, @transitions_url).with(
@@ -651,7 +666,7 @@ RSpec.describe JiraService do
         it 'calls the api with transition ids separated by semicolon' do
           allow(@jira_service).to receive_messages(jira_issue_transition_id: '1;2;3')
 
-          @jira_service.close_issue(resource, ExternalIssue.new('JIRA-123', project))
+          close_issue
 
           1.upto(3) do |transition_id|
             expect(WebMock).to have_requested(:post, @transitions_url).with(
@@ -702,6 +717,14 @@ RSpec.describe JiraService do
           body: /mentioned this issue in/
         ).once
       end
+
+      it 'tracks usage' do
+        expect(Gitlab::UsageDataCounters::HLLRedisCounter)
+          .to receive(:track_event)
+          .with('i_ecosystem_jira_service_cross_reference', values: user.id)
+
+        subject
+      end
     end
 
     context 'when resource is a commit' do

spec/services/merge_requests/merge_service_spec.rb

@@ -161,7 +161,7 @@ RSpec.describe MergeRequests::MergeService do
           commit = double('commit', safe_message: "Fixes #{jira_issue.to_reference}")
           allow(merge_request).to receive(:commits).and_return([commit])
 
-          expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue).once
+          expect_any_instance_of(JiraService).to receive(:close_issue).with(merge_request, jira_issue, user).once
 
           service.execute(merge_request)
         end