Skip to content
Snippets Groups Projects
Commit 9ca1beff authored by charlie ablett's avatar charlie ablett :red_circle:
Browse files

Add in-memory TimeToMerge virtual VSA stage

- Update specs

Changelog: added
EE: true
parent b53ca218
No related branches found
No related tags found
1 merge request!117986Add in-memory MR time virtual VSA stage
Showing
with 295 additions and 80 deletions
......@@ -19,8 +19,8 @@ module CycleAnalytics
# Stage 1, Stage 2
#
# The class also adds two "in-memory" stages into the distinct calculation which
# represent the CycleTime and LeadTime metrics. These metrics are calculated as
# pre-defined VSA stages.
# represent the CycleTime and LeadTime metric for issues and TimeToMerge metric for MRs.
# These metrics are calculated as pre-defined VSA stages.
class DistinctStageLoader
def initialize(group:)
@group = group
......@@ -29,8 +29,7 @@ def initialize(group:)
def stages
[
*persisted_stages,
add_stage_event_hash_id(in_memory_lead_time_stage),
add_stage_event_hash_id(in_memory_cycle_time_stage)
*in_memory_stages.map { |stage| add_stage_event_hash_id(stage) }
].uniq(&:stage_event_hash_id)
end
......@@ -42,6 +41,14 @@ def persisted_stages
@persisted_stages ||= ::Analytics::CycleAnalytics::Stage.distinct_stages_within_hierarchy(group)
end
def in_memory_stages
[
in_memory_lead_time_stage,
in_memory_cycle_time_stage,
in_memory_time_to_merge_stage
]
end
def in_memory_lead_time_stage
::Analytics::CycleAnalytics::Stage.new(
name: 'lead time', # not visible to the user
......@@ -60,6 +67,15 @@ def in_memory_cycle_time_stage
)
end
def in_memory_time_to_merge_stage
::Analytics::CycleAnalytics::Stage.new(
name: 'time to merge',
start_event_identifier: Summary::TimeToMerge.start_event_identifier,
end_event_identifier: Summary::TimeToMerge.end_event_identifier,
namespace: group
)
end
# rubocop: disable CodeReuse/ActiveRecord
def add_stage_event_hash_id(stage)
# find or create the stage event hash
......
......@@ -14,7 +14,7 @@ def initialize(stage, options:)
end
def data
[lead_time, cycle_time].tap do |array|
[lead_time, cycle_time, time_to_merge].tap do |array|
array << serialize(lead_time_for_changes, with_unit: true) if lead_time_for_changes.value.present?
array << serialize(time_to_restore_service, with_unit: true) if time_to_restore_service.value.present?
array << serialize(change_failure_rate, with_unit: true) if change_failure_rate.value.present?
......@@ -41,6 +41,15 @@ def cycle_time
)
end
def time_to_merge
serialize(
Summary::TimeToMerge.new(
stage: stage, current_user: current_user, options: options
),
with_unit: true
)
end
def lead_time_for_changes
@lead_time_for_changes ||= Summary::LeadTimeForChanges.new(
stage: stage,
......
# frozen_string_literal: true
module Gitlab
module Analytics
module CycleAnalytics
module Summary
class TimeToMerge < BaseTime
def title
_('Time to Merge')
end
def self.start_event_identifier
:merge_request_created
end
def self.end_event_identifier
:merge_request_merged
end
def links
namespace = @stage.namespace
return [] if namespace.is_a?(::Group)
helpers = Gitlab::Routing.url_helpers
dashboard_link = helpers.project_analytics_merge_request_analytics_path(namespace.project)
[
{ "name" => title, "url" => dashboard_link,
"label" => s_('ValueStreamAnalytics|Merge request analytics') },
{ "name" => title,
"url" => helpers.help_page_path('user/analytics/index', anchor: 'definitions'),
"docs_link" => true,
"label" => s_('ValueStreamAnalytics|Go to docs') }
]
end
end
end
end
end
end
......@@ -62,7 +62,7 @@
Analytics::CycleAnalytics::DataLoaderService.new(group: group, model: Issue).execute
# Calculating Cycle Time and Lead Time
expect(Gitlab::Analytics::CycleAnalytics::Aggregated::DataCollector).to receive(:new).twice.and_call_original
expect(Gitlab::Analytics::CycleAnalytics::Aggregated::DataCollector).to receive(:new).thrice.and_call_original
subject
......
......@@ -41,11 +41,12 @@
end
it 'returns correct value' do
expected_cycle_time = (closed_at - first_mentioned_in_commit_at).to_i
expected_cycle_time = (closed_at - first_mentioned_in_commit_at).to_f
subject
expect(json_response.last["value"].to_i).to eq(expected_cycle_time)
lead_time = json_response.find { |result| result['identifier'] == 'cycle_time' }.symbolize_keys
expect(lead_time).to include({ value: expected_cycle_time.to_s, title: 'Cycle Time', unit: 'days' })
end
context 'when analytics_disabled features are disabled' do
......
......@@ -2,15 +2,20 @@
require 'spec_helper'
RSpec.describe Gitlab::Analytics::CycleAnalytics::DistinctStageLoader do
RSpec.describe Gitlab::Analytics::CycleAnalytics::DistinctStageLoader, feature_category: :value_stream_management do
let_it_be(:group) { create(:group) }
let_it_be(:stage_1) { create(:cycle_analytics_stage, namespace: group, start_event_identifier: :merge_request_created, end_event_identifier: :merge_request_merged) }
let_it_be(:stage_1) { create(:cycle_analytics_stage, namespace: group, start_event_identifier: :merge_request_created, end_event_identifier: :merge_request_closed) }
let_it_be(:common_stage_params) { { start_event_identifier: :issue_created, end_event_identifier: :issue_first_associated_with_milestone } }
let_it_be(:stage_2) { create(:cycle_analytics_stage, namespace: group, **common_stage_params) }
let_it_be(:stage_duplicate) { create(:cycle_analytics_stage, namespace: group, **common_stage_params) }
let_it_be(:stage_triplicate) { create(:cycle_analytics_stage, namespace: group, **common_stage_params) }
let_it_be(:project) { create(:project, group: group) }
let(:lead_time_name) { 'lead time' }
let(:cycle_time_name) { 'cycle time' }
let(:time_to_merge_name) { 'time to merge' }
let(:in_memory_stages_names) { [lead_time_name, cycle_time_name, time_to_merge_name] }
let(:in_memory_stages_count) { in_memory_stages_names.count }
subject(:distinct_stages) { described_class.new(group: group).stages }
......@@ -20,45 +25,53 @@
expect(distinct_stage_hash_ids).to eq(distinct_stage_hash_ids.uniq)
end
context 'when lead time and cycle time are not defined as stages' do
it 'returns in-memory stages' do
lead_time = distinct_stages.find { |stage| stage.name == 'lead time' }
cycle_time = distinct_stages.find { |stage| stage.name == 'cycle time' }
context 'when in-memory stages are not defined as stages', :aggregate_failures do
it 'creates three stage event hash records' do
expect { distinct_stages }.to change { Analytics::CycleAnalytics::StageEventHash.count }.by(3)
end
end
context 'when all in-memory stages have been defined' do
let(:lead_time) { distinct_stages.find { |stage| stage.name == lead_time_name } }
let(:cycle_time) { distinct_stages.find { |stage| stage.name == cycle_time_name } }
let(:time_to_merge) { distinct_stages.find { |stage| stage.name == time_to_merge_name } }
let(:in_memory_stages) { [lead_time, cycle_time, time_to_merge] }
expect(lead_time).to be_present
expect(cycle_time).to be_present
expect(lead_time.stage_event_hash_id).not_to be_nil
expect(cycle_time.stage_event_hash_id).not_to be_nil
it 'returns in-memory stages' do
# all should be present
expect(in_memory_stages.compact.count).to eq in_memory_stages_count
expect(lead_time.stage_event_hash_id).not_to eq(cycle_time.stage_event_hash_id)
# all should have unique stage event hash IDs
expect(in_memory_stages.map(&:stage_event_hash_id).count).to eq in_memory_stages_count
end
it 'creates two stage event hash records' do
expect { distinct_stages }.to change { Analytics::CycleAnalytics::StageEventHash.count }.by(2)
it 'has distinct values for all in-memory stages' do
expect(in_memory_stages.map(&:stage_event_hash_id).uniq.count).to eq in_memory_stages_count
end
it 'returns 4 stages' do
expect(distinct_stages.size).to eq(4)
it 'returns total number of stages - in-memory + persisted' do
expect(distinct_stages.size).to eq(in_memory_stages_count + 2)
end
end
context 'when lead time and cycle time are persisted stages' do
context 'when a subset of in-memory stages are already defined' do
let_it_be(:cycle_time) do
create(:cycle_analytics_stage,
namespace: group,
start_event_identifier: :issue_created,
end_event_identifier: :issue_first_associated_with_milestone)
start_event_identifier: Gitlab::Analytics::CycleAnalytics::Summary::LeadTime.start_event_identifier,
end_event_identifier: Gitlab::Analytics::CycleAnalytics::Summary::LeadTime.end_event_identifier)
end
let_it_be(:lead_tiem) do
let_it_be(:lead_time) do
create(:cycle_analytics_stage,
namespace: group,
start_event_identifier: :issue_created,
end_event_identifier: :issue_first_associated_with_milestone)
start_event_identifier: Gitlab::Analytics::CycleAnalytics::Summary::CycleTime.start_event_identifier,
end_event_identifier: Gitlab::Analytics::CycleAnalytics::Summary::CycleTime.end_event_identifier)
end
it 'does not create extra stage event hash records' do
expect { distinct_stages }.to change { Analytics::CycleAnalytics::StageEventHash.count }
# only creates time_to_merge because that hasn't been defined yet
expect { distinct_stages }.to change { Analytics::CycleAnalytics::StageEventHash.count }.by(1)
end
end
end
......@@ -13,7 +13,9 @@
let(:options) { { from: from, to: to, current_user: user } }
let(:stage) { Analytics::CycleAnalytics::Stage.new(namespace: group) }
subject { described_class.new(stage, options: options).data }
subject do
described_class.new(stage, options: options).data
end
around do |example|
freeze_time { example.run }
......@@ -23,20 +25,10 @@
group.add_owner(user)
end
describe '#identifier' do
it 'returns identifiers for each metric' do
identifiers = subject.pluck(:identifier)
expect(identifiers).to eq(%i[lead_time cycle_time])
end
end
context 'when the use_aggregated_data_collector option is given' do
context 'when aggregated data is available yet' do
context 'when aggregated data is not available yet' do
it 'shows no value' do
lead_time, cycle_time, * = subject
expect(lead_time[:value]).to eq('-')
expect(cycle_time[:value]).to eq('-')
expect_values(lead_time: '-', cycle_time: '-', time_to_merge: '-')
end
end
......@@ -45,21 +37,64 @@
issue = create(:closed_issue, project: project, created_at: 1.day.ago, closed_at: Time.current)
issue.metrics.update!(first_mentioned_in_commit_at: 2.days.ago)
merge_request = create(:merge_request, :merged, created_at: 5.days.ago, project: project)
merge_request.metrics.update!(merged_at: 1.day.ago)
options[:use_aggregated_data_collector] = true
stub_licensed_features(cycle_analytics_for_groups: true)
Analytics::CycleAnalytics::DataLoaderService.new(group: group, model: Issue).execute
Analytics::CycleAnalytics::DataLoaderService.new(group: group, model: model).execute
end
it 'loads the lead and cycle time' do
lead_time, cycle_time, * = subject
context 'when only Issue model is specified' do
let(:model) { Issue }
expect(lead_time[:value]).to eq('1.0')
expect(cycle_time[:value]).to eq('2.0')
it 'loads the lead time, cycle time and time to merge' do
# this only loads Issue models, so Time to Merge is not filled in.
expect_values(lead_time: '1.0', cycle_time: '2.0', time_to_merge: '-')
end
end
context 'when only MR model is specified' do
let(:model) { MergeRequest }
it 'shows time to merge' do
# this only shows MergeRequest models, so the first two are not filled in.
expect_values(lead_time: '-', cycle_time: '-', time_to_merge: '4.0')
end
end
context 'when some other model is specified' do
let(:model) { Epic }
it 'shows none of the values' do
expect_values(lead_time: '-', cycle_time: '-', time_to_merge: '-')
end
end
end
def expect_values(lead_time:, cycle_time:, time_to_merge:)
expect(subject.as_json).to contain_exactly(
a_hash_including({
"identifier" => 'lead_time',
"value" => lead_time
}),
a_hash_including({
"identifier" => 'cycle_time',
"value" => cycle_time
}),
a_hash_including({
"identifier" => 'time_to_merge',
"value" => time_to_merge
})
)
end
end
describe '#lead_time' do
let(:lead_time) do
subject.find { |result| result[:identifier] == :lead_time }
end
describe 'issuable filter parameters' do
let_it_be(:label) { create(:group_label, group: group) }
......@@ -73,7 +108,7 @@
end
it 'returns the correct lead time' do
expect(subject.first[:value]).to eq('1.0')
expect(lead_time[:value]).to eq('1.0')
end
end
......@@ -83,7 +118,7 @@
end
it 'returns `-`' do
expect(subject.first[:value]).to eq('-')
expect(lead_time[:value]).to eq('-')
end
end
......@@ -93,7 +128,7 @@
end
it 'returns the correct lead time' do
expect(subject.first[:value]).to eq('1.0')
expect(lead_time[:value]).to eq('1.0')
end
end
......@@ -103,7 +138,7 @@
end
it 'returns `-`' do
expect(subject.first[:value]).to eq('-')
expect(lead_time[:value]).to eq('-')
end
end
end
......@@ -118,7 +153,7 @@
end
it 'finds the lead time of issues created after it' do
expect(subject.first[:value]).to eq('2.0')
expect(lead_time[:value]).to eq('2.0')
end
context 'with subgroups' do
......@@ -131,7 +166,7 @@
end
it 'finds the lead time of issues from them' do
expect(subject.first[:value]).to eq('3.0')
expect(lead_time[:value]).to eq('3.0')
end
end
......@@ -144,7 +179,7 @@
it 'finds the lead time of issues from those projects' do
# Median of 1, 2, 4, not including new issue
expect(subject.first[:value]).to eq('2.0')
expect(lead_time[:value]).to eq('2.0')
end
end
......@@ -153,7 +188,7 @@
let(:to) { Time.zone.now }
it 'finds the lead time of issues from 3 days ago' do
expect(subject.first[:value]).to eq('1.5')
expect(lead_time[:value]).to eq('1.5')
end
end
end
......@@ -169,13 +204,18 @@
it 'does not find the lead time of issues from them' do
# Median of 2, 3, not including first issue
expect(subject.first[:value]).to eq('2.5')
expect(lead_time[:value]).to eq('2.5')
end
end
end
describe '#cycle_time' do
let(:created_at) { 6.days.ago }
let(:cycle_time) do
subject.find do |result|
result[:identifier] == :cycle_time
end
end
context 'with `from` date' do
let(:from) { 7.days.ago }
......@@ -191,7 +231,7 @@
end
it 'finds the cycle time of issues created after it' do
expect(subject.second[:value]).to eq('2.0')
expect(cycle_time[:value]).to eq('2.0')
end
context 'with subgroups' do
......@@ -207,7 +247,7 @@
end
it 'finds the cycle time of issues from them' do
expect(subject.second[:value]).to eq('3.0')
expect(cycle_time[:value]).to eq('3.0')
end
end
......@@ -221,7 +261,7 @@
it 'finds the cycle time of issues from those projects' do
# Median of 1, 2, 4, not including new issue
expect(subject.second[:value]).to eq('2.0')
expect(cycle_time[:value]).to eq('2.0')
end
end
......@@ -232,7 +272,7 @@
it 'finds the cycle time of issues created between `from` and `to`' do
# Median of 1, 2, 4
expect(subject.second[:value]).to eq('2.0')
expect(cycle_time[:value]).to eq('2.0')
end
end
end
......@@ -253,15 +293,101 @@
it 'does not find the cycle time of issues from them' do
# Median of 2, 3, not including first issue
expect(subject.second[:value]).to eq('2.5')
expect(cycle_time[:value]).to eq('2.5')
end
end
end
describe '#time_to_merge' do
let(:created_at) { 6.days.ago }
let(:time_to_merge) do
subject.find { |result| result[:identifier] == :time_to_merge }
end
context 'with `from` date' do
let(:from) { 7.days.ago }
before do
mr1 = create(:merge_request, :merged, project: project, created_at: created_at)
mr2 = create(:merge_request, :merged, project: project, created_at: created_at)
mr3 = create(:merge_request, :merged, project: project_2, created_at: created_at)
mr1.metrics.update!(merged_at: 1.day.ago)
mr2.metrics.update!(merged_at: 2.days.ago)
mr3.metrics.update!(merged_at: 4.days.ago)
end
it 'finds the time to merge of MRs created after it' do
expect(time_to_merge).to include({ value: '4.0', title: "Time to Merge", unit: "days" })
end
context 'with subgroups' do
let_it_be(:subgroup) { create(:group, parent: group) }
let_it_be(:project_3) { create(:project, namespace: subgroup) }
before do
mr4 = create(:merge_request, :merged, created_at: created_at, project: project_3)
mr5 = create(:merge_request, :merged, created_at: created_at, project: project_3)
mr4.metrics.update!(merged_at: 3.days.ago)
mr5.metrics.update!(merged_at: 5.days.ago)
end
it 'finds the time to merge of MRs from them' do
expect(time_to_merge).to include({ value: '3.0', title: "Time to Merge", unit: "days" })
end
end
context 'with projects specified in options' do
before do
mr4 = create(:merge_request, :merged, created_at: created_at, project: create(:project, namespace: group))
mr4.metrics.update!(merged_at: 3.days.ago)
end
subject { described_class.new(stage, options: { from: from, current_user: user, projects: [project.id, project_2.id] }).data }
it 'finds the time to merge of MRs from those projects' do
# Median of 1, 2, 4, not including new issue
expect(time_to_merge).to include({ value: '4.0', title: "Time to Merge", unit: "days" })
end
end
context 'when `from` and `to` parameters are provided' do
let(:from) { 5.days.ago }
let(:to) { 2.days.ago }
let(:created_at) { from }
it 'finds the time to merge of MRs created between `from` and `to`' do
expect(time_to_merge).to include({ value: '3.0', title: "Time to Merge", unit: "days" })
end
end
end
context 'with other projects' do
let(:from) { 4.days.ago }
let(:created_at) { from }
before do
mr1 = create(:merge_request, :merged, created_at: created_at, project: create(:project, namespace: create(:group)))
mr2 = create(:merge_request, :merged, created_at: created_at, project: project)
mr3 = create(:merge_request, :merged, created_at: created_at, project: project_2)
mr1.metrics.update!(merged_at: 1.day.ago)
mr2.metrics.update!(merged_at: 2.days.ago)
mr3.metrics.update!(merged_at: 3.days.ago)
end
it 'does not find the time to merge of MRs from them' do
# Median of 2, 3, not including first MR
expect(time_to_merge).to include({ value: '1.5', title: "Time to Merge", unit: "days" })
end
end
end
describe 'dora4 metrics' do
let(:lead_time_for_changes) { subject[2] }
let(:time_to_restore_service) { subject[3] }
let(:change_failure_rate) { subject[4] }
let(:lead_time_for_changes) { subject.find { |result| result[:identifier] == :lead_time_for_changes } }
let(:time_to_restore_service) { subject.find { |result| result[:identifier] == :time_to_restore_service } }
let(:change_failure_rate) { subject.find { |result| result[:identifier] == :change_failure_rate } }
before do
stub_licensed_features(dora4_analytics: true)
......@@ -284,7 +410,7 @@
end
it 'does not return any dora4 metrics' do
expect(subject.size).to eq 2
expect(subject.size).to eq 3
end
end
......
......@@ -55,19 +55,19 @@
describe '#time_summary' do
let(:issue) { create(:issue, project: project) }
let(:merge_request) { create(:merge_request, :merged, created_at: 9.days.ago, project: project) }
before do
# lead_time: 1 day, cycle_time: 2 days
# lead_time: 1 day, cycle_time: 2 days, time_to_merge: 8 days
issue.update!(created_at: 5.days.ago)
issue.metrics.update!(first_mentioned_in_commit_at: 4.days.ago)
issue.update!(closed_at: 3.days.ago)
merge_request.metrics.update!(merged_at: 1.day.ago)
end
it 'returns medians for lead time and cycle type' do
expect(subject.time_summary.map { |summary| summary[:value] }).to contain_exactly('1.0', '2.0')
it 'returns medians for lead time, cycle time and time to merge' do
expect(subject.time_summary.map { |summary| summary[:value] }).to contain_exactly('1.0', '2.0', '8.0')
end
end
end
......@@ -2,7 +2,7 @@
require 'spec_helper'
RSpec.describe Analytics::CycleAnalytics::ConsistencyCheckService, :aggregate_failures do
RSpec.describe Analytics::CycleAnalytics::ConsistencyCheckService, :aggregate_failures, feature_category: :value_stream_management do
let_it_be_with_refind(:group) { create(:group) }
let_it_be_with_refind(:subgroup) { create(:group, parent: group) }
......@@ -27,9 +27,11 @@
expect(service_response).to be_success
expect(service_response.payload[:reason]).to eq(:group_processed)
all_stage_events = event_model.all
expect(all_stage_events.size).to eq(1)
expect(all_stage_events.first[event_model.issuable_id_column]).to eq(record2.id)
# stage events where the end criteria are not met are excluded.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/408320
target_stage_events = event_model.where.not(end_event_timestamp: nil)
expect(target_stage_events.size).to eq(1)
expect(target_stage_events.first[event_model.issuable_id_column]).to eq(record2.id)
end
context 'when running out of allotted time' do
......@@ -69,9 +71,11 @@
}
end
all_stage_events = event_model.all
expect(all_stage_events.size).to eq(1)
expect(all_stage_events.first[event_model.issuable_id_column]).to eq(record2.id)
# stage events where the end criteria are not met are excluded.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/408320
target_stage_events = event_model.where.not(end_event_timestamp: nil)
expect(target_stage_events.size).to eq(1)
expect(target_stage_events.first[event_model.issuable_id_column]).to eq(record2.id)
end
end
end
......
......@@ -46613,6 +46613,9 @@ msgstr ""
msgid "Time spent must be formatted correctly. For example: 1h 30m."
msgstr ""
 
msgid "Time to Merge"
msgstr ""
msgid "Time to Restore Service"
msgstr ""
 
......@@ -49292,6 +49295,9 @@ msgstr ""
msgid "ValueStreamAnalytics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed."
msgstr ""
 
msgid "ValueStreamAnalytics|Merge request analytics"
msgstr ""
msgid "ValueStreamAnalytics|New Value Stream"
msgstr ""
 
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment