Commit e49a221c authored by James Lopez's avatar James Lopez

Merge branch 'bvl-document-worker-context' into 'master'

Document the contexts we apply to workers

See merge request gitlab-org/gitlab!23995
parents 6ae01f77 7de59ad6
...@@ -13,7 +13,7 @@ module MergeRequests ...@@ -13,7 +13,7 @@ module MergeRequests
diffs.each_batch(of: BATCH_SIZE) do |relation, index| diffs.each_batch(of: BATCH_SIZE) do |relation, index|
ids = relation.pluck_primary_key.map { |id| [id] } ids = relation.pluck_primary_key.map { |id| [id] }
DeleteDiffFilesWorker.bulk_perform_in(index * 5.minutes, ids) DeleteDiffFilesWorker.bulk_perform_in(index * 5.minutes, ids) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
end end
......
...@@ -9,7 +9,7 @@ module MergeRequests ...@@ -9,7 +9,7 @@ module MergeRequests
def self.enqueue! def self.enqueue!
ids = MergeRequestDiff.ids_for_external_storage_migration(limit: MAX_JOBS) ids = MergeRequestDiff.ids_for_external_storage_migration(limit: MAX_JOBS)
MigrateExternalDiffsWorker.bulk_perform_async(ids.map { |id| [id] }) MigrateExternalDiffsWorker.bulk_perform_async(ids.map { |id| [id] }) # rubocop:disable Scalability/BulkPerformWithContext
end end
def initialize(merge_request_diff) def initialize(merge_request_diff)
......
...@@ -11,7 +11,7 @@ class UserProjectAccessChangedService ...@@ -11,7 +11,7 @@ class UserProjectAccessChangedService
if blocking if blocking
AuthorizedProjectsWorker.bulk_perform_and_wait(bulk_args) AuthorizedProjectsWorker.bulk_perform_and_wait(bulk_args)
else else
AuthorizedProjectsWorker.bulk_perform_async(bulk_args) AuthorizedProjectsWorker.bulk_perform_async(bulk_args) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
end end
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class AdminEmailWorker class AdminEmailWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category_not_owned! feature_category_not_owned!
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Ci module Ci
class ArchiveTracesCronWorker class ArchiveTracesCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :continuous_integration feature_category :continuous_integration
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class ContainerExpirationPolicyWorker class ContainerExpirationPolicyWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :container_registry feature_category :container_registry
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class ExpireBuildArtifactsWorker class ExpireBuildArtifactsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :continuous_integration feature_category :continuous_integration
......
...@@ -4,7 +4,7 @@ class GitlabUsagePingWorker ...@@ -4,7 +4,7 @@ class GitlabUsagePingWorker
LEASE_TIMEOUT = 86400 LEASE_TIMEOUT = 86400
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category_not_owned! feature_category_not_owned!
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class ImportExportProjectCleanupWorker class ImportExportProjectCleanupWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :importers feature_category :importers
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class IssueDueSchedulerWorker class IssueDueSchedulerWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :issue_tracking feature_category :issue_tracking
...@@ -10,7 +10,7 @@ class IssueDueSchedulerWorker ...@@ -10,7 +10,7 @@ class IssueDueSchedulerWorker
def perform def perform
project_ids = Issue.opened.due_tomorrow.group(:project_id).pluck(:project_id).map { |id| [id] } project_ids = Issue.opened.due_tomorrow.group(:project_id).pluck(:project_id).map { |id| [id] }
MailScheduler::IssueDueWorker.bulk_perform_async(project_ids) MailScheduler::IssueDueWorker.bulk_perform_async(project_ids) # rubocop:disable Scalability/BulkPerformWithContext
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
end end
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Namespaces module Namespaces
class PruneAggregationSchedulesWorker class PruneAggregationSchedulesWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class PagesDomainRemovalCronWorker class PagesDomainRemovalCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :pages feature_category :pages
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class PagesDomainSslRenewalCronWorker class PagesDomainSslRenewalCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :pages feature_category :pages
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class PagesDomainVerificationCronWorker class PagesDomainVerificationCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :pages feature_category :pages
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module PersonalAccessTokens module PersonalAccessTokens
class ExpiringWorker class ExpiringWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :authentication_and_authorization feature_category :authentication_and_authorization
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class PipelineScheduleWorker class PipelineScheduleWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :continuous_integration feature_category :continuous_integration
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class PruneOldEventsWorker class PruneOldEventsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category_not_owned! feature_category_not_owned!
......
...@@ -4,7 +4,7 @@ ...@@ -4,7 +4,7 @@
# table. # table.
class PruneWebHookLogsWorker class PruneWebHookLogsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :integrations feature_category :integrations
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class RemoveExpiredGroupLinksWorker class RemoveExpiredGroupLinksWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :authentication_and_authorization feature_category :authentication_and_authorization
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class RemoveExpiredMembersWorker class RemoveExpiredMembersWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :authentication_and_authorization feature_category :authentication_and_authorization
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class RemoveUnreferencedLfsObjectsWorker class RemoveUnreferencedLfsObjectsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class RepositoryArchiveCacheWorker class RepositoryArchiveCacheWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module RepositoryCheck module RepositoryCheck
class DispatchWorker class DispatchWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
include ::EachShardWorker include ::EachShardWorker
include ExclusiveLeaseGuard include ExclusiveLeaseGuard
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class RequestsProfilesWorker class RequestsProfilesWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class ScheduleMigrateExternalDiffsWorker class ScheduleMigrateExternalDiffsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
include Gitlab::ExclusiveLeaseHelpers include Gitlab::ExclusiveLeaseHelpers
feature_category :source_code_management feature_category :source_code_management
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class StuckCiJobsWorker class StuckCiJobsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :continuous_integration feature_category :continuous_integration
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class StuckImportJobsWorker class StuckImportJobsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :importers feature_category :importers
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class StuckMergeJobsWorker class StuckMergeJobsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class TrendingProjectsWorker class TrendingProjectsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
......
...@@ -276,6 +276,125 @@ class SomeCrossCuttingConcernWorker ...@@ -276,6 +276,125 @@ class SomeCrossCuttingConcernWorker
end end
``` ```
## Worker context
To have some more information about workers in the logs, we add
[metadata to the jobs in the form of an
`ApplicationContext`](logging.md#logging-context-metadata-through-rails-or-grape-requests).
In most cases, when scheduling a job from a request, this context will
already be deducted from the request and added to the scheduled
job.
When a job runs, the context that was active when it was scheduled
will be restored. This causes the context to be propagated to any job
scheduled from within the running job.
All this means that in most cases, to add context to jobs, we don't
need to do anything.
There are however some instances when there would be no context
present when the job is scheduled, or the context that is present is
likely to be incorrect. For these instances we've added rubocop-rules
to draw attention and avoid incorrect metadata in our logs.
As with most our cops, there are perfectly valid reasons for disabling
them. In this case it could be that the context from the request is
correct. Or maybe you've specified a context already in a way that
isn't picked up by the cops. In any case, please leave a code-comment
pointing to which context will be used when disabling the cops.
When you do provide objects to the context, please make sure that the
route for namespaces and projects is preloaded. This can be done using
the `.with_route` scope defined on all `Routable`s.
### Cron-Workers
The context is automatically cleared for workers in the cronjob-queue
(which `include CronjobQueue`), even when scheduling them from
requests. We do this to avoid incorrect metadata when other jobs are
scheduled from the cron-worker.
Cron-Workers themselves run instance wide, so they aren't scoped to
users, namespaces, projects or other resources that should be added to
the context.
However, they often schedule other jobs that _do_ require context.
That is why there needs to be an indication of context somewhere in
the worker. This can be done by using one of the following methods
somewhere within the worker:
1. Wrap the code that schedules jobs in the `with_context` helper:
```ruby
def perform
deletion_cutoff = Gitlab::CurrentSettings
.deletion_adjourned_period.days.ago.to_date
projects = Project.with_route.with_namespace
.aimed_for_deletion(deletion_cutoff)
projects.find_each(batch_size: 100).with_index do |project, index|
delay = index * INTERVAL
with_context(project: project) do
AdjournedProjectDeletionWorker.perform_in(delay, project.id)
end
end
end
```
1. Use the a batch scheduling method that provides context:
```ruby
def schedule_projects_in_batch(projects)
ProjectImportScheduleWorker.bulk_perform_async_with_contexts(
projects,
arguments_proc: -> (project) { project.id },
context_proc: -> (project) { { project: project } }
)
end
```
or when scheduling with delays:
```ruby
diffs.each_batch(of: BATCH_SIZE) do |diffs, index|
DeleteDiffFilesWorker
.bulk_perform_in_with_contexts(index * 5.minutes,
diffs,
arguments_proc: -> (diff) { diff.id },
context_proc: -> (diff) { { project: diff.merge_request.target_project } })
end
```
### Jobs scheduled in bulk
Often, when scheduling jobs in bulk, these jobs should have a separate
context rather than the overarching context.
If that is the case, `bulk_perform_async` can be replaced by the
`bulk_perform_async_with_context` helper, and instead of
`bulk_perform_in` use `bulk_perform_in_with_context`.
For example:
```ruby
ProjectImportScheduleWorker.bulk_perform_async_with_contexts(
projects,
arguments_proc: -> (project) { project.id },
context_proc: -> (project) { { project: project } }
)
```
Each object from the enumerable in the first argument is yielded into 2
blocks:
The `arguments_proc` which needs to return the list of arguments the
job needs to be scheduled with.
The `context_proc` which needs to return a hash with the context
information for the job.
## Tests ## Tests
Each Sidekiq worker must be tested using RSpec, just like any other class. These Each Sidekiq worker must be tested using RSpec, just like any other class. These
......
...@@ -41,7 +41,7 @@ class ElasticsearchIndexedNamespace < ApplicationRecord ...@@ -41,7 +41,7 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
jobs = batch_ids.map { |id| [id, :index] } jobs = batch_ids.map { |id| [id, :index] }
ElasticNamespaceIndexerWorker.bulk_perform_async(jobs) ElasticNamespaceIndexerWorker.bulk_perform_async(jobs) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
...@@ -58,7 +58,7 @@ class ElasticsearchIndexedNamespace < ApplicationRecord ...@@ -58,7 +58,7 @@ class ElasticsearchIndexedNamespace < ApplicationRecord
jobs = batch_ids.map { |id| [id, :delete] } jobs = batch_ids.map { |id| [id, :delete] }
ElasticNamespaceIndexerWorker.bulk_perform_async(jobs) ElasticNamespaceIndexerWorker.bulk_perform_async(jobs) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
......
...@@ -18,7 +18,7 @@ module Elastic ...@@ -18,7 +18,7 @@ module Elastic
end end
args.each_slice(BULK_PERFORM_SIZE) do |args| args.each_slice(BULK_PERFORM_SIZE) do |args|
ElasticFullIndexWorker.bulk_perform_async(args) ElasticFullIndexWorker.bulk_perform_async(args) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
end end
......
...@@ -53,7 +53,7 @@ module Geo ...@@ -53,7 +53,7 @@ module Geo
end end
end end
Geo::FileRemovalWorker.bulk_perform_async(paths_to_remove) Geo::FileRemovalWorker.bulk_perform_async(paths_to_remove) # rubocop:disable Scalability/BulkPerformWithContext
end end
# rubocop: enable CodeReuse/ActiveRecord # rubocop: enable CodeReuse/ActiveRecord
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class AdjournedGroupDeletionWorker class AdjournedGroupDeletionWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
INTERVAL = 5.minutes.to_i INTERVAL = 5.minutes.to_i
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class AdjournedProjectsDeletionCronWorker class AdjournedProjectsDeletionCronWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
INTERVAL = 5.minutes.to_i INTERVAL = 5.minutes.to_i
......
...@@ -4,7 +4,7 @@ class ClearSharedRunnersMinutesWorker ...@@ -4,7 +4,7 @@ class ClearSharedRunnersMinutesWorker
LEASE_TIMEOUT = 3600 LEASE_TIMEOUT = 3600
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :continuous_integration feature_category :continuous_integration
......
...@@ -27,14 +27,14 @@ class ElasticNamespaceIndexerWorker ...@@ -27,14 +27,14 @@ class ElasticNamespaceIndexerWorker
# https://www.rubydoc.info/github/mperham/sidekiq/Sidekiq%2FClient:push_bulk # https://www.rubydoc.info/github/mperham/sidekiq/Sidekiq%2FClient:push_bulk
namespace.all_projects.find_in_batches do |batch| namespace.all_projects.find_in_batches do |batch|
args = batch.map { |project| [:index, project.class.to_s, project.id, project.es_id] } args = batch.map { |project| [:index, project.class.to_s, project.id, project.es_id] }
ElasticIndexerWorker.bulk_perform_async(args) ElasticIndexerWorker.bulk_perform_async(args) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
def delete_from_index(namespace) def delete_from_index(namespace)
namespace.all_projects.find_in_batches do |batch| namespace.all_projects.find_in_batches do |batch|
args = batch.map { |project| [:delete, project.class.to_s, project.id, project.es_id] } args = batch.map { |project| [:delete, project.class.to_s, project.id, project.es_id] }
ElasticIndexerWorker.bulk_perform_async(args) ElasticIndexerWorker.bulk_perform_async(args) # rubocop:disable Scalability/BulkPerformWithContext
end end
end end
end end
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Geo module Geo
class ContainerRepositorySyncDispatchWorker < Geo::Scheduler::Secondary::SchedulerWorker class ContainerRepositorySyncDispatchWorker < Geo::Scheduler::Secondary::SchedulerWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
def perform def perform
unless Gitlab.config.geo.registry_replication.enabled unless Gitlab.config.geo.registry_replication.enabled
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
module Geo module Geo
class FileDownloadDispatchWorker < Geo::Scheduler::Secondary::SchedulerWorker class FileDownloadDispatchWorker < Geo::Scheduler::Secondary::SchedulerWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
private private
......
...@@ -4,7 +4,7 @@ module Geo ...@@ -4,7 +4,7 @@ module Geo
class MetricsUpdateWorker class MetricsUpdateWorker
include ApplicationWorker include ApplicationWorker
include ExclusiveLeaseGuard include ExclusiveLeaseGuard
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :geo_replication feature_category :geo_replication
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Geo module Geo
class PruneEventLogWorker class PruneEventLogWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
include ::Gitlab::Geo::LogHelpers include ::Gitlab::Geo::LogHelpers
......
...@@ -4,7 +4,7 @@ module Geo ...@@ -4,7 +4,7 @@ module Geo
module RepositoryVerification module RepositoryVerification
module Secondary module Secondary
class ShardWorker < Geo::Scheduler::Secondary::SchedulerWorker class ShardWorker < Geo::Scheduler::Secondary::SchedulerWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
attr_accessor :shard_name attr_accessor :shard_name
def perform(shard_name) def perform(shard_name)
......
...@@ -4,7 +4,7 @@ module Geo ...@@ -4,7 +4,7 @@ module Geo
module Scheduler module Scheduler
class PerShardSchedulerWorker class PerShardSchedulerWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
include ::Gitlab::Geo::LogHelpers include ::Gitlab::Geo::LogHelpers
include ::EachShardWorker include ::EachShardWorker
......
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
module Geo module Geo
class SidekiqCronConfigWorker class SidekiqCronConfigWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :geo_replication feature_category :geo_replication
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class HistoricalDataWorker class HistoricalDataWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :license_compliance feature_category :license_compliance
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class LdapAllGroupsSyncWorker class LdapAllGroupsSyncWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :authentication_and_authorization feature_category :authentication_and_authorization
worker_has_external_dependencies! worker_has_external_dependencies!
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class LdapSyncWorker class LdapSyncWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :authentication_and_authorization feature_category :authentication_and_authorization
worker_has_external_dependencies! worker_has_external_dependencies!
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class PseudonymizerWorker class PseudonymizerWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :integrations feature_category :integrations
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class UpdateAllMirrorsWorker class UpdateAllMirrorsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :source_code_management feature_category :source_code_management
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
class UpdateMaxSeatsUsedForGitlabComSubscriptionsWorker class UpdateMaxSeatsUsedForGitlabComSubscriptionsWorker
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
feature_category :license_compliance feature_category :license_compliance
worker_resource_boundary :cpu worker_resource_boundary :cpu
......
...@@ -22,7 +22,7 @@ namespace :gitlab do ...@@ -22,7 +22,7 @@ namespace :gitlab do
[:index, 'Project', id, nil] # es_id is unused for :index [:index, 'Project', id, nil] # es_id is unused for :index
end end
ElasticIndexerWorker.bulk_perform_async(args) ElasticIndexerWorker.bulk_perform_async(args) # rubocop:disable Scalability/BulkPerformWithContext
print "." print "."
end end
......
...@@ -17,7 +17,7 @@ module Gitlab ...@@ -17,7 +17,7 @@ module Gitlab
def self.execute_all_async(data) def self.execute_all_async(data)
args = files.map { |file| [file, data] } args = files.map { |file| [file, data] }
FileHookWorker.bulk_perform_async(args) FileHookWorker.bulk_perform_async(args) # rubocop:disable Scalability/BulkPerformWithContext
end end
def self.execute(file, data) def self.execute(file, data)
......
# frozen_string_literal: true
require_relative '../../migration_helpers'
require_relative '../../code_reuse_helpers'
module RuboCop
module Cop
module Scalability
class BulkPerformWithContext < RuboCop::Cop::Cop
include RuboCop::MigrationHelpers
include RuboCop::CodeReuseHelpers
MSG = <<~MSG
Prefer using `Worker.bulk_perform_async_with_contexts` and
`Worker.bulk_perform_in_with_context` over the methods without a context
if your worker deals with specific projects or namespaces
The context is required to add metadata to our logs.
If there is already a parent context that will apply to the jobs
being scheduled, please disable this cop with a comment explaing which
context will be applied.
Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context
MSG
def_node_matcher :schedules_in_batch_without_context?, <<~PATTERN
(send (...) {:bulk_perform_async :bulk_perform_in} _*)
PATTERN
def on_send(node)
return if in_migration?(node) || in_spec?(node)
return unless schedules_in_batch_without_context?(node)
return if name_of_receiver(node) == "BackgroundMigrationWorker"
add_offense(node, location: :expression)
end
private
def in_spec?(node)
file_path_for_node(node).end_with?("_spec.rb")
end
end
end
end
end
# frozen_string_literal: true
module RuboCop
module Cop
module Scalability
class CronWorkerContext < RuboCop::Cop::Cop
MSG = <<~MSG
Manually define an ApplicationContext for cronjob-workers. The context
is required to add metadata to our logs.
If there is no relevant metadata, please disable the cop with a comment
explaining this.
Read more about it https://docs.gitlab.com/ee/development/sidekiq_style_guide.html#worker-context
MSG
def_node_matcher :includes_cronjob_queue?, <<~PATTERN
(send nil? :include (const nil? :CronjobQueue))
PATTERN
def_node_search :defines_contexts?, <<~PATTERN
(send nil? :with_context _)
PATTERN
def_node_search :schedules_with_batch_context?, <<~PATTERN
(send (...) {:bulk_perform_async_with_contexts :bulk_perform_in_with_contexts} (...))
PATTERN
def on_send(node)
return unless includes_cronjob_queue?(node)
return if defines_contexts?(node.parent)
return if schedules_with_batch_context?(node.parent)
add_offense(node.arguments.first, location: :expression)
end
end
end
end
end
...@@ -44,6 +44,8 @@ require_relative 'cop/qa/element_with_pattern' ...@@ -44,6 +44,8 @@ require_relative 'cop/qa/element_with_pattern'
require_relative 'cop/qa/ambiguous_page_object_name' require_relative 'cop/qa/ambiguous_page_object_name'
require_relative 'cop/sidekiq_options_queue' require_relative 'cop/sidekiq_options_queue'
require_relative 'cop/scalability/file_uploads' require_relative 'cop/scalability/file_uploads'
require_relative 'cop/scalability/bulk_perform_with_context'
require_relative 'cop/scalability/cron_worker_context'
require_relative 'cop/destroy_all' require_relative 'cop/destroy_all'
require_relative 'cop/ruby_interpolation_in_translation' require_relative 'cop/ruby_interpolation_in_translation'
require_relative 'code_reuse_helpers' require_relative 'code_reuse_helpers'
......
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require_relative '../../../support/helpers/expect_offense'
require_relative '../../../../rubocop/cop/scalability/bulk_perform_with_context'
describe RuboCop::Cop::Scalability::BulkPerformWithContext do
include CopHelper
include ExpectOffense
subject(:cop) { described_class.new }
it "adds an offense when calling bulk_perform_async" do
inspect_source(<<~CODE.strip_indent)
Worker.bulk_perform_async(args)
CODE
expect(cop.offenses.size).to eq(1)
end
it "adds an offense when calling bulk_perform_in" do
inspect_source(<<~CODE.strip_indent)
diffs.each_batch(of: BATCH_SIZE) do |relation, index|
ids = relation.pluck_primary_key.map { |id| [id] }
DeleteDiffFilesWorker.bulk_perform_in(index * 5.minutes, ids)
end
CODE
expect(cop.offenses.size).to eq(1)
end
it "does not add an offense for migrations" do
allow(cop).to receive(:in_migration?).and_return(true)
inspect_source(<<~CODE.strip_indent)
Worker.bulk_perform_in(args)
CODE
expect(cop.offenses.size).to eq(0)
end
it "does not add an offence for specs" do
allow(cop).to receive(:in_spec?).and_return(true)
inspect_source(<<~CODE.strip_indent)
Worker.bulk_perform_in(args)
CODE
expect(cop.offenses.size).to eq(0)
end
it "does not add an offense for scheduling BackgroundMigrations" do
inspect_source(<<~CODE.strip_indent)
BackgroundMigrationWorker.bulk_perform_in(args)
CODE
expect(cop.offenses.size).to eq(0)
end
end
# frozen_string_literal: true
require 'fast_spec_helper'
require 'rubocop'
require_relative '../../../support/helpers/expect_offense'
require_relative '../../../../rubocop/cop/scalability/cron_worker_context'
describe RuboCop::Cop::Scalability::CronWorkerContext do
include CopHelper
include ExpectOffense
subject(:cop) { described_class.new }
it 'adds an offense when including CronjobQueue' do
inspect_source(<<~CODE.strip_indent)
class SomeWorker
include CronjobQueue
end
CODE
expect(cop.offenses.size).to eq(1)
end
it 'does not add offenses for other workers' do
expect_no_offenses(<<~CODE.strip_indent)
class SomeWorker
end
CODE
end
it 'does not add an offense when the class defines a context' do
expect_no_offenses(<<~CODE.strip_indent)
class SomeWorker
include CronjobQueue
with_context user: 'bla'
end
CODE
end
it 'does not add an offense when the worker calls `with_context`' do
expect_no_offenses(<<~CODE.strip_indent)
class SomeWorker
include CronjobQueue
def perform
with_context(user: 'bla') do
# more work
end
end
end
CODE
end
it 'does not add an offense when the worker calls `bulk_perform_async_with_contexts`' do
expect_no_offenses(<<~CODE.strip_indent)
class SomeWorker
include CronjobQueue
def perform
SomeOtherWorker.bulk_perform_async_with_contexts(contexts_for_arguments)
end
end
CODE
end
it 'does not add an offense when the worker calls `bulk_perform_in_with_contexts`' do
expect_no_offenses(<<~CODE.strip_indent)
class SomeWorker
include CronjobQueue
def perform
SomeOtherWorker.bulk_perform_in_with_contexts(contexts_for_arguments)
end
end
CODE
end
end
...@@ -10,7 +10,7 @@ describe CronjobQueue do ...@@ -10,7 +10,7 @@ describe CronjobQueue do
end end
include ApplicationWorker include ApplicationWorker
include CronjobQueue include CronjobQueue # rubocop:disable Scalability/CronWorkerContext
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment