Skip to content

Commit

Permalink
DEBUG-3182 DI: remove hard dependency on tracing (#4223)
Browse files Browse the repository at this point in the history
  • Loading branch information
p-datadog authored Dec 17, 2024
1 parent 28d1ef8 commit 7349680
Show file tree
Hide file tree
Showing 13 changed files with 76 additions and 30 deletions.
2 changes: 2 additions & 0 deletions lib/datadog/di/component.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require_relative '../core'

module Datadog
module DI
# Component for dynamic instrumentation.
Expand Down
2 changes: 0 additions & 2 deletions lib/datadog/di/init.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
# enable dynamic instrumentation for third-party libraries used by the
# application.

require_relative '../tracing'
require_relative '../tracing/contrib'
require_relative '../di'

# Code tracking is required for line probes to work; see the comments
Expand Down
18 changes: 16 additions & 2 deletions lib/datadog/di/probe_notification_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,17 @@ def build_snapshot(probe, rv: nil, snapshot: nil, path: nil,
version: 2,
},
# TODO add tests that the trace/span id is correctly propagated
"dd.trace_id": Datadog::Tracing.active_trace&.id&.to_s,
"dd.span_id": Datadog::Tracing.active_span&.id&.to_s,
"dd.trace_id": active_trace&.id&.to_s,
"dd.span_id": active_span&.id&.to_s,
ddsource: 'dd_debugger',
message: probe.template && evaluate_template(probe.template,
duration: duration ? duration * 1000 : 0),
timestamp: timestamp,
}
end

private

def build_status(probe, message:, status:)
{
service: settings.service,
Expand Down Expand Up @@ -200,6 +202,18 @@ def get_local_variables(trace_point)
map[name] = value
end
end

def active_trace
if defined?(Datadog::Tracing)
Datadog::Tracing.active_trace
end
end

def active_span
if defined?(Datadog::Tracing)
Datadog::Tracing.active_span
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/datadog/di/transport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require 'ostruct'
require_relative 'error'
require_relative '../core/transport/http/adapters/net'

module Datadog
module DI
Expand Down
3 changes: 3 additions & 0 deletions sig/datadog/di/probe_notification_builder.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@ module Datadog
def timestamp_now: () -> Integer

def get_local_variables: (TracePoint trace_point) -> Hash[Symbol,untyped]

def active_trace: () -> Datadog::Tracing::TraceSegment?
def active_span: () -> Datadog::Tracing::SpanOperation?
end
end
end
7 changes: 2 additions & 5 deletions spec/datadog/di/code_tracker_spec.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
require 'datadog/di'
require "datadog/di/spec_helper"
require "datadog/di/code_tracker"

RSpec.describe Datadog::DI::CodeTracker do
di_test

before(:all) do
Datadog::DI.deactivate_tracking!
end
deactivate_code_tracking

let(:tracker) do
described_class.new
Expand Down
57 changes: 37 additions & 20 deletions spec/datadog/di/instrumenter_spec.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
require "datadog/di/spec_helper"
require 'datadog/di'
require 'datadog/di/instrumenter'
require 'datadog/di/code_tracker'
require 'datadog/di/serializer'
require 'datadog/di/probe'
require_relative 'hook_line'
require_relative 'hook_method'
require 'logger'

# The examples below use a local code tracker when they set line probes,
# for better test encapsulation and to avoid having to clear/reset global state.
RSpec.describe Datadog::DI::Instrumenter do
di_test

Expand Down Expand Up @@ -55,6 +61,18 @@
%i[caller_locations duration probe rv serialized_entry_args]
end

shared_context 'with code tracking' do
let!(:code_tracker) do
Datadog::DI::CodeTracker.new.tap do |tracker|
tracker.start
end
end

after do
code_tracker.stop
end
end

describe '.hook_method' do
after do
instrumenter.unhook(probe)
Expand Down Expand Up @@ -582,6 +600,12 @@
end

context 'when hooking two identical but different probes' do
include_context 'with code tracking'

before do
load File.join(File.dirname(__FILE__), 'hook_line_recursive.rb')
end

let(:probe) do
Datadog::DI::Probe.new(**base_probe_args.merge(
type_name: 'HookTestClass', method_name: 'hook_test_method'
Expand Down Expand Up @@ -810,12 +834,10 @@
end

context 'with code tracking' do
let(:code_tracker) { Datadog::DI.code_tracker }
include_context 'with code tracking'

before do
expect(di_internal_settings).to receive(:untargeted_trace_points).and_return(false)
Datadog::DI.activate_tracking!
code_tracker.clear
end

let(:probe) do
Expand Down Expand Up @@ -906,7 +928,6 @@

context 'when hooking same line twice with identical but different probes' do
before(:all) do
Datadog::DI.activate_tracking!
require_relative 'hook_line_basic'
end

Expand Down Expand Up @@ -952,24 +973,22 @@
end

context 'when code tracking is available' do
before do
Datadog::DI.activate_tracking!
require_relative 'hook_line_targeted'
include_context 'with code tracking'

before do
path = File.join(File.dirname(__FILE__), 'hook_line_targeted.rb')
expect(Datadog::DI.code_tracker.send(:registry)[path]).to be_a(RubyVM::InstructionSequence)
load path
expect(code_tracker.send(:registry)[path]).to be_a(RubyVM::InstructionSequence)
end

let(:code_tracker) { Datadog::DI.code_tracker }

let(:probe) do
Datadog::DI::Probe.new(file: 'hook_line_targeted.rb', line_no: 3,
id: 1, type: :log)
end

it 'targets the trace point' do
path = File.join(File.dirname(__FILE__), 'hook_line_targeted.rb')
target = Datadog::DI.code_tracker.send(:registry)[path]
target = code_tracker.send(:registry)[path]
expect(target).to be_a(RubyVM::InstructionSequence)

expect_any_instance_of(TracePoint).to receive(:enable).with(target: target, target_line: 3).and_call_original
Expand Down Expand Up @@ -1000,13 +1019,12 @@
end

context 'when method is recursive' do
before(:all) do
Datadog::DI.activate_tracking!
include_context 'with code tracking'

before do
load File.join(File.dirname(__FILE__), 'hook_line_recursive.rb')
end

let(:code_tracker) { Datadog::DI.code_tracker }

context 'non-enriched probe' do
let(:probe_args) do
{file: 'hook_line_recursive.rb', line_no: 3}
Expand Down Expand Up @@ -1039,13 +1057,12 @@
end

context 'when method is infinitely recursive' do
before(:all) do
Datadog::DI.activate_tracking!
include_context 'with code tracking'

before do
require_relative 'hook_line_recursive'
end

let(:code_tracker) { Datadog::DI.code_tracker }

# We need to use a rate limiter, otherwise the stack is exhausted
# very slowly and this test burns 100% CPU for a long time performing
# snapshot building etc.
Expand Down
2 changes: 2 additions & 0 deletions spec/datadog/di/probe_manager_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require "datadog/di/spec_helper"
require 'datadog/di/probe_manager'
require 'datadog/di/instrumenter'
require 'logger'

class ProbeManagerSpecTestClass; end

Expand Down
1 change: 1 addition & 0 deletions spec/datadog/di/probe_notifier_worker_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require "datadog/di/spec_helper"
require "datadog/di/probe_notifier_worker"
require 'logger'

RSpec.describe Datadog::DI::ProbeNotifierWorker do
di_test
Expand Down
2 changes: 2 additions & 0 deletions spec/datadog/di/remote_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
require "datadog/di/spec_helper"
require 'datadog/di'
require 'spec_helper'
require 'logger'

RSpec.describe Datadog::DI::Remote do
di_test
Expand Down
8 changes: 8 additions & 0 deletions spec/datadog/di/spec_helper.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
module DIHelpers
module ClassMethods
def deactivate_code_tracking
before(:all) do
if Datadog::DI.respond_to?(:deactivate_tracking!)
Datadog::DI.deactivate_tracking!
end
end
end

def ruby_2_only
if RUBY_VERSION >= '3'
before(:all) do
Expand Down
1 change: 1 addition & 0 deletions spec/datadog/di/transport_spec.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
require "datadog/di/spec_helper"
require "datadog/di/transport"
require 'datadog/core/configuration/agent_settings_resolver'

RSpec.describe Datadog::DI::Transport do
di_test
Expand Down
2 changes: 1 addition & 1 deletion spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -306,4 +306,4 @@ def initialize(*args, &block)
# Code tracking calls out to the current DI component, which may reference
# mock objects in the test suite. Disable it and tests that need code tracking
# will enable it back for themselves.
Datadog::DI.deactivate_tracking! if Datadog::DI.respond_to?(:deactivate_tracking!)
Datadog::DI.deactivate_tracking! if defined?(Datadog::DI) && Datadog::DI.respond_to?(:deactivate_tracking!)

0 comments on commit 7349680

Please sign in to comment.