Merge branch '299096-project-dora-metrics-api-support-time-to-restore-service' into 'master'

Add Time to Restore Service DORA metric

See merge request gitlab-org/gitlab!82510

db/migrate/20220309100648_add_time_to_restore_service_dora_metric.rb

+# frozen_string_literal: true
+
+class AddTimeToRestoreServiceDoraMetric < Gitlab::Database::Migration[1.0]
+  def change
+    add_column :dora_daily_metrics, :time_to_restore_service_in_seconds, :integer
+  end
+end

db/post_migrate/20220309154855_add_index_on_issues_closed_incidents.rb

+# frozen_string_literal: true
+
+class AddIndexOnIssuesClosedIncidents < Gitlab::Database::Migration[1.0]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_on_issues_closed_incidents_by_project_id_and_closed_at'
+
+  def up
+    add_concurrent_index :issues, [:project_id, :closed_at], where: "issue_type = 1 AND state_id = 2", name: INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :issues, INDEX_NAME
+  end
+end

db/schema_migrations/20220309100648

+3385dc0dc2a3d306e01a719b7a21197ea8468976d37abab932beade4780bb4ff
\ No newline at end of file

db/schema_migrations/20220309154855

+9e62675366f9c2f0fc159a9748409dbcaea240c813ab19ea26d24c966e5fd6c8
\ No newline at end of file

db/structure.sql

@@ -14461,6 +14461,7 @@ CREATE TABLE dora_daily_metrics (
     date date NOT NULL,
     deployment_frequency integer,
     lead_time_for_changes_in_seconds integer,
+    time_to_restore_service_in_seconds integer,
     CONSTRAINT dora_daily_metrics_deployment_frequency_positive CHECK ((deployment_frequency >= 0)),
     CONSTRAINT dora_daily_metrics_lead_time_for_changes_in_seconds_positive CHECK ((lead_time_for_changes_in_seconds >= 0))
 );
@@ -28291,6 +28292,8 @@ CREATE INDEX index_on_identities_lower_extern_uid_and_provider ON identities USI
 
 CREATE UNIQUE INDEX index_on_instance_statistics_recorded_at_and_identifier ON analytics_usage_trends_measurements USING btree (identifier, recorded_at);
 
+CREATE INDEX index_on_issues_closed_incidents_by_project_id_and_closed_at ON issues USING btree (project_id, closed_at) WHERE ((issue_type = 1) AND (state_id = 2));
+
 CREATE INDEX index_on_label_links_all_columns ON label_links USING btree (target_id, label_id, target_type);
 
 CREATE INDEX index_on_merge_request_assignees_state ON merge_request_assignees USING btree (state) WHERE (state = 2);

doc/api/dora/metrics.md

@@ -9,6 +9,7 @@ type: reference, api
 
 > - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/279039) in GitLab 13.10.
 > - The legacy key/value pair `{ "<date>" => "<value>" }` was removed from the payload in GitLab 14.0.
+> `time_to_restore_service` metric was introduced in GitLab 14.9.
 
 All methods require at least the Reporter role.
 
