Merge branch 'sarnold-variables-builder-scoped-vars' into 'master'

Add CI Variables builder, prometheus timing

See merge request gitlab-org/gitlab!72348

app/models/ci/pipeline.rb

@@ -780,6 +780,10 @@ module Ci
       strong_memoize(:legacy_trigger) { trigger_requests.first }
     end
 
+    def variables_builder
+      @variables_builder ||= ::Gitlab::Ci::Variables::Builder.new(self)
+    end
+
     def persisted_variables
       Gitlab::Ci::Variables::Collection.new.tap do |variables|
         break variables unless persisted?
@@ -1254,6 +1258,12 @@ module Ci
       self.builds.latest.build_matchers(project)
     end
 
+    def predefined_vars_in_builder_enabled?
+      strong_memoize(:predefined_vars_in_builder_enabled) do
+        Feature.enabled?(:ci_predefined_vars_in_builder, project, default_enabled: :yaml)
+      end
+    end
+
     private
 
     def add_message(severity, content)

app/models/concerns/ci/contextable.rb

@@ -10,8 +10,10 @@ module Ci
     # Variables in the environment name scope.
     #
     def scoped_variables(environment: expanded_environment_name, dependencies: true)
-      Gitlab::Ci::Variables::Collection.new.tap do |variables|
-        variables.concat(predefined_variables)
+      track_duration do
+        variables = pipeline.variables_builder.scoped_variables(self, environment: environment, dependencies: dependencies)
+
+        variables.concat(predefined_variables) unless pipeline.predefined_vars_in_builder_enabled?
         variables.concat(project.predefined_variables)
         variables.concat(pipeline.predefined_variables)
         variables.concat(runner.predefined_variables) if runnable? && runner
@@ -25,9 +27,23 @@ module Ci
         variables.concat(trigger_request.user_variables) if trigger_request
         variables.concat(pipeline.variables)
         variables.concat(pipeline.pipeline_schedule.job_variables) if pipeline.pipeline_schedule
+
+        variables
       end
     end
 
+    def track_duration
+      start_time = ::Gitlab::Metrics::System.monotonic_time
+      result = yield
+      duration = ::Gitlab::Metrics::System.monotonic_time - start_time
+
+      ::Gitlab::Ci::Pipeline::Metrics
+        .pipeline_builder_scoped_variables_histogram
+        .observe({}, duration.seconds)
+
+      result
+    end
+
     ##
     # Variables that do not depend on the environment name.
     #

config/feature_flags/development/ci_predefined_vars_in_builder.yml

+---
+name: ci_predefined_vars_in_builder
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/72348
+rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/231300
+milestone: '14.4'
+type: development
+group: group::pipeline authoring
+default_enabled: false

doc/administration/monitoring/prometheus/gitlab_metrics.md

@@ -41,6 +41,7 @@ The following metrics are available:
 | `gitlab_cache_misses_total`                                      | Counter     | 10.2    | Cache read miss                                                                                                       | `controller`, `action`                                    |
 | `gitlab_cache_operation_duration_seconds`                        | Histogram   | 10.2    | Cache access time                                                                                                     |                                                           |
 | `gitlab_cache_operations_total`                                  | Counter     | 12.2    | Cache operations by controller or action                                                                              | `controller`, `action`, `operation`                       |
+| `gitlab_ci_pipeline_builder_scoped_variables_duration`           | Histogram   | 14.5   | Time in seconds it takes to create the scoped variables for a CI/CD job
 | `gitlab_ci_pipeline_creation_duration_seconds`                   | Histogram   | 13.0    | Time in seconds it takes to create a CI/CD pipeline                                                                   |                                                           |
 | `gitlab_ci_pipeline_size_builds`                                 | Histogram   | 13.1    | Total number of builds within a pipeline grouped by a pipeline source                                                 | `source`                                                  |
 | `job_waiter_started_total`                                       | Counter     | 12.9    | Number of batches of jobs started where a web request is waiting for the jobs to complete                             | `worker`                                                  |

lib/gitlab/ci/pipeline/metrics.rb

@@ -51,6 +51,15 @@ module Gitlab
           ::Gitlab::Metrics.histogram(name, comment, labels, buckets)
         end
 
+        def self.pipeline_builder_scoped_variables_histogram
+          name = :gitlab_ci_pipeline_builder_scoped_variables_duration
+          comment = 'Pipeline variables builder scoped_variables duration'
+          labels = {}
+          buckets = [0.01, 0.05, 0.1, 0.3, 0.5, 1, 2, 5, 10, 30, 60, 120]
+
+          ::Gitlab::Metrics.histogram(name, comment, labels, buckets)
+        end
+
         def self.pipeline_processing_events_counter
           name = :gitlab_ci_pipeline_processing_events_total
           comment = 'Total amount of pipeline processing events'

