Merge branch '371885-handle-missing-dora-data' into 'master'

Expose dates where DORA data is missing

See merge request gitlab-org/gitlab!96527

ee/lib/gitlab/insights/executors/dora_executor.rb

@@ -5,6 +5,13 @@ module Insights
     module Executors
       class DoraExecutor
         DoraExecutorError = Class.new(StandardError)
+        FORMATTERS = {
+          'day' => '%d %b %y',
+          'month' => '%B %Y'
+        }.freeze
+        DEFAULT_VALUES = {
+          'deployment_frequency' => 0
+        }.freeze
 
         def initialize(query_params:, current_user:, insights_entity:, projects: {}, chart_type:)
           @query_params = query_params
@@ -23,14 +30,36 @@ def execute
 
           raise(DoraExecutorError, result[:message]) if result[:status] == :error
 
-          reduced_data = Gitlab::Insights::Reducers::DoraReducer.reduce(result[:data], period: group_by, metric: metric)
-          serializer.present(reduced_data)
+          serializer.present(format_data(result[:data]))
         end
 
         private
 
         attr_reader :query_params, :current_user, :insights_entity, :projects, :chart_type
 
+        def format_data(data)
+          input = data.each_with_object({}) { |item, hash| hash[item['date']] = format_value(item['value']) }
+
+          Gitlab::Analytics::DateFiller.new(input,
+                                            from: start_date,
+                                            to: Date.today,
+                                            period: group_by.to_sym,
+                                            default_value: DEFAULT_VALUES[metric],
+                                            date_formatter: -> (date) { date.strftime(FORMATTERS[group_by]) }
+                                           ).fill
+        end
+
+        def format_value(value)
+          case metric
+          when 'lead_time_for_changes', 'time_to_restore_service'
+            value ? value.fdiv(1.day).round(1) : nil
+          when 'change_failure_rate'
+            value ? (value * 100).round(2) : 0
+          else
+            value
+          end
+        end
+
         def dora_api_params
           params = {
             interval: dora_interval,
@@ -62,9 +91,9 @@ def metric
         def start_date
           case group_by
           when 'day'
-            period_limit.days.ago.to_date
+            (period_limit - 1).days.ago.to_date
           when 'month'
-            period_limit.months.ago.to_date
+            (period_limit - 1).months.ago.to_date
           end
         end
 
@@ -88,12 +117,11 @@ def group_by
         end
 
         def serializer
-          case chart_type
-          when 'bar', 'line'
-            Gitlab::Insights::Serializers::Chartjs::BarSerializer
-          else
+          unless %w[bar line].include?(chart_type)
             raise DoraExecutor::DoraExecutorError, "Unsupported chart type is given: #{chart_type}"
           end
+
+          Gitlab::Insights::Serializers::Chartjs::BarSerializer
         end
       end
     end

ee/lib/gitlab/insights/reducers/dora_reducer.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Insights
-    module Reducers
-      class DoraReducer < BaseReducer
-        def initialize(data, period:, metric:)
-          @data = data
-          @period = period
-          @metric = metric
-        end
-
-        def reduce
-          data.each_with_object({}) do |item, hash|
-            hash[format_date(item['date'])] = format_value(item['value'])
-          end
-        end
-
-        private
-
-        attr_reader :data, :period, :metric
-
-        def format_date(date)
-          Date.parse(date).strftime(period_format)
-        end
-
-        def period_format
-          case period
-          when 'day'
-            '%d %b %y'
-          when 'month'
-            '%B %Y'
-          else
-            raise Gitlab::Insights::Executors::DoraExecutor::DoraExecutorError, "Unknown period is given: #{period}"
-          end
-        end
-
-        def format_value(value)
-          case metric
-          when 'lead_time_for_changes', 'time_to_restore_service'
-            value ? value.fdiv(1.day).round(1) : nil
-          when 'deployment_frequency'
-            value
-          when 'change_failure_rate'
-            value ? (value * 100).round(2) : 0
-          else
-            raise Gitlab::Insights::Executors::DoraExecutor::DoraExecutorError, "Unknown metric is given: #{period}"
-          end
-        end
-      end
-    end
-  end
-end

ee/spec/lib/gitlab/insights/executors/dora_executor_spec.rb

@@ -43,21 +43,27 @@
 
     create(:dora_daily_metrics,
            deployment_frequency: 5,
+           lead_time_for_changes_in_seconds: 100,
            environment: environment1,
            date: date1)
 
     create(:dora_daily_metrics,
            deployment_frequency: 20,
+           lead_time_for_changes_in_seconds: 10000,
+           incidents_count: 5,
            environment: environment2,
            date: date1)
 
     create(:dora_daily_metrics,
            deployment_frequency: 50,
+           lead_time_for_changes_in_seconds: 20000,
+           incidents_count: 15,
            environment: environment1,
            date: date2)
 
     create(:dora_daily_metrics,
            deployment_frequency: 100,
+           lead_time_for_changes_in_seconds: 40000,
            environment: environment3,
            date: date2)
   end
@@ -84,7 +90,27 @@
     let(:insights_entity) { group }
 
     it_behaves_like 'serialized_data examples' do
-      let(:expected_result) { [50, 25] }
+      let(:expected_result) { [0, 50, 0, 0, 25] }
+    end
+
+    context 'when requesting the lead_time_for_changes metric' do
+      before do
+        query_params[:metric] = 'lead_time_for_changes'
+      end
+
+      it_behaves_like 'serialized_data examples' do
+        let(:expected_result) { [nil, 0.2, nil, nil, 0.1] }
+      end
+    end
+
+    context 'when requesting the change_failure_rate metric' do
+      before do
+        query_params[:metric] = 'change_failure_rate'
+      end
+
+      it_behaves_like 'serialized_data examples' do
+        let(:expected_result) { [nil, 30, nil, nil, 20] }
+      end
     end
 
     context 'when filtering environment tiers' do
@@ -93,7 +119,7 @@
       end
 
       it_behaves_like 'serialized_data examples' do
-        let(:expected_result) { [150, 25] }
+        let(:expected_result) { [0, 150, 0, 0, 25] }
       end
     end
 
@@ -102,7 +128,7 @@
         let(:projects) { { only: [project2.id] } }
 
         it_behaves_like 'serialized_data examples' do
-          let(:expected_result) { [20] }
+          let(:expected_result) { [0, 0, 0, 0, 20] }
         end
       end
 
@@ -110,7 +136,7 @@
         let(:projects) { { only: [project2.full_path] } }
 
         it_behaves_like 'serialized_data examples' do
-          let(:expected_result) { [20] }
+          let(:expected_result) { [0, 0, 0, 0, 20] }
         end
       end
     end
@@ -142,7 +168,7 @@
     let(:insights_entity) { project1 }
 
     it_behaves_like 'serialized_data examples' do
-      let(:expected_result) { [50, 5] }
+      let(:expected_result) { [0, 50, 0, 0, 5] }
     end
 
     context 'when filtering projects' do
@@ -150,7 +176,7 @@
         let(:projects) { { only: [project1.id] } }
 
         it_behaves_like 'serialized_data examples' do
-          let(:expected_result) { [50, 5] }
+          let(:expected_result) { [0, 50, 0, 0, 5] }
         end
       end
 
@@ -159,7 +185,7 @@
 
         # ignores the filter
         it_behaves_like 'serialized_data examples' do
-          let(:expected_result) { [50, 5] }
+          let(:expected_result) { [0, 50, 0, 0, 5] }
         end
       end
     end

ee/spec/lib/gitlab/insights/loader_spec.rb

@@ -73,7 +73,8 @@
     let(:data_source_params) do
       {
         metric: 'time_to_restore_service',
-        group_by: 'day'
+        group_by: 'day',
+        period_limit: 3
       }
     end
 
@@ -97,7 +98,7 @@
     end
 
     it 'returns the serialized data' do
-      expect(serialized_data['datasets'].first['data']).to eq([2])
+      expect(serialized_data['datasets'].first['data']).to eq([nil, nil, 2])
     end
   end
 end

ee/spec/lib/gitlab/insights/reducers/dora_reducer_spec.rb

-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-
-RSpec.describe Gitlab::Insights::Reducers::DoraReducer do
-  context 'when metric=change_failure_rate' do
-    it 'converts to percentage' do
-      data = [
-        { 'value' => 0.5, 'date' => '2020-01-01' },
-        { 'value' => 0.1, 'date' => '2020-01-02' }
-      ]
-
-      result = described_class
-        .reduce(data, period: 'day', metric: 'change_failure_rate')
-        .to_a
-
-      expect(result).to eq({ '01 Jan 20' => 50, '02 Jan 20' => 10 }.to_a)
-    end
-  end
-
-  context 'when metric=deployment_frequency' do
-    it 'uses the value as is' do
-      data = [
-        { 'value' => 100, 'date' => '2020-01-01' },
-        { 'value' => 20, 'date' => '2020-02-01' }
-      ]
-
-      result = described_class
-        .reduce(data, period: 'month', metric: 'deployment_frequency')
-        .to_a
-
-      expect(result).to eq({ 'January 2020' => 100, 'February 2020' => 20 }.to_a)
-    end
-  end
-
-  context 'when metric=lead_time_for_changes' do
-    it 'converts from seconds to days' do
-      data = [
-        { 'value' => 86400, 'date' => '2020-01-01' },
-        { 'value' => 43200, 'date' => '2020-01-02' },
-        { 'value' => nil, 'date' => '2020-01-03' }
-      ]
-
-      result = described_class
-        .reduce(data, period: 'day', metric: 'lead_time_for_changes')
-        .to_a
-
-      expect(result).to match({ '01 Jan 20' => 1, '02 Jan 20' => 0.5, '03 Jan 20' => nil }.to_a)
-    end
-  end
-
-  context 'when metric=time_to_restore_service' do
-    it 'converts from seconds to days' do
-      data = [
-        { 'value' => 86400, 'date' => '2020-01-01' },
-        { 'value' => 43200, 'date' => '2020-01-02' },
-        { 'value' => nil, 'date' => '2020-01-03' }
-      ]
-
-      result = described_class
-        .reduce(data, period: 'day', metric: 'time_to_restore_service')
-        .to_a
-
-      expect(result).to eq({ '01 Jan 20' => 1, '02 Jan 20' => 0.5, '03 Jan 20' => nil }.to_a)
-    end
-  end
-
-  context 'when unknown metric is given' do
-    it 'raises error' do
-      data = [
-        { 'value' => 86400, 'date' => '2020-01-01' },
-        { 'value' => 43200, 'date' => '2020-01-02' }
-      ]
-
-      expect do
-        described_class.reduce(data, period: 'day', metric: 'unknown')
-      end.to raise_error /Unknown metric is given/
-    end
-  end
-
-  context 'when unknown period is given' do
-    it 'raises error' do
-      data = [
-        { 'value' => 86400, 'date' => '2020-01-01' },
-        { 'value' => 43200, 'date' => '2020-01-02' }
-      ]
-
-      expect do
-        described_class.reduce(data, period: 'unknown', metric: 'time_to_restore_service')
-      end.to raise_error /Unknown period is given/
-    end
-  end
-end

lib/gitlab/analytics/date_filler.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Analytics
+    # This class generates a date => value hash without gaps in the data points.
+    #
+    # Simple usage:
+    #
+    # > # We have the following data for the last 5 day:
+    # > input = { 3.days.ago.to_date => 10, Date.today => 5 }
+    #
+    # > # Format this data, so we can chart the complete date range:
+    # > Gitlab::Analytics::DateFiller.new(input, from: 4.days.ago, to: Date.today, default_value: 0).fill
+    # > {
+    # >  Sun, 28 Aug 2022=>0,
+    # >  Mon, 29 Aug 2022=>10,
+    # >  Tue, 30 Aug 2022=>0,
+    # >  Wed, 31 Aug 2022=>0,
+    # >  Thu, 01 Sep 2022=>5
+    # > }
+    #
+    # Parameters:
+    #
+    # **input**
+    # A Hash containing data for the series or the chart. The key is a Date object
+    # or an object which can be converted to Date.
+    #
+    # **from**
+    # Start date of the range
+    #
+    # **to**
+    # End date of the range
+    #
+    # **period**
+    # Specifies the period in wich the dates should be generated. Options:
+    #
+    # - :day, generate date-value pair for each day in the given period
+    # - :week, generate date-value pair for each week (beginning of the week date)
+    # - :month, generate date-value pair for each week (beginning of the month date)
+    #
+    # Note: the Date objects in the `input` should follow the same pattern (beginning of ...)
+    #
+    # **default_value**
+    #
+    # Which value use when the `input` Hash does not contain data for the given day.
+    #
+    # **date_formatter**
+    #
+    # How to format the dates in the resulting hash.
+    class DateFiller
+      DEFAULT_DATE_FORMATTER = -> (date) { date }
+      PERIOD_STEPS = {
+        day: 1.day,
+        week: 1.week,
+        month: 1.month
+      }.freeze
+
+      def initialize(
+        input,
+        from:,
+        to:,
+        period: :day,
+        default_value: nil,
+        date_formatter: DEFAULT_DATE_FORMATTER)
+        @input = input.transform_keys(&:to_date)
+        @from = from.to_date
+        @to = to.to_date
+        @period = period
+        @default_value = default_value
+        @date_formatter = date_formatter
+      end
+
+      def fill
+        data = {}
+
+        current_date = from
+        loop do
+          transformed_date = transform_date(current_date)
+          break if transformed_date > to
+
+          formatted_date = date_formatter.call(transformed_date)
+
+          value = input.delete(transformed_date)
+          data[formatted_date] = value.nil? ? default_value : value
+
+          current_date = (current_date + PERIOD_STEPS.fetch(period)).to_date
+        end
+
+        raise "Input contains values which doesn't fall under the given period!" if input.any?
+
+        data
+      end
+
+      private
+
+      attr_reader :input, :from, :to, :period, :default_value, :date_formatter
+
+      def transform_date(date)
+        case period
+        when :day
+          date.beginning_of_day.to_date
+        when :week
+          date.beginning_of_week.to_date
+        when :month
+          date.beginning_of_month.to_date
+        else
+          raise "Unknown period given: #{period}"
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/analytics/date_filler_spec.rb

+# frozen_string_literal: true
+require 'fast_spec_helper'
+
+RSpec.describe Gitlab::Analytics::DateFiller do
+  let(:default_value) { 0 }
+  let(:formatter) { Gitlab::Analytics::DateFiller::DEFAULT_DATE_FORMATTER }
+
+  subject(:filler_result) do
+    described_class.new(data,
+                        from: from,
+                        to: to,
+                        period: period,
+                        default_value: default_value,
+                        date_formatter: formatter).fill.to_a
+  end
+
+  context 'when unknown period is given' do
+    it 'raises error' do
+      input = { 3.days.ago.to_date => 10, Date.today => 5 }
+
+      expect do
+        described_class.new(input, from: 4.days.ago, to: Date.today, period: :unknown).fill
+      end.to raise_error(/Unknown period given/)
+    end
+  end
+
+  context 'when period=:day' do
+    let(:from) { Date.new(2021, 5, 25) }
+    let(:to) { Date.new(2021, 6, 5) }
+    let(:period) { :day }
+
+    let(:expected_result) do
+      {
+        Date.new(2021, 5, 25) => 1,
+        Date.new(2021, 5, 26) => default_value,
+        Date.new(2021, 5, 27) => default_value,
+        Date.new(2021, 5, 28) => default_value,
+        Date.new(2021, 5, 29) => default_value,
+        Date.new(2021, 5, 30) => default_value,
+        Date.new(2021, 5, 31) => default_value,
+        Date.new(2021, 6, 1) => default_value,
+        Date.new(2021, 6, 2) => default_value,
+        Date.new(2021, 6, 3) => 10,
+        Date.new(2021, 6, 4) => default_value,
+        Date.new(2021, 6, 5) => default_value
+      }
+    end
+
+    let(:data) do
+      {
+        Date.new(2021, 6, 3) => 10, # deliberatly not sorted
+        Date.new(2021, 5, 27) => nil,
+        Date.new(2021, 5, 25) => 1
+      }
+    end
+
+    it { is_expected.to eq(expected_result.to_a) }
+
+    context 'when a custom default value is given' do
+      let(:default_value) { 'MISSING' }
+
+      it do
+        is_expected.to eq(expected_result.to_a)
+      end
+    end
+
+    context 'when a custom date formatter is given' do
+      let(:formatter) { -> (date) { date.to_s } }
+
+      it do
+        expected_result.transform_keys!(&:to_s)
+
+        is_expected.to eq(expected_result.to_a)
+      end
+    end
+
+    context 'when the data contains dates outside of the requested period' do
+      before do
+        data[Date.new(2022, 6, 1)] = 5
+      end
+
+      it 'raises error' do
+        expect { filler_result }.to raise_error(/Input contains values which doesn't/)
+      end
+    end
+  end
+
+  context 'when period=:week' do
+    let(:from) { Date.new(2021, 5, 16) }
+    let(:to) { Date.new(2021, 6, 7) }
+    let(:period) { :week }
+    let(:data) do
+      {
+        Date.new(2021, 5, 24) => nil,
+        Date.new(2021, 6, 7) => 10
+      }
+    end
+
+    let(:expected_result) do
+      {
+        Date.new(2021, 5, 10) => 0,
+        Date.new(2021, 5, 17) => 0,
+        Date.new(2021, 5, 24) => 0,
+        Date.new(2021, 5, 31) => 0,
+        Date.new(2021, 6, 7) => 10
+      }
+    end
+
+    it do
+      is_expected.to eq(expected_result.to_a)
+    end
+  end
+
+  context 'when period=:month' do
+    let(:from) { Date.new(2021, 5, 1) }
+    let(:to) { Date.new(2021, 7, 1) }
+    let(:period) { :month }
+    let(:data) do
+      {
+        Date.new(2021, 5, 1) => 100
+      }
+    end
+
+    let(:expected_result) do
+      {
+        Date.new(2021, 5, 1) => 100,
+        Date.new(2021, 6, 1) => 0,
+        Date.new(2021, 7, 1) => 0
+      }
+    end
+
+    it do
+      is_expected.to eq(expected_result.to_a)
+    end
+  end
+end