Rename project_services to integrations in codebase

As part of the Mass-integration project, we decided in &2504 (closed) to rename project services to integrations.

To reflect the new name and in accordance with the Gitlab/NamespacedClass cop !51236 (merged):

• The project_services directory should be renamed to integrations
• The models should be namespaced with Integrations::, i.e. Integrations::Jira.

Checklist

• Rename classes namespaced Integrations::
• Remove Rubocop TODOs https://gitlab.com/gitlab-org/gitlab/-/blob/9a65f0bd374515fc8e720fbb6e4889653dfae527/.rubocop_manual_todo.yml#L1482-1531
• We should not break the usage metrics, details #201855 (comment 562526950)
• Manage Single table inheritance #201855 (comment 564903142)
• GraphQL considerations #201855 (comment 566482827)

added backend backstage [DEPRECATED] groupecosystem [DEPRECATED] labels

changed weight to 3

added to epic &2504 (closed)

Setting devopsenablement based on ~"group::ecosystem".

added devopssystems label

Note that the UI work of renaming Project Services to Integrations is handled in #119429 (closed) so this issue is purely about renaming on the backend

changed title from Rename project_services to integrations to Rename project_services to integrations in codebase

added [deprecated] Accepting merge requests label

added devopscreate label and removed devopssystems label

added sectiondev label

added Category:Integrations label

added priority3 label

changed milestone to %13.8

changed milestone to %13.11

changed the description

marked this issue as related to #321982

changed the description

removed backstage [DEPRECATED] label

added Deliverable label

assigned to @.luke

removed [deprecated] Accepting merge requests label

added workflowready for development label

changed health status to at risk

@mnohr @deuley I'm marking this issue as "at risk" for %13.11 due to last week being taken up with rapid actionshift db load from primary and mimemagic gem dependency #325851 (closed) work.

Sounds good, thanks for the heads up.

@mnohr shall we proactively move to %13.12 ?

Due to the need to focus on database performance in %13.11, I am moving this to %13.12

changed milestone to %13.12

removed Deliverable label

removed the health status

unassigned @.luke

added [deprecated] Accepting merge requests label

added Stretch label

Related to this issue, we should not change the metric definitions because that will break the SiSense dashboards. A lot of metrics having _service_ as part of the name will be impacted by this change, see https://docs.gitlab.com/ee/development/usage_ping/dictionary.html.

One piece of relevant code related to that is https://gitlab.com/gitlab-org/gitlab/-/blob/b8ea06e5d3945845c5b357745c40ae4117c02828/lib/gitlab/usage_data.rb#L417-424 but it could be more places affected by this renaming.

One possible approach is to rename the SiSense dashboards before merging these changes. We should create an issue in the GitLab Data Team project to have some help about approaching this. Also relevant is the process to deprecate and remove a metric !59000 (merged) (linking the merge request until we have the docs page updated).

changed the description

Another point, what happens with the Single table inheritance after the class names change. We currently save type as AsanaService, JiraService, etc.

We will need a migration to update the type values or maybe a different strategy like using sti_name, reference:

• https://docs.gitlab.com/ee/development/single_table_inheritance.html
• https://stackoverflow.com/questions/11434678/how-to-migrate-the-database-when-renaming-an-activerecord-model-that-is-a-subcla
• https://stackoverflow.com/questions/65006386/rename-single-table-inheritance-rails-model-class-name-without-changing-existing
• https://stackoverflow.com/questions/23293177/rails-sti-how-to-change-mapping-between-class-name-value-of-the-type-column

set weight to 3

set weight to 5

changed the description

I'm changing the weight of this issue because I expect surprises when doing this rename, see: #201855 (comment 562526950), #201855 (comment 564903142) and maybe more.

I don't know if we can split this into more issues, let's leave this as-is for the time being.

mentioned in merge request !60649 (merged)

@arturoherrero I'll pick this one up, and will try handling the change in code-only. If we need to migrate data then I'll report back on this issue.

assigned to @.luke

removed [deprecated] Accepting merge requests label

Another consideration is the GraphQL ServiceType enum, which has values of ASANA_SERVICE, BAMBOO_SERVICE, etc.

We'll need to continue to support these values. We could deprecate them if we need to.

About deprecating GraphQL, we could do that in %14.0, related #198358 (closed).

added workflowin dev label and removed workflowready for development label

changed the description

mentioned in commit 60bd6c6d

mentioned in merge request !60968 (merged)

mentioned in commit d387578d

mentioned in commit a77224ef

mentioned in commit 531660f6

mentioned in commit c40b1bfe

mentioned in issue #330442 (closed)

mentioned in commit 6766b156

mentioned in merge request !61210 (merged)

mentioned in commit 7d501dd4

A first MR has been merged !60968 (merged) containing changes needed to keep the classes working as STI models. It has moved across Assembla, Asana and Bamboo integrations.

QA'ing that change on staging: No errors in Sentry for those services.

I've been able to save the integration successfully. I haven't yet QA'd the integrations against a real servce. I've asked in Slack about login details.

