Skip to content
Snippets Groups Projects
Commit 8ad412e0 authored by Stan Hu's avatar Stan Hu Committed by Ruben Davila
Browse files

Merge branch '21170-cycle-analytics' into 'master'

Cycle Analytics: first iteration

## What does this MR do?

- Implement the first iteration of the "Cycle Analytics" feature.

## What are the relevant issue numbers?

- Closes #21170 

## Screenshots

![cycle_analytics_screencast.gif](/uploads/d23c3c912caa6935fd47b53ca3a56b97/cycle_analytics.gif)

## Backend Tasks

- [x]  Implementation
    - [x]  Phases
        - [x]  Issue (Tracker)
        - [x]  Plan (Board)
        - [x]  Code (IDE)
        - [x]  Test (CI)
        - [x]  Review (MR)
        - [x]  Staging (CD)
        - [x]  Production (Total)
    - [x]  Make heuristics more modular
    - [x]  Scope to project
    - [x]  Date range (30 days, 90 days)
    - [x]  Access restriction
- [x]  Test
    - [x]  Find a better way to test these phases
    - [x]  Phases
        - [x]  Issue (Tracker)
        - [x]  Plan (Board)
        - [x]  Code (IDE)
        - [x]  Test (CI)
        - [x]  Review (MR)
        - [x]  Staging (CD)
        - [x]  Production (Total)
    - [x]  Test for "end case happens before start case"
    - [x]  Consolidate helper
- [x]  Miniboss review
- [x]  Performance testing with mock data
- [x]  Improve performance
    - [x]  Pre-calculate "merge requests closing issues
    - [x]  Pre-calculate everything else
- [x]  Test performance against 10k issues
- [x]  Test all pre-calculation code
    - [x]  Ci::Pipeline -> build start/finish
    - [x]  Ci::Pipeline#merge_requests
    - [x]  Issue -> record default metrics after save
    - [x]  MergeRequest -> record default metrics after save
    - [x]  Deployment -> Update "first_deployed_to_production_at" for MR metrics
    - [x]  Git Push -> Update "first commit mention" for issue metrics
    - [x]  Merge request create/update/refresh -> Update "merge requests closing issues"
- [x]  Remove `MergeRequestsClosingIssues` when necessary
- [x]  Changes to unblock Fatih
    - [x]  Add summary data
    - [x]  `stats` should be array
    - [x]  Let `stats` be `null` if all `stats` are null
- [x]  Indexes for "merge requests closing issues"
- [x]  Test summary data
- [x]  Scope everything to project
    - [x]  Find out why tests were passing
- [x]  Filter should include issues/MRs which have made it to production within the range
- [x]  Don't create duplicate `MergeRequestsClosingIssues`
- [x]  Fix tests
- [x]  MySQL median
- [x]  Assign to Douwe for review
- [x]  Fix conflicts
- [x]  Implement suggestions from Yorick's review
    - [x]  Test on PG
    - [x]  Test on MySQL
- [x]  Refactor
    - [x]  Cleanup
        - [x]  What happens if we have no data at all?
        - [x]  Extract common queries to methods / scopes
    - [x]  Remove unused queries
    - [x]  Downtime for foreign key migrations
    - [x]  Find a way around "if issue.metrics.present?" all over the place
    - [x]  Find a way around "if merge_request.metrics.present?" all over the place
    - [x]  Test migrations on a fresh database
        - [x]  MySQL
        - [x]  Pg
- [x]  Access issues
    - While the project is public and the visibility is set to "Everyone with access", you cannot visit the cycle analytics page when signed out.
- [x]  CHANGELOG
- [x]  Implement suggestions from Douwe's review
    - [x]  First set of comments
    - [x]  Second set of comments
    - [x]  Third set of comments
    - [x]  Fourth set of comments
- [x]  Make sure build is green
- [ ]  Make issue for "polish"
- [ ]  EE MR


See merge request !5986
parent 84e6b80b
No related branches found
No related tags found
1 merge request!8889WIP: Port of 25624-anticipate-obstacles-to-removing-turbolinks to EE.
Showing
with 545 additions and 1 deletion
Loading
Loading
@@ -22,6 +22,7 @@ v 8.12.0 (unreleased)
- Pass the "Remember me" value to the U2F authentication form
- Display stages in valid order in stages dropdown on build page
- Only update projects.last_activity_at once per hour when creating a new event
- Cycle analytics (first iteration) !5986
- Remove vendor prefixes for linear-gradient CSS (ClemMakesApps)
- Move pushes_since_gc from the database to Redis
- Add font color contrast to external label in admin area (ClemMakesApps)
Loading
Loading
Loading
Loading
@@ -320,6 +320,7 @@ group :test do
gem 'webmock', '~> 1.21.0'
gem 'test_after_commit', '~> 0.4.2'
gem 'sham_rack', '~> 1.3.6'
gem 'timecop', '~> 0.8.0'
end
 
group :production do
Loading
Loading
Loading
Loading
@@ -980,6 +980,7 @@ DEPENDENCIES
teaspoon-jasmine (~> 2.2.0)
test_after_commit (~> 0.4.2)
thin (~> 1.7.0)
timecop (~> 0.8.0)
turbolinks (~> 2.5.0)
u2f (~> 0.2.1)
uglifier (~> 2.7.2)
Loading
Loading
((global) => {
const COOKIE_NAME = 'cycle_analytics_help_dismissed';
gl.CycleAnalytics = class CycleAnalytics {
constructor() {
const that = this;
this.isHelpDismissed = $.cookie(COOKIE_NAME);
this.vue = new Vue({
el: '#cycle-analytics',
name: 'CycleAnalytics',
created: this.fetchData(),
data: this.decorateData({ isLoading: true }),
methods: {
dismissLanding() {
that.dismissLanding();
}
}
});
}
fetchData(options) {
options = options || { startDate: 30 };
$.ajax({
url: $('#cycle-analytics').data('request-path'),
method: 'GET',
dataType: 'json',
contentType: 'application/json',
data: { start_date: options.startDate }
}).done((data) => {
this.vue.$data = this.decorateData(data);
this.initDropdown();
})
.error((data) => {
this.handleError(data);
})
.always(() => {
this.vue.isLoading = false;
})
}
decorateData(data) {
data.summary = data.summary || [];
data.stats = data.stats || [];
data.isHelpDismissed = this.isHelpDismissed;
data.isLoading = data.isLoading || false;
data.summary.forEach((item) => {
item.value = item.value || '-';
});
data.stats.forEach((item) => {
item.value = item.value || '- - -';
})
return data;
}
handleError(data) {
this.vue.$data = {
hasError: true,
isHelpDismissed: this.isHelpDismissed
};
new Flash('There was an error while fetching cycle analytics data.', 'alert');
}
dismissLanding() {
this.vue.isHelpDismissed = true;
$.cookie(COOKIE_NAME, true);
}
initDropdown() {
const $dropdown = $('.js-ca-dropdown');
const $label = $dropdown.find('.dropdown-label');
$dropdown.find('li a').off('click').on('click', (e) => {
e.preventDefault();
const $target = $(e.currentTarget);
const value = $target.data('value');
$label.text($target.text().trim());
this.vue.isLoading = true;
this.fetchData({ startDate: value });
})
}
}
})(window.gl || (window.gl = {}));
Loading
Loading
@@ -94,6 +94,11 @@
break;
case "projects:merge_requests:conflicts":
window.mcui = new MergeConflictResolver()
break;
case 'projects:merge_requests:index':
shortcut_handler = new ShortcutsNavigation();
Issuable.init();
break;
case 'dashboard:activity':
new Activities();
break;
Loading
Loading
@@ -185,6 +190,9 @@
new gl.ProtectedBranchCreate();
new gl.ProtectedBranchEditList();
break;
case 'projects:cycle_analytics:show':
new gl.CycleAnalytics();
break;
}
switch (path.first()) {
case 'admin':
Loading
Loading
#cycle-analytics {
margin: 24px auto 0;
width: 800px;
position: relative;
.panel {
.content-block {
padding: 24px 0;
border-bottom: none;
position: relative;
}
.column {
text-align: center;
.header {
font-size: 30px;
line-height: 38px;
font-weight: normal;
margin: 0;
}
.text {
color: $layout-link-gray;
margin: 0;
}
&:last-child {
text-align: right;
}
}
.dropdown {
position: relative;
top: 13px;
}
}
.bordered-box {
border: 1px solid $border-color;
@include border-radius($border-radius-default);
position: relative;
}
.content-list {
li {
padding: 18px $gl-padding $gl-padding;
.container-fluid {
padding: 0;
}
}
.title-col {
p {
margin: 0;
&.title {
line-height: 19px;
font-size: 15px;
font-weight: 600;
}
&:text {
color: #8c8c8c;
}
}
}
.value-col {
text-align: right;
span {
line-height: 42px;
}
}
}
.landing {
margin-bottom: $gl-padding;
overflow: hidden;
.dismiss-icon {
position: absolute;
right: $gl-padding;
cursor: pointer;
color: #b2b2b2;
}
svg {
margin: 0 20px;
float: left;
width: 136px;
height: 136px;
}
.inner-content {
width: 480px;
float: left;
h4 {
color: $gl-text-color;
font-size: 17px;
}
p {
color: #8c8c8c;
margin-bottom: $gl-padding;
}
}
}
.fa-spinner {
font-size: 28px;
position: relative;
margin-left: -20px;
left: 50%;
margin-top: 36px;
}
}
class Projects::CycleAnalyticsController < Projects::ApplicationController
include ActionView::Helpers::DateHelper
include ActionView::Helpers::TextHelper
before_action :authorize_read_cycle_analytics!
def show
@cycle_analytics = CycleAnalytics.new(@project, from: parse_start_date)
respond_to do |format|
format.html
format.json { render json: cycle_analytics_json }
end
end
private
def parse_start_date
case cycle_analytics_params[:start_date]
when '30' then 30.days.ago
when '90' then 90.days.ago
else 90.days.ago
end
end
def cycle_analytics_params
return {} unless params[:cycle_analytics].present?
{ start_date: params[:cycle_analytics][:start_date] }
end
def cycle_analytics_json
cycle_analytics_view_data = [[:issue, "Issue", "Time before an issue gets scheduled"],
[:plan, "Plan", "Time before an issue starts implementation"],
[:code, "Code", "Time until first merge request"],
[:test, "Test", "Total test time for all commits/merges"],
[:review, "Review", "Time between merge request creation and merge/close"],
[:staging, "Staging", "From merge request merge until deploy to production"],
[:production, "Production", "From issue creation until deploy to production"]]
stats = cycle_analytics_view_data.reduce([]) do |stats, (stage_method, stage_text, stage_description)|
value = @cycle_analytics.send(stage_method).presence
stats << {
title: stage_text,
description: stage_description,
value: value && !value.zero? ? distance_of_time_in_words(value) : nil
}
stats
end
issues = @cycle_analytics.summary.new_issues
commits = @cycle_analytics.summary.commits
deploys = @cycle_analytics.summary.deploys
summary = [
{ title: "New Issue".pluralize(issues), value: issues },
{ title: "Commit".pluralize(commits), value: commits },
{ title: "Deploy".pluralize(deploys), value: deploys }
]
{
summary: summary,
stats: stats
}
end
end
Loading
Loading
@@ -46,6 +46,10 @@ module GitlabRoutingHelper
namespace_project_environments_path(project.namespace, project, *args)
end
 
def project_cycle_analytics_path(project, *args)
namespace_project_cycle_analytics_path(project.namespace, project, *args)
end
def project_builds_path(project, *args)
namespace_project_builds_path(project.namespace, project, *args)
end
Loading
Loading
Loading
Loading
@@ -56,6 +56,16 @@ module Ci
pipeline.finished_at = Time.now
end
 
after_transition [:created, :pending] => :running do |pipeline|
MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)).
update_all(latest_build_started_at: pipeline.started_at, latest_build_finished_at: nil)
end
after_transition any => [:success] do |pipeline|
MergeRequest::Metrics.where(merge_request_id: pipeline.merge_requests.map(&:id)).
update_all(latest_build_finished_at: pipeline.finished_at)
end
before_transition do |pipeline|
pipeline.update_duration
end
Loading
Loading
@@ -280,6 +290,16 @@ module Ci
project.execute_services(data, :pipeline_hooks)
end
 
# Merge requests for which the current pipeline is running against
# the merge request's latest commit.
def merge_requests
@merge_requests ||=
begin
project.merge_requests.where(source_branch: self.ref).
select { |merge_request| merge_request.pipeline.try(:id) == self.id }
end
end
private
 
def pipeline_data
Loading
Loading
Loading
Loading
@@ -28,10 +28,13 @@ module Issuable
loaded? && to_a.all? { |note| note.association(:award_emoji).loaded? }
end
end
has_many :label_links, as: :target, dependent: :destroy
has_many :labels, through: :label_links
has_many :todos, as: :target, dependent: :destroy
 
has_one :metrics
validates :author, presence: true
validates :title, presence: true, length: { within: 0..255 }
 
Loading
Loading
@@ -81,6 +84,7 @@ module Issuable
acts_as_paranoid
 
after_save :update_assignee_cache_counts, if: :assignee_id_changed?
after_save :record_metrics
 
