Merge branch 'track-project-ci-minutes-monthly' into 'master'

Track CI minutes on a monthly basis at project level

See merge request gitlab-org/gitlab!53460

changelogs/unreleased/track-project-ci-minutes-monthly.yml

+---
+title: Track CI minutes on a monthly basis at project level
+merge_request: 53460
+author:
+type: added

db/migrate/20210205084357_create_ci_project_monthly_usage.rb

+# frozen_string_literal: true
+
+class CreateCiProjectMonthlyUsage < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    with_lock_retries do
+      create_table :ci_project_monthly_usages, if_not_exists: true do |t|
+        t.references :project, foreign_key: { on_delete: :cascade }, index: false, null: false
+        t.date :date, null: false
+        t.decimal :amount_used, null: false, default: 0.0, precision: 18, scale: 2
+
+        t.index [:project_id, :date], unique: true
+      end
+    end
+
+    add_check_constraint :ci_project_monthly_usages, "(date = date_trunc('month', date))", 'ci_project_monthly_usages_year_month_constraint'
+  end
+
+  def down
+    with_lock_retries do
+      drop_table :ci_project_monthly_usages
+    end
+  end
+end

db/schema_migrations/20210205084357

+5bd622f36126b06c7c585ee14a8140750843d36092e79b6cc35b62c06afb1178
\ No newline at end of file

db/structure.sql

@@ -10721,6 +10721,23 @@ CREATE SEQUENCE ci_platform_metrics_id_seq
 
 ALTER SEQUENCE ci_platform_metrics_id_seq OWNED BY ci_platform_metrics.id;
 
+CREATE TABLE ci_project_monthly_usages (
+    id bigint NOT NULL,
+    project_id bigint NOT NULL,
+    date date NOT NULL,
+    amount_used numeric(18,2) DEFAULT 0.0 NOT NULL,
+    CONSTRAINT ci_project_monthly_usages_year_month_constraint CHECK ((date = date_trunc('month'::text, (date)::timestamp with time zone)))
+);
+
+CREATE SEQUENCE ci_project_monthly_usages_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE ci_project_monthly_usages_id_seq OWNED BY ci_project_monthly_usages.id;
+
 CREATE TABLE ci_refs (
     id bigint NOT NULL,
     project_id bigint NOT NULL,
@@ -18785,6 +18802,8 @@ ALTER TABLE ONLY ci_pipelines_config ALTER COLUMN pipeline_id SET DEFAULT nextva
 
 ALTER TABLE ONLY ci_platform_metrics ALTER COLUMN id SET DEFAULT nextval('ci_platform_metrics_id_seq'::regclass);
 
+ALTER TABLE ONLY ci_project_monthly_usages ALTER COLUMN id SET DEFAULT nextval('ci_project_monthly_usages_id_seq'::regclass);
+
 ALTER TABLE ONLY ci_refs ALTER COLUMN id SET DEFAULT nextval('ci_refs_id_seq'::regclass);
 
 ALTER TABLE ONLY ci_resource_groups ALTER COLUMN id SET DEFAULT nextval('ci_resource_groups_id_seq'::regclass);
@@ -19910,6 +19929,9 @@ ALTER TABLE ONLY ci_pipelines
 ALTER TABLE ONLY ci_platform_metrics
     ADD CONSTRAINT ci_platform_metrics_pkey PRIMARY KEY (id);
 
+ALTER TABLE ONLY ci_project_monthly_usages
+    ADD CONSTRAINT ci_project_monthly_usages_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY ci_refs
     ADD CONSTRAINT ci_refs_pkey PRIMARY KEY (id);
 
@@ -21747,6 +21769,8 @@ CREATE INDEX index_ci_pipelines_on_user_id_and_created_at_and_config_source ON c
 
 CREATE INDEX index_ci_pipelines_on_user_id_and_created_at_and_source ON ci_pipelines USING btree (user_id, created_at, source);
 
+CREATE UNIQUE INDEX index_ci_project_monthly_usages_on_project_id_and_date ON ci_project_monthly_usages USING btree (project_id, date);
+
 CREATE UNIQUE INDEX index_ci_refs_on_project_id_and_ref_path ON ci_refs USING btree (project_id, ref_path);
 
 CREATE UNIQUE INDEX index_ci_resource_groups_on_project_id_and_key ON ci_resource_groups USING btree (project_id, key);
@@ -25266,6 +25290,9 @@ ALTER TABLE ONLY resource_iteration_events
 ALTER TABLE ONLY status_page_settings
     ADD CONSTRAINT fk_rails_506e5ba391 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY ci_project_monthly_usages
+    ADD CONSTRAINT fk_rails_508bcd4aa6 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY project_repository_storage_moves
     ADD CONSTRAINT fk_rails_5106dbd44a FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 

ee/app/models/ci/minutes/namespace_monthly_usage.rb

@@ -22,13 +22,13 @@ module Ci
         current_month.safe_find_or_create_by(namespace: namespace)
       end
 
-      def self.increase_usage(namespace, amount)
+      def self.increase_usage(usage, amount)
         return unless amount > 0
 
         # The use of `update_counters` ensures we do a SQL update rather than
         # incrementing the counter for the object in memory and then save it.
         # This is better for concurrent updates.
-        update_counters(self.find_or_create_current(namespace), amount_used: amount)
+        update_counters(usage, amount_used: amount)
       end
     end
   end

ee/app/models/ci/minutes/project_monthly_usage.rb

+# frozen_string_literal: true
+
+module Ci
+  module Minutes
+    # Track usage of Shared Runners minutes at root project level.
+    # This class ensures that we keep 1 record per project per month.
+    class ProjectMonthlyUsage < ApplicationRecord
+      self.table_name = "ci_project_monthly_usages"
+
+      belongs_to :project
+
+      scope :current_month, -> { where(date: Time.current.beginning_of_month) }
+
+      # We should pretty much always use this method to access data for the current month
+      # since this will lazily create an entry if it doesn't exist.
+      # For example, on the 1st of each month, when we update the usage for a project,
+      # we will automatically generate new records and reset usage for the current month.
+      def self.find_or_create_current(project)
+        current_month.safe_find_or_create_by(project: project)
+      end
+
+      def self.increase_usage(usage, amount)
+        return unless amount > 0
+
+        # The use of `update_counters` ensures we do a SQL update rather than
+        # incrementing the counter for the object in memory and then save it.
+        # This is better for concurrent updates.
+        update_counters(usage, amount_used: amount)
+      end
+    end
+  end
+end

ee/app/services/ci/minutes/update_build_minutes_service.rb

@@ -31,7 +31,13 @@ module Ci
       def track_usage_of_monthly_minutes(consumption)
         return unless Feature.enabled?(:ci_minutes_monthly_tracking, project, default_enabled: :yaml)
 
-        ::Ci::Minutes::NamespaceMonthlyUsage.increase_usage(namespace, consumption)
+        namespace_usage = ::Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace)
+        project_usage = ::Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project)
+
+        ActiveRecord::Base.transaction do
+          ::Ci::Minutes::NamespaceMonthlyUsage.increase_usage(namespace_usage, consumption)
+          ::Ci::Minutes::ProjectMonthlyUsage.increase_usage(project_usage, consumption)
+        end
       end
 
       def namespace_statistics

ee/spec/factories/ci/minutes/project_monthly_usages.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ci_project_monthly_usage, class: 'Ci::Minutes::ProjectMonthlyUsage' do
+    amount_used { 100 }
+    project factory: :project
+    date { Time.current.utc.beginning_of_month }
+  end
+end

ee/spec/models/ci/minutes/namespace_monthly_usage_spec.rb

@@ -66,40 +66,27 @@ RSpec.describe Ci::Minutes::NamespaceMonthlyUsage do
   end
 
   describe '.increase_usage' do
-    subject { described_class.increase_usage(namespace, amount) }
+    subject { described_class.increase_usage(usage, amount) }
 