The old JiraService model has been renamed Integrations::Jira in !61743 (merged) and I've QA'd on staging, and it looks fine. No errors in staging Sentry.

mentioned in issue #330670 (closed)

@arturoherrero I'll drop the weight on this issue from 5 to 3 as we've managed to slice off some of the work into separate issues #330442 (closed) and #330443 (closed).

@.luke This turns out to be more work than we expect, setting weight 2 but feel free to increase this if you consider that.

set weight to 3

marked this issue as related to #330442 (closed)

marked this issue as related to #330443 (closed)

mentioned in commit 82aa8087

mentioned in merge request !61559 (merged)

added typemaintenance label

added typefeature label

mentioned in issue #330832 (closed)

mentioned in commit ecb392af

mentioned in commit 708008a3

mentioned in merge request !61743 (merged)

mentioned in merge request !61744 (merged)

mentioned in commit 0a55d0dd

mentioned in commit 90fa8df3

mentioned in commit f92e3791

mentioned in commit c49208bd

mentioned in commit eabf5573

mentioned in commit 0b8ca085

mentioned in commit cfc3def5

mentioned in commit a968a76e

mentioned in commit 008440cb

mentioned in merge request !61854 (merged)

mentioned in commit 0a86c92b

mentioned in commit 6ffa3a16

mentioned in commit 51a8e8ed

mentioned in commit c1867b98

mentioned in commit 113623b0

mentioned in commit 6e729f7f

marked this issue as related to #331181 (closed)

mentioned in commit ffb63dc1

mentioned in merge request !61957 (merged)

mentioned in commit 135459aa

mentioned in commit 0344b1bc

mentioned in commit 11af3673

mentioned in commit 6bec51ff

mentioned in commit d1b3a36e

changed milestone to %14.0

set weight to 1

mentioned in commit 8c381b0e

mentioned in commit 86b2aebf

mentioned in commit e0b3c001

mentioned in commit 890a5123

mentioned in merge request !62265 (merged)

mentioned in commit a120e465

mentioned in issue #330300 (closed)

mentioned in commit 26a83f55

mentioned in commit 2b881190

mentioned in commit f418c03f

mentioned in merge request !62359 (merged)

added Deliverable label and removed Stretch label

mentioned in commit 0f407a69

@.luke I'll start working on this issue too if that's alright

But I wanted to check first if you have any unpushed MRs, it looks like after !62359 (merged) and !61957 (merged) the remaining files are:

Monitoring services:

• monitoring_service.rb
• mock_monitoring_service.rb
• prometheus_service.rb

Slash commands services:

• slash_commands_service.rb
• slack_slash_commands_service.rb
• mattermost_slash_commands_service.rb

Data fields models:

• issue_tracker_data.rb
• jira_tracker_data.rb
• open_project_tracker_data.rb
• app/models/project_services/data_fields.rb: rename to app/models/concerns/integrations/has_data_fields.rb?
• app/models/concerns/services/data_fields.rb: rename to app/models/concerns/integrations/data_fields.rb?

Others:

• pushover_service.rb
• slack_mattermost/notifier.rb

ee/app/models/project_services:

• github_service.rb
• github_service/remote_project.rb
• github_service/status_message.rb
• github_service/status_notifier.rb
• gitlab_slack_application_service.rb (not a ChatService for some reason )

I think I'll start with the data fields models as I've already worked with them a bit, let me know your current status and thoughts on the proposed rename above!

Thanks, @toupeira! Yay, looking forward to working alongside you.

Yes, those are the groups of integrations, and working on them in those groups you've defined looks great.

For the integration models, these are the steps I've been taken per model. It's fairly obvious, but having it written down makes the manual task a bit quicker:

