diff --git a/lib/datadog/di/el/compiler.rb b/lib/datadog/di/el/compiler.rb index 8a7487012c4..e93d865cf60 100644 --- a/lib/datadog/di/el/compiler.rb +++ b/lib/datadog/di/el/compiler.rb @@ -17,7 +17,7 @@ module EL # @api private class Compiler def compile(ast) - Expression.new(compile_partial(ast)) + compile_partial(ast) end private diff --git a/lib/datadog/di/el/expression.rb b/lib/datadog/di/el/expression.rb index 69d4d1d6118..959ed88bd7b 100644 --- a/lib/datadog/di/el/expression.rb +++ b/lib/datadog/di/el/expression.rb @@ -7,11 +7,13 @@ module EL # # @api private class Expression - def initialize(compiled_expr) + def initialize(dsl_expr, compiled_expr) unless String === compiled_expr raise ArgumentError, "compiled_expr must be a string" end + @dsl_expr = dsl_expr + cls = Class.new(Evaluator) cls.class_exec do eval(<<-RUBY, Object.new.send(:binding), __FILE__, __LINE__ + 1) # standard:disable Security/Eval @@ -24,6 +26,7 @@ def evaluate(context) @evaluator = cls.new end + attr_reader :dsl_expr attr_reader :evaluator def evaluate(context) diff --git a/lib/datadog/di/probe_builder.rb b/lib/datadog/di/probe_builder.rb index 4b861518ec2..98ee87056d8 100644 --- a/lib/datadog/di/probe_builder.rb +++ b/lib/datadog/di/probe_builder.rb @@ -30,6 +30,13 @@ def build_from_remote_config(config) # The validations here are not yet comprehensive. type = config.fetch('type') type_symbol = PROBE_TYPES[type] or raise ArgumentError, "Unrecognized probe type: #{type}" + cond = if cond_spec = config['when'] + unless cond_spec['dsl'] && cond_spec['json'] + raise ArgumentError, "Malformed condition specification for probe: #{config}" + end + compiled = EL::Compiler.new.compile(cond_spec['json']) + EL::Expression.new(cond_spec['dsl'], compiled) + end Probe.new( id: config.fetch("id"), type: type_symbol, @@ -47,7 +54,7 @@ def build_from_remote_config(config) max_capture_depth: config["capture"]&.[]("maxReferenceDepth"), max_capture_attribute_count: config["capture"]&.[]("maxFieldCount"), rate_limit: config["sampling"]&.[]("snapshotsPerSecond"), - condition: (cond = config.dig('when', 'json')) && EL::Compiler.new.compile(cond), # steep:ignore + condition: cond, ) rescue KeyError => exc raise ArgumentError, "Malformed remote configuration entry for probe: #{exc.class}: #{exc}: #{config}" @@ -59,7 +66,11 @@ def build_template_segments(segments) if str = segment['str'] str elsif ast = segment['json'] - EL::Compiler.new.compile(ast) + unless dsl = segment['dsl'] + raise ArgumentError, "Missing dsl for json in segment: #{segment}" + end + compiled = EL::Compiler.new.compile(ast) + EL::Expression.new(dsl, compiled) else # TODO report to telemetry? end diff --git a/lib/datadog/di/probe_notification_builder.rb b/lib/datadog/di/probe_notification_builder.rb index 16c4924d90b..41bdd87275b 100644 --- a/lib/datadog/di/probe_notification_builder.rb +++ b/lib/datadog/di/probe_notification_builder.rb @@ -188,7 +188,10 @@ def evaluate_template(template_segments, context) raise ArgumentError, "Invalid template segment type: #{segment}" end rescue => exc - evaluation_errors << "#{exc.class}: #{exc}" + evaluation_errors << { + message: "#{exc.class}: #{exc}", + expr: segment.dsl_expr, + } '[evaluation error]' end.join [message, evaluation_errors] diff --git a/sig/datadog/di/el/expression.rbs b/sig/datadog/di/el/expression.rbs index ce1b187221a..4e4e18ca20a 100644 --- a/sig/datadog/di/el/expression.rbs +++ b/sig/datadog/di/el/expression.rbs @@ -2,13 +2,15 @@ module Datadog module DI module EL class Expression - @compiled_expr: untyped + @dsl_expr: untyped - def initialize: (untyped compiled_expr) -> void + @evaluator: untyped - attr_reader compiled_expr: untyped + def initialize: (untyped dsl_expr, untyped compiled_expr) -> void - def ==: (untyped other) -> untyped + attr_reader dsl_expr: untyped + + attr_reader evaluator: untyped def evaluate: (untyped context) -> untyped diff --git a/sig/datadog/di/probe.rbs b/sig/datadog/di/probe.rbs index 7606632728c..39a9f5e3f25 100644 --- a/sig/datadog/di/probe.rbs +++ b/sig/datadog/di/probe.rbs @@ -25,6 +25,7 @@ module Datadog @executed_on_line: bool? def initialize: (id: String, type: Symbol, ?file: String?, ?line_no: Integer?, ?type_name: String?, ?method_name: String?, ?template: String?, ?template_segments: Array[untyped]?, ?capture_snapshot: bool, + ?condition: DI::EL::Expression?, ?max_capture_depth: Integer, ?max_capture_attribute_count: Integer?, ?rate_limit: Integer) -> void attr_reader condition: DI::EL::Expression? diff --git a/spec/datadog/di/el/integration_spec.rb b/spec/datadog/di/el/integration_spec.rb index ac04630d54b..bdc8a433ae8 100644 --- a/spec/datadog/di/el/integration_spec.rb +++ b/spec/datadog/di/el/integration_spec.rb @@ -40,9 +40,10 @@ class ELTestIvarClass let(:expected) { spec.fetch('compiled') } let(:compiled) { compiler.compile(ast) } + let(:expr) { Datadog::DI::EL::Expression.new('(expression)', compiled) } let(:evaluated) do - compiled.evaluate(context) + expr.evaluate(context) end let(:context) do diff --git a/spec/datadog/di/instrumenter_spec.rb b/spec/datadog/di/instrumenter_spec.rb index f5f0378ea21..dbfbce6191a 100644 --- a/spec/datadog/di/instrumenter_spec.rb +++ b/spec/datadog/di/instrumenter_spec.rb @@ -933,6 +933,7 @@ context 'when condition is met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', # We use "arg1" here, actual variable name is not currently available "ref('arg1') == 41" ) @@ -944,6 +945,7 @@ context 'when condition is not met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', # We use "arg1" here, actual variable name is not currently available "ref('arg1') == 42" ) @@ -957,6 +959,7 @@ context 'when condition is met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "ref('kwarg') == 42" ) end @@ -967,6 +970,7 @@ context 'when condition is not met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "ref('kwarg') == 41" ) end @@ -980,6 +984,7 @@ let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "unknown_function('kwarg') == 42" ) end @@ -1451,6 +1456,7 @@ context 'when condition is met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "ref('local') == 42" ) end @@ -1466,6 +1472,7 @@ context 'when condition is not met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "ref('local') == 43" ) end @@ -1489,6 +1496,7 @@ context 'when condition is met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "iref('@ivar') == 42" ) end @@ -1504,6 +1512,7 @@ context 'when condition is not met' do let(:condition) do Datadog::DI::EL::Expression.new( + '(expression)', "iref('@ivar') == 43" ) end diff --git a/spec/datadog/di/integration/everything_from_remote_config_spec.rb b/spec/datadog/di/integration/everything_from_remote_config_spec.rb index e6db9bd18b0..f81dc8b7fb7 100644 --- a/spec/datadog/di/integration/everything_from_remote_config_spec.rb +++ b/spec/datadog/di/integration/everything_from_remote_config_spec.rb @@ -398,13 +398,13 @@ def assert_received_and_errored where: { typeName: 'EverythingFromRemoteConfigSpecTestClass', methodName: 'target_method', }, - when: {json: {foo: 'bar'}}, + when: {json: {foo: 'bar'}, dsl: '(expression)'}, } end let(:propagate_all_exceptions) { false } - it 'catches the exception' do + it 'catches the exception and reports probe status error' do expect_lazy_log(logger, :debug, /di: unhandled exception handling a probe in DI remote receiver: Datadog::DI::Error::InvalidExpression: Unknown operation: foo/) do_rc(expect_add_probe: false) @@ -412,11 +412,111 @@ def assert_received_and_errored payload = payloads.first expect(payload).to be_a(Hash) + expect(payload).to match( + ddsource: 'dd_debugger', + debugger: { + diagnostics: { + parentId: nil, + probeId: '11', + probeVersion: 0, + runtimeId: String, + status: 'ERROR', + }, + }, + path: '/debugger/v1/diagnostics', + service: 'rspec', + timestamp: Integer, + message: String, + ) expect(payload[:message]).to match( /Instrumentation for probe .* failed: Unknown operation: foo/, ) end end + + context 'when there is a message template' do + let(:probe_spec) do + { + id: '11', name: 'bar', type: 'LOG_PROBE', + where: { + typeName: 'EverythingFromRemoteConfigSpecTestClass', methodName: 'target_method', + }, + segments: [ + # String segment + {str: 'hello '}, + # Expression segment - valid at runtime + {json: {eq: [{ref: '@ivar'}, 51]}, dsl: '(good expression)'}, + # Another expression which fails evaluation at runtime + {json: {filter: [{ref: '@ivar'}, 'x']}, dsl: '(failing expression)'}, + ], + } + end + + let(:expected_snapshot_payload) do + { + path: '/debugger/v1/input', + # We do not have active span/trace in the test. + "dd.span_id": nil, + "dd.trace_id": nil, + "debugger.snapshot": { + captures: nil, + evaluationErrors: [ + {'expr' => '(failing expression)', 'message' => 'Datadog::DI::Error::ExpressionEvaluationError: Bad collection type for filter: NilClass'}, + ], + id: LOWERCASE_UUID_REGEXP, + language: 'ruby', + probe: { + id: '11', + location: { + method: 'target_method', + type: 'EverythingFromRemoteConfigSpecTestClass', + }, + version: 0, + }, + stack: Array, + timestamp: Integer, + }, + ddsource: 'dd_debugger', + duration: Integer, + host: nil, + logger: { + method: 'target_method', + name: nil, + thread_id: nil, + thread_name: 'Thread.main', + version: 2, + }, + # false is the result of first expression evaluation + # second expression fails evaluation + message: 'hello false[evaluation error]', + service: 'rspec', + timestamp: Integer, + } + end + + it 'evaluates expressions and reports errors' do + expect_lazy_log(logger, :debug, /di: received log probe/) + + do_rc + assert_received_and_installed + + # invocation + + expect(EverythingFromRemoteConfigSpecTestClass.new.target_method).to eq 42 + + component.probe_notifier_worker.flush + + # assertions + + expect(payloads.length).to eq 2 + + emitting_payload = payloads.shift + expect(emitting_payload).to match(expected_emitting_payload) + + snapshot_payload = payloads.shift + expect(order_hash_keys(snapshot_payload)).to match(deep_stringify_keys(order_hash_keys(expected_snapshot_payload))) + end + end end context 'line probe' do @@ -514,7 +614,7 @@ def assert_received_and_errored where: { sourceFile: 'hook_line_load.rb', lines: [14], }, - when: {json: {'contains' => [{'ref' => 'bar'}, 'baz']}}, + when: {json: {'contains' => [{'ref' => 'bar'}, 'baz']}, dsl: '(expression)'}, } end diff --git a/spec/datadog/di/integration/instrumentation_spec.rb b/spec/datadog/di/integration/instrumentation_spec.rb index d70046a228d..75effb5519a 100644 --- a/spec/datadog/di/integration/instrumentation_spec.rb +++ b/spec/datadog/di/integration/instrumentation_spec.rb @@ -509,7 +509,7 @@ def run_test let(:segments) do [ {str: 'hello '}, - {json: {ref: '@duration'}}, + {json: {ref: '@duration'}, dsl: '@duration'}, {str: ' ms'}, ] end @@ -538,7 +538,7 @@ def run_test let(:segments) do [ {str: 'hello '}, - {json: {ref: '@return'}}, + {json: {ref: '@return'}, dsl: '@return'}, ] end @@ -560,7 +560,7 @@ def run_test let(:segments) do [ {str: 'hello '}, - {json: {ref: '@exception'}}, + {json: {ref: '@exception'}, dsl: '@exception'}, ] end diff --git a/spec/datadog/di/integration/probe_notification_builder_spec.rb b/spec/datadog/di/integration/probe_notification_builder_spec.rb index 8f55fdcd5be..742dfc939a6 100644 --- a/spec/datadog/di/integration/probe_notification_builder_spec.rb +++ b/spec/datadog/di/integration/probe_notification_builder_spec.rb @@ -141,7 +141,7 @@ let(:segments) do [ {str: 'hello'}, - {json: {ref: 'bar'}}, + {json: {ref: 'bar'}, dsl: '(expression)'}, ] end @@ -173,7 +173,7 @@ let(:segments) do [ {str: 'hello'}, - {json: {substring: ['bar', 'baz', 3]}}, + {json: {substring: ['bar', 'baz', 3]}, dsl: '(expression)'}, ] end @@ -181,7 +181,9 @@ payload = builder.build_snapshot(context) expect(payload).to be_a(Hash) expect(payload[:message]).to eq "hello[evaluation error]" - expect(payload[:"debugger.snapshot"][:evaluationErrors]).to eq ['ArgumentError: bad value for range'] + expect(payload[:"debugger.snapshot"][:evaluationErrors]).to eq [ + {message: 'ArgumentError: bad value for range', expr: '(expression)'} + ] # We asked to not create a snapshot expect(payload.fetch(:"debugger.snapshot").fetch(:captures)).to be nil @@ -192,8 +194,8 @@ let(:segments) do [ {str: 'hello'}, - {json: {substring: ['bar', 'baz', 3]}}, - {json: {filter: ['bar', 'baz']}}, + {json: {substring: ['bar', 'baz', 3]}, dsl: '(bar baz 3)'}, + {json: {filter: ['bar', 'baz']}, dsl: '(bar baz)'}, {str: 'hello'}, ] end @@ -203,8 +205,8 @@ expect(payload).to be_a(Hash) expect(payload[:message]).to eq "hello[evaluation error][evaluation error]hello" expect(payload[:"debugger.snapshot"][:evaluationErrors]).to eq [ - 'ArgumentError: bad value for range', - 'Datadog::DI::Error::ExpressionEvaluationError: Bad collection type for filter: String', + {message: 'ArgumentError: bad value for range', expr: '(bar baz 3)'}, + {message: 'Datadog::DI::Error::ExpressionEvaluationError: Bad collection type for filter: String', expr: '(bar baz)'}, ] # We asked to not create a snapshot diff --git a/spec/datadog/di/probe_notification_builder_spec.rb b/spec/datadog/di/probe_notification_builder_spec.rb index ef4320c4a21..75911cb8f2a 100644 --- a/spec/datadog/di/probe_notification_builder_spec.rb +++ b/spec/datadog/di/probe_notification_builder_spec.rb @@ -346,9 +346,9 @@ let(:template_segments) do [ - compiler.compile('ref' => 'hello'), + Datadog::DI::EL::Expression.new('(expression)', compiler.compile('ref' => 'hello')), ' ', - compiler.compile('ref' => 'world'), + Datadog::DI::EL::Expression.new('(expression)', compiler.compile('ref' => 'world')), ] end