From 5ef8dcac7874865c15afcacc64cdeda82bc660a6 Mon Sep 17 00:00:00 2001
From: Jonas Larsen <jlarsen@gitlab.com>
Date: Thu, 16 Feb 2023 20:29:21 +0000
Subject: [PATCH] Add a new danger rule to prevent adding new metrics to
UsageData class
---
danger/product_intelligence/Dangerfile | 2 +
.../danger/product_intelligence_spec.rb | 47 ++++++++++++++++++-
tooling/danger/product_intelligence.rb | 12 +++++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/danger/product_intelligence/Dangerfile b/danger/product_intelligence/Dangerfile
index 1be549f139f18..86888fe542e64 100644
--- a/danger/product_intelligence/Dangerfile
+++ b/danger/product_intelligence/Dangerfile
@@ -3,3 +3,5 @@
product_intelligence.check!
product_intelligence.check_affected_scopes!
+
+product_intelligence.check_usage_data_insertions!
diff --git a/spec/tooling/danger/product_intelligence_spec.rb b/spec/tooling/danger/product_intelligence_spec.rb
index fab8b0c61fa27..c4cd0e5bfb6e9 100644
--- a/spec/tooling/danger/product_intelligence_spec.rb
+++ b/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
diff --git a/tooling/danger/product_intelligence.rb b/tooling/danger/product_intelligence.rb
index 58e327408a126..d25f966504f26 100644
--- a/tooling/danger/product_intelligence.rb
+++ b/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)
--
GitLab