Merge pull request #2956 from zendesk/grosser/ref

reword tracer PR

.env.example

@@ -108,9 +108,12 @@ PERIODICAL=stop_expired_deploys:60,remove_expired_locks:60,report_system_stats:6
 # RELEASE_TAG_IN_REPO_RETRIES=10 # how often to retry finding a tag after it is released to github
 
 ## NewRelic: optional report performance stats see https://docs.newrelic.com/docs/agents/ruby-agent/configuration/ruby-agent-configuration
-# NEW_RELIC_LICENSE_KEY: my-key
-# NEW_RELIC_APP_NAME: Samson
-# NEW_RELIC_LOG_FILE_PATH: STDOUT
+# NEW_RELIC_LICENSE_KEY=
+# NEW_RELIC_APP_NAME=Samson
+# NEW_RELIC_LOG_FILE_PATH=STDOUT
+#
+# optional: show graphs during/after deploy
+# NEW_RELIC_API_KEY=
 
 ## Memcache: configure servers or we use localhost
 # MEMCACHIER_SERVERS=a:123,b:234

Gemfile

@@ -42,7 +42,6 @@ gem 'coderay'
 gem 'net-http-persistent'
 gem 'concurrent-ruby'
 gem 'vault'
-gem 'newrelic_rpm'
 gem 'lograge'
 gem 'logstash-event'
 gem 'diffy'

Gemfile.lock

@@ -94,6 +94,7 @@ PATH
   remote: plugins/new_relic
   specs:
     samson_new_relic (0.0.0)
+      newrelic_rpm
 
 PATH
   remote: plugins/pipelines
@@ -583,7 +584,6 @@ DEPENDENCIES
   momentjs-rails
   mysql2
   net-http-persistent
-  newrelic_rpm
   octokit
   omniauth
   omniauth-bitbucket

app/models/deploy_service.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 class DeployService
-  include ::Samson::PerformanceTracer
+  extend ::Samson::PerformanceTracer::Tracers
   attr_reader :user
 
   def initialize(user)

app/models/docker_builder_service.rb

@@ -2,7 +2,7 @@
 require 'docker'
 
 class DockerBuilderService
-  include ::Samson::PerformanceTracer
+  extend ::Samson::PerformanceTracer::Tracers
 
   def initialize(build)
     @build = build

app/models/git_repository.rb

@@ -2,7 +2,7 @@
 # Responsible for all git knowledge of a repo
 # Caches a local mirror (not a full checkout) and creates a workspace when deploying
 class GitRepository
-  include ::Samson::PerformanceTracer
+  extend ::Samson::PerformanceTracer::Tracers
 
   attr_accessor :executor # others set this to listen in on commands being executed
 
@@ -132,7 +132,7 @@ def clone!
   def create_workspace(temp_dir)
     executor.execute "git clone #{repo_cache_dir} #{temp_dir}"
   end
-  add_tracer :create_workspace!
+  add_tracer :create_workspace
 
   def update!
     executor.execute("cd #{repo_cache_dir}", 'git fetch -p')

app/models/image_builder.rb

@@ -4,9 +4,9 @@
 
 class ImageBuilder
   class << self
-    DIGEST_SHA_REGEX = /Digest:.*(sha256:[0-9a-f]{64})/i
+    extend ::Samson::PerformanceTracer::Tracers
 
-    include ::Samson::PerformanceTracer
+    DIGEST_SHA_REGEX = /Digest:.*(sha256:[0-9a-f]{64})/i
 
     def build_image(dir, build, executor, tag_as_latest:, **args)
       if DockerRegistry.all.empty?

app/models/job_execution.rb

@@ -2,7 +2,7 @@
 require 'shellwords'
 
 class JobExecution
-  include ::Samson::PerformanceTracer
+  extend ::Samson::PerformanceTracer::Tracers
 
   cattr_accessor(:cancel_timeout, instance_writer: false) { 15.seconds }
 

app/models/multi_lock.rb

@@ -4,7 +4,7 @@ class MultiLock
   cattr_accessor(:locks) { {} }
 
   class << self
-    include ::Samson::PerformanceTracer
+    extend ::Samson::PerformanceTracer::Tracers
 
     def lock(id, holder, options)
       locked = wait_for_lock(id, holder, options)

config/application.rb

@@ -17,22 +17,11 @@
 Bundler.require(:assets) if Rails.env.development? || ENV["PRECOMPILE"]
 
 ###
-# Railties need to be loaded before the application is defined
+# Railties need to be loaded before the application is initialized
 if ['development', 'staging'].include?(Rails.env) && ENV["SERVER_MODE"]
   require 'rack-mini-profiler' # side effect: removes expires headers
   Rack::MiniProfiler.config.authorization_mode = :allow_all
 end
-
-if ['staging', 'production'].include?(Rails.env)
-  require 'newrelic_rpm'
-else
-  # avoids circular dependencies warning
-  # https://discuss.newrelic.com/t/circular-require-in-ruby-agent-lib-new-relic-agent-method-tracer-rb/42737
-  require 'new_relic/control'
-
-  # needed even in dev/test mode
-  require 'new_relic/agent/method_tracer'
-end
 # END Railties
 ###
 

config/initializers/docker.rb

@@ -16,6 +16,7 @@
     end
   rescue
     warn "Unable to verify local docker!"
-    ErrorNotifier.notify($!, sync: true) # sync to avoid background threads breaking boot_check.rb thread checker
+    # errors and hooks they trigger cause background threads, that would break boot_check.rb thread checker
+    ErrorNotifier.notify($!) unless Rails.env.development?
   end
 end

docs/plugins.md

@@ -25,7 +25,7 @@ Available plugins:
  - [Kubernetes](https://github.com/zendesk/samson/tree/master/plugins/kubernetes)
  - [Ledger](https://github.com/zendesk/samson/tree/master/plugins/ledger)
  - [Release Number From CI](https://github.com/redbubble/samson-release-number-from-ci)
- - [NewRelic monitoring](https://github.com/zendesk/samson/tree/master/plugins/new_relic)
+ - [NewRelic](https://github.com/zendesk/samson/tree/master/plugins/new_relic)
  - [Pipelined deploys](https://github.com/zendesk/samson/tree/master/plugins/pipelines)
  - [Slack deploys](https://github.com/zendesk/samson/tree/master/plugins/slack_app)
  - [Slack notifications](https://github.com/zendesk/samson/tree/master/plugins/slack_webhooks)

lib/samson/hooks.rb

@@ -58,7 +58,7 @@ class UserError < StandardError
       :release_deploy_conditions,
       :stage_clone,
       :stage_permitted_params,
-      :performance_tracer,
+      :trace_method,
       :asynchronous_performance_tracer
     ].freeze
 

lib/samson/performance_tracer.rb

@@ -1,41 +1,22 @@
 # frozen_string_literal: true
 module Samson
   module PerformanceTracer
-    # It's used to trace the hook fire event,
-    # We can't use the samson hook to fetch the available plugins.
-    TRACER_PLUGINS = ['SamsonNewRelic', 'SamsonDatadogTracer::APM'].freeze
-
     class << self
-      def included(clazz)
-        clazz.extend ClassMethods
-      end
-
-      def trace_execution_scoped(scope_name)
-        # Tracing the scope is restricted to avoid into slow startup
-        # Refer Samson::BootCheck
-        if ['staging', 'production'].include?(Rails.env)
-          plugins = TRACER_PLUGINS.map(&:safe_constantize).compact
-          execution = using_plugins plugins, scope_name do
-            yield
-          end
-          execution.call
-        else
-          yield
-        end
+      # NOTE: this cannot be a hook since it is used from Hooks#fire
+      def handlers
+        @handlers ||= []
       end
 
-      def using_plugins(plugins, scope_name, &block)
-        plugins.inject(block) { |inner, plugin| plugin.trace_method_execution_scope(scope_name) { inner } }
+      def trace_execution_scoped(scope_name, &block)
+        handlers.inject(block) { |inner, plugin| -> { plugin.trace_execution_scoped(scope_name, &inner) } }.call
       end
     end
 
-    # for Newrelic and Datadog -> for tracer plugins.
-    module ClassMethods
+    module Tracers
       def add_tracer(method)
-        Samson::Hooks.fire(:performance_tracer, self, method)
+        Samson::Hooks.fire(:trace_method, self, method)
       end
 
-      # TODO: Add asynchronous tracer for Datadog.
       def add_asynchronous_tracer(method, options)
         Samson::Hooks.fire(:asynchronous_performance_tracer, self, method, options)
       end

plugins/datadog_tracer/lib/samson_datadog_tracer/samson_plugin.rb

@@ -3,22 +3,61 @@ module SamsonDatadogTracer
   class Engine < Rails::Engine
   end
 
-  def self.enabled?
-    !!ENV['DATADOG_TRACER']
+  class << self
+    def enabled?
+      !!ENV['DATADOG_TRACER']
+    end
+
+    def trace_execution_scoped(scope_name, &block)
+      if enabled?
+        Datadog.tracer.trace("Custom/Hooks/#{scope_name}", &block)
+      else
+        yield
+      end
+    end
+
+    # We are not using super to make running newrelic (which uses alias) and datadog possible
+    def trace_method(klass, method)
+      wrap_method klass, method, "apm_tracer" do |&block|
+        Datadog.tracer.trace("#{klass}###{method}", &block)
+      end
+    end
+
+    private
+
+    def wrap_method(klass, method, scope, &callback)
+      visibility = method_visibility(klass, method)
+      without = "without_#{scope}_#{method}"
+      if klass.method_defined?(without) || klass.private_method_defined?(without)
+        raise "#{scope} wrapper already defined for #{method}"
+      end
+      klass.alias_method without, method
+      klass.define_method(method) do |*args, &block|
+        callback.call { send(without, *args, &block) } # rubocop:disable Performance/RedundantBlockCall
+      end
+      klass.send visibility, method
+      klass.send visibility, without
+    end
+
+    def method_visibility(klass, method)
+      if klass.protected_method_defined?(method)
+        :protected
+      elsif klass.private_method_defined?(method)
+        :private
+      else
+        :public
+      end
+    end
   end
 end
 
-Samson::Hooks.callback :performance_tracer do |klass, method|
+require 'samson/performance_tracer'
+Samson::PerformanceTracer.handlers << SamsonDatadogTracer
+
+Samson::Hooks.callback :trace_method do |klass, method|
   if SamsonDatadogTracer.enabled?
-    klass.class_eval do
-      include SamsonDatadogTracer::APM
-      helper = SamsonDatadogTracer::APM::Helpers
-      trace_method method
-
-      if method_defined?(method) || private_method_defined?(method)
-        alias_method helper.untracer_method_name(method), method
-        alias_method method, helper.tracer_method_name(method)
-      end
-    end
+    SamsonDatadogTracer.trace_method klass, method
   end
 end
+
+# TODO: support :asynchronous_performance_tracer hook, see lib/samson/performance_tracer.rb