From a74622c2cdf9b9e828b86a4a50d2edc4059192f8 Mon Sep 17 00:00:00 2001 From: Pavel Shutsin Date: Tue, 30 Aug 2022 16:44:45 +0200 Subject: [PATCH] Add basic DORA configuration model It will be used to store specific project configuraiton of DORA calculations Changelog: added --- .../development/dora_configuration.yml | 8 +++ db/docs/dora_configurations.yml | 9 +++ ...0114228_create_dora_configuration_table.rb | 16 +++++ db/schema_migrations/20220830114228 | 1 + db/structure.sql | 25 ++++++++ ee/app/models/dora/configuration.rb | 12 ++++ .../dora/lead_time_for_changes_metric.rb | 29 +++++++++ ee/spec/factories/dora/configurations.rb | 8 +++ ee/spec/models/dora/configuration_spec.rb | 16 +++++ .../dora/lead_time_for_changes_metric_spec.rb | 61 +++++++++++++++---- lib/gitlab/database/gitlab_schemas.yml | 1 + 11 files changed, 174 insertions(+), 12 deletions(-) create mode 100644 config/feature_flags/development/dora_configuration.yml create mode 100644 db/docs/dora_configurations.yml create mode 100644 db/migrate/20220830114228_create_dora_configuration_table.rb create mode 100644 db/schema_migrations/20220830114228 create mode 100644 ee/app/models/dora/configuration.rb create mode 100644 ee/spec/factories/dora/configurations.rb create mode 100644 ee/spec/models/dora/configuration_spec.rb diff --git a/config/feature_flags/development/dora_configuration.yml b/config/feature_flags/development/dora_configuration.yml new file mode 100644 index 00000000000000..38a050571d8222 --- /dev/null +++ b/config/feature_flags/development/dora_configuration.yml @@ -0,0 +1,8 @@ +--- +name: dora_configuration +introduced_by_url: "https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96561" +rollout_issue_url: "https://gitlab.com/gitlab-org/gitlab/-/issues/372545" +milestone: '15.4' +type: development +group: group::optimize +default_enabled: false diff --git a/db/docs/dora_configurations.yml b/db/docs/dora_configurations.yml new file mode 100644 index 00000000000000..e13cf088670a99 --- /dev/null +++ b/db/docs/dora_configurations.yml @@ -0,0 +1,9 @@ +--- +table_name: dora_configurations +classes: +- Dora::Configuration +feature_categories: +- continuous_delivery +description: Stores project specific configurations for DORA4 calculations. +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/96561 +milestone: '15.4' diff --git a/db/migrate/20220830114228_create_dora_configuration_table.rb b/db/migrate/20220830114228_create_dora_configuration_table.rb new file mode 100644 index 00000000000000..ee5960d14b6d4f --- /dev/null +++ b/db/migrate/20220830114228_create_dora_configuration_table.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +class CreateDoraConfigurationTable < Gitlab::Database::Migration[2.0] + disable_ddl_transaction! + + def up + create_table :dora_configurations do |t| + t.references :project, null: false, index: { unique: true }, foreign_key: { on_delete: :cascade } + t.text :branches_for_lead_time_for_changes, null: false, array: true, default: [] + end + end + + def down + drop_table :dora_configurations + end +end diff --git a/db/schema_migrations/20220830114228 b/db/schema_migrations/20220830114228 new file mode 100644 index 00000000000000..44b26221fd5bc6 --- /dev/null +++ b/db/schema_migrations/20220830114228 @@ -0,0 +1 @@ +fad5bab727bdaed1d17950d320baecd995dcc8a91816e2cfcdff6d1b393c637d \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index ea00b4049cdec9..f17a51a41b2d61 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -14838,6 +14838,21 @@ CREATE SEQUENCE dingtalk_tracker_data_id_seq ALTER SEQUENCE dingtalk_tracker_data_id_seq OWNED BY dingtalk_tracker_data.id; +CREATE TABLE dora_configurations ( + id bigint NOT NULL, + project_id bigint NOT NULL, + branches_for_lead_time_for_changes text[] DEFAULT '{}'::text[] NOT NULL +); + +CREATE SEQUENCE dora_configurations_id_seq + START WITH 1 + INCREMENT BY 1 + NO MINVALUE + NO MAXVALUE + CACHE 1; + +ALTER SEQUENCE dora_configurations_id_seq OWNED BY dora_configurations.id; + CREATE TABLE dora_daily_metrics ( id bigint NOT NULL, environment_id bigint NOT NULL, @@ -23433,6 +23448,8 @@ ALTER TABLE ONLY diff_note_positions ALTER COLUMN id SET DEFAULT nextval('diff_n ALTER TABLE ONLY dingtalk_tracker_data ALTER COLUMN id SET DEFAULT nextval('dingtalk_tracker_data_id_seq'::regclass); +ALTER TABLE ONLY dora_configurations ALTER COLUMN id SET DEFAULT nextval('dora_configurations_id_seq'::regclass); + ALTER TABLE ONLY dora_daily_metrics ALTER COLUMN id SET DEFAULT nextval('dora_daily_metrics_id_seq'::regclass); ALTER TABLE ONLY draft_notes ALTER COLUMN id SET DEFAULT nextval('draft_notes_id_seq'::regclass); @@ -25285,6 +25302,9 @@ ALTER TABLE ONLY diff_note_positions ALTER TABLE ONLY dingtalk_tracker_data ADD CONSTRAINT dingtalk_tracker_data_pkey PRIMARY KEY (id); +ALTER TABLE ONLY dora_configurations + ADD CONSTRAINT dora_configurations_pkey PRIMARY KEY (id); + ALTER TABLE ONLY dora_daily_metrics ADD CONSTRAINT dora_daily_metrics_pkey PRIMARY KEY (id); @@ -28524,6 +28544,8 @@ CREATE UNIQUE INDEX index_diff_note_positions_on_note_id_and_diff_type ON diff_n CREATE INDEX index_dingtalk_tracker_data_on_integration_id ON dingtalk_tracker_data USING btree (integration_id); +CREATE UNIQUE INDEX index_dora_configurations_on_project_id ON dora_configurations USING btree (project_id); + CREATE UNIQUE INDEX index_dora_daily_metrics_on_environment_id_and_date ON dora_daily_metrics USING btree (environment_id, date); CREATE INDEX index_draft_notes_on_author_id ON draft_notes USING btree (author_id); @@ -34286,6 +34308,9 @@ ALTER TABLE ONLY approval_project_rules_protected_branches ALTER TABLE ONLY packages_composer_cache_files ADD CONSTRAINT fk_rails_b82cea43a0 FOREIGN KEY (namespace_id) REFERENCES namespaces(id) ON DELETE SET NULL; +ALTER TABLE ONLY dora_configurations + ADD CONSTRAINT fk_rails_b9b8d90ddb FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE; + ALTER TABLE ONLY merge_trains ADD CONSTRAINT fk_rails_b9d67af01d FOREIGN KEY (target_project_id) REFERENCES projects(id) ON DELETE CASCADE; diff --git a/ee/app/models/dora/configuration.rb b/ee/app/models/dora/configuration.rb new file mode 100644 index 00000000000000..75306006803cbf --- /dev/null +++ b/ee/app/models/dora/configuration.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +module Dora + class Configuration < ApplicationRecord + self.table_name = 'dora_configurations' + + belongs_to :project + + validates :project_id, uniqueness: true, presence: true + validates :branches_for_lead_time_for_changes, length: { minimum: 0, allow_nil: false, message: :blank } + end +end diff --git a/ee/app/models/dora/lead_time_for_changes_metric.rb b/ee/app/models/dora/lead_time_for_changes_metric.rb index a344417cda1050..86aaa2ae25f285 100644 --- a/ee/app/models/dora/lead_time_for_changes_metric.rb +++ b/ee/app/models/dora/lead_time_for_changes_metric.rb @@ -30,9 +30,38 @@ def data_queries ) .where(eligible_deployments) + if Feature.enabled?(:dora_configuration, environment.project) + merge_requests = MergeRequest.arel_table + dora_configurations = Dora::Configuration.arel_table + + query = query + .join(merge_requests).on( + merge_requests[:id].eq(deployment_merge_requests[:merge_request_id]) + ) + .outer_join(dora_configurations).on( + dora_configurations[:project_id].eq(deployments[:project_id]) + ) + .where(eligible_merge_requests) + end + { lead_time_for_changes_in_seconds: query.to_sql } end + + private + + def eligible_merge_requests + merge_requests = MergeRequest.arel_table + dora_configurations = Dora::Configuration.arel_table + + [ + dora_configurations[:branches_for_lead_time_for_changes].eq(nil), + dora_configurations[:branches_for_lead_time_for_changes].eq([]), + merge_requests[:target_branch].eq( + Arel::Nodes::NamedFunction.new("ANY", [dora_configurations[:branches_for_lead_time_for_changes]]) + ) + ].reduce(&:or) + end end end diff --git a/ee/spec/factories/dora/configurations.rb b/ee/spec/factories/dora/configurations.rb new file mode 100644 index 00000000000000..5e89ecbc0ec72d --- /dev/null +++ b/ee/spec/factories/dora/configurations.rb @@ -0,0 +1,8 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :dora_configuration, class: 'Dora::Configuration' do + project + branches_for_lead_time_for_changes { %w[master main] } + end +end diff --git a/ee/spec/models/dora/configuration_spec.rb b/ee/spec/models/dora/configuration_spec.rb new file mode 100644 index 00000000000000..b2b9f5faf0dc5a --- /dev/null +++ b/ee/spec/models/dora/configuration_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Dora::Configuration, type: :model do + subject { build :dora_configuration } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + end + + it { is_expected.to validate_presence_of(:project_id) } + it { is_expected.to validate_uniqueness_of(:project_id) } + it { is_expected.to allow_value([]).for(:branches_for_lead_time_for_changes) } + it { is_expected.not_to allow_value(nil).for(:branches_for_lead_time_for_changes) } +end diff --git a/ee/spec/models/dora/lead_time_for_changes_metric_spec.rb b/ee/spec/models/dora/lead_time_for_changes_metric_spec.rb index 50f8bfc7b8ad52..8abaa7f683994c 100644 --- a/ee/spec/models/dora/lead_time_for_changes_metric_spec.rb +++ b/ee/spec/models/dora/lead_time_for_changes_metric_spec.rb @@ -4,30 +4,67 @@ RSpec.describe Dora::LeadTimeForChangesMetric do describe '#data_queries' do - subject { described_class.new(environment, date.to_date).data_queries } + subject(:data_queries) { described_class.new(environment, date.to_date).data_queries } + + let(:query_result) { Deployment.connection.execute(data_queries[:lead_time_for_changes_in_seconds]).first['percentile_cont'] } let_it_be(:project) { create(:project, :repository) } let_it_be(:environment) { create(:environment, project: project) } let_it_be(:date) { 1.day.ago } - around do |example| - freeze_time { example.run } - end - - it 'returns median of time between merge and deployment' do - create(:merge_request, :with_merged_metrics, project: project, merged_at: date - 1.day) + before_all do + create(:merge_request, :with_merged_metrics, project: project, target_branch: 'main', merged_at: date - 1.day) merge_requests = [ - create(:merge_request, :with_merged_metrics, project: project, merged_at: date - 1.day), - create(:merge_request, :with_merged_metrics, project: project, merged_at: date - 2.days), - create(:merge_request, :with_merged_metrics, project: project, merged_at: date - 5.days) + create(:merge_request, :with_merged_metrics, project: project, target_branch: 'main', merged_at: date - 1.day), + create(:merge_request, :with_merged_metrics, project: project, target_branch: 'staging', merged_at: date - 5.days), + create(:merge_request, :with_merged_metrics, project: project, target_branch: 'production', merged_at: date - 7.days) ] # Deployment finished on the date create(:deployment, :success, environment: environment, finished_at: date, merge_requests: merge_requests) + end + + around do |example| + freeze_time { example.run } + end + + context 'with dora_configuration disabled' do + before do + stub_feature_flags(dora_configuration: false) + end + + it 'returns median of time between merge and deployment' do + expect(query_result).to eql 5.days.to_f + end + end + + context 'with dora_configuration enabled' do + context 'without configuration object' do + it 'returns median of time between merge and deployment' do + expect(query_result).to eql 5.days.to_f + end + end + + context 'with empty branches configuration' do + before do + create :dora_configuration, project: project, branches_for_lead_time_for_changes: [] + end + + it 'returns median of time between merge and deployment' do + expect(query_result).to eql 5.days.to_f + end + end + + context 'with filled branches configuration' do + before do + create :dora_configuration, project: project, branches_for_lead_time_for_changes: %w[main staging] + end - expect(subject.size).to eq 1 - expect(Deployment.connection.execute(subject[:lead_time_for_changes_in_seconds]).first['percentile_cont']).to eql 2.days.to_f + it 'returns median of time between merge and deployment for MRs with target branch from configuration whitelist' do + expect(query_result).to eql 3.days.to_f + end + end end end end diff --git a/lib/gitlab/database/gitlab_schemas.yml b/lib/gitlab/database/gitlab_schemas.yml index 1c2d04561b4e5c..9ac38198be22b5 100644 --- a/lib/gitlab/database/gitlab_schemas.yml +++ b/lib/gitlab/database/gitlab_schemas.yml @@ -183,6 +183,7 @@ design_management_versions: :gitlab_main design_user_mentions: :gitlab_main detached_partitions: :gitlab_shared diff_note_positions: :gitlab_main +dora_configurations: :gitlab_main dora_daily_metrics: :gitlab_main draft_notes: :gitlab_main elastic_index_settings: :gitlab_main -- GitLab