Skip to content

Commit

Permalink
add an exception boundary around probe parsing, ensure subsequent pro…
Browse files Browse the repository at this point in the history
…bes are parsed and installed
  • Loading branch information
p committed Nov 13, 2024
1 parent ab31e4c commit c35a76a
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 25 deletions.
57 changes: 33 additions & 24 deletions lib/datadog/di/remote.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,37 +44,46 @@ def receivers(telemetry)
repository.contents.each do |content|
case content.path.product
when PRODUCT
probe_spec = parse_content(content)
probe = ProbeBuilder.build_from_remote_config(probe_spec)
payload = component.probe_notification_builder.build_received(probe)
component.probe_notifier_worker.add_status(payload)
component.logger.info("Received probe from RC: #{probe.type} #{probe.location}")

begin
# TODO test exception capture
probe_manager.add_probe(probe)
content.applied
probe_spec = parse_content(content)
probe = ProbeBuilder.build_from_remote_config(probe_spec)
payload = component.probe_notification_builder.build_received(probe)
component.probe_notifier_worker.add_status(payload)
component.logger.info("Received probe from RC: #{probe.type} #{probe.location}")

begin
# TODO test exception capture
probe_manager.add_probe(probe)
content.applied
rescue => exc
raise if component.settings.dynamic_instrumentation.internal.propagate_all_exceptions

component.logger.warn("Unhandled exception adding probe in DI remote receiver: #{exc.class}: #{exc}")
component.telemetry&.report(exc, description: "Unhandled exception adding probe in DI remote receiver")

# If a probe fails to install, we will mark the content
# as errored. On subsequent remote configuration application
# attemps, probe manager will raise the "previously errored"
# exception and we'll rescue it here, again marking the
# content as errored but with a somewhat different exception
# message.
# TODO stack trace must be redacted or not sent at all
content.errored("Error applying dynamic instrumentation configuration: #{exc.class.name} #{exc.message}: #{Array(exc.backtrace).join("\n")}")
end

# Important: even if processing fails for this probe config,
# we need to note it as being current so that we do not
# try to remove instrumentation that is still supposed to be
# active.
current_probe_ids[probe_spec.fetch('id')] = true
rescue => exc
raise if component.settings.dynamic_instrumentation.internal.propagate_all_exceptions

component.logger.warn("Unhandled exception adding probe in DI remote receiver: #{exc.class}: #{exc}")
component.telemetry&.report(exc, description: "Unhandled exception adding probe in DI remote receiver")
component.logger.warn("Unhandled exception handling probe in DI remote receiver: #{exc.class}: #{exc}")
component.telemetry&.report(exc, description: "Unhandled exception handling probe in DI remote receiver")

# If a probe fails to install, we will mark the content
# as errored. On subsequent remote configuration application
# attemps, probe manager will raise the "previously errored"
# exception and we'll rescue it here, again marking the
# content as errored but with a somewhat different exception
# message.
# TODO stack trace must be redacted or not sent at all
content.errored("Error applying dynamic instrumentation configuration: #{exc.class.name} #{exc.message}: #{Array(exc.backtrace).join("\n")}")
end

# Important: even if processing fails for this probe config,
# we need to note it as being current so that we do not
# try to remove instrumentation that is still supposed to be
# active.
current_probe_ids[probe_spec.fetch('id')] = true
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ def target_method
Datadog::DI::Component.build!(settings, agent_settings)
end

let(:propagate_all_exceptions) { true }

let(:settings) do
Datadog::Core::Configuration::Settings.new.tap do |settings|
settings.remote.enabled = true
settings.dynamic_instrumentation.enabled = true
settings.dynamic_instrumentation.internal.development = true
settings.dynamic_instrumentation.internal.propagate_all_exceptions = true
settings.dynamic_instrumentation.internal.propagate_all_exceptions = propagate_all_exceptions
end
end

Expand Down Expand Up @@ -299,5 +301,28 @@ def assert_received_and_installed
expect(order_hash_keys(snapshot_payload)).to match(deep_stringify_keys(order_hash_keys(expected_snapshot_payload)))
end
end

context 'unknown type probe followed by method probe' do

# If exceptions are propagated, remote config processing will stop
# at the first, failing, probe specification.
let(:propagate_all_exceptions) { false }

let(:unknown_probe_spec) do
{id: '12', name: 'foo', type: 'UNKNOWN_PROBE'}
end

let(:probe_configs) do
{'datadog/2/LIVE_DEBUGGING/foo1/bar1' => unknown_probe_spec,
'datadog/2/LIVE_DEBUGGING/foo2/bar2' => probe_spec}
end

it 'installs the second, known, probe' do
do_rc
assert_received_and_installed

expect(probe_manager.installed_probes.length).to eq 1
end
end
end
end

0 comments on commit c35a76a

Please sign in to comment.