Restructure metrics for time frame

lib/gitlab/usage_data_counters/hll_redis_counter.rb

@@ -12,6 +12,7 @@ module Gitlab
 
       KNOWN_EVENTS_PATH = 'lib/gitlab/usage_data_counters/known_events.yml'.freeze
       ALLOWED_AGGREGATIONS = %i(daily weekly).freeze
+      COUNTS_TYPES = %i(weekly monthly).freeze
 
       # Track event on entity_id
       # Increment a Redis HLL counter for unique event_name and entity_id
@@ -68,29 +69,43 @@ module Gitlab
         end
 
         def unique_events_data
+          {
+            counts_weekly: counts_for(:weekly),
+            counts_monthly: counts_for(:monthly)
+          }
+        end
+
+        def known_event?(event_name)
+          event_for(event_name).present?
+        end
+
+        private
+
+        # type is one of weekly, monthly
+        def counts_for(type)
+          return unless type.in?(COUNTS_TYPES)
+
+          period = if type == :weekly
+                     { start_date: 7.days.ago.to_date, end_date: Date.current }
+                   else
+                     { start_date: 4.weeks.ago.to_date, end_date: Date.current }
+                   end
+
           categories.each_with_object({}) do |category, category_results|
             events_names = events_for_category(category)
 
             event_results = events_names.each_with_object({}) do |event, hash|
-              hash["#{event}_weekly"] = unique_events(event_names: event, start_date: 7.days.ago.to_date, end_date: Date.current)
-              hash["#{event}_monthly"] = unique_events(event_names: event, start_date: 28.days.ago.to_date, end_date: Date.current)
+              hash["#{event}"] = unique_events(period.merge(event_names: event))
             end
 
             if eligible_for_totals?(events_names)
-              event_results["#{category}_total_unique_counts_weekly"] = unique_events(event_names: events_names, start_date: 7.days.ago.to_date, end_date: Date.current)
-              event_results["#{category}_total_unique_counts_monthly"] = unique_events(event_names: events_names, start_date: 4.weeks.ago.to_date, end_date: Date.current)
+              event_results["total_unique_counts"] = unique_events(period.merge(event_names: events_names))
             end
 
             category_results["#{category}"] = event_results
           end
         end
 
-        def known_event?(event_name)
-          event_for(event_name).present?
-        end
-
-        private
-
         # Allow to add totals for events that are in the same redis slot, category and have the same aggregation level
         # and if there are more than 1 event
         def eligible_for_totals?(events_names)

spec/lib/gitlab/usage_data_counters/hll_redis_counter_spec.rb

@@ -238,20 +238,28 @@ RSpec.describe Gitlab::UsageDataCounters::HLLRedisCounter, :clean_gitlab_redis_s
 
     it 'returns the number of unique events for all known events' do
       results = {
-       'category1' => {
-          'event1_slot_weekly' => 1,
-          'event1_slot_monthly' => 1,
-          'event2_slot_weekly' => 1,
-          'event2_slot_monthly' => 2,
-          'category1_total_unique_counts_weekly' => 2,
-          'category1_total_unique_counts_monthly' => 3
-       },
-       'category2' => {
-          'event3_weekly' => 1,
-          'event3_monthly' => 1,
-          'event4_weekly' => 1,
-          'event4_monthly' => 1
-       }
+        counts_weekly: {
+          "category1" => {
+            "event1_slot" => 1,
+            "event2_slot" => 1,
+            "total_unique_counts" => 2
+          },
+          "category2" => {
+            "event3" => 1,
+            "event4" => 1
+          }
+        },
+        counts_monthly: {
+          "category1" => {
+            "event1_slot" => 1,
+            "event2_slot" => 2,
+            "total_unique_counts" => 3
+          },
+          "category2" => {
+            "event3" => 1,
+            "event4" => 1
+          }
+        }
       }
 
       expect(subject.unique_events_data).to eq(results)

spec/lib/gitlab/usage_data_spec.rb

@@ -1167,18 +1167,18 @@ RSpec.describe Gitlab::UsageData, :aggregate_failures do
     it 'has all know_events' do
       expect(subject).to have_key(:redis_hll_counters)
 
-      expect(subject[:redis_hll_counters].keys).to match_array(categories)
+      expect(subject[:redis_hll_counters].keys).to match_array([:counts_weekly, :counts_monthly])
 
-      categories.each do |category|
-        keys = ::Gitlab::UsageDataCounters::HLLRedisCounter.events_for_category(category)
+      [:counts_weekly, :counts_monthly].each do |type|
+        categories.each do |category|
+          metrics = ::Gitlab::UsageDataCounters::HLLRedisCounter.events_for_category(category)
 
-        metrics = keys.map { |key| "#{key}_weekly" } + keys.map { |key| "#{key}_monthly" }
+          if ineligible_total_categories.exclude?(category)
+            metrics.append("total_unique_counts")
+          end
 
-        if ineligible_total_categories.exclude?(category)
-          metrics.append("#{category}_total_unique_counts_weekly", "#{category}_total_unique_counts_monthly")
+          expect(subject[:redis_hll_counters][type][category].keys).to match_array(metrics)
         end
-
-        expect(subject[:redis_hll_counters][category].keys).to match_array(metrics)
       end
     end
   end