Expand the instrumentation FF coverage [RUN ALL RSPEC] [RUN AS-IF-FOSS]
What does this MR do?
I noticed that we need to add more code under the method_instrumentation_disable_initialization
feature flag as it exclusively serves for the instrumentation init statements.
We could've probably jumped over this step and remove this code alongside the FF removal, but I prefer to move in tiny steps here because it's an initializer.
Even though the code shouldn't do anything outside the Instrumentation, I have a tiny concern about the possible implicit effects of no longer calling the require_dependency file
.
The diff may look messy, it's worth checking the whole file: https://gitlab.com/gitlab-org/gitlab/-/blob/257d5ebd1636a8d35c130f782d148c1c5c04d3ff/config/initializers/zz_metrics.rb#L167
production
again: https://gitlab.slack.com/archives/C101F3796/p1631013557358300 . I will re-enable and monitor it after this MR is released.
How to setup and validate locally (strongly suggested)
Make sure that the application restarts with and enabled/disabled FF:
rails c >
::Feature.disable(:method_instrumentation_disable_initialization)
bash >
gdk restart rails-web
and
rails c >
::Feature.enable(:method_instrumentation_disable_initialization)
bash >
gdk restart rails-web
Check for logs and that the app started fine.
Does this MR meet the acceptance criteria?
Conformity
-
I have included changelog trailers, or none are needed. (Does this MR need a changelog?) -
I have added/updated documentation, or it's not needed. (Is documentation required?) -
I have properly separated EE content from FOSS, or this MR is FOSS only. (Where should EE code go?) -
I have added information for database reviewers in the MR description, or it's not needed. (Does this MR have database related changes?) -
I have self-reviewed this MR per code review guidelines. -
This MR does not harm performance, or I have asked a reviewer to help assess the performance impact. (Merge request performance guidelines) -
I have followed the style guides. -
This change is backwards compatible across updates, or this does not apply.
Availability and Testing
-
I have added/updated tests following the Testing Guide, or it's not needed. (Consider all test levels. See the Test Planning Process.) -
I have tested this MR in all supported browsers, or it's not needed. -
I have informed the Infrastructure department of a default or new setting change per definition of done, or it's not needed.
Security
N/A
Related to #217978 (closed)