DEBUG-2334 enable dynamic instrumentation line probe benchmarks (#4122)

Author: p-datadog

benchmarks/di_instrument.rb

@@ -69,19 +69,24 @@ def test_method_for_line_probe
     end
   end
 
-  def run_benchmark
+  attr_reader :instrumenter
+
+  def logger
+    @logger ||= Logger.new(STDERR)
+  end
+
+  def configure
     settings = Datadog.configuration
-    # We benchmark untargeted and targeted trace points; untargeted ones
-    # are prohibited by default, permit them.
-    begin
-      settings.dynamic_instrumentation.internal.untargeted_trace_points = true
-    rescue NoMethodError
-      settings.dynamic_instrumentation.untargeted_trace_points = true
-    end
+    yield settings if block_given?
+
     redactor = Datadog::DI::Redactor.new(settings)
     serializer = Datadog::DI::Serializer.new(settings, redactor)
-    logger = Logger.new(STDERR)
-    instrumenter = Datadog::DI::Instrumenter.new(settings, serializer, logger)
+    @instrumenter = Datadog::DI::Instrumenter.new(settings, serializer, logger,
+      code_tracker: Datadog::DI.code_tracker)
+  end
+
+  def run_benchmark
+    configure
 
     m = Target.instance_method(:test_method_for_line_probe)
     file, line = m.source_location
@@ -103,9 +108,12 @@ def run_benchmark
     calls = 0
     probe = Datadog::DI::Probe.new(id: 1, type: :log,
       type_name: 'DIInstrumentBenchmark::Target', method_name: 'test_method')
-    instrumenter.hook_method(probe) do
+    rv = instrumenter.hook_method(probe) do
       calls += 1
     end
+    unless rv
+      raise "Method probe was not successfully installed"
+    end
 
     Benchmark.ips do |x|
       benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
@@ -131,21 +139,31 @@ def run_benchmark
 
     instrumenter.unhook(probe)
 
-=begin Line probes require more of DI code to be merged
+    # We benchmark untargeted and targeted trace points; untargeted ones
+    # are prohibited by default, permit them.
+    # In order to install untargeted trace point, we currently need to
+    # disable code tracking.
+    Datadog::DI.deactivate_tracking!
+    configure do |c|
+      c.dynamic_instrumentation.internal.untargeted_trace_points = true
+    end
+
     calls = 0
     probe = Datadog::DI::Probe.new(id: 1, type: :log,
       file: file, line_no: line + 1)
-    instrumenter.hook_line(probe) do
+    rv = instrumenter.hook_line(probe) do
       calls += 1
     end
+    unless rv
+      raise "Line probe (in method) was not successfully installed"
+    end
 
     Benchmark.ips do |x|
       benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
       x.config(
         **benchmark_time,
       )
-
-      x.report('line instrumentation') do
+      x.report('line instrumentation - untargeted') do
         Target.new.test_method_for_line_probe
       end
 
@@ -161,7 +179,13 @@ def run_benchmark
       raise "Expected at least 1000 calls to the method, got #{calls}"
     end
 
+    instrumenter.unhook(probe)
+
     Datadog::DI.activate_tracking!
+    configure do |c|
+      c.dynamic_instrumentation.internal.untargeted_trace_points = false
+    end
+
     if defined?(DITarget)
       raise "DITarget is already defined, this should not happen"
     end
@@ -173,13 +197,15 @@ def run_benchmark
     m = DITarget.instance_method(:test_method_for_line_probe)
     targeted_file, targeted_line = m.source_location
 
-    instrumenter.unhook(probe)
     calls = 0
     probe = Datadog::DI::Probe.new(id: 1, type: :log,
       file: targeted_file, line_no: targeted_line + 1)
-    instrumenter.hook_line(probe) do
+    rv = instrumenter.hook_line(probe) do
       calls += 1
     end
+    unless rv
+      raise "Line probe (targeted) was not successfully installed"
+    end
 
     Benchmark.ips do |x|
       benchmark_time = VALIDATE_BENCHMARK_MODE ? { time: 0.01, warmup: 0 } : { time: 10, warmup: 2 }
@@ -207,7 +233,6 @@ def run_benchmark
     # target code is approximately what it was prior to hook installation.
 
     instrumenter.unhook(probe)
-=end
 
     calls = 0
 

lib/datadog/di/instrumenter.rb

@@ -266,7 +266,11 @@ def hook_line(probe, &block)
             # If trace point is not targeted, we must verify that the invocation
             # is the file & line that we want, because untargeted trace points
             # are invoked for *each* line of Ruby executed.
-            if iseq || tp.lineno == probe.line_no && probe.file_matches?(tp.path)
+            # TODO find out exactly when the path in trace point is relative.
+            # Looks like this is the case when line trace point is not targeted?
+            if iseq || tp.lineno == probe.line_no && (
+              probe.file == tp.path || probe.file_matches?(tp.path)
+            )
               if rate_limiter.nil? || rate_limiter.allow?
                 # & is to stop steep complaints, block is always present here.
                 block&.call(probe: probe, trace_point: tp, caller_locations: caller_locations)
@@ -305,7 +309,9 @@ def hook_line(probe, &block)
           else
             tp.enable
           end
+          # TracePoint#enable returns false when it succeeds.
         end
+        true
       end
 
       def unhook_line(probe)

spec/datadog/di/validate_benchmarks_spec.rb

@@ -1,8 +1,5 @@
 require "datadog/di/spec_helper"
 
-# rubocop:disable Style/BlockComments
-
-=begin benchmarks require DI code to be merged
 RSpec.describe "Dynamic instrumentation benchmarks", :memcheck_valgrind_skip do
   di_test
 
@@ -29,6 +26,3 @@
     expect(benchmarks_to_validate).to contain_exactly(*all_benchmarks)
   end
 end
-=end
-
-# rubocop:enable Style/BlockComments