Merge branch 'dblessing-add-sidekiq-mailer-argument-docs' into 'master'

Docs on adding/removing args from Sidekiq workers/mailers

See merge request gitlab-org/gitlab!33625

doc/development/README.md

@@ -63,7 +63,7 @@ Complementary reads:
 - [Working with Gitaly](gitaly.md)
 - [Manage feature flags](feature_flags/index.md)
 - [Licensed feature availability](licensed_feature_availability.md)
-- [View sent emails or preview mailers](emails.md)
+- [Dealing with email/mailers](emails.md)
 - [Shell commands](shell_commands.md) in the GitLab codebase
 - [`Gemfile` guidelines](gemfile.md)
 - [Pry debugging](pry_debugging.md)

doc/development/emails.md

 # Dealing with email in development
 
+## Ensuring compatibility with mailer Sidekiq jobs
+
+A Sidekiq job is enqueued whenever `deliver_later` is called on an `ActionMailer`.
+If a mailer argument needs to be added or removed, it is important to ensure
+both backward and forward compatibility. Adhere to the Sidekiq Style Guide steps for
+[changing the arguments for a worker](sidekiq_style_guide.md#changing-the-arguments-for-a-worker).
+
+In the following example from [`NotificationService`](https://gitlab.com/gitlab-org/gitlab/-/blob/33ccb22e4fc271dbaac94b003a7a1a2915a13441/app/services/notification_service.rb#L74)
+adding or removing an argument in this mailer's definition may cause problems
+during deployment before all Rails and Sidekiq nodes have the updated code.
+
+```ruby
+mailer.unknown_sign_in_email(user, ip, time).deliver_later
+```
+
 ## Sent emails
 
 To view rendered emails "sent" in your development instance, visit

doc/development/sidekiq_style_guide.md

@@ -537,18 +537,74 @@ possible situations:
 
 ### Changing the arguments for a worker
 
-Jobs need to be backwards- and forwards-compatible between consecutive versions
-of the application.
-
-This can be done by following this process:
-
-1. **Do not remove arguments from the `perform` function.**. Instead, use the
-   following approach
-   1. Provide a default value (usually `nil`) and use a comment to mark the
-      argument as deprecated
-   1. Stop using the argument in `perform_async`.
-   1. Ignore the value in the worker class, but do not remove it until the next
-      major release.
+Jobs need to be backward and forward compatible between consecutive versions
+of the application. Adding or removing an argument may cause problems
+during deployment before all Rails and Sidekiq nodes have the updated code.
+
+#### Remove an argument
+
+**Do not remove arguments from the `perform` function.**. Instead, use the
+following approach:
+
+1. Provide a default value (usually `nil`) and use a comment to mark the
+   argument as deprecated
+1. Stop using the argument in `perform_async`.
+1. Ignore the value in the worker class, but do not remove it until the next
+   major release.
+
+In the following example, if you want to remove `arg2`, first set a `nil` default value,
+and then update locations where `ExampleWorker.perform_async` is called.
+
+```ruby
+class ExampleWorker
+  def perform(object_id, arg1, arg2 = nil)
+    # ...
+  end
+end
+```
+
+#### Add an argument
+
+There are two options for safely adding new arguments to Sidekiq workers:
+
+1. Set up a [multi-step deployment](#multi-step-deployment) in which the new argument is first added to the worker
+1. Use a [parameter hash](#parameter-hash) for additional arguments. This is perhaps the most flexible option.
+1. Use a parameter hash for additional arguments. This is perhaps the most flexible option.
+
+##### Multi-step deployment
+
+This approach requires multiple merge requests and for the first merge request
+to be merged and deployed before additional changes are merged.
+
+1. In an initial merge request, add the argument to the worker with a default
+   value:
+
+    ```ruby
+    class ExampleWorker
+      def perform(object_id, new_arg = nil)
+        # ...
+      end
+    end
+    ```
+
+1. Merge and deploy the worker with the new argument.
+1. In a further merge request, update `ExampleWorker.perform_async` calls to
+   use the new argument.
+
+##### Parameter hash  
+
+This approach will not require multiple deployments if an existing worker already
+utilizes a parameter hash.
+
+1. Use a parameter hash in the worker to allow for future flexibility:
+
+    ```ruby
+    class ExampleWorker
+      def perform(object_id, params = {})
+        # ...
+      end
+    end
+    ```
 
 ### Removing workers