Merge branch...

Merge branch '377341-add-a-new-danger-rule-to-prevent-adding-new-metrics-to-usagedata-class' into 'master'

Add a new danger rule to prevent adding new metrics to UsageData class

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/111687



Merged-by: Ethan Urie <eurie@gitlab.com>
Approved-by: Ethan Urie <eurie@gitlab.com>
Approved-by: Dzmitry Meshcharakou <12459192-dmeshcharakou@users.noreply.gitlab.com>
Approved-by: Nao Hashizume <nhashizume@gitlab.com>
Reviewed-by: Dzmitry Meshcharakou <12459192-dmeshcharakou@users.noreply.gitlab.com>
Co-authored-by: j_lar <jlarsen@gitlab.com>

danger/product_intelligence/Dangerfile

@@ -3,3 +3,5 @@
 product_intelligence.check!
 
 product_intelligence.check_affected_scopes!
+
+product_intelligence.check_usage_data_insertions!

spec/tooling/danger/product_intelligence_spec.rb

@@ -18,7 +18,7 @@
   let(:has_product_intelligence_label) { true }
 
   before do
-    allow(fake_helper).to receive(:changed_lines).and_return(changed_lines)
+    allow(fake_helper).to receive(:changed_lines).and_return(changed_lines) if defined?(changed_lines)
     allow(fake_helper).to receive(:labels_to_add).and_return(labels_to_add)
     allow(fake_helper).to receive(:ci?).and_return(ci_env)
     allow(fake_helper).to receive(:mr_has_labels?).with('product intelligence').and_return(has_product_intelligence_label)
@@ -175,4 +175,49 @@
       end
     end
   end
+
+  describe '#check_usage_data_insertions!' do
+    context 'when usage_data.rb is modified' do
+      let(:modified_files) { ['lib/gitlab/usage_data.rb'] }
+
+      before do
+        allow(fake_helper).to receive(:changed_lines).with("lib/gitlab/usage_data.rb").and_return(changed_lines)
+      end
+
+      context 'and has insertions' do
+        let(:changed_lines) { ['+ ci_runners: count(::Ci::CiRunner),'] }
+
+        it 'produces warning' do
+          expect(product_intelligence).to receive(:warn).with(/usage_data\.rb has been deprecated/)
+
+          product_intelligence.check_usage_data_insertions!
+        end
+      end
+
+      context 'and changes are not insertions' do
+        let(:changed_lines) { ['- ci_runners: count(::Ci::CiRunner),'] }
+
+        it 'doesnt do anything' do
+          expect(product_intelligence).not_to receive(:warn)
+
+          product_intelligence.check_usage_data_insertions!
+        end
+      end
+    end
+
+    context 'when usage_data.rb is not modified' do
+      context 'and another file has insertions' do
+        let(:modified_files) { ['tooling/danger/product_intelligence.rb'] }
+
+        it 'doesnt do anything' do
+          expect(fake_helper).to receive(:changed_lines).with("lib/gitlab/usage_data.rb").and_return([])
+          allow(fake_helper).to receive(:changed_lines).with("tooling/danger/product_intelligence.rb").and_return(["+ Inserting"])
+
+          expect(product_intelligence).not_to receive(:warn)
+
+          product_intelligence.check_usage_data_insertions!
+        end
+      end
+    end
+  end
 end

tooling/danger/product_intelligence.rb

@@ -22,6 +22,11 @@ module ProductIntelligence
 
       MSG
 
+      CHANGED_USAGE_DATA_MESSAGE = <<~MSG
+        Notice that implementing metrics directly in usage_data.rb has been deprecated. ([Deprecated Usage Metrics](https://docs.gitlab.com/ee/development/service_ping/usage_data.html#usage-data-metrics-guide))
+        Please use [Instrumentation Classes](https://docs.gitlab.com/ee/development/service_ping/metrics_instrumentation.html) instead.
+      MSG
+
       WORKFLOW_LABELS = [
         APPROVED_LABEL,
         REVIEW_LABEL
@@ -47,6 +52,13 @@ def check_affected_scopes!
         helper.labels_to_add.concat(missing_labels) unless missing_labels.empty?
       end
 
+      def check_usage_data_insertions!
+        usage_data_changes = helper.changed_lines("lib/gitlab/usage_data.rb")
+        return if usage_data_changes.none? { |change| change.start_with?("+") }
+
+        warn format(CHANGED_USAGE_DATA_MESSAGE)
+      end
+
       private
 
       def convert_to_table(items)