Skip to content

Fix zz_metrics not running outside of production

Matthias Käppler requested to merge 407145-fix-zz-metrics into master

What does this MR do and why?

This MR fixes a regression with the zz_metrics initializer, which installs a number of important ActiveSupport::Notification handlers.

Due to a bug fix for a past problem where this initializer interfered with migration template generation, it at some point stopped running altogether outside of production environments (I suspect after we moved to zeitwerk with newer versions of Rails), which creates a test gap.

I am fixing this by making sure the initializer always runs for Puma and Sidekiq, regardless of the Rails env used.

I also tried a variant where we also run this initializer in rspec runs, and the pipeline passed with pipeline:run-all-rspec applied: https://gitlab.com/gitlab-org/gitlab/-/pipelines/846140083 This suggests that the issue linked about any_instance has also been fixed meanwhile. However, I still think it's safer not to do that since these collectors could have knock-on effects when run in controlled environment such as a test suite.

Original issues/MRs:

Screenshots or screen recordings

Screenshots are required for UI changes, and strongly recommended for all other merge requests.

How to set up and validate locally

Run bin/rails g migration test-migration

This does apply our migration template, so the original issue seems to still be resolved:

[13:36:33] gl-gck/gitlab-rails::407145-fix-zz-metrics  cat db/migrate/20230424113702_test_migration.rb
# frozen_string_literal: true

# See https://docs.gitlab.com/ee/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.

class TestMigration < Gitlab::Database::Migration[2.1]
  # When using the methods "add_concurrent_index" or "remove_concurrent_index"
  # you must disable the use of transactions
  # as these methods can not run in an existing transaction.
  # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure
  # that either of them is the _only_ method called in the migration,
  # any other changes should go in a separate migration.
  # This ensures that upon failure _only_ the index creation or removing fails
  # and can be retried or reverted easily.
  #
  # To disable transactions uncomment the following line and remove these
  # comments:
  # disable_ddl_transaction!
  #
  # Configure the `gitlab_schema` to perform data manipulation (DML).
  # Visit: https://docs.gitlab.com/ee/development/database/migrations_for_multiple_databases.html
  # restrict_gitlab_migration gitlab_schema: :gitlab_main

  def change
  end
end

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Related to #407145 (closed)

Edited by Matthias Käppler

Merge request reports

Loading