-    context 'when usage for current month exists' do
-      let!(:usage) { create(:ci_namespace_monthly_usage, namespace: namespace, amount_used: 100.0) }
+    let(:usage) { create(:ci_namespace_monthly_usage, namespace: namespace, amount_used: 100.0) }
 
-      context 'when amount is greater than 0' do
-        let(:amount) { 10.5 }
+    context 'when amount is greater than 0' do
+      let(:amount) { 10.5 }
 
-        it 'updates the current month usage' do
-          subject
+      it 'updates the current month usage' do
+        subject
 
-          expect(usage.reload.amount_used).to eq(110.5)
-        end
-      end
-
-      context 'when amount is less or equal to 0' do
-        let(:amount) { -2.0 }
-
-        it 'does not update the current month usage' do
-          subject
-
-          expect(usage.reload.amount_used).to eq(100.0)
-        end
+        expect(usage.reload.amount_used).to eq(110.5)
       end
     end
 
-    context 'when usage for current month does not exist' do
-      let(:amount) { 17.0 }
+    context 'when amount is less or equal to 0' do
+      let(:amount) { -2.0 }
 
-      it 'creates a new record for the current month and records the usage' do
-        expect { subject }.to change { described_class.count }.by(1)
+      it 'does not update the current month usage' do
+        subject
 
-        current_usage = described_class.find_or_create_current(namespace)
-        expect(current_usage.amount_used).to eq(17.0)
+        expect(usage.reload.amount_used).to eq(100.0)
       end
     end
   end

ee/spec/models/ci/minutes/project_monthly_usage_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Ci::Minutes::ProjectMonthlyUsage do
+  let_it_be(:project) { create(:project) }
+
+  describe 'unique index' do
+    before do
+      create(:ci_project_monthly_usage, project: project)
+    end
+
+    it 'raises unique index violation' do
+      expect { create(:ci_project_monthly_usage, project: project) }
+        .to raise_error { ActiveRecord::RecordNotUnique }
+    end
+
+    it 'does not raise exception if unique index is not violated' do
+      expect { create(:ci_project_monthly_usage, project: project, date: 1.month.ago.utc.beginning_of_month) }
+        .to change { described_class.count }.by(1)
+    end
+  end
+
+  describe '.find_or_create_current' do
+    subject { described_class.find_or_create_current(project) }
+
+    shared_examples 'creates usage record' do
+      it 'creates new record and resets minutes consumption' do
+        freeze_time do
+          expect { subject }.to change { described_class.count }.by(1)
+
+          expect(subject.amount_used).to eq(0)
+          expect(subject.project).to eq(project)
+          expect(subject.date).to eq(Time.current.beginning_of_month)
+        end
+      end
+    end
+
+    context 'when project usage does not exist' do
+      it_behaves_like 'creates usage record'
+    end
+
+    context 'when project usage exists for previous months' do
+      before do
+        create(:ci_project_monthly_usage, project: project, date: 2.months.ago.utc.beginning_of_month)
+      end
+
+      it_behaves_like 'creates usage record'
+    end
+
+    context 'when project usage exists for the current month' do
+      it 'returns the existing usage' do
+        freeze_time do
+          usage = create(:ci_project_monthly_usage, project: project)
+
+          expect(subject).to eq(usage)
+        end
+      end
+    end
+
+    context 'when a usage for another project exists for the current month' do
+      let!(:usage) { create(:ci_project_monthly_usage) }
+
+      it_behaves_like 'creates usage record'
+    end
+  end
+
+  describe '.increase_usage' do
+    subject { described_class.increase_usage(usage, amount) }
+
+    let(:usage) { create(:ci_project_monthly_usage, project: project, amount_used: 100.0) }
+
+    context 'when amount is greater than 0' do
+      let(:amount) { 10.5 }
+
+      it 'updates the current month usage' do
+        subject
+
+        expect(usage.reload.amount_used).to eq(110.5)
+      end
+    end
+
+    context 'when amount is less or equal to 0' do
+      let(:amount) { -2.0 }
+
+      it 'does not update the current month usage' do
+        subject
+
+        expect(usage.reload.amount_used).to eq(100.0)
+      end
+    end
+  end
+end

ee/spec/services/ci/minutes/update_build_minutes_service_spec.rb

@@ -15,6 +15,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
     end
 
     let(:namespace_amount_used) { Ci::Minutes::NamespaceMonthlyUsage.find_or_create_current(namespace).amount_used }
+    let(:project_amount_used) { Ci::Minutes::ProjectMonthlyUsage.find_or_create_current(project).amount_used }
 
     subject { described_class.new(project, nil).execute(build) }
 
@@ -24,6 +25,9 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
 
         expect(namespace_amount_used)
           .to eq((namespace.namespace_statistics.reload.shared_runners_seconds.to_f / 60).round(2))
+
+        expect(project_amount_used)
+          .to eq((project.statistics.reload.shared_runners_seconds.to_f / 60).round(2))
       end
     end
 
@@ -45,6 +49,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
         subject
 
         expect(namespace_amount_used).to eq((60 * 2).to_f)
+        expect(project_amount_used).to eq((60 * 2).to_f)
       end
 
       it_behaves_like 'new tracking matches legacy tracking'
@@ -56,6 +61,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
 
         it 'does not track usage on a monthly basis' do
           expect(namespace_amount_used).to eq(0)
+          expect(project_amount_used).to eq(0)
         end
       end
 
@@ -67,6 +73,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
           project.statistics.update!(shared_runners_seconds: usage_in_seconds)
           namespace.create_namespace_statistics(shared_runners_seconds: usage_in_seconds)
           create(:ci_namespace_monthly_usage, namespace: namespace, amount_used: usage_in_minutes)
+          create(:ci_project_monthly_usage, project: project, amount_used: usage_in_minutes)
         end
 
         it 'updates statistics and adds duration with applied cost factor' do
@@ -83,6 +90,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
           subject
 
           expect(namespace_amount_used).to eq(usage_in_minutes + 60 * 2)
+          expect(project_amount_used).to eq(usage_in_minutes + 60 * 2)
         end
 
         it_behaves_like 'new tracking matches legacy tracking'
@@ -96,6 +104,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
             subject
 
             expect(namespace_amount_used).to eq(usage_in_minutes)
+            expect(project_amount_used).to eq(usage_in_minutes)
           end
         end
       end
@@ -117,6 +126,7 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
           subject
 
           expect(namespace_amount_used).to eq(60 * 2)
+          expect(project_amount_used).to eq(60 * 2)
         end
 
         it 'stores the same information in both legacy and new tracking' do
@@ -124,20 +134,9 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
 
           expect(namespace_amount_used)
             .to eq((root_ancestor.namespace_statistics.reload.shared_runners_seconds.to_f / 60).round(2))
-        end
-      end
-
-      context 'when cost factor has non-zero fractional part' do
-        let(:cost_factor) { 1.234 }
-
-        it 'truncates the result product value' do
-          subject
 
-          expect(project.statistics.reload.shared_runners_seconds)
-            .to eq((build.duration.to_i * 1.234).to_i)
-
-          expect(namespace.namespace_statistics.reload.shared_runners_seconds)
-            .to eq((build.duration.to_i * 1.234).to_i)
+          expect(project_amount_used)
+            .to eq((project.statistics.reload.shared_runners_seconds.to_f / 60).round(2))
         end
       end
     end
@@ -151,9 +150,13 @@ RSpec.describe Ci::Minutes::UpdateBuildMinutesService do
         expect(namespace.namespace_statistics).to be_nil
       end
 
-      it 'does not track monthly usage' do
+      it 'does not track namespace monthly usage' do
         expect { subject }.not_to change { Ci::Minutes::NamespaceMonthlyUsage.count }
       end
+
+      it 'does not track project monthly usage' do
+        expect { subject }.not_to change { Ci::Minutes::ProjectMonthlyUsage.count }
+      end
     end
   end
 end