Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DEBUG-3251 dependency inject logger into DI component #4262

Merged
merged 2 commits into from
Jan 8, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/datadog/core/configuration/components.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def initialize(settings)
@runtime_metrics = self.class.build_runtime_metrics_worker(settings)
@health_metrics = self.class.build_health_metrics(settings)
@appsec = Datadog::AppSec::Component.build_appsec_component(settings, telemetry: telemetry)
@dynamic_instrumentation = Datadog::DI::Component.build(settings, agent_settings, telemetry: telemetry)
@dynamic_instrumentation = Datadog::DI::Component.build(settings, agent_settings, @logger, telemetry: telemetry)

self.class.configure_tracing(settings)
end
Expand Down
20 changes: 10 additions & 10 deletions lib/datadog/di/component.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,22 @@ module DI
# resources and installed tracepoints upon shutdown.
class Component
class << self
def build(settings, agent_settings, telemetry: nil)
def build(settings, agent_settings, logger, telemetry: nil)
p-datadog marked this conversation as resolved.
Show resolved Hide resolved
return unless settings.respond_to?(:dynamic_instrumentation) && settings.dynamic_instrumentation.enabled

unless settings.respond_to?(:remote) && settings.remote.enabled
Datadog.logger.debug("Dynamic Instrumentation could not be enabled because Remote Configuration Management is not available. To enable Remote Configuration, see https://docs.datadoghq.com/agent/remote_config")
logger.debug("Dynamic Instrumentation could not be enabled because Remote Configuration Management is not available. To enable Remote Configuration, see https://docs.datadoghq.com/agent/remote_config")
return
end

return unless environment_supported?(settings)
return unless environment_supported?(settings, logger)

new(settings, agent_settings, Datadog.logger, code_tracker: DI.code_tracker, telemetry: telemetry).tap do |component|
new(settings, agent_settings, logger, code_tracker: DI.code_tracker, telemetry: telemetry).tap do |component|
DI.add_current_component(component)
end
end

def build!(settings, agent_settings, telemetry: nil)
def build!(settings, agent_settings, logger, telemetry: nil)
unless settings.respond_to?(:dynamic_instrumentation) && settings.dynamic_instrumentation.enabled
raise "Requested DI component but DI is not enabled in settings"
end
Expand All @@ -40,27 +40,27 @@ def build!(settings, agent_settings, telemetry: nil)
raise "Requested DI component but remote config is not enabled in settings"
end

unless environment_supported?(settings)
unless environment_supported?(settings, logger)
raise "DI does not support the environment (development or Ruby version too low or not MRI)"
end

new(settings, agent_settings, Datadog.logger, code_tracker: DI.code_tracker, telemetry: telemetry)
new(settings, agent_settings, logger, code_tracker: DI.code_tracker, telemetry: telemetry)
end

# Checks whether the runtime environment is supported by
# dynamic instrumentation. Currently we only require that, if Rails
# is used, that Rails environment is not development because
# DI does not currently support code unloading and reloading.
def environment_supported?(settings)
def environment_supported?(settings, logger)
# TODO add tests?
unless settings.dynamic_instrumentation.internal.development
if Datadog::Core::Environment::Execution.development?
Datadog.logger.debug("Not enabling dynamic instrumentation because we are in development environment")
logger.debug("Not enabling dynamic instrumentation because we are in development environment")
return false
end
end
if RUBY_ENGINE != 'ruby' || RUBY_VERSION < '2.6'
Datadog.logger.debug("Not enabling dynamic instrumentation because of unsupported Ruby version")
logger.debug("Not enabling dynamic instrumentation because of unsupported Ruby version")
return false
end
true
Expand Down
8 changes: 4 additions & 4 deletions sig/datadog/di/component.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ module Datadog

@probe_manager: untyped

def self.build: (untyped settings, untyped agent_settings, ?telemetry: untyped?) -> (nil | untyped)
def self.build: (untyped settings, untyped agent_settings, Core::Logger logger, ?telemetry: untyped?) -> (nil | untyped)

def self.build!: (untyped settings, untyped agent_settings, ?telemetry: untyped?) -> untyped
def self.environment_supported?: (untyped settings) -> (false | true)
def self.build!: (untyped settings, untyped agent_settings, Core::Logger logger, ?telemetry: untyped?) -> untyped
def self.environment_supported?: (untyped settings, Core::Logger logger) -> (false | true)

def initialize: (untyped settings, untyped agent_settings, untyped logger, ?code_tracker: untyped?, ?telemetry: untyped?) -> void
def initialize: (untyped settings, untyped agent_settings, Core::Logger logger, ?code_tracker: untyped?, ?telemetry: untyped?) -> void

attr_reader settings: untyped

Expand Down
11 changes: 8 additions & 3 deletions spec/datadog/di/component_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@
instance_double_agent_settings
end

let(:logger) do
instance_double(Logger)
end

context 'when dynamic instrumentation is enabled' do
let(:dynamic_instrumentation_enabled) { true }

Expand All @@ -32,7 +36,7 @@
end

it 'returns a Datadog::DI::Component instance' do
component = described_class.build(settings, agent_settings)
component = described_class.build(settings, agent_settings, logger)
expect(component).to be_a(described_class)
component.shutdown!
end
Expand All @@ -44,7 +48,8 @@
end

it 'returns nil' do
component = described_class.build(settings, agent_settings)
expect(logger).to receive(:debug).with(/Dynamic Instrumentation could not be enabled because Remote Configuration Management is not available/)
component = described_class.build(settings, agent_settings, logger)
expect(component).to be nil
end
end
Expand All @@ -54,7 +59,7 @@
let(:dynamic_instrumentation_enabled) { false }

it 'returns nil' do
component = described_class.build(settings, agent_settings)
component = described_class.build(settings, agent_settings, logger)
expect(component).to be nil
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ def target_method

let(:telemetry) { instance_double(Datadog::Core::Telemetry::Component) }

let(:logger) { instance_double(Logger) }

let(:repository) { Datadog::Core::Remote::Configuration::Repository.new }

let(:transaction) do
Expand Down Expand Up @@ -59,7 +61,7 @@ def target_method
let(:receiver) { remote.receivers(telemetry)[0] }

let(:component) do
Datadog::DI::Component.build!(settings, agent_settings)
Datadog::DI::Component.build!(settings, agent_settings, logger)
end

let(:propagate_all_exceptions) { true }
Expand Down Expand Up @@ -239,6 +241,8 @@ def do_rc
end

it 'adds a probe to pending list' do
expect(logger).to receive(:info).with(/Received probe from RC:/)

do_rc

expect(payloads).to be_a(Array)
Expand Down Expand Up @@ -268,6 +272,8 @@ def assert_received_and_installed
end

it 'instruments code and adds probe to installed list' do
expect(logger).to receive(:info).with(/Received probe from RC:/)

do_rc
assert_received_and_installed

Expand All @@ -276,6 +282,8 @@ def assert_received_and_installed

context 'and target method is invoked' do
it 'notifies about execution' do
expect(logger).to receive(:info).with(/Received probe from RC:/)

do_rc
assert_received_and_installed

Expand Down Expand Up @@ -317,6 +325,9 @@ def assert_received_and_installed
end

it 'installs the second, known, probe' do
expect(logger).to receive(:warn).with(/Unrecognized probe type:/)
expect(logger).to receive(:info).with(/Received probe from RC:/)

do_rc
assert_received_and_installed

Expand Down
6 changes: 5 additions & 1 deletion spec/datadog/di/integration/instrumentation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@ def mutating_method(greeting)
instance_double_agent_settings
end

let(:logger) do
instance_double(Logger)
end

let(:component) do
Datadog::DI::Component.build!(settings, agent_settings)
Datadog::DI::Component.build!(settings, agent_settings, logger)
end

let(:expected_installed_payload) do
Expand Down
Loading