From 42d54619d16c7bddd43d0225445dd2844aec882f Mon Sep 17 00:00:00 2001
From: Niko Belokolodov <nbelokolodov@gitlab.com>
Date: Fri, 2 Aug 2024 20:04:45 +0000
Subject: [PATCH] Add a Dangerbot rule to suggest adding specs for Redis/HLL
metrics migration
---
danger/analytics_instrumentation/Dangerfile | 2 ++
.../danger/analytics_instrumentation_spec.rb | 34 +++++++++++++++++++
tooling/danger/analytics_instrumentation.rb | 14 ++++++--
3 files changed, 48 insertions(+), 2 deletions(-)
diff --git a/danger/analytics_instrumentation/Dangerfile b/danger/analytics_instrumentation/Dangerfile
index 4e57697c92ba3..df4474ffc59ba 100644
--- a/danger/analytics_instrumentation/Dangerfile
+++ b/danger/analytics_instrumentation/Dangerfile
@@ -11,3 +11,5 @@ analytics_instrumentation.check_usage_data_insertions!
analytics_instrumentation.check_deprecated_data_sources!
analytics_instrumentation.check_removed_metric_fields!
+
+analytics_instrumentation.warn_about_migrated_redis_keys_specs!
diff --git a/spec/tooling/danger/analytics_instrumentation_spec.rb b/spec/tooling/danger/analytics_instrumentation_spec.rb
index 09db343d3ccdd..6b85c6531c472 100644
--- a/spec/tooling/danger/analytics_instrumentation_spec.rb
+++ b/spec/tooling/danger/analytics_instrumentation_spec.rb
@@ -443,4 +443,38 @@
end
end
end
+
+ describe '#warn_about_migrated_redis_keys_specs!' do
+ let(:redis_hll_file) { 'lib/gitlab/usage_data_counters/hll_redis_key_overrides.yml' }
+ let(:total_counter_file) { 'lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml' }
+
+ subject(:check_redis_keys_files_overrides) { analytics_instrumentation.warn_about_migrated_redis_keys_specs! }
+
+ before do
+ allow(fake_helper).to receive(:changed_lines).with(redis_hll_file).and_return([file_diff_hll])
+ allow(fake_helper).to receive(:changed_lines).with(total_counter_file).and_return([file_diff_total])
+ end
+
+ context 'when new keys added to overrides files' do
+ let(:file_diff_hll) { "+user_viewed_cluster_configuration-user: user_viewed_cluster_configuration" }
+ let(:file_diff_total) { "+user_viewed_cluster_configuration-user: USER_VIEWED_CLUSTER_CONFIGURATION" }
+
+ it 'adds suggestion to add specs' do
+ expect(analytics_instrumentation).to receive(:warn)
+
+ check_redis_keys_files_overrides
+ end
+ end
+
+ context 'when no new keys added to overrides files' do
+ let(:file_diff_hll) { "-user_viewed_cluster_configuration-user: user_viewed_cluster_configuration" }
+ let(:file_diff_total) { "-user_viewed_cluster_configuration-user: USER_VIEWED_CLUSTER_CONFIGURATION" }
+
+ it 'adds suggestion to add specs' do
+ expect(analytics_instrumentation).not_to receive(:warn)
+
+ check_redis_keys_files_overrides
+ end
+ end
+ end
end
diff --git a/tooling/danger/analytics_instrumentation.rb b/tooling/danger/analytics_instrumentation.rb
index 10ff990f9f4da..6afa97562ad81 100644
--- a/tooling/danger/analytics_instrumentation.rb
+++ b/tooling/danger/analytics_instrumentation.rb
@@ -97,12 +97,22 @@ def check_removed_metric_fields!
end
end
- def modified_config_files
- helper.modified_files.select { |f| f.include?('config/metrics') && f.end_with?('yml') }
+ def warn_about_migrated_redis_keys_specs!
+ override_files_changes = ["lib/gitlab/usage_data_counters/hll_redis_key_overrides.yml",
+ "lib/gitlab/usage_data_counters/total_counter_redis_key_overrides.yml"].map do |filename|
+ helper.changed_lines(filename).filter { |line| line.start_with?("+") }
+ end
+ return if override_files_changes.flatten.none?
+
+ warn "Redis keys overrides were added. Please consider cover keys merging with specs. See the [related issue](https://gitlab.com/gitlab-org/gitlab/-/issues/475191) for details"
end
private
+ def modified_config_files
+ helper.modified_files.select { |f| f.include?('config/metrics') && f.end_with?('yml') }
+ end
+
def comment_removed_metric(filename, has_removed_url, has_removed_milestone)
mr_has_milestone = !helper.mr_milestone.nil?
milestone = mr_has_milestone ? helper.mr_milestone['title'] : '[PLEASE SET MILESTONE]'
--
GitLab