Add DevOps adoption end_time column

Used to specify end of datetime range for
specific devops adoption snapshot

changelogs/unreleased/292516-daily-updates-for-devops-adoption.yml

+---
+title: Add DevOps adoption end_time column
+merge_request: 50257
+author:
+type: added

db/migrate/20201216154457_add_devops_adoption_snapshot_range_end.rb

+# frozen_string_literal: true
+
+class AddDevopsAdoptionSnapshotRangeEnd < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :analytics_devops_adoption_snapshots, :end_time, :datetime_with_timezone
+  end
+end

db/migrate/20201221124036_add_devops_snapshot_index.rb

+# frozen_string_literal: true
+
+class AddDevopsSnapshotIndex < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_on_snapshots_segment_id_end_time'
+
+  def up
+    add_concurrent_index :analytics_devops_adoption_snapshots, [:segment_id, :end_time], name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :analytics_devops_adoption_snapshots, INDEX_NAME
+  end
+end

db/post_migrate/20201216185336_add_devops_adoption_snapshot_not_null.rb

+# frozen_string_literal: true
+
+class AddDevopsAdoptionSnapshotNotNull < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    with_lock_retries do
+      execute(
+        <<~SQL
+          LOCK TABLE analytics_devops_adoption_snapshots IN ACCESS EXCLUSIVE MODE;
+
+          UPDATE analytics_devops_adoption_snapshots SET end_time = date_trunc('month', recorded_at) - interval '1 millisecond';
+
+          ALTER TABLE analytics_devops_adoption_snapshots ALTER COLUMN end_time SET NOT NULL;
+      SQL
+      )
+    end
+  end
+
+  def down
+    with_lock_retries do
+      execute(<<~SQL)
+        ALTER TABLE analytics_devops_adoption_snapshots ALTER COLUMN end_time DROP NOT NULL;
+      SQL
+    end
+  end
+end

db/schema_migrations/20201216154457

+c878874bbb9bf2314b90c1328d6e27846fe71513ba1ce688a262d36761b66665
\ No newline at end of file

db/schema_migrations/20201216185336

+3647e8b944aedeb58c41d9b3f08c8fb657fab231b0ff25c276f6d2aaa2ea93ae
\ No newline at end of file

db/schema_migrations/20201221124036

+15517956f3b5d7ce2c05d196a34881fa169df7b1bf5ccf0dbfbee74fb3143ba7
\ No newline at end of file

db/structure.sql

@@ -9003,7 +9003,8 @@ CREATE TABLE analytics_devops_adoption_snapshots (
     runner_configured boolean NOT NULL,
     pipeline_succeeded boolean NOT NULL,
     deploy_succeeded boolean NOT NULL,
-    security_scan_succeeded boolean NOT NULL
+    security_scan_succeeded boolean NOT NULL,
+    end_time timestamp with time zone NOT NULL
 );
 
 CREATE SEQUENCE analytics_devops_adoption_snapshots_id_seq
@@ -22102,6 +22103,8 @@ CREATE UNIQUE INDEX index_on_segment_selections_project_id_segment_id ON analyti
 
 CREATE INDEX index_on_segment_selections_segment_id ON analytics_devops_adoption_segment_selections USING btree (segment_id);
 
+CREATE INDEX index_on_snapshots_segment_id_end_time ON analytics_devops_adoption_snapshots USING btree (segment_id, end_time);
+
 CREATE INDEX index_on_snapshots_segment_id_recorded_at ON analytics_devops_adoption_snapshots USING btree (segment_id, recorded_at);
 
 CREATE INDEX index_on_users_lower_email ON users USING btree (lower((email)::text));

ee/app/models/analytics/devops_adoption/snapshot.rb

 # frozen_string_literal: true
 
 class Analytics::DevopsAdoption::Snapshot < ApplicationRecord
-  SNAPSHOT_TIME_PERIOD = 1.month
-
   belongs_to :segment, inverse_of: :snapshots
 
   validates :segment, presence: true
   validates :recorded_at, presence: true
