Make Auto-creation of issues for alerts off by default (!34107) · Merge requests · GitLab.org / GitLab

Allison Browne requested to merge 217034-auto-creation-of-issues-for-alerts-off-by-default into master Jun 08, 2020

What does this MR do?

User Overview

This makes issue creation from alerts off by default, if the user navigates to Settings > Operations > Incident Management.

Existing projects using alerting should appear to not change behavior (they should have create_issue on still).

Technical Overview

Change column migration

This adds a migration to change the default to false for create_issue on the project_incident_management_settings table.

Downtime is not required: https://docs.gitlab.com/ee/development/migration_style_guide.html#what-requires-downtime

disable_ddl_transaction! is not required:

For the reasons mentioned above, it’s safe to use change_column_default in a single-transaction migration without requiring disable_ddl_transaction!.

https://docs.gitlab.com/ee/development/migration_style_guide.html#changing-the-column-default

with_lock_retries are not required on a smaller table:

https://docs.gitlab.com/ee/development/migration_style_guide.html#when-to-use-the-helper-method

Data Migration

The intent is to keep the behavior of existing projects using alerting the same.

This thread discusses more reasoning: !34107 (comment 358820762)

I do not disable ddl transactions because the query only takes 46 ms on gitlab.com https://docs.gitlab.com/ee/development/migration_style_guide.html#heavy-operations-in-a-single-transaction

explain
WITH project_ids AS (
  SELECT DISTINCT issues.project_id AS id
  FROM issues
  LEFT OUTER JOIN project_incident_management_settings
              ON project_incident_management_settings.project_id = issues.project_id
  INNER JOIN label_links
         ON label_links.target_type = ‘Issue’
            AND label_links.target_id = issues.id
  INNER JOIN labels
         ON labels.id = label_links.label_id
  WHERE  ( project_incident_management_settings.project_id IS NULL )
         AND labels.title = ‘incident’
         AND labels.color = ‘#CC0033’
         AND labels.description =
  ‘Denotes a disruption to IT services and the associated issues require immediate attention’
)
INSERT INTO project_incident_management_settings (project_id, create_issue, send_email, issue_template_key)
SELECT project_ids.id, TRUE, FALSE, NULL
FROM project_ids;

Summary:

Time: 46.526 ms
  - planning: 1.277 ms
  - execution: 45.249 ms
    - I/O read: 0.000 ms
    - I/O write: 0.000 ms

Shared buffers:
  - hits: 48324 (~377.50 MiB) from the buffer pool
  - reads: 0 from the OS file cache, including disk I/O
  - dirtied: 0
  - writes: 0

 ModifyTable on public.project_incident_management_settings  (cost=151.47..151.53 rows=3 width=38) (actual time=45.171..45.171 rows=0 loops=1)
   Buffers: shared hit=48324
   CTE project_ids
     ->  Unique  (cost=151.46..151.47 rows=3 width=4) (actual time=45.167..45.167 rows=0 loops=1)
           Buffers: shared hit=48324
           ->  Sort  (cost=151.46..151.47 rows=3 width=4) (actual time=45.167..45.167 rows=0 loops=1)
                 Sort Key: issues.project_id
                 Sort Method: quicksort  Memory: 25kB
                 Buffers: shared hit=48324
                 ->  Nested Loop Anti Join  (cost=1.84..151.43 rows=3 width=4) (actual time=45.161..45.161 rows=0 loops=1)
                       Buffers: shared hit=48324
                       ->  Nested Loop  (cost=1.56..150.54 rows=3 width=4) (actual time=0.074..35.286 rows=5676 loops=1)
                             Buffers: shared hit=34171
                             ->  Nested Loop  (cost=1.00..148.43 rows=3 width=4) (actual time=0.037..9.365 rows=5676 loops=1)
                                   Buffers: shared hit=5773
                                   ->  Index Scan using index_labels_on_title on public.labels  (cost=0.43..68.92 rows=1 width=4) (actual time=0.025..0.248 rows=61 loops=1)
                                         Index Cond: ((labels.title)::text = 'incident'::text)
                                         Filter: (((labels.color)::text = '#CC0033'::text) AND ((labels.description)::text = 'Denotes a disruption to IT services and the associated issues require immediate attention'::text))
                                         Rows Removed by Filter: 72
                                         Buffers: shared hit=137
                                   ->  Index Scan using index_label_links_on_label_id on public.label_links  (cost=0.56..78.24 rows=126 width=8) (actual time=0.007..0.132 rows=93 loops=61)
                                         Index Cond: (label_links.label_id = labels.id)
                                         Filter: ((label_links.target_type)::text = 'Issue'::text)
                                         Rows Removed by Filter: 0
                                         Buffers: shared hit=5636
                             ->  Index Scan using issues_pkey on public.issues  (cost=0.56..0.70 rows=1 width=8) (actual time=0.004..0.004 rows=1 loops=5676)
                                   Index Cond: (issues.id = label_links.target_id)
                                   Buffers: shared hit=28398
                       ->  Index Only Scan using project_incident_management_settings_pkey on public.project_incident_management_settings project_incident_management_settings_1  (cost=0.28..0.29 rows=1 width=4) (actual time=0.001..0.001 rows=1 loops=5676)
                             Index Cond: (project_incident_management_settings_1.project_id = issues.project_id)
                             Heap Fetches: 2800
                             Buffers: shared hit=14153
   ->  CTE Scan on project_ids  (cost=0.00..0.06 rows=3 width=38) (actual time=45.169..45.169 rows=0 loops=1)
         Buffers: shared hit=48324

Migration output


▶ rake db:migrate

== 20200608214008 ChangeColumnDefaultProjectIncidentManagementSettings: migrating
-- change_column_default(:project_incident_management_settings, :create_issue, {:from=>true, :to=>false})
   -> 0.0016s
== 20200608214008 ChangeColumnDefaultProjectIncidentManagementSettings: migrated (0.0017s)

== 20200609212701 AddIncidentSettingsToAllExistingProjects: migrating =========
-- execute("      WITH project_ids AS (\n        SELECT DISTINCT issues.project_id AS id\n        FROM issues\n        LEFT OUTER JOIN project_incident_management_settings\n                    ON project_incident_management_settings.project_id = issues.project_id\n        INNER JOIN label_links\n               ON label_links.target_type = 'Issue'\n                  AND label_links.target_id = issues.id\n        INNER JOIN labels\n               ON labels.id = label_links.label_id\n        WHERE  ( project_incident_management_settings.project_id IS NULL )\n               -- Use incident labels even though they could be manually added by users who\n               -- are not using alert funtionality.\n               AND labels.title = 'incident'\n               AND labels.color = '#CC0033'\n               AND labels.description = 'Denotes a disruption to IT services and the associated issues require immediate attention'\n      )\n      INSERT INTO project_incident_management_settings (project_id, create_issue, send_email, issue_template_key)\n      SELECT project_ids.id, TRUE, FALSE, NULL\n      FROM project_ids\n      ON CONFLICT (project_id) DO NOTHING;\n")
   -> 0.0206s
== 20200609212701 AddIncidentSettingsToAllExistingProjects: migrated (0.0206s)


▶ rake db:rollback
== 20200609212701 AddIncidentSettingsToAllExistingProjects: reverting =========
== 20200609212701 AddIncidentSettingsToAllExistingProjects: reverted (0.0000s)

== 20200608214008 ChangeColumnDefaultProjectIncidentManagementSettings: reverting
-- change_column_default(:project_incident_management_settings, :create_issue, {:from=>false, :to=>true})
   -> 0.0072s
== 20200608214008 ChangeColumnDefaultProjectIncidentManagementSettings: reverted (0.0073s)

Screenshots

Database Structure	Settings Screen

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Tested in all supported browsers
Informed Infrastructure department of a default or new setting change, if applicable per definition of done

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

Label as security and @ mention @gitlab-com/gl-security/appsec
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
Security reports checked/validated by a reviewer from the AppSec team

Closes #217034 (closed)

Edited Nov 10, 2020 by Rémy Coutable

Make Auto-creation of issues for alerts off by default