Commit 1458985e authored by Aleksei Lipniagov's avatar Aleksei Lipniagov

Disable method instrumentation initialization

We are deprecating the method instrumentation the way we implemented it,
as we are not using the data produced by the instrumented classes and
methods while the approach:
- redefines every method with metaprogramming/monkey patching
- makes some assumptions on the method arity which is tricky to test
and may be prone to errors
- appears almost on every trace, so it could mask the real issue
with the argument mismatch
- does something that the profiler should do,
but with the custom solution
- adds performance & memory penalty to our call chain
The implementation itself would be removed in next MRs, to have smaller
iterations and reduce the risks.

Changelog: removed
parent e90a5bf2
---
name: method_instrumentation_disable_initialization
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69091
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/339665
milestone: '14.3'
type: development
group: group::memory
default_enabled: false
......@@ -162,49 +162,6 @@ if Gitlab::Metrics.enabled? && !Rails.env.test? && !(Rails.env.development? && d
config.middleware.use(Gitlab::Metrics::ElasticsearchRackMiddleware)
end
# We are removing the Instrumentation module entirely in steps.
# More in https://gitlab.com/gitlab-org/gitlab/-/issues/217978.
unless ::Feature.enabled?(:method_instrumentation_disable_initialization)
# This instruments all methods residing in app/models that (appear to) use any
# of the ActiveRecord methods. This has to take place _after_ initializing as
# for some unknown reason calling eager_load! earlier breaks Devise.
Gitlab::Application.config.after_initialize do
# We should move all the logic of this file to somewhere else
# and require it after `Rails.application.initialize!` in `environment.rb` file.
models_path = Rails.root.join('app', 'models').to_s
Dir.glob("**/*.rb", base: models_path).sort.each do |file|
require_dependency file
end
regex = Regexp.union(
ActiveRecord::Querying.public_instance_methods(false).map(&:to_s)
)
Gitlab::Metrics::Instrumentation
.instrument_class_hierarchy(ActiveRecord::Base) do |klass, method|
# Instrumenting the ApplicationSetting class can lead to an infinite
# loop. Since the data is cached any way we don't really need to
# instrument it.
if klass == ApplicationSetting
false
else
loc = method.source_location
loc && loc[0].start_with?(models_path) && method.source =~ regex
end
end
# Ability is in app/models, is not an ActiveRecord model, but should still
# be instrumented.
Gitlab::Metrics::Instrumentation.instrument_methods(Ability)
end
Gitlab::Metrics::Instrumentation.configure do |config|
instrument_classes(config)
end
end
GC::Profiler.enable
module TrackNewRedisConnections
......
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