lib/gitlab/ci/variables/builder.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Variables
+      class Builder
+        include ::Gitlab::Utils::StrongMemoize
+
+        def initialize(pipeline)
+          @pipeline = pipeline
+        end
+
+        def scoped_variables(job, environment:, dependencies:)
+          Gitlab::Ci::Variables::Collection.new.tap do |variables|
+            variables.concat(predefined_variables(job)) if pipeline.predefined_vars_in_builder_enabled?
+          end
+        end
+
+        private
+
+        attr_reader :pipeline
+
+        def predefined_variables(job)
+          Gitlab::Ci::Variables::Collection.new.tap do |variables|
+            variables.append(key: 'CI_JOB_NAME', value: job.name)
+            variables.append(key: 'CI_JOB_STAGE', value: job.stage)
+            variables.append(key: 'CI_JOB_MANUAL', value: 'true') if job.action?
+            variables.append(key: 'CI_PIPELINE_TRIGGERED', value: 'true') if job.trigger_request
+
+            variables.append(key: 'CI_NODE_INDEX', value: job.options[:instance].to_s) if job.options&.include?(:instance)
+            variables.append(key: 'CI_NODE_TOTAL', value: ci_node_total_value(job).to_s)
+
+            # legacy variables
+            variables.append(key: 'CI_BUILD_NAME', value: job.name)
+            variables.append(key: 'CI_BUILD_STAGE', value: job.stage)
+            variables.append(key: 'CI_BUILD_TRIGGERED', value: 'true') if job.trigger_request
+            variables.append(key: 'CI_BUILD_MANUAL', value: 'true') if job.action?
+          end
+        end
+
+        def ci_node_total_value(job)
+          parallel = job.options&.dig(:parallel)
+          parallel = parallel.dig(:total) if parallel.is_a?(Hash)
+          parallel || 1
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/ci/variables/builder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Gitlab::Ci::Variables::Builder do
+  let(:builder) { described_class.new(pipeline) }
+  let(:pipeline) { create(:ci_pipeline) }
+  let(:job) { create(:ci_build, pipeline: pipeline) }
+
+  describe '#scoped_variables' do
+    let(:environment) { job.expanded_environment_name }
+    let(:dependencies) { true }
+
+    subject { builder.scoped_variables(job, environment: environment, dependencies: dependencies) }
+
+    it 'returns the expected variables' do
+      keys = %w[CI_JOB_NAME
+                CI_JOB_STAGE
+                CI_NODE_TOTAL
+                CI_BUILD_NAME
+                CI_BUILD_STAGE]
+
+      subject.map { |env| env[:key] }.tap do |names|
+        expect(names).to include(*keys)
+      end
+    end
+
+    context 'feature flag disabled' do
+      before do
+        stub_feature_flags(ci_predefined_vars_in_builder: false)
+      end
+
+      it 'returns no variables' do
+        expect(subject.map { |env| env[:key] }).to be_empty
+      end
+    end
+  end
+end

spec/models/ci/build_spec.rb

@@ -2759,7 +2759,10 @@ RSpec.describe Ci::Build do
           let(:job_dependency_var) { { key: 'job_dependency', value: 'value', public: true, masked: false } }
 
           before do
-            allow(build).to receive(:predefined_variables) { [build_pre_var] }
+            allow_next_instance_of(Gitlab::Ci::Variables::Builder) do |builder|
+              allow(builder).to receive(:predefined_variables) { [build_pre_var] }
+            end
+
             allow(build).to receive(:yaml_variables) { [build_yaml_var] }
             allow(build).to receive(:persisted_variables) { [] }
             allow(build).to receive(:job_jwt_variables) { [job_jwt_var] }
@@ -3411,75 +3414,122 @@ RSpec.describe Ci::Build do
   end
 
   describe '#scoped_variables' do
-    context 'when build has not been persisted yet' do
-      let(:build) do
-        described_class.new(
-          name: 'rspec',
-          stage: 'test',
-          ref: 'feature',
-          project: project,
-          pipeline: pipeline,
-          scheduling_type: :stage
-        )
-      end
+    before do
+      pipeline.clear_memoization(:predefined_vars_in_builder_enabled)
+    end
 
-      let(:pipeline) { create(:ci_pipeline, project: project, ref: 'feature') }
+    it 'records a prometheus metric' do
+      histogram = double(:histogram)
+      expect(::Gitlab::Ci::Pipeline::Metrics).to receive(:pipeline_builder_scoped_variables_histogram)
+        .and_return(histogram)
 
-      it 'does not persist the build' do
-        expect(build).to be_valid
-        expect(build).not_to be_persisted
+      expect(histogram).to receive(:observe)
+        .with({}, a_kind_of(ActiveSupport::Duration))
 
-        build.scoped_variables
+      build.scoped_variables
+    end
 
-        expect(build).not_to be_persisted
-      end
+    shared_examples 'calculates scoped_variables' do
+      context 'when build has not been persisted yet' do
+        let(:build) do
+          described_class.new(
+            name: 'rspec',
+            stage: 'test',
+            ref: 'feature',
+            project: project,
+            pipeline: pipeline,
+            scheduling_type: :stage
+          )
+        end
 
-      it 'returns static predefined variables' do
-        keys = %w[CI_JOB_NAME
-                  CI_COMMIT_SHA
-                  CI_COMMIT_SHORT_SHA
-                  CI_COMMIT_REF_NAME
-                  CI_COMMIT_REF_SLUG
-                  CI_JOB_STAGE]
+        let(:pipeline) { create(:ci_pipeline, project: project, ref: 'feature') }
 
-        variables = build.scoped_variables
+        it 'does not persist the build' do
+          expect(build).to be_valid
+          expect(build).not_to be_persisted
 
-        variables.map { |env| env[:key] }.tap do |names|
-          expect(names).to include(*keys)
+          build.scoped_variables
+
+          expect(build).not_to be_persisted
         end
 
-        expect(variables)
-          .to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true, masked: false)
+        it 'returns static predefined variables' do
+          keys = %w[CI_JOB_NAME
+                    CI_COMMIT_SHA
+                    CI_COMMIT_SHORT_SHA
+                    CI_COMMIT_REF_NAME
+                    CI_COMMIT_REF_SLUG
+                    CI_JOB_STAGE]
+
+          variables = build.scoped_variables
+
+          variables.map { |env| env[:key] }.tap do |names|
+            expect(names).to include(*keys)
+          end
+
+          expect(variables)
+            .to include(key: 'CI_COMMIT_REF_NAME', value: 'feature', public: true, masked: false)
+        end
+
+        it 'does not return prohibited variables' do
+          keys = %w[CI_JOB_ID
+                    CI_JOB_URL
+                    CI_JOB_TOKEN
+                    CI_BUILD_ID
+                    CI_BUILD_TOKEN
+                    CI_REGISTRY_USER
+                    CI_REGISTRY_PASSWORD
+                    CI_REPOSITORY_URL
+                    CI_ENVIRONMENT_URL
+                    CI_DEPLOY_USER
+                    CI_DEPLOY_PASSWORD]
+
+          build.scoped_variables.map { |env| env[:key] }.tap do |names|
+            expect(names).not_to include(*keys)
+          end
+        end
       end
 
-      it 'does not return prohibited variables' do
-        keys = %w[CI_JOB_ID
-                  CI_JOB_URL
-                  CI_JOB_TOKEN
-                  CI_BUILD_ID
-                  CI_BUILD_TOKEN
-                  CI_REGISTRY_USER
-                  CI_REGISTRY_PASSWORD
-                  CI_REPOSITORY_URL
-                  CI_ENVIRONMENT_URL
-                  CI_DEPLOY_USER
-                  CI_DEPLOY_PASSWORD]
+      context 'with dependency variables' do
+        let!(:prepare) { create(:ci_build, name: 'prepare', pipeline: pipeline, stage_idx: 0) }
+        let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 1, options: { dependencies: ['prepare'] }) }
 
-        build.scoped_variables.map { |env| env[:key] }.tap do |names|
-          expect(names).not_to include(*keys)
+        let!(:job_variable) { create(:ci_job_variable, :dotenv_source, job: prepare) }
+
+        it 'inherits dependent variables' do
+          expect(build.scoped_variables.to_hash).to include(job_variable.key => job_variable.value)
         end
       end
     end
 
-    context 'with dependency variables' do
-      let!(:prepare) { create(:ci_build, name: 'prepare', pipeline: pipeline, stage_idx: 0) }
-      let!(:build) { create(:ci_build, pipeline: pipeline, stage_idx: 1, options: { dependencies: ['prepare'] }) }
+    it_behaves_like 'calculates scoped_variables'
 
-      let!(:job_variable) { create(:ci_job_variable, :dotenv_source, job: prepare) }
+    it 'delegates to the variable builders' do
+      expect_next_instance_of(Gitlab::Ci::Variables::Builder) do |builder|
+        expect(builder)
+          .to receive(:scoped_variables).with(build, hash_including(:environment, :dependencies))
+          .and_call_original
 
-      it 'inherits dependent variables' do
-        expect(build.scoped_variables.to_hash).to include(job_variable.key => job_variable.value)
+        expect(builder).to receive(:predefined_variables).and_call_original
       end
+
+      build.scoped_variables
+    end
+
+    context 'when ci builder feature flag is disabled' do
+      before do
+        stub_feature_flags(ci_predefined_vars_in_builder: false)
+      end
+
+      it 'does not delegate to the variable builders' do
+        expect_next_instance_of(Gitlab::Ci::Variables::Builder) do |builder|
+          expect(builder).not_to receive(:predefined_variables)
+        end
+
+        build.scoped_variables
+      end
+
+      it_behaves_like 'calculates scoped_variables'
     end
   end