Merge branch 'am-remove-force-refresh-param-from-usage_data' into 'master'

Add ::Gitlab::Usage::ServicePing::Generate class

See merge request gitlab-org/gitlab!79735

app/controllers/admin/application_settings_controller.rb

@@ -62,11 +62,11 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController
   def usage_data
     respond_to do |format|
       format.html do
-        usage_data_json = Gitlab::Json.pretty_generate(Gitlab::UsageData.data)
+        usage_data_json = Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true))
 
         render html: Gitlab::Highlight.highlight('payload.json', usage_data_json, language: 'json')
       end
-      format.json { render json: Gitlab::UsageData.to_json }
+      format.json { render json: Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true).to_json }
     end
   end
 

app/controllers/admin/instance_review_controller.rb

@@ -16,7 +16,7 @@ class Admin::InstanceReviewController < Admin::ApplicationController
     }
 
     if Gitlab::CurrentSettings.usage_ping_enabled?
-      data = ::Gitlab::UsageData.data
+      data = Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true)
       counts = data[:counts]
 
       result[:instance_review].merge!(

app/services/service_ping/build_payload_service.rb

@@ -19,7 +19,7 @@ module ServicePing
     end
 
     def raw_payload
-      @raw_payload ||= ::Gitlab::UsageData.data(force_refresh: true)
+      @raw_payload ||= ::Gitlab::Usage::ServicePingReport.for(mode: :values)
     end
   end
 end

app/services/service_ping/submit_service.rb

@@ -33,7 +33,7 @@ module ServicePing
         }
         submit_payload({ error: error_payload }, url: error_url)
 
-        usage_data = Gitlab::UsageData.data(force_refresh: true)
+        usage_data = Gitlab::Usage::ServicePingReport.for(mode: :values)
         response = submit_usage_data_payload(usage_data)
       end
 

doc/administration/troubleshooting/gitlab_rails_cheat_sheet.md

@@ -1324,7 +1324,7 @@ has more information about Service Ping.
 ### Generate or get the cached Service Ping
 
 ```ruby
-Gitlab::UsageData.to_json
+Gitlab::Usage::ServicePingReport.for(mode: :values, cached: true)
 ```
 
 ### Generate a fresh new Service Ping
@@ -1332,7 +1332,7 @@ Gitlab::UsageData.to_json
 This also refreshes the cached Service Ping displayed in the Admin Area
 
 ```ruby
-Gitlab::UsageData.to_json(force_refresh: true)
+Gitlab::Usage::ServicePingReport.for(mode: :values)
 ```
 
 ### Generate and print

doc/development/service_ping/index.md

@@ -198,9 +198,9 @@ sequenceDiagram
 ## How Service Ping works
 
 1. The Service Ping [cron job](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/workers/gitlab_service_ping_worker.rb#L24) is set in Sidekiq to run weekly.
-1. When the cron job runs, it calls [`Gitlab::UsageData.to_json`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb#L49).
-1. `Gitlab::UsageData.to_json` [cascades down](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb) to ~400+ other counter method calls.
-1. The response of all methods calls are [merged together](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb#L68) into a single JSON payload in `Gitlab::UsageData.to_json`.
+1. When the cron job runs, it calls [`Gitlab::Usage::ServicePingReport.for(mode: :values)`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb).
+1. `Gitlab::Usage::ServicePingReport.for(mode: :values)` [cascades down](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb) to ~400+ other counter method calls.
+1. The response of all methods calls are [merged together](https://gitlab.com/gitlab-org/gitlab/-/blob/master/lib/gitlab/usage_data.rb#L68) into a single JSON payload.
 1. The JSON payload is then [posted to the Versions application](https://gitlab.com/gitlab-org/gitlab/-/blob/master/app/services/service_ping/submit_service.rb#L20)
    If a firewall exception is needed, the required URL depends on several things. If
    the hostname is `version.gitlab.com`, the protocol is `TCP`, and the port number is `443`,

ee/spec/config/metrics/every_metric_definition_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe 'Every metric definition' do
   include UsageDataHelpers
 
-  let(:usage_ping) { Gitlab::UsageData.data(force_refresh: true) }
+  let(:usage_ping) { Gitlab::Usage::ServicePingReport.for(mode: :values) }
   let(:ignored_usage_ping_key_patterns) do
     %w(
       license_add_ons

ee/spec/lib/ee/gitlab/usage_data_non_sql_metrics_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Gitlab::UsageDataNonSqlMetrics do
   describe '.data' do
     it 'does make instrumentations_class DB calls' do
       recorder = ActiveRecord::QueryRecorder.new do
-        described_class.data(force_refresh: true)
+        described_class.data
       end
 
       expect(recorder.count).to eq(65)

ee/spec/lib/ee/gitlab/usage_data_spec.rb

@@ -801,7 +801,7 @@ RSpec.describe Gitlab::UsageData do
   it 'clears memoized values' do
     allow(described_class).to receive(:clear_memoization)
 
-    described_class.data(force_refresh: true)
+    described_class.data
 
     described_class::EE_MEMOIZED_VALUES.each do |key|
       expect(described_class).to have_received(:clear_memoization).with(key)

lib/api/usage_data_non_sql_metrics.rb

@@ -18,7 +18,7 @@ module API
       get 'non_sql_metrics' do
         Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534')
 
-        data = Gitlab::UsageDataNonSqlMetrics.data(force_refresh: true)
+        data = Gitlab::UsageDataNonSqlMetrics.data
 
         present data
       end

lib/api/usage_data_queries.rb

@@ -18,7 +18,7 @@ module API
       get 'queries' do
         Gitlab::QueryLimiting.disable!('https://gitlab.com/gitlab-org/gitlab/-/issues/325534')
 
-        queries = Gitlab::UsageDataQueries.data(force_refresh: true)
+        queries = Gitlab::UsageDataQueries.data
 
         present queries
       end

lib/gitlab/usage/service_ping_report.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Usage
+    class ServicePingReport
+      class << self
+        def for(mode:, cached: false)
+          case mode.to_sym
+          when :values
+            usage_data(cached)
+          end
+        end
+
+        private
+
+        def usage_data(cached)
+          Rails.cache.fetch('usage_data', force: !cached, expires_in: 2.weeks) do
+            Gitlab::UsageData.data
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/usage_data.rb

@@ -41,14 +41,8 @@ module Gitlab
       include Gitlab::Utils::StrongMemoize
       include Gitlab::Usage::TimeFrame
 
-      def data(force_refresh: false)
-        Rails.cache.fetch('usage_data', force: force_refresh, expires_in: 2.weeks) do
-          uncached_data
-        end
-      end
-
-      def to_json(force_refresh: false)
-        data(force_refresh: force_refresh).to_json
+      def data
+        uncached_data
       end
 
       def license_usage_data

lib/tasks/gitlab/usage_data.rake

@@ -4,17 +4,17 @@ namespace :gitlab do
   namespace :usage_data do
     desc 'GitLab | UsageData | Generate raw SQLs for usage ping in YAML'
     task dump_sql_in_yaml: :environment do
-      puts Gitlab::UsageDataQueries.data(force_refresh: true).to_yaml
+      puts Gitlab::UsageDataQueries.data.to_yaml
     end
 
     desc 'GitLab | UsageData | Generate raw SQLs for usage ping in JSON'
     task dump_sql_in_json: :environment do
-      puts Gitlab::Json.pretty_generate(Gitlab::UsageDataQueries.data(force_refresh: true))
+      puts Gitlab::Json.pretty_generate(Gitlab::UsageDataQueries.data)
     end
 
     desc 'GitLab | UsageData | Generate usage ping in JSON'
     task generate: :environment do
-      puts Gitlab::Json.pretty_generate(Gitlab::UsageData.data(force: true))
+      puts Gitlab::Json.pretty_generate(Gitlab::Usage::ServicePingReport.for(mode: :values))
     end
 
     desc 'GitLab | UsageData | Generate usage ping and send it to Versions Application'

spec/controllers/admin/instance_review_controller_spec.rb

@@ -23,7 +23,7 @@ RSpec.describe Admin::InstanceReviewController do
         stub_application_setting(usage_ping_enabled: true)
         stub_usage_data_connections
         stub_database_flavor_check
-        ::Gitlab::UsageData.data(force_refresh: true)
+        ::Gitlab::Usage::ServicePingReport.for(mode: :values)
         subject
       end
 

spec/lib/gitlab/usage/service_ping_report_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Usage::ServicePingReport, :use_clean_rails_memory_store_caching do
+  let(:usage_data) { { uuid: "1111" } }
+
+  context 'for mode: :values' do
+    it 'generates the service ping' do
+      expect(Gitlab::UsageData).to receive(:data)
+
+      described_class.for(mode: :values)
+    end
+  end
+
+  context 'when using cached' do
+    context 'for cached: true' do
+      let(:new_usage_data) { { uuid: "1112" } }
+
+      it 'caches the values' do
+        allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data)
+
+        expect(described_class.for(mode: :values)).to eq(usage_data)
+        expect(described_class.for(mode: :values, cached: true)).to eq(usage_data)
+
+        expect(Rails.cache.fetch('usage_data')).to eq(usage_data)
+      end
+
+      it 'writes to cache and returns fresh data' do
+        allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data)
+
+        expect(described_class.for(mode: :values)).to eq(usage_data)
+        expect(described_class.for(mode: :values)).to eq(new_usage_data)
+        expect(described_class.for(mode: :values, cached: true)).to eq(new_usage_data)
+
+        expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data)
+      end
+    end
+
+    context 'when no caching' do
+      let(:new_usage_data) { { uuid: "1112" } }
+
+      it 'returns fresh data' do
+        allow(Gitlab::UsageData).to receive(:data).and_return(usage_data, new_usage_data)
+
+        expect(described_class.for(mode: :values)).to eq(usage_data)
+        expect(described_class.for(mode: :values)).to eq(new_usage_data)
+
+        expect(Rails.cache.fetch('usage_data')).to eq(new_usage_data)
+      end
+    end
+  end
+end

spec/lib/gitlab/usage_data_spec.rb

@@ -13,7 +13,7 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
   end
 
   describe '.data' do
-    subject { described_class.data(force_refresh: true) }
+    subject { described_class.data }
 
     it 'includes basic top and second level keys' do
       is_expected.to include(:counts)

spec/services/service_ping/submit_service_ping_service_spec.rb

@@ -55,7 +55,7 @@ RSpec.describe ServicePing::SubmitService do
   shared_examples 'does not run' do
     it do
       expect(Gitlab::HTTP).not_to receive(:post)
-      expect(Gitlab::UsageData).not_to receive(:data)
+      expect(Gitlab::Usage::ServicePingReport).not_to receive(:for)
 
       subject.execute
     end
@@ -118,7 +118,7 @@ RSpec.describe ServicePing::SubmitService do
     it 'generates service ping' do
       stub_response(body: with_dev_ops_score_params)
 
-      expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_call_original
+      expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_call_original
 
       subject.execute
     end
@@ -151,7 +151,7 @@ RSpec.describe ServicePing::SubmitService do
     it 'forces a refresh of usage data statistics before submitting' do
       stub_response(body: with_dev_ops_score_params)
 
-      expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_call_original
+      expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_call_original
 
       subject.execute
     end
@@ -167,7 +167,7 @@ RSpec.describe ServicePing::SubmitService do
         recorded_at = Time.current
         usage_data = { uuid: 'uuid', recorded_at: recorded_at }
 
-        expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data)
+        expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data)
 
         subject.execute
 
@@ -190,7 +190,7 @@ RSpec.describe ServicePing::SubmitService do
         recorded_at = Time.current
         usage_data = { uuid: 'uuid', recorded_at: recorded_at }
 
-        expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data)
+        expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data)
 
         subject.execute
 
@@ -235,7 +235,7 @@ RSpec.describe ServicePing::SubmitService do
         recorded_at = Time.current
         usage_data = { uuid: 'uuid', recorded_at: recorded_at }
 
-        expect(Gitlab::UsageData).to receive(:data).with(force_refresh: true).and_return(usage_data)
+        expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data)
 
         subject.execute
 
@@ -260,7 +260,7 @@ RSpec.describe ServicePing::SubmitService do
 
     context 'and usage data is empty string' do
       before do
-        allow(Gitlab::UsageData).to receive(:data).and_return({})
+        allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return({})
       end
 
       it_behaves_like 'does not send a blank usage ping payload'
@@ -269,7 +269,7 @@ RSpec.describe ServicePing::SubmitService do
     context 'and usage data is nil' do
       before do
         allow(ServicePing::BuildPayloadService).to receive(:execute).and_return(nil)
-        allow(Gitlab::UsageData).to receive(:data).and_return(nil)
+        allow(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(nil)
       end
 
       it_behaves_like 'does not send a blank usage ping payload'
@@ -282,10 +282,10 @@ RSpec.describe ServicePing::SubmitService do
                                                      .and_raise(described_class::SubmissionError, 'SubmissionError')
       end
 
-      it 'calls UsageData .data method' do
+      it 'calls Gitlab::Usage::ServicePingReport .for method' do
         usage_data = build_usage_data
 
-        expect(Gitlab::UsageData).to receive(:data).and_return(usage_data)
+        expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data)
 
         subject.execute
       end
@@ -326,10 +326,10 @@ RSpec.describe ServicePing::SubmitService do
         end
       end
 
-      it 'calls UsageData .data method' do
+      it 'calls Gitlab::Usage::ServicePingReport .for method' do
         usage_data = build_usage_data
 
-        expect(Gitlab::UsageData).to receive(:data).and_return(usage_data)
+        expect(Gitlab::Usage::ServicePingReport).to receive(:for).with(mode: :values).and_return(usage_data)
 
         # SubmissionError is raised as a result of 404 in response from HTTP Request
         expect { subject.execute }.to raise_error(described_class::SubmissionError)