+  validates :end_time, presence: true
   validates :issue_opened, inclusion: { in: [true, false] }
   validates :merge_request_opened, inclusion: { in: [true, false] }
   validates :merge_request_approved, inclusion: { in: [true, false] }
@@ -19,17 +18,13 @@ class Analytics::DevopsAdoption::Snapshot < ApplicationRecord
     inner_select = model
       .default_scoped
       .distinct
-      .select("FIRST_VALUE(id) OVER (PARTITION BY segment_id ORDER BY recorded_at DESC) as id")
+      .select("FIRST_VALUE(id) OVER (PARTITION BY segment_id ORDER BY end_time DESC) as id")
       .where(segment_id: ids)
 
     joins("INNER JOIN (#{inner_select.to_sql}) latest_snapshots ON latest_snapshots.id = analytics_devops_adoption_snapshots.id")
   end
 
   def start_time
-    (recorded_at - SNAPSHOT_TIME_PERIOD).at_beginning_of_day
-  end
-
-  def end_time
-    recorded_at
+    end_time.beginning_of_month
   end
 end

ee/app/services/analytics/devops_adoption/snapshots/calculate_and_save_service.rb

@@ -6,7 +6,7 @@ module Analytics
       class CalculateAndSaveService
         attr_reader :segment, :range_end
 
-        def initialize(segment:, range_end: Time.zone.now)
+        def initialize(segment:, range_end:)
           @segment = segment
           @range_end = range_end
         end

ee/app/services/analytics/devops_adoption/snapshots/create_service.rb

@@ -7,6 +7,7 @@ module Analytics
         ALLOWED_ATTRIBUTES = [
           :segment,
           :segment_id,
+          :end_time,
           :recorded_at,
           :issue_opened,
           :merge_request_opened,

ee/app/workers/analytics/devops_adoption/create_all_snapshots_worker.rb

@@ -14,7 +14,8 @@ module Analytics
 
       # rubocop: disable CodeReuse/ActiveRecord
       def perform
-        range_end = Time.zone.now
+        range_end = 1.month.ago.end_of_month
+
         ::Analytics::DevopsAdoption::Segment.all.pluck(:id).each.with_index do |segment_id, i|
           CreateSnapshotWorker.perform_in(i * WORKERS_GAP, segment_id, range_end)
         end

ee/db/fixtures/development/31_devops_adoption.rb

@@ -31,7 +31,8 @@ Gitlab::Seeder.quiet do
 
     # create snapshots for the last 5 months
     5.downto(1).each do |index|
-      recorded_at = index.months.ago.at_end_of_month
+      end_time = index.months.ago.at_end_of_month
+      recorded_at = end_time + 1.day
 
       segments.each do |segment|
         Analytics::DevopsAdoption::Snapshot.create!(
@@ -43,7 +44,8 @@ Gitlab::Seeder.quiet do
           pipeline_succeeded: booleans.sample,
           deploy_succeeded: booleans.sample,
           security_scan_succeeded: booleans.sample,
-          recorded_at: recorded_at
+          recorded_at: recorded_at,
+          end_time: end_time
         )
 
         segment.update!(last_recorded_at: recorded_at)

ee/lib/analytics/devops_adoption/snapshot_calculator.rb

@@ -7,14 +7,14 @@ module Analytics
 
       ADOPTION_FLAGS = %i[issue_opened merge_request_opened merge_request_approved runner_configured pipeline_succeeded deploy_succeeded security_scan_succeeded].freeze
 
-      def initialize(segment:, range_end: Time.zone.now)
+      def initialize(segment:, range_end:)
         @segment = segment
         @range_end = range_end
-        @range_start = Analytics::DevopsAdoption::Snapshot.new(recorded_at: range_end).start_time
+        @range_start = Snapshot.new(end_time: range_end).start_time
       end
 
       def calculate
-        params = { recorded_at: range_end, segment: segment }
+        params = { recorded_at: Time.zone.now, end_time: range_end, segment: segment }
 
         ADOPTION_FLAGS.each do |flag|
           params[flag] = send(flag) # rubocop:disable GitlabSecurity/PublicSend

ee/spec/factories/analytics/devops_adoption/snapshots.rb

@@ -5,6 +5,7 @@ FactoryBot.define do
     association :segment, factory: :devops_adoption_segment
 
     recorded_at { Time.zone.now }
+    end_time { 1.month.ago.end_of_month }
     issue_opened { true }
     merge_request_opened { false }
     merge_request_approved { false }

ee/spec/lib/analytics/devops_adoption/snapshot_calculator_spec.rb

@@ -3,22 +3,29 @@
 require 'spec_helper'
 
 RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
-  subject(:data) { described_class.new(segment: segment).calculate }
-
   let_it_be(:group1) { create(:group) }
   let_it_be(:project2) { create(:project) }
   let_it_be(:segment) { create(:devops_adoption_segment, groups: [group1], projects: [project2]) }
   let_it_be(:subgroup) { create(:group, parent: group1) }
   let_it_be(:project) { create(:project, group: group1) }
   let_it_be(:subproject) { create(:project, group: subgroup) }
+  let_it_be(:range_end) { Time.zone.parse('2020-12-01').end_of_month }
+
+  subject(:data) { described_class.new(segment: segment, range_end: range_end).calculate }
+
+  describe 'end_time' do
+    it 'equals to range_end' do
+      expect(data[:end_time]).to be_like_time range_end
+    end
+  end
 
   describe 'issue_opened' do
     subject { data[:issue_opened] }
 
-    let_it_be(:old_issue) { create(:issue, project: subproject, created_at: 1.year.ago) }
+    let_it_be(:old_issue) { create(:issue, project: subproject, created_at: 1.year.ago(range_end)) }
 
-    context 'with an issue opened within 30 days' do
-      let_it_be(:fresh_issue) { create(:issue, project: project2, created_at: 3.weeks.ago) }
+    context 'with an issue opened within month' do
+      let_it_be(:fresh_issue) { create(:issue, project: project2, created_at: 3.weeks.ago(range_end)) }
 
       it { is_expected.to eq true }
     end
@@ -29,10 +36,10 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
   describe 'merge_request_opened' do
     subject { data[:merge_request_opened] }
 
-    let!(:old_merge_request) { create(:merge_request, source_project: subproject, created_at: 1.year.ago) }
+    let!(:old_merge_request) { create(:merge_request, source_project: subproject, created_at: 1.year.ago(range_end)) }
 
-    context 'with a merge request opened within 30 days' do
-      let!(:fresh_merge_request) { create(:merge_request, source_project: project2, created_at: 3.weeks.ago) }
+    context 'with a merge request opened within month' do
+      let!(:fresh_merge_request) { create(:merge_request, source_project: project2, created_at: 3.weeks.ago(range_end)) }
 
       it { is_expected.to eq true }
     end
@@ -43,11 +50,11 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
   describe 'merge_request_approved' do
     subject { data[:merge_request_approved] }
 
-    let!(:old_merge_request) { create(:merge_request, source_project: subproject, created_at: 1.year.ago) }
-    let!(:old_approval) { create(:approval, merge_request: old_merge_request, created_at: 6.months.ago) }
+    let!(:old_merge_request) { create(:merge_request, source_project: subproject, created_at: 1.year.ago(range_end)) }
+    let!(:old_approval) { create(:approval, merge_request: old_merge_request, created_at: 6.months.ago(range_end)) }
 
-    context 'with a merge request approved within 30 days' do
-      let!(:fresh_approval) { create(:approval, merge_request: old_merge_request, created_at: 3.weeks.ago) }
+    context 'with a merge request approved within month' do
+      let!(:fresh_approval) { create(:approval, merge_request: old_merge_request, created_at: 3.weeks.ago(range_end)) }
 
       it { is_expected.to eq true }
     end
@@ -59,11 +66,11 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
     subject { data[:runner_configured] }
 
     let!(:inactive_runner) { create(:ci_runner, :project, active: false) }
-    let!(:ci_runner_project) { create(:ci_runner_project, project: project, runner: inactive_runner )}
+    let!(:ci_runner_project) { create(:ci_runner_project, project: project, runner: inactive_runner) }
 
     context 'with active runner present' do
       let!(:active_runner) { create(:ci_runner, :project, active: true) }
-      let!(:ci_runner_project) { create(:ci_runner_project, project: subproject, runner: active_runner )}
+      let!(:ci_runner_project) { create(:ci_runner_project, project: subproject, runner: active_runner) }
 
       it { is_expected.to eq true }
     end
@@ -74,11 +81,11 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
   describe 'pipeline_succeeded' do
     subject { data[:pipeline_succeeded] }
 
-    let!(:failed_pipeline) { create(:ci_pipeline, :failed, project: project2, updated_at: 1.day.ago) }
-    let!(:old_pipeline) { create(:ci_pipeline, :success, project: project2, updated_at: 100.days.ago) }
+    let!(:failed_pipeline) { create(:ci_pipeline, :failed, project: project2, updated_at: 1.day.ago(range_end)) }
+    let!(:old_pipeline) { create(:ci_pipeline, :success, project: project2, updated_at: 100.days.ago(range_end)) }
 
-    context 'with successful pipeline in last 30 days' do
-      let!(:fresh_pipeline) { create(:ci_pipeline, :success, project: project2, updated_at: 1.week.ago) }
+    context 'with successful pipeline within month' do
+      let!(:fresh_pipeline) { create(:ci_pipeline, :success, project: project2, updated_at: 1.week.ago(range_end)) }
 
       it { is_expected.to eq true }
     end
@@ -89,7 +96,7 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
   describe 'deploy_succeeded' do
     subject { data[:deploy_succeeded] }
 
-    let!(:old_deployment) { create(:deployment, :success, updated_at: 100.days.ago) }
+    let!(:old_deployment) { create(:deployment, :success, updated_at: 100.days.ago(range_end)) }
     let!(:old_group) do
       create(:group).tap do |g|
         g.projects << old_deployment.project
@@ -98,8 +105,8 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
 
     let(:segment) { create(:devops_adoption_segment, groups: [old_group]) }
 
-    context 'with any deployment in last 30 days' do
-      let!(:fresh_deployment) { create(:deployment, :success, updated_at: 1.day.ago) }
+    context 'with successful deployment within month' do
+      let!(:fresh_deployment) { create(:deployment, :success, updated_at: 1.day.ago(range_end)) }
       let!(:fresh_group) do
         create(:group).tap do |g|
           g.projects << fresh_deployment.project
@@ -117,10 +124,10 @@ RSpec.describe Analytics::DevopsAdoption::SnapshotCalculator do
   describe 'security_scan_succeeded' do
     subject { data[:security_scan_succeeded] }
 
-    let!(:old_security_scan) { create :security_scan, build: create(:ci_build, project: project2), created_at: 100.days.ago }
+    let!(:old_security_scan) { create :security_scan, build: create(:ci_build, project: project2), created_at: 100.days.ago(range_end) }
 
-    context 'with successful security scan in last 30 days' do
-      let!(:fresh_security_scan) { create :security_scan, build: create(:ci_build, project: project2), created_at: 10.days.ago }
+    context 'with successful security scan within month' do
+      let!(:fresh_security_scan) { create :security_scan, build: create(:ci_build, project: project2), created_at: 10.days.ago(range_end) }
 
       it { is_expected.to eq true }
     end

ee/spec/models/analytics/devops_adoption/snapshot_spec.rb

@@ -7,16 +7,17 @@ RSpec.describe Analytics::DevopsAdoption::Snapshot, type: :model do
 
   it { is_expected.to validate_presence_of(:segment) }
   it { is_expected.to validate_presence_of(:recorded_at) }
+  it { is_expected.to validate_presence_of(:end_time) }
 
   describe '.latest_snapshot_for_segment_ids' do
-    it 'returns the latest snapshot for the given segment ids' do
+    it 'returns the latest snapshot for the given segment ids based on snapshot end_time' do
       segment_1 = create(:devops_adoption_segment)
-      segment_1_latest_snapshot = create(:devops_adoption_snapshot, segment: segment_1, recorded_at: 1.week.ago)
-      create(:devops_adoption_snapshot, segment: segment_1, recorded_at: 2.weeks.ago)
+      segment_1_latest_snapshot = create(:devops_adoption_snapshot, segment: segment_1, end_time: 1.week.ago)
+      create(:devops_adoption_snapshot, segment: segment_1, end_time: 2.weeks.ago)
 
       segment_2 = create(:devops_adoption_segment)
-      segment_2_latest_snapshot = create(:devops_adoption_snapshot, segment: segment_2, recorded_at: 1.year.ago)
-      create(:devops_adoption_snapshot, segment: segment_2, recorded_at: 2.years.ago)
+      segment_2_latest_snapshot = create(:devops_adoption_snapshot, segment: segment_2, end_time: 1.year.ago)
+      create(:devops_adoption_snapshot, segment: segment_2, end_time: 2.years.ago)
 
       latest_snapshot_for_segments = described_class.latest_snapshot_for_segment_ids([segment_1.id, segment_2.id])
 
@@ -24,21 +25,14 @@ RSpec.describe Analytics::DevopsAdoption::Snapshot, type: :model do
     end
   end
 
-  describe '#end_time' do
-    subject(:segment) { described_class.new(recorded_at: 5.days.ago) }
-
-    it 'equals to recorded_at' do
-      expect(segment.end_time).to eq(segment.recorded_at)
-    end
-  end
-
   describe '#start_time' do
-    subject(:segment) { described_class.new(recorded_at: 3.days.ago) }
+    subject(:segment) { described_class.new(end_time: end_time) }
 
-    it 'calcualtes a one-month period from end_time' do
-      expected_end_time = (segment.end_time - 1.month).at_beginning_of_day
+    let(:end_time) { DateTime.parse('2020-12-17') }
+    let(:expected_start_time) { DateTime.parse('2020-12-01') }
 
-      expect(segment.start_time).to eq(expected_end_time)
+    it 'is start of the month of end_time' do
+      expect(segment.start_time).to eq(expected_start_time)
     end
   end
 end

ee/spec/services/analytics/devops_adoption/snapshots/calculate_and_save_service_spec.rb

@@ -5,7 +5,7 @@ require 'spec_helper'
 RSpec.describe Analytics::DevopsAdoption::Snapshots::CalculateAndSaveService do
   let(:segment_mock) { instance_double('Analytics::DevopsAdoption::Segment') }
 
-  subject { described_class.new(segment: segment_mock) }
+  subject { described_class.new(segment: segment_mock, range_end: Time.zone.now.end_of_month) }
 
   it 'creates a snapshot with whatever snapshot calculator returns' do
     allow_next_instance_of(Analytics::DevopsAdoption::SnapshotCalculator) do |calc|

ee/spec/services/analytics/devops_adoption/snapshots/create_service_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe Analytics::DevopsAdoption::Snapshots::CreateService do
       result[attribute] = rand(2).odd?
     end
     params[:recorded_at] = Time.zone.now
+    params[:end_time] = 1.month.ago.end_of_month
     params[:segment] = segment
     params
   end

ee/spec/workers/analytics/devops_adoption/create_all_snapshots_worker_spec.rb

@@ -11,8 +11,9 @@ RSpec.describe Analytics::DevopsAdoption::CreateAllSnapshotsWorker do
 
     it 'schedules workers for each individual segment' do
       freeze_time do
-        expect(Analytics::DevopsAdoption::CreateSnapshotWorker).to receive(:perform_in).with(0, segment1.id, Time.zone.now)
-        expect(Analytics::DevopsAdoption::CreateSnapshotWorker).to receive(:perform_in).with(5, segment2.id, Time.zone.now)
+        end_time = 1.month.ago.end_of_month
+        expect(Analytics::DevopsAdoption::CreateSnapshotWorker).to receive(:perform_in).with(0, segment1.id, end_time)
+        expect(Analytics::DevopsAdoption::CreateSnapshotWorker).to receive(:perform_in).with(5, segment2.id, end_time)
 
         worker.perform
       end