Merge branch 'am-redis-hll-keys-issue' into 'master'

Fix Redis HLL weekly keys

See merge request gitlab-org/gitlab!50358

lib/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -336,12 +336,10 @@ def validate_aggregation_operator!(operator)
         end
         def weekly_redis_keys(events:, start_date:, end_date:, context: '')
-          weeks = end_date.to_date.cweek - start_date.to_date.cweek
-          weeks = 1 if weeks == 0
-
-          (0..(weeks - 1)).map do |week_increment|
-            events.map { |event| redis_key(event, start_date + week_increment * 7.days, context) }
-          end.flatten
+          end_date = end_date.end_of_week - 1.week
+          (start_date.to_date..end_date.to_date).map do |date|
+            events.map { |event| redis_key(event, date, context) }
+          end.flatten.uniq
         end
       end
     end

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -283,6 +283,50 @@
       context 'when no slot is set' do
         it { expect(described_class.unique_events(event_names: [no_slot], start_date: 7.days.ago, end_date: Date.current)).to eq(1) }
       end
+
+      context 'when data crosses into new year' do
+        it 'does not raise error' do
+          expect { described_class.unique_events(event_names: [weekly_event], start_date: DateTime.parse('2020-12-26'), end_date: DateTime.parse('2021-02-01')) }
+            .not_to raise_error
+        end
+      end
+    end
+  end
+
+  describe '.weekly_redis_keys' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:weekly_event) { 'g_compliance_dashboard' }
+    let(:redis_event) { described_class.send(:event_for, weekly_event) }
+
+    subject(:weekly_redis_keys) { described_class.send(:weekly_redis_keys, events: [redis_event], start_date: DateTime.parse(start_date), end_date: DateTime.parse(end_date)) }
+
+    where(:start_date, :end_date, :keys) do
+      '2020-12-21' | '2020-12-21' | []
+      '2020-12-21' | '2020-12-20' | []
+      '2020-12-21' | '2020-11-21' | []
+      '2021-01-01' | '2020-12-28' | []
+      '2020-12-21' | '2020-12-28' | ['g_{compliance}_dashboard-2020-52']
+      '2020-12-21' | '2021-01-01' | ['g_{compliance}_dashboard-2020-52']
+      '2020-12-27' | '2021-01-01' | ['g_{compliance}_dashboard-2020-52']
+      '2020-12-26' | '2021-01-04' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53']
+      '2020-12-26' | '2021-01-11' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53', 'g_{compliance}_dashboard-2021-01']
+      '2020-12-26' | '2021-01-17' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53', 'g_{compliance}_dashboard-2021-01']
+      '2020-12-26' | '2021-01-18' | ['g_{compliance}_dashboard-2020-52', 'g_{compliance}_dashboard-2020-53', 'g_{compliance}_dashboard-2021-01', 'g_{compliance}_dashboard-2021-02']
+    end
+
+    with_them do
+      it "returns the correct keys" do
+        expect(subject).to match(keys)
+      end
+    end
+
+    it 'returns 1 key for last for week' do
+      expect(described_class.send(:weekly_redis_keys, events: [redis_event], start_date: 7.days.ago.to_date, end_date: Date.current).size).to eq 1
+    end
+
+    it 'returns 4 key for last for weeks' do
+      expect(described_class.send(:weekly_redis_keys, events: [redis_event], start_date: 4.weeks.ago.to_date, end_date: Date.current).size).to eq 4
     end
   end

spec/lib/gitlab/usage_data_spec.rb

@@ -54,6 +54,32 @@
       expect { subject }.to raise_error('Stopped calculating recorded_at')
     end
+
+    context 'when generating usage ping in critical weeks' do
+      it 'does not raise error when generated in last week of the year' do
+        travel_to(DateTime.parse('2020-12-29')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      it 'does not raise error when generated in first week of the year' do
+        travel_to(DateTime.parse('2021-01-01')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      it 'does not raise error when generated in second week of the year' do
+        travel_to(DateTime.parse('2021-01-07')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+
+      it 'does not raise error when generated in 3rd week of the year' do
+        travel_to(DateTime.parse('2021-01-14')) do
+          expect { subject }.not_to raise_error
+        end
+      end
+    end
   end
   describe 'usage_activity_by_stage_package' do

spec/services/ci/build_report_result_service_spec.rb

@@ -6,12 +6,6 @@
   describe '#execute', :clean_gitlab_redis_shared_state do
     subject(:build_report_result) { described_class.new.execute(build) }
-    around do |example|
-      travel_to(DateTime.parse('2020-07-01')) do
-        example.run
-      end
-    end
-
     context 'when build is finished' do
       let(:build) { create(:ci_build, :success, :test_reports) }

spec/support/shared_examples/lib/gitlab/usage_data_counters/incident_management_activity_shared_examples.rb

@@ -3,8 +3,8 @@
 RSpec.shared_examples 'an incident management tracked event' do |event|
   describe ".track_event", :clean_gitlab_redis_shared_state do
     let(:counter) { Gitlab::UsageDataCounters::HLLRedisCounter }
-    let(:start_time) { 1.minute.ago }
-    let(:end_time) { 1.minute.from_now }
+    let(:start_time) { 1.week.ago }
+    let(:end_time) { 1.week.from_now }
     it "tracks the event using redis" do
       # Allow other subsequent calls