1. Rename and move the model (FooService becomes Integrations::Foo)
2. Move the specs (feature and unit - update the unit's RSpec.describe block)
3. Add to StiType::NAMESPACED_INTEGRATIONS
4. Update Api::Helpers::ServicesHelpers.service_classes
5. class: the factory (if one exists)
6. class_name: the Project association
7. Remove from .rubocop_manual_todo.yml (and also search through .rubocop_todo.yml and even .rubocop.yml to see if the file was being excluded from other cops, and if those exclusions are autocorrectable, autocorrect it, otherwise leave it)
8. Search through codebase for <Integration-Name>:: collisions with other files and proactively change them to reference the constant on the root namespace - i.e., !62265 (comment 581548111) - I don't rely on our test suite to necessarily pick these problems up, so just go and change everything even if it's likely not a problem
9. Search through codebase for the old model name, and fix most things, except where used as a translation key, or where used in SQL (i.e., where(type: 'FooService')) and... yeah use your discretion .

When each MR is merged, we tend to get merge conflicts on other open MRs due to steps 3, 4, 6 and 7 (4 and 6 because I've been alphabetising as I go, which tends to cause conflicts). If we're aware of each others' MRs, we could resolve those conflicts for each other's in-flight MRs and notify the other to pull the changes when they wake up.

For the "base" integration classes, I've started to rename those according to #331181 (closed) also - just because we're touching the file anyway. But I've been leaving out the abstract_class part because I don't wish to introduce functional changes in these MRs.

I'm going to peel off the Monitoring services group today.

Update: This turned out to be more difficult than I hoped, because PrometheusService is used as a GraphQL GlobalID which will need to remain compatible for arguments when we rename the model. I've come up with a possible WIP around this that I'll look into more tomorrow.

thoughts on the proposed rename above!

• app/models/project_services/data_fields.rb: rename to app/models/concerns/integrations/has_data_fields.rb?
• app/models/concerns/services/data_fields.rb: rename to app/models/concerns/integrations/data_fields.rb?

I like Integrations::HasDataFields! Perhaps would the other concern be better named Integrations::DataField (unpluralised, as in, this is "data field" model)?

@.luke thanks so much for the pointers, that's very helpful!

Perhaps would the other concern be better named Integrations::DataField (unpluralised, as in, this is "data field" model)?

Hmm I think plural would make more sense since each record has multiple "fields" (columns).

But also in light of the other base models, I think I'll go with Integrations::BaseDataFields and will make it into a proper class rather than a concern, there's only three models that use it and this should be a small enough change to not cause any trouble.

@.luke I'll continue from the bottom of the list, with the EE and "Others" models.

@.luke are the slash commands still up for grabs? If yes I'll continue with those tomorrow, unless you were just about to jump on them too

@toupeira Slash commands are still up for grabs the only group I'm currently working on is still the monitoring group !62645 (merged).

assigned to @toupeira

mentioned in commit c5cb85ca

set weight to 5

set weight to 2

mentioned in commit e99c340d

mentioned in merge request !62605 (merged)

mentioned in commit 9ca7dbeb

mentioned in commit 60fadaa6

mentioned in commit 4cff7f5c

mentioned in commit b40e49cf

mentioned in commit 28e1e593

mentioned in commit 981dd58b

mentioned in commit 48ead07e

mentioned in merge request !62645 (merged)

mentioned in commit 823b693c

mentioned in commit 7925124d

mentioned in commit b555855e

mentioned in commit 248d197c

mentioned in merge request !62676 (merged)

mentioned in commit 37b3150f

mentioned in commit 28199904

mentioned in commit 0a55f68f

mentioned in commit ea5f1015

mentioned in commit 19350d3b

mentioned in commit c646f407

mentioned in commit a91d9bc6

mentioned in commit 698f79e1

mentioned in commit f7947dd7

mentioned in commit 11ab8030

mentioned in commit f79aa673

mentioned in merge request !62934 (merged)

mentioned in commit e24ab64d

mentioned in merge request !62937 (merged)

mentioned in commit eb0e7cd8

mentioned in commit f7471e0b

mentioned in commit 72222e9f

mentioned in commit 724dc902

mentioned in merge request !63015 (merged)

mentioned in commit 280b5f5a

mentioned in commit de83a68b

mentioned in commit 77eea334

mentioned in commit 358f455d

mentioned in commit 7b60aba6

mentioned in commit 5b6a279f

mentioned in commit 637d18c1

mentioned in issue #331064 (closed)

@arturoherrero @deuley Just an update on this issue. The only remaining models to be renamed are the monitoring integrations. There's been a hurdle to overcome first, and I'm happy to say that I have something I'm just tidying up for review that will get us over that hurdle. I need to stage the hurdle-overcoming MR first, and then the monitoring integrations MR will merge after that. We're cutting it fine in terms of delivering all parts for %14.0. However, so long as the hurdle-overcoming MR is merged in %14.0 the remaining work will be negligible (like, a weight 0.5). I'll keep you both updated.

mentioned in merge request !63743 (merged)

mentioned in commit fbf9ce5e

added workflowin review label and removed workflowin dev label

mentioned in commit 4272b14c

mentioned in commit 5c5e67ff

mentioned in commit d7a79c9e

mentioned in commit e0e915c0

mentioned in epic &6177 (closed)

marked this issue as related to #333506 (closed)

mentioned in commit 9e5caf15

mentioned in commit 787b86fb

mentioned in commit ae981dab

marked this issue as related to #333759 (closed)

set weight to 5

added workflowverification label and removed workflowin review label

The final MR !62645 (merged) has been merged, so I'll put this into workflowverification. If we don't see any related errors after a couple of days then I'll close it. cc @toupeira

I'm closing this. Great work, 20 related merge requests!

mentioned in commit 576c8e86

mentioned in commit b86b6f90

mentioned in commit edith007/gitlab@bda8a64d

mentioned in commit edith007/gitlab@9f572bd0

closed

I'll set the milestone to %14.1 because !62645 (merged) was the final MR.

changed milestone to %14.1

mentioned in issue #335882 (closed)

mentioned in issue #336014 (closed)

mentioned in merge request !66928 (merged)

added 1 deleted label and removed devopscreate label

removed typefeature label

added devopsmanage label and removed 1 deleted label