@@ -20,14 +21,14 @@ Get project-level DORA metrics.
 GET /projects/:id/dora/metrics
 ```
 
-| Attribute          | Type           | Required | Description                      |
-|--------------      |--------        |----------|-----------------------           |
-| `id`               | integer/string | yes      | The ID or [URL-encoded path of the project](../index.md#namespaced-path-encoding) can be accessed by the authenticated user. |
-| `metric`           | string         | yes      | The [metric name](../../user/analytics/ci_cd_analytics.md#supported-metrics-in-gitlab). One of `deployment_frequency` or `lead_time_for_changes`.  |
-| `start_date`       | string         | no       | Date range to start from. ISO 8601 Date format, for example `2021-03-01`. Default is 3 months ago. |
-| `end_date`         | string         | no       | Date range to end at. ISO 8601 Date format, for example `2021-03-01`. Default is the current date. |
-| `interval`         | string         | no       | The bucketing interval. One of `all`, `monthly` or `daily`. Default is `daily`.   |
-| `environment_tier` | string         | no       | The [tier of the environment](../../ci/environments/index.md#deployment-tier-of-environments). Default is `production`.                     |
+| Attribute          | Type           | Required | Description                                                                                                                                                                 |
+|--------------      |--------        |----------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| `id`               | integer/string | yes      | The ID or [URL-encoded path of the project](../index.md#namespaced-path-encoding) can be accessed by the authenticated user.                                                |
+| `metric`           | string         | yes      | The [metric name](../../user/analytics/ci_cd_analytics.md#supported-metrics-in-gitlab). One of `deployment_frequency`, `lead_time_for_changes` or `time_to_restore_service`.|
+| `start_date`       | string         | no       | Date range to start from. ISO 8601 Date format, for example `2021-03-01`. Default is 3 months ago.                                                                          |
+| `end_date`         | string         | no       | Date range to end at. ISO 8601 Date format, for example `2021-03-01`. Default is the current date.                                                                          |
+| `interval`         | string         | no       | The bucketing interval. One of `all`, `monthly` or `daily`. Default is `daily`.                                                                                             |
+| `environment_tier` | string         | no       | The [tier of the environment](../../ci/environments/index.md#deployment-tier-of-environments). Default is `production`.                                                     |
 
 Example request:
 
@@ -63,7 +64,7 @@ GET /groups/:id/dora/metrics
 | Attribute          | Type           | Required | Description                      |
 |--------------      |--------        |----------|-----------------------           |
 | `id`               | integer/string | yes      | The ID or [URL-encoded path of the project](../index.md#namespaced-path-encoding) can be accessed by the authenticated user. |
-| `metric`           | string         | yes      | The [metric name](../../user/analytics/ci_cd_analytics.md#supported-metrics-in-gitlab). One of `deployment_frequency` or `lead_time_for_changes`.  |
+| `metric`           | string         | yes      | The [metric name](../../user/analytics/ci_cd_analytics.md#supported-metrics-in-gitlab). One of `deployment_frequency`, `lead_time_for_changes` or `time_to_restore_service`. |
 | `start_date`       | string         | no       | Date range to start from. ISO 8601 Date format, for example `2021-03-01`. Default is 3 months ago. |
 | `end_date`         | string         | no       | Date range to end at. ISO 8601 Date format, for example `2021-03-01`. Default is the current date. |
 | `interval`         | string         | no       | The bucketing interval. One of `all`, `monthly` or `daily`. Default is `daily`.   |
@@ -97,6 +98,7 @@ API response has a different meaning depending on the provided `metric` query
 parameter:
 
 | `metric` query parameter | Description of `value` in response                                                                                                                           |
-| ------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------ |
+| ------------------------ |--------------------------------------------------------------------------------------------------------------------------------------------------------------|
 | `deployment_frequency`   | The number of successful deployments during the time period.                                                                                                 |
 | `lead_time_for_changes`  | The median number of seconds between the merge of the merge request (MR) and the deployment of the MR's commits for all MRs deployed during the time period. |
+| `time_to_restore_service`  | The median number of seconds an incident was open during the time period. Available only for production environment                                          |

doc/api/graphql/reference/index.md

@@ -18098,6 +18098,7 @@ All supported DORA metric types.
 | ----- | ----------- |
 | <a id="dorametrictypedeployment_frequency"></a>`DEPLOYMENT_FREQUENCY` | Deployment frequency. |
 | <a id="dorametrictypelead_time_for_changes"></a>`LEAD_TIME_FOR_CHANGES` | Lead time for changes. |
+| <a id="dorametrictypetime_to_restore_service"></a>`TIME_TO_RESTORE_SERVICE` | Time to restore service. |
 
 ### `EntryType`
 

ee/app/graphql/types/dora_metric_type_enum.rb

@@ -5,7 +5,8 @@ module Types
     graphql_name 'DoraMetricType'
     description 'All supported DORA metric types.'
 
-    value 'DEPLOYMENT_FREQUENCY', description: 'Deployment frequency.', value: 'deployment_frequency'
-    value 'LEAD_TIME_FOR_CHANGES', description: 'Lead time for changes.', value: 'lead_time_for_changes'
+    value 'DEPLOYMENT_FREQUENCY', description: 'Deployment frequency.', value: Dora::DailyMetrics::METRIC_DEPLOYMENT_FREQUENCY
+    value 'LEAD_TIME_FOR_CHANGES', description: 'Lead time for changes.', value: Dora::DailyMetrics::METRIC_LEAD_TIME_FOR_CHANGES
+    value 'TIME_TO_RESTORE_SERVICE', description: 'Time to restore service.', value: Dora::DailyMetrics::METRIC_TIME_TO_RESTORE_SERVICE
   end
 end

ee/app/models/dora/daily_metrics.rb

@@ -15,7 +15,8 @@ module Dora
     INTERVAL_DAILY = 'daily'
     METRIC_DEPLOYMENT_FREQUENCY = 'deployment_frequency'
     METRIC_LEAD_TIME_FOR_CHANGES = 'lead_time_for_changes'
-    AVAILABLE_METRICS = [METRIC_DEPLOYMENT_FREQUENCY, METRIC_LEAD_TIME_FOR_CHANGES].freeze
+    METRIC_TIME_TO_RESTORE_SERVICE = 'time_to_restore_service'
+    AVAILABLE_METRICS = [METRIC_DEPLOYMENT_FREQUENCY, METRIC_LEAD_TIME_FOR_CHANGES, METRIC_TIME_TO_RESTORE_SERVICE].freeze
     AVAILABLE_INTERVALS = [INTERVAL_ALL, INTERVAL_MONTHLY, INTERVAL_DAILY].freeze
 
     scope :for_environments, -> (environments) do
@@ -32,6 +33,7 @@ module Dora
 
         deployment_frequency = deployment_frequency(environment, date)
         lead_time_for_changes = lead_time_for_changes(environment, date)
+        time_to_restore_service = time_to_restore_service(environment, date)
 
         # This query is concurrent safe upsert with the unique index.
         connection.execute(<<~SQL)
@@ -39,18 +41,21 @@ module Dora
             environment_id,
             date,
             deployment_frequency,
-            lead_time_for_changes_in_seconds
+            lead_time_for_changes_in_seconds,
+            time_to_restore_service_in_seconds
           )
           VALUES (
             #{environment.id},
             #{connection.quote(date.to_s)},
             (#{deployment_frequency}),
-            (#{lead_time_for_changes})
+            (#{lead_time_for_changes}),
+            (#{time_to_restore_service})
           )
           ON CONFLICT (environment_id, date)
           DO UPDATE SET
             deployment_frequency = (#{deployment_frequency}),
-            lead_time_for_changes_in_seconds = (#{lead_time_for_changes})
+            lead_time_for_changes_in_seconds = (#{lead_time_for_changes}),
+            time_to_restore_service_in_seconds = (#{time_to_restore_service})
         SQL
       end
 
@@ -84,6 +89,9 @@ module Dora
         when METRIC_LEAD_TIME_FOR_CHANGES
           # Median
           '(PERCENTILE_CONT(0.5) WITHIN GROUP(ORDER BY lead_time_for_changes_in_seconds)) AS data'
+        when METRIC_TIME_TO_RESTORE_SERVICE
+          # Median
+          '(PERCENTILE_CONT(0.5) WITHIN GROUP(ORDER BY time_to_restore_service_in_seconds)) AS data'
         else
           raise ArgumentError, 'Unknown metric'
         end
@@ -129,6 +137,20 @@ module Dora
          deployments[:finished_at].lteq(date.end_of_day),
          deployments[:status].eq(Deployment.statuses[:success])].reduce(&:and)
       end
+
+      def time_to_restore_service(environment, date)
+        # Non-production environments are ignored as we assume all Incidents happen on production
+        # See https://gitlab.com/gitlab-org/gitlab/-/issues/299096#note_550275633 for details
+        return Arel.sql('NULL') unless environment.production?
+
+        Issue.incident.closed.select(
+          Arel.sql(
+            'PERCENTILE_CONT(0.5) WITHIN GROUP(ORDER BY EXTRACT(EPOCH FROM (issues.closed_at - issues.created_at)))'
+          )
+        ).where("closed_at >= ? AND closed_at <= ?", date.beginning_of_day, date.end_of_day)
+          .where(project_id: environment.project_id)
+          .to_sql
+      end
     end
   end
 end

ee/app/models/ee/issue.rb

@@ -82,6 +82,18 @@ module EE
         after_transition do |issue|
           issue.refresh_blocking_and_blocked_issues_cache!
         end
+
+        after_transition any => :closed do |issue|
+          next unless issue.incident?
+
+          related_production_env = issue.project.environments.production.first
+
+          next unless related_production_env
+
+          issue.run_after_commit do
+            ::Dora::DailyMetrics::RefreshWorker.perform_async(related_production_env.id, issue.closed_at.to_date.to_s)
+          end
+        end
       end
     end
 

ee/spec/graphql/types/dora_metric_type_enum_spec.rb

@@ -6,7 +6,8 @@ RSpec.describe Types::DoraMetricTypeEnum do
   it 'includes a value for each DORA metric type' do
     expect(described_class.values).to match(
       'DEPLOYMENT_FREQUENCY' => have_attributes(value: 'deployment_frequency'),
-      'LEAD_TIME_FOR_CHANGES' => have_attributes(value: 'lead_time_for_changes')
+      'LEAD_TIME_FOR_CHANGES' => have_attributes(value: 'lead_time_for_changes'),
+      'TIME_TO_RESTORE_SERVICE' => have_attributes(value: 'time_to_restore_service')
     )
   end
 end

ee/spec/models/dora/daily_metrics_spec.rb

@@ -150,6 +150,41 @@ RSpec.describe Dora::DailyMetrics, type: :model do
       end
     end
 
+    context 'with closed issues' do
+      before do
+        create(:issue, :incident, :closed, project: project, created_at: date - 7.days, closed_at: date)
+        create(:issue, :incident, :closed, project: project, created_at: date - 5.days, closed_at: date)
+        create(:issue, :incident, :closed, project: project, created_at: date - 3.days, closed_at: date)
+        create(:issue, :incident, :closed, project: project, created_at: date - 1.day, closed_at: date)
+
+        # Issues which shouldn't be included in calculation
+        create(:issue, :closed, project: project, created_at: date - 1.year, closed_at: date) # not an incident
+        create(:issue, :incident, project: project, created_at: date - 1.year) # not closed yet
+        create(:issue, :incident, :closed, created_at: date - 1.year, closed_at: date) # different project
+        create(:issue, :incident, :closed, project: project, created_at: date - 1.year, closed_at: date + 1.day) # different date
+      end
+
+      context 'for production environment' do
+        let_it_be(:environment) { create(:environment, :production, project: project) }
+
+        it 'inserts the daily metrics with time_to_restore_service' do
+          subject
+
+          metrics = environment.dora_daily_metrics.find_by_date(date)
+          expect(metrics.time_to_restore_service_in_seconds).to eq(4.days.to_i) # median
+        end
+      end
+
+      context 'for non-production environment' do
+        it 'does not calculate time_to_restore_service daily metric' do
+          subject
+
+          metrics = environment.dora_daily_metrics.find_by_date(date)
+          expect(metrics.time_to_restore_service_in_seconds).to be_nil
+        end
+      end
+    end
+
     context 'when date is invalid type' do
       let(:date) { '2021-02-03' }
 
@@ -215,19 +250,21 @@ RSpec.describe Dora::DailyMetrics, type: :model do
       end
     end
 
-    context 'when metric is lead time for changes' do
+    shared_examples 'median metric' do |metric|
+      subject { described_class.aggregate_for!(metric, interval) }
+
       before_all do
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: 100, date: '2021-01-01')
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: 90, date: '2021-01-01')
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: 80, date: '2021-01-02')
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: 70, date: '2021-01-02')
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: 60, date: '2021-01-03')
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: 50, date: '2021-01-03')
-        create(:dora_daily_metrics, lead_time_for_changes_in_seconds: nil, date: '2021-01-04')
+        column_name = :"#{metric}_in_seconds"
+
+        create(:dora_daily_metrics, column_name => 100, :date => '2021-01-01')
+        create(:dora_daily_metrics, column_name => 90, :date => '2021-01-01')
+        create(:dora_daily_metrics, column_name => 80, :date => '2021-01-02')
+        create(:dora_daily_metrics, column_name => 70, :date => '2021-01-02')
+        create(:dora_daily_metrics, column_name => 60, :date => '2021-01-03')
+        create(:dora_daily_metrics, column_name => 50, :date => '2021-01-03')
+        create(:dora_daily_metrics, column_name => nil, :date => '2021-01-04')
       end
 
-      let(:metric) { described_class::METRIC_LEAD_TIME_FOR_CHANGES }
-
       context 'when interval is all' do
         let(:interval) { described_class::INTERVAL_ALL }
 
@@ -262,6 +299,14 @@ RSpec.describe Dora::DailyMetrics, type: :model do
       end
     end
 
+    context 'when metric is lead time for changes' do
+      include_examples 'median metric', described_class::METRIC_LEAD_TIME_FOR_CHANGES
+    end
+
+    context 'when metric is time_to_restore_service' do
+      include_examples 'median metric', described_class::METRIC_TIME_TO_RESTORE_SERVICE
+    end
+
     context 'when metric is unknown' do
       let(:metric) { 'unknown' }
       let(:interval) { described_class::INTERVAL_ALL }

ee/spec/models/issue_spec.rb

@@ -353,6 +353,45 @@ RSpec.describe Issue do
     it { is_expected.to have_one(:status_page_published_incident) }
   end
 
+  describe 'state machine' do
+    context 'daily dora metrics refresh' do
+      let_it_be(:production_env) { create(:environment, :production) }
+
+      context 'when incident is closed' do
+        let(:issue) { create(:issue, :incident, project: production_env.project) }
+
+        it 'schedules Dora::DailyMetrics::RefreshWorker' do
+          freeze_time do
+            expect(::Dora::DailyMetrics::RefreshWorker)
+              .to receive(:perform_async).with(production_env.id, Time.current.to_date.to_s)
+
+            issue.close!
+          end
+        end
+      end
+
+      context 'when there is no production env' do
+        let(:issue) { create(:issue, :incident) }
+
+        it 'does not schedule Dora::DailyMetrics::RefreshWorker' do
+          expect(::Dora::DailyMetrics::RefreshWorker).not_to receive(:perform_async)
+
+          issue.close!
+        end
+      end
+
+      context 'when issue is not an incident' do
+        let(:issue) { create(:issue, project: production_env.project) }
+
+        it 'does not schedule Dora::DailyMetrics::RefreshWorker' do
+          expect(::Dora::DailyMetrics::RefreshWorker).not_to receive(:perform_async)
+
+          issue.close!
+        end
+      end
+    end
+  end
+
   it_behaves_like 'an editable mentionable with EE-specific mentions' do
     subject { create(:issue, project: create(:project, :repository)) }
 

ee/spec/requests/api/dora/metrics_spec.rb

@@ -3,16 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe API::Dora::Metrics do
-  describe 'GET /projects/:id/dora/metrics' do
-    subject { get api(url, user), params: params }
+  let_it_be(:group) { create(:group) }
+  let_it_be(:project) { create(:project, group: group) }
+  let_it_be(:production) { create(:environment, :production, project: project) }
+  let_it_be(:maintainer) { create(:user) }
+  let_it_be(:guest) { create(:user) }
 
-    let_it_be(:project) { create(:project) }
-    let_it_be(:production) { create(:environment, :production, project: project) }
-    let_it_be(:maintainer) { create(:user) }
-    let_it_be(:guest) { create(:user) }
+  shared_examples 'common dora metrics endpoint' do
+    using RSpec::Parameterized::TableSyntax
 
-    let(:url) { "/projects/#{project.id}/dora/metrics" }
-    let(:params) { { metric: :deployment_frequency } }
     let(:user) { maintainer }
 
     around do |example|
@@ -22,26 +21,45 @@ RSpec.describe API::Dora::Metrics do
     end
 
     before_all do
-      project.add_maintainer(maintainer)
-      project.add_guest(guest)
-      create(:dora_daily_metrics, deployment_frequency: 1, environment: production, date: '2021-01-01')
-      create(:dora_daily_metrics, deployment_frequency: 2, environment: production, date: '2021-01-02')
+      create(:dora_daily_metrics,
+             deployment_frequency: 1,
+             lead_time_for_changes_in_seconds: 3,
+             time_to_restore_service_in_seconds: 5,
+             environment: production,
+             date: '2021-01-01')
+      create(:dora_daily_metrics,
+             deployment_frequency: 2,
+             lead_time_for_changes_in_seconds: 4,
+             time_to_restore_service_in_seconds: 6,
+             environment: production,
+             date: '2021-01-02')
     end
 
     before do
       stub_licensed_features(dora4_analytics: true)
     end
 
-    it 'returns data' do
-      subject
+    where(:metric, :value1, :value2) do
+      :deployment_frequency    | 1  | 2
+      :lead_time_for_changes   | 3  | 4
+      :time_to_restore_service | 5  | 6
+    end
+
+    with_them do
+      let(:params) { { metric: metric } }
 
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(json_response).to eq([{ 'date' => '2021-01-01', 'value' => 1 },
-                                   { 'date' => '2021-01-02', 'value' => 2 }])
+      it 'returns data' do
+        subject
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response).to match_array([{ 'date' => '2021-01-01', 'value' => value1 },
+                                              { 'date' => '2021-01-02', 'value' => value2 }])
+      end
     end
 
     context 'when user is guest' do
       let(:user) { guest }
+      let(:params) { { metric: :deployment_frequency } }
 
       it 'returns authorization error' do
         subject
@@ -52,53 +70,25 @@ RSpec.describe API::Dora::Metrics do
     end
   end
 
-  describe 'GET /groups/:id/dora/metrics' do
-    subject { get api(url, user), params: params }
+  describe 'GET /projects/:id/dora/metrics' do
+    subject { get api("/projects/#{project.id}/dora/metrics", user), params: params }
 
-    let_it_be(:group) { create(:group) }
-    let_it_be(:project) { create(:project, group: group) }
-    let_it_be(:production) { create(:environment, :production, project: project) }
-    let_it_be(:maintainer) { create(:user) }
-    let_it_be(:guest) { create(:user) }
+    before_all do
+      project.add_maintainer(maintainer)
+      project.add_guest(guest)
+    end
 
-    let(:url) { "/groups/#{group.id}/dora/metrics" }
-    let(:params) { { metric: :deployment_frequency } }
-    let(:user) { maintainer }
+    include_examples 'common dora metrics endpoint'
+  end
 
-    around do |example|
-      freeze_time do
-        example.run
-      end
-    end
+  describe 'GET /groups/:id/dora/metrics' do
+    subject { get api("/groups/#{group.id}/dora/metrics", user), params: params }
 
     before_all do
       group.add_maintainer(maintainer)
       group.add_guest(guest)
-      create(:dora_daily_metrics, deployment_frequency: 1, environment: production, date: 1.day.ago.to_date)
-      create(:dora_daily_metrics, deployment_frequency: 2, environment: production, date: Time.current.to_date)
-    end
-
-    before do
-      stub_licensed_features(dora4_analytics: true)
     end
 
-    it 'returns data' do
-      subject
-
-      expect(response).to have_gitlab_http_status(:ok)
-      expect(json_response).to eq([{ 'date' => 1.day.ago.to_date.to_s, 'value' => 1 },
-                                   { 'date' => Time.current.to_date.to_s, 'value' => 2 }])
-    end
-
-    context 'when user is guest' do
-      let(:user) { guest }
-
-      it 'returns authorization error' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:unauthorized)
-        expect(json_response['message']).to eq('You do not have permission to access dora metrics.')
-      end
-    end
+    include_examples 'common dora metrics endpoint'
   end
 end