refactored metrics fetcher - merged into stage and events

lib/gitlab/cycle_analytics/base_event_fetcher.rb

 module Gitlab
   module CycleAnalytics
     class BaseEventFetcher
-      include MetricsTables
+      include BaseQuery
 
       attr_reader :projections, :query, :stage, :order
 
-      def initialize(fetcher:, options:, stage:)
-        @fetcher = fetcher
-        @project = fetcher.project
-        @options = options
+      def initialize(project:, stage:, options:)
+        @project = project
         @stage = stage
+        @options = options
       end
 
       def fetch
@@ -20,8 +19,6 @@ module Gitlab
         end.compact
       end
 
-      def custom_query(_base_query); end
-
       private
 
       def update_author!
@@ -31,7 +28,21 @@ module Gitlab
       end
 
       def event_result
-        @event_result ||= @fetcher.events.to_a
+        @event_result ||= ActiveRecord::Base.connection.exec_query(events_query.to_sql).to_a
+      end
+
+      def events_query
+        diff_fn = subtract_datetimes_diff(base_query, @options[:start_time_attrs], @options[:end_time_attrs])
+
+        base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *projections).order(order.desc)
+      end
+
+      def order
+        @order || default_order
+      end
+
+      def default_order
+        @options[:start_time_attrs].is_a?(Array) ? @options[:start_time_attrs].first : @options[:start_time_attrs]
       end
 
       def serialize(_event)

lib/gitlab/cycle_analytics/base_query.rb

+module Gitlab
+  module CycleAnalytics
+    module BaseQuery
+      include MetricsTables
+      include Gitlab::Database::Median
+      include Gitlab::Database::DateTime
+
+      private
+
+      def base_query
+        @base_query ||= stage_query
+      end
+
+      def stage_query
+        query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])).
+          join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])).
+          where(issue_table[:project_id].eq(@project.id)).
+          where(issue_table[:deleted_at].eq(nil)).
+          where(issue_table[:created_at].gteq(@options[:from]))
+
+        # Load merge_requests
+        query = query.join(mr_table, Arel::Nodes::OuterJoin).
+          on(mr_table[:id].eq(mr_closing_issues_table[:merge_request_id])).
+          join(mr_metrics_table).
+          on(mr_table[:id].eq(mr_metrics_table[:merge_request_id]))
+
+        query
+      end
+    end
+  end
+end

lib/gitlab/cycle_analytics/base_stage.rb

 module Gitlab
   module CycleAnalytics
     class BaseStage
-      include MetricsTables
-
-      attr_accessor :start_time_attrs, :end_time_attrs
+      include BaseQuery
 
       def initialize(project:, options:)
         @project = project
         @options = options
-        @fetcher = Gitlab::CycleAnalytics::MetricsFetcher.new(project: project,
-                                                              from: options[:from],
-                                                              branch: options[:branch],
-                                                              stage: self)
       end
 
       def event
-        @event ||= Gitlab::CycleAnalytics::Event[stage].new(fetcher: @fetcher,
-                                                            options: @options,
-                                                            stage: stage)
+        @event ||= Gitlab::CycleAnalytics::Event[name].new(project: @project,
+                                                           stage: name,
+                                                           options: event_options)
       end
 
       def events
@@ -29,17 +23,31 @@ module Gitlab
       end
 
       def title
-        stage.to_s.capitalize
+        name.to_s.capitalize
       end
 
       def median
-        @fetcher.median
+        cte_table = Arel::Table.new("cte_table_for_#{name}")
+
+        # Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time).
+        # Next, we find the first of the start_time_attrs that isn't `NULL` (call this start_time).
+        # We compute the (end_time - start_time) interval, and give it an alias based on the current
+        # cycle analytics stage.
+        interval_query = Arel::Nodes::As.new(
+          cte_table,
+          subtract_datetimes(base_query, @start_time_attrs, @end_time_attrs, name.to_s))
+
+        median_datetime(cte_table, interval_query, name)
+      end
+
+      def name
+        raise NotImplementedError.new("Expected #{self.name} to implement name")
       end
 
       private
 
-      def stage
-        class_name_for('Stage')
+      def event_options
+        @options.merge(start_time_attrs: @start_time_attrs, end_time_attrs: @end_time_attrs)
       end
     end
   end

lib/gitlab/cycle_analytics/code_stage.rb

@@ -8,7 +8,7 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :code
       end
 

lib/gitlab/cycle_analytics/issue_stage.rb

@@ -9,7 +9,7 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :issue
       end
 

lib/gitlab/cycle_analytics/metrics_fetcher.rb

