Draft: Attribute reactive cache jobs to the owner of the computation
What does this MR do and why?
ReactiveCachingWorkerand friends are declaredfeature_category :not_owned, so Sidekiq apdex and Kibana logs attribute their jobs to whoever enqueued them, not to who owns the actual computation.- A Kibana check found that license scanning report comparisons enqueued from the MR widget (
cached_widgetpolling) get counted ascode_review_workflowinstead ofsoftware_composition_analysis, skewing that team's Sidekiq metrics. - This MR lets
ReactiveCachingmodels opt a specific computation into its own feature category, and uses that to attribute the license scanning comparison to Software Composition Analysis. - This should be reviewed by, or at least surfaced to, the Software Composition Analysis group, since it moves load onto their Sidekiq apdex.
Detailed context for AI agents
Root cause
WorkerAttributes.get_feature_category (app/workers/concerns/worker_attributes.rb, around line 60) falls back to Gitlab::ApplicationContext.current_context_attribute('meta.feature_category') for any worker declared not_owned. That's the feature category of whatever HTTP endpoint or worker enqueued the job. Gitlab::SidekiqMiddleware::ServerMetrics uses that value as the feature_category label on sidekiq_jobs_* metrics, and it's also what shows up as json.meta.feature_category in Kibana. This is documented in doc/development/feature_categorization/_index.md.
ReactiveCachingWorker, ReactiveCaching::LowUrgencyWorker and ExternalServiceReactiveCachingWorker are all not_owned. The worker's first argument (related_class, for example MergeRequest) only affects logging (meta.related_class) - it does not affect feature category, despite that being a common assumption in the originating discussion.
A Kibana breakdown of ReactiveCachingWorker jobs with meta.feature_category = code_review_workflow over 24h in production showed:
Projects::MergeRequests::ContentController#cached_widget/related_class: MergeRequest: 220,363 jobs (about 85%)GraphqlController#execute:getMrSastReport/related_class: Security::MergeRequestSecurityReportGenerationService: 33,437 jobs (about 13%) - handled by the companion MR- Various MR page controllers /
related_class: Group(seat counting): about 7,000 jobs (about 3%), owned by Fulfillment, out of scope here
Other reactive caching consumers are already correctly attributed because their endpoints declare their own category: test_reports/coverage_reports are code_testing, codequality_reports is code_quality, license_scanning_reports* are software_composition_analysis, terraform_reports is infrastructure_as_code (app/controllers/projects/merge_requests_controller.rb).
The remaining slice is MergeRequest#compare_reports, called from EE::MergeRequest#has_denied_policies? (ee/app/models/ee/merge_request.rb, around line 437), which runs on every cached_widget poll via MergeRequestPollCachedWidgetEntity's policy_violation field, and only for the license scanning comparison.
Mechanism
app/models/concerns/reactive_caching.rb: newclass_attribute :reactive_cache_feature_category, defaultnil. A new private methodwith_reactive_cache_feature_category(*args, &block)wraps both places a reactive cache job is enqueued:refresh_reactive_cache!(worker_class.perform_async) andenqueuing_update(the worker's own re-enqueue viabackground_update_worker.perform_in).- The attribute can be a category symbol/string, or a callable that receives the arguments passed to
with_reactive_cacheand returns a category ornil.nil(default): no change, job keeps the caller's category.- Set: the enqueue is wrapped in
Gitlab::ApplicationContext.with_context(feature_category: category.to_s), someta.feature_categoryand the metric label become that category.
- Because the worker's self re-enqueue is wrapped too, the refresh chain keeps the category for the life of the cache, not just the first job.
ee/app/models/ee/merge_request.rb, in theprependedblock:self.reactive_cache_feature_category = ->(service_class_name, *) { :software_composition_analysis if service_class_name == 'Ci::CompareLicenseScanningReportsService' }.MergeRequest#compare_reports(service_class, ...)callswith_reactive_cache(service_class.name, user_id, report_type), so the lambda receives the service class name as a string (used instead of the constant to avoid loading the service class when the model loads) and returnsnilfor every other comparison, which keeps those on the caller's category.doc/development/reactive_caching.md: new#### self.reactive_cache_feature_categorysection under "Configurable options".
Alternatives considered and rejected
- Hardcoding
feature_category :continuous_integration(or similar) directly on the worker, as the originating MR (!254437 (closed)) did: this overrides caller attribution for every consumer, lumping code testing, code quality, SCA, vulnerability management and seat counting under one group. - Calling
Gitlab::ApplicationContext.with_context(feature_category: ...)inline at the two enqueue call sites: works, but has no precedent in application code. A concern-level attribute next to the existingreactive_cache_work_typeis the established way this concern is configured. - Pushing the category inside the worker's
perform, asCounters::CleanupRefreshWorkerandProjects::FinalizeProjectStatisticsRefreshWorkerdo: this only fixes logs, becauseServerMetricsbuilds its labels beforeperformruns. - Moving the license policy check out of
cached_widget: a larger, product/frontend-scoped change.
Verification performed
bundle exec rspec spec/models/concerns/reactive_caching_spec.rb: 66 examples, 0 failures. Newdescribe '.reactive_cache_feature_category'covers: not set keeps the caller's category; set to a symbol attributes both the initial job and the scheduled refresh (LowUrgencyWorker); a lambda uses the returned category; a lambda returningnilkeeps the caller's category. Assertions run against the real Sidekiq fake job hash'smeta.feature_category, not stubs.bundle exec rspec ee/spec/models/merge_request_spec.rb:1038(the#has_denied_policies?block): 11 examples, 0 failures. New context "when the comparison is not cached yet" (using the:with_cyclonedx_reportsfactory) asserts the enqueuedReactiveCachingWorkerjob's args includeCi::CompareLicenseScanningReportsServiceandmeta.feature_category == 'software_composition_analysis', confirmed by hand to fail without the EE declaration (gotcode_review_workflow).- A throwaway request spec hit
GET /:project/-/merge_requests/:iid/cached_widget.jsonwith license scanning results present: before this change the enqueued job carriedmeta.feature_category=code_review_workflow(callerProjects::MergeRequests::ContentController#cached_widget); after, it carriessoftware_composition_analysis. - A throwaway spec ran a job through the full production Sidekiq server middleware chain and confirmed the
ServerMetricslabel followsmeta.feature_category(and falls back tonot_ownedwhen the caller has none). - RuboCop clean on all changed Ruby files. Vale and markdownlint clean on the doc change (lefthook pre-commit).
Out of scope
EE::Group#billable_members_countseat count jobs (about 3% of the slice, owned by Fulfillment).- The SAST report query slice - covered by the companion MR.
- A general service-class-to-category mapping for every
compare_reportsconsumer - not needed, since their endpoints already carry the right category. - No changelog trailer: not user facing.
References
- Origin discussion: !254437 (comment 3861759678)
- This MR: !256681 (closed)
- Companion MR: !256683 (merged)
Screenshots or screen recordings
No UI change.
How to set up and validate locally
- Run the concern spec:
bundle exec rspec spec/models/concerns/reactive_caching_spec.rb. - Run the EE spec:
bundle exec rspec ee/spec/models/merge_request_spec.rb:1038. - Optionally, open the MR page of a project with license scanning results on the MR head pipeline, then check the
sidekiqlog forReactiveCachingWorkerentries withmeta.related_class=MergeRequest.meta.feature_categoryshould besoftware_composition_analysisfor the license comparison job.
MR acceptance checklist
Evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.