def update_assignee_cache_counts
# make sure we flush the cache for both the old *and* new assignee
Loading
Loading
@@ -286,4 +290,9 @@ module Issuable
def can_move?(*)
false
end
def record_metrics
metrics = self.metrics || create_metrics
metrics.record!
end
end
class CycleAnalytics
include Gitlab::Database::Median
include Gitlab::Database::DateTime
def initialize(project, from:)
@project = project
@from = from
end
def summary
@summary ||= Summary.new(@project, from: @from)
end
def issue
calculate_metric(:issue,
Issue.arel_table[:created_at],
[Issue::Metrics.arel_table[:first_associated_with_milestone_at],
Issue::Metrics.arel_table[:first_added_to_board_at]])
end
def plan
calculate_metric(:plan,
[Issue::Metrics.arel_table[:first_associated_with_milestone_at],
Issue::Metrics.arel_table[:first_added_to_board_at]],
Issue::Metrics.arel_table[:first_mentioned_in_commit_at])
end
def code
calculate_metric(:code,
Issue::Metrics.arel_table[:first_mentioned_in_commit_at],
MergeRequest.arel_table[:created_at])
end
def test
calculate_metric(:test,
MergeRequest::Metrics.arel_table[:latest_build_started_at],
MergeRequest::Metrics.arel_table[:latest_build_finished_at])
end
def review
calculate_metric(:review,
MergeRequest.arel_table[:created_at],
MergeRequest::Metrics.arel_table[:merged_at])
end
def staging
calculate_metric(:staging,
MergeRequest::Metrics.arel_table[:merged_at],
MergeRequest::Metrics.arel_table[:first_deployed_to_production_at])
end
def production
calculate_metric(:production,
Issue.arel_table[:created_at],
MergeRequest::Metrics.arel_table[:first_deployed_to_production_at])
end
private
def calculate_metric(name, start_time_attrs, end_time_attrs)
cte_table = Arel::Table.new("cte_table_for_#{name}")
# Build a `SELECT` query. We find the first of the `end_time_attrs` that isn't `NULL` (call this end_time).
# Next, we find the first of the start_time_attrs that isn't `NULL` (call this start_time).
# We compute the (end_time - start_time) interval, and give it an alias based on the current
# cycle analytics stage.
interval_query = Arel::Nodes::As.new(
cte_table,
subtract_datetimes(base_query, end_time_attrs, start_time_attrs, name.to_s))
median_datetime(cte_table, interval_query, name)
end
# Join table with a row for every <issue,merge_request> pair (where the merge request
# closes the given issue) with issue and merge request metrics included. The metrics
# are loaded with an inner join, so issues / merge requests without metrics are
# automatically excluded.
def base_query
arel_table = MergeRequestsClosingIssues.arel_table
# Load issues
query = arel_table.join(Issue.arel_table).on(Issue.arel_table[:id].eq(arel_table[:issue_id])).
join(Issue::Metrics.arel_table).on(Issue.arel_table[:id].eq(Issue::Metrics.arel_table[:issue_id])).
where(Issue.arel_table[:project_id].eq(@project.id)).
where(Issue.arel_table[:deleted_at].eq(nil)).
where(Issue.arel_table[:created_at].gteq(@from))
# Load merge_requests
query = query.join(MergeRequest.arel_table, Arel::Nodes::OuterJoin).
on(MergeRequest.arel_table[:id].eq(arel_table[:merge_request_id])).
join(MergeRequest::Metrics.arel_table).
on(MergeRequest.arel_table[:id].eq(MergeRequest::Metrics.arel_table[:merge_request_id]))
# Limit to merge requests that have been deployed to production after `@from`
query.where(MergeRequest::Metrics.arel_table[:first_deployed_to_production_at].gteq(@from))
end
end
class CycleAnalytics
class Summary
def initialize(project, from:)
@project = project
@from = from
end
def new_issues
@project.issues.created_after(@from).count
end
def commits
repository = @project.repository.raw_repository
if @project.default_branch
repository.log(ref: @project.default_branch, after: @from).count
end
end
def deploys
@project.deployments.where("created_at > ?", @from).count
end
end
end
Loading
Loading
@@ -42,4 +42,38 @@ class Deployment < ActiveRecord::Base
 
project.repository.is_ancestor?(commit.id, sha)
end
def update_merge_request_metrics!
return unless environment.update_merge_request_metrics?
merge_requests = project.merge_requests.
joins(:metrics).
where(target_branch: self.ref, merge_request_metrics: { first_deployed_to_production_at: nil }).
where("merge_request_metrics.merged_at <= ?", self.created_at)
if previous_deployment
merge_requests = merge_requests.where("merge_request_metrics.merged_at >= ?", previous_deployment.created_at)
end
# Need to use `map` instead of `select` because MySQL doesn't allow `SELECT`ing from the same table
# that we're updating.
merge_request_ids =
if Gitlab::Database.postgresql?
merge_requests.select(:id)
elsif Gitlab::Database.mysql?
merge_requests.map(&:id)
end
MergeRequest::Metrics.
where(merge_request_id: merge_request_ids, first_deployed_to_production_at: nil).
update_all(first_deployed_to_production_at: self.created_at)
end
def previous_deployment
@previous_deployment ||=
project.deployments.joins(:environment).
where(environments: { name: self.environment.name }, ref: self.ref).
where.not(id: self.id).
take
end
end
Loading
Loading
@@ -43,4 +43,8 @@ class Environment < ActiveRecord::Base
 
last_deployment.includes_commit?(commit)
end
def update_merge_request_metrics?
self.name == "production"
end
end
Loading
Loading
@@ -23,6 +23,8 @@ class Issue < ActiveRecord::Base
 
has_many :events, as: :target, dependent: :destroy
 
has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
validates :project, presence: true
 
scope :cared, ->(user) { where(assignee_id: user) }
Loading
Loading
@@ -36,6 +38,8 @@ class Issue < ActiveRecord::Base
scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') }
scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') }
 
scope :created_after, -> (datetime) { where("created_at >= ?", datetime) }
attr_spammable :title, spam_title: true
attr_spammable :description, spam_description: true
 
Loading
Loading
class Issue::Metrics < ActiveRecord::Base
belongs_to :issue
def record!
if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank?
self.first_associated_with_milestone_at = Time.now
end
if issue_assigned_to_list_label? && self.first_added_to_board_at.blank?
self.first_added_to_board_at = Time.now
end
self.save
end
private
def issue_assigned_to_list_label?
issue.labels.any? { |label| label.lists.present? }
end
end
Loading
Loading
@@ -16,6 +16,8 @@ class MergeRequest < ActiveRecord::Base
 
has_many :events, as: :target, dependent: :destroy
 
has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
serialize :merge_params, Hash
 
after_create :ensure_merge_request_diff, unless: :importing?
Loading
Loading
@@ -501,6 +503,19 @@ class MergeRequest < ActiveRecord::Base
target_project
end
 
# If the merge request closes any issues, save this information in the
# `MergeRequestsClosingIssues` model. This is a performance optimization.
# Calculating this information for a number of merge requests requires
# running `ReferenceExtractor` on each of them separately.
def cache_merge_request_closes_issues!(current_user = self.author)
transaction do
self.merge_requests_closing_issues.delete_all
closes_issues(current_user).each do |issue|
self.merge_requests_closing_issues.create!(issue: issue)
end
end
end
def closes_issue?(issue)
closes_issues.include?(issue)
end
Loading
Loading
@@ -508,7 +523,8 @@ class MergeRequest < ActiveRecord::Base
# Return the set of issues that will be closed if this merge request is accepted.
def closes_issues(current_user = self.author)
if target_branch == project.default_branch
messages = commits.map(&:safe_message) << description
messages = [description]
messages.concat(commits.map(&:safe_message)) if merge_request_diff
 
Gitlab::ClosingIssueExtractor.new(project, current_user).
closed_by_message(messages.join("\n"))
Loading
Loading
class MergeRequest::Metrics < ActiveRecord::Base
belongs_to :merge_request
def record!
if merge_request.merged? && self.merged_at.blank?
self.merged_at = Time.now
end
self.save
end
end
class MergeRequestsClosingIssues < ActiveRecord::Base
belongs_to :merge_request
belongs_to :issue
validates :merge_request_id, uniqueness: { scope: :issue_id }, presence: true
validates :issue_id, presence: true
end
Loading
Loading
@@ -46,6 +46,7 @@ class ProjectPolicy < BasePolicy
can! :create_issue
can! :create_note
can! :upload_file
can! :read_cycle_analytics
end
 
def reporter_access!
Loading
Loading
@@ -204,6 +205,7 @@ class ProjectPolicy < BasePolicy
can! :read_commit_status
can! :read_container_image
can! :download_code
can! :read_cycle_analytics
 
# NOTE: may be overridden by IssuePolicy
can! :read_issue
Loading
Loading
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