-module Gitlab
-  module CycleAnalytics
-    class MetricsFetcher
-      include Gitlab::Database::Median
-      include Gitlab::Database::DateTime
-      include MetricsTables
-
-      attr_reader :project
-
-      DEPLOYMENT_METRIC_STAGES = %i[production staging]
-
-      def initialize(project:, from:, branch:, stage:)
-        @project = project
-        @from = from
-        @branch = branch
-        @stage = stage
-      end
-
-      def median
-        cte_table = Arel::Table.new("cte_table_for_#{@stage.stage}")
-
-        # Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time).
-        # Next, we find the first of the start_time_attrs that isn't `NULL` (call this start_time).
-        # We compute the (end_time - start_time) interval, and give it an alias based on the current
-        # cycle analytics stage.
-        interval_query = Arel::Nodes::As.new(
-          cte_table,
-          subtract_datetimes(base_query_for(@stage.stage), @stage.start_time_attrs, @stage.end_time_attrs, @stage.stage.to_s))
-
-        median_datetime(cte_table, interval_query, @stage.stage)
-      end
-
-      def events
-        ActiveRecord::Base.connection.exec_query(events_query.to_sql)
-      end
-
-      private
-
-      def events_query
-        base_query = base_query_for(@stage.stage)
-
-        diff_fn = subtract_datetimes_diff(base_query, @stage.start_time_attrs, @stage.end_time_attrs)
-
-        @stage.event.custom_query(base_query)
-
-        base_query.project(extract_diff_epoch(diff_fn).as('total_time'), *@stage.event.projections).order(order.desc)
-      end
-
-      def order
-        @stage.event.order || default_order
-      end
-
-      def default_order
-        @stage.start_time_attrs.is_a?(Array) ? @stage.start_time_attrs.first : @stage.start_time_attrs
-      end
-
-      # Join table with a row for every <issue,merge_request> pair (where the merge request
-      # closes the given issue) with issue and merge request metrics included. The metrics
-      # are loaded with an inner join, so issues / merge requests without metrics are
-      # automatically excluded.
-      def base_query_for(name)
-        # Load issues
-        query = mr_closing_issues_table.join(issue_table).on(issue_table[:id].eq(mr_closing_issues_table[:issue_id])).
-          join(issue_metrics_table).on(issue_table[:id].eq(issue_metrics_table[:issue_id])).
-          where(issue_table[:project_id].eq(@project.id)).
-          where(issue_table[:deleted_at].eq(nil)).
-          where(issue_table[:created_at].gteq(@from))
-
-        query = query.where(build_table[:ref].eq(@branch)) if name == :test && @branch
-
-        # Load merge_requests
-        query = query.join(mr_table, Arel::Nodes::OuterJoin).
-          on(mr_table[:id].eq(mr_closing_issues_table[:merge_request_id])).
-          join(mr_metrics_table).
-          on(mr_table[:id].eq(mr_metrics_table[:merge_request_id]))
-
-        if DEPLOYMENT_METRIC_STAGES.include?(name)
-          # Limit to merge requests that have been deployed to production after `@from`
-          query.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@from))
-        end
-
-        query
-      end
-    end
-  end
-end

lib/gitlab/cycle_analytics/plan_event_fetcher.rb

@@ -8,8 +8,10 @@ module Gitlab
         super(*args)
       end
 
-      def custom_query(base_query)
+      def events_query
         base_query.join(mr_diff_table).on(mr_diff_table[:merge_request_id].eq(mr_table[:id]))
+
+        super
       end
 
       private

lib/gitlab/cycle_analytics/plan_stage.rb

@@ -9,7 +9,7 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :plan
       end
 

lib/gitlab/cycle_analytics/production_helper.rb

+module Gitlab
+  module CycleAnalytics
+    module ProductionHelper
+      def stage_query
+        super.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@options[:from]))
+      end
+    end
+  end
+end

lib/gitlab/cycle_analytics/production_stage.rb

 module Gitlab
   module CycleAnalytics
     class ProductionStage < BaseStage
+      include ProductionHelper
+
       def initialize(*args)
         @start_time_attrs = issue_table[:created_at]
         @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at]
@@ -8,13 +10,18 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :production
       end
 
       def description
         "From issue creation until deploy to production"
       end
+
+      def query
+        # Limit to merge requests that have been deployed to production after `@from`
+        query.where(mr_metrics_table[:first_deployed_to_production_at].gteq(@from))
+      end
     end
   end
 end

lib/gitlab/cycle_analytics/review_stage.rb

@@ -8,7 +8,7 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :review
       end
 

lib/gitlab/cycle_analytics/staging_event_fetcher.rb

@@ -14,8 +14,10 @@ module Gitlab
         super
       end
 
-      def custom_query(base_query)
+      def events_query
         base_query.join(build_table).on(mr_metrics_table[:pipeline_id].eq(build_table[:commit_id]))
+
+        super
       end
 
       private

lib/gitlab/cycle_analytics/staging_stage.rb

 module Gitlab
   module CycleAnalytics
     class StagingStage < BaseStage
+      include ProductionHelper
+
       def initialize(*args)
         @start_time_attrs = mr_metrics_table[:merged_at]
         @end_time_attrs = mr_metrics_table[:first_deployed_to_production_at]
@@ -8,7 +10,7 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :staging
       end
 

lib/gitlab/cycle_analytics/summary/commit.rb

@@ -23,8 +23,11 @@ module Gitlab
           cmd << "--after=#{@from.iso8601}"
           cmd << sha
 
-          raw_output = IO.popen(cmd) { |io| io.read }
-          raw_output.lines.count
+          output, status = Gitlab::Popen.popen(cmd) { |io| io.read }
+
+          raise IOError, output unless status.zero?
+
+          output.lines.count
         end
 
         def ref

lib/gitlab/cycle_analytics/test_stage.rb

@@ -8,13 +8,21 @@ module Gitlab
         super(*args)
       end
 
-      def stage
+      def name
         :test
       end
 
       def description
         "Total test time for all commits/merges"
       end
+
+      def stage_query
+        if @options[:branch]
+          super.where(build_table[:ref].eq(@options[:branch]))
+        else
+          super
+        end
+      end
     end
   end
 end

spec/lib/gitlab/cycle_analytics/shared_event_spec.rb

@@ -8,10 +8,11 @@ shared_examples 'default query config' do
                                                stage: stage_name)
   end
 
-  let(:event) { described_class.new(fetcher: fetcher, options: {}, stage: stage_name) }
+  let(project)
+  let(:event) { described_class.new(project: project, stage: stage_name, options: {}) }
 
   it 'has the stage attribute' do
-    expect(event.stage).not_to be_nil
+    expect(event.name).not_to be_nil
   end
 
   it 'has the projection attributes' do