From dc25c26cc82b2d6b0c6563799e03d6c52ff53310 Mon Sep 17 00:00:00 2001 From: Tony Hsu Date: Fri, 23 Aug 2024 13:16:36 +0200 Subject: [PATCH] Implement --- lib/datadog/core/environment/cgroup.rb | 3 +- lib/datadog/core/environment/container.rb | 5 +- lib/datadog/core/metrics/client.rb | 12 ++- lib/datadog/core/remote/component.rb | 4 +- lib/datadog/core/remote/negotiation.rb | 12 +-- lib/datadog/core/telemetry/event.rb | 3 + lib/datadog/core/telemetry/logging.rb | 2 +- lib/datadog/opentelemetry/sdk/propagator.rb | 4 +- .../contrib/action_cable/instrumentation.rb | 21 ++-- .../action_controller/instrumentation.rb | 5 +- .../active_record/configuration/resolver.rb | 5 +- .../tracing/contrib/elasticsearch/patcher.rb | 3 +- lib/datadog/tracing/contrib/grape/endpoint.rb | 11 ++- .../tracing/contrib/http/instrumentation.rb | 3 +- .../contrib/httpclient/instrumentation.rb | 9 +- .../tracing/contrib/httpclient/patcher.rb | 16 +-- .../tracing/contrib/httprb/instrumentation.rb | 3 +- lib/datadog/tracing/contrib/httprb/patcher.rb | 16 +-- .../tracing/contrib/lograge/patcher.rb | 3 +- .../tracing/contrib/opensearch/patcher.rb | 3 +- lib/datadog/tracing/contrib/patcher.rb | 4 +- lib/datadog/tracing/contrib/presto/patcher.rb | 15 +-- .../tracing/distributed/propagation.rb | 6 +- lib/datadog/tracing/sampling/rate_sampler.rb | 3 +- lib/datadog/tracing/sampling/rule.rb | 3 +- lib/datadog/tracing/sampling/rule_sampler.rb | 8 +- lib/datadog/tracing/transport/http/client.rb | 2 +- lib/datadog/tracing/transport/io/client.rb | 2 +- lib/datadog/tracing/workers.rb | 4 +- lib/datadog/tracing/workers/trace_writer.rb | 3 +- sig/datadog/core/telemetry/logging.rbs | 2 +- spec/datadog/core/environment/cgroup_spec.rb | 6 ++ .../core/environment/container_spec.rb | 18 +++- spec/datadog/core/metrics/client_spec.rb | 58 ++++++----- spec/datadog/core/remote/negotiation_spec.rb | 16 +-- .../configuration/resolver_spec.rb | 14 +++ .../tracing/contrib/grape/tracer_spec.rb | 1 + .../tracing/contrib/lograge/patcher_spec.rb | 4 +- spec/datadog/tracing/contrib/patcher_spec.rb | 99 ++++++++++--------- .../tracing/sampling/rate_sampler_spec.rb | 12 +++ .../tracing/sampling/rule_sampler_spec.rb | 8 +- spec/datadog/tracing/sampling/rule_spec.rb | 2 + 42 files changed, 229 insertions(+), 204 deletions(-) diff --git a/lib/datadog/core/environment/cgroup.rb b/lib/datadog/core/environment/cgroup.rb index 94ba65c2818..26ac7ca3d43 100644 --- a/lib/datadog/core/environment/cgroup.rb +++ b/lib/datadog/core/environment/cgroup.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative 'ext' +require_relative '../telemetry/logging' module Datadog module Core @@ -33,10 +34,10 @@ def descriptors(process = 'self') end end rescue StandardError => e - # TODO: add telemetry logs Datadog.logger.error( "Error while parsing cgroup. Cause: #{e.class.name} #{e.message} Location: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Error while parsing cgroup') end end end diff --git a/lib/datadog/core/environment/container.rb b/lib/datadog/core/environment/container.rb index f4c3c822030..c2ed8f4a86d 100644 --- a/lib/datadog/core/environment/container.rb +++ b/lib/datadog/core/environment/container.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative 'cgroup' +require_relative '../telemetry/logging' module Datadog module Core @@ -35,6 +36,7 @@ def task_uid descriptor.task_uid end + # rubocop:disable Metrics/MethodLength def descriptor @descriptor ||= Descriptor.new.tap do |descriptor| begin @@ -78,14 +80,15 @@ def descriptor break end rescue StandardError => e - # TODO: add telemetry logs Datadog.logger.error( "Error while parsing container info. Cause: #{e.class.name} #{e.message} " \ "Location: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Error while parsing container info') end end end + # rubocop:enable Metrics/MethodLength end end end diff --git a/lib/datadog/core/metrics/client.rb b/lib/datadog/core/metrics/client.rb index a4929c403e6..41884124a14 100644 --- a/lib/datadog/core/metrics/client.rb +++ b/lib/datadog/core/metrics/client.rb @@ -2,6 +2,7 @@ require_relative '../utils/time' require_relative '../utils/only_once' +require_relative '../telemetry/logging' require_relative '../configuration/ext' require_relative 'ext' @@ -97,10 +98,10 @@ def count(stat, value = nil, options = nil, &block) statsd.count(stat, value, metric_options(options)) rescue StandardError => e - # TODO: add telemetry logs Datadog.logger.error( "Failed to send count stat. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Failed to send count stat') end def distribution(stat, value = nil, options = nil, &block) @@ -111,10 +112,10 @@ def distribution(stat, value = nil, options = nil, &block) statsd.distribution(stat, value, metric_options(options)) rescue StandardError => e - # TODO: add telemetry logs Datadog.logger.error( "Failed to send distribution stat. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Failed to send distribution stat') end def increment(stat, options = nil) @@ -124,10 +125,10 @@ def increment(stat, options = nil) statsd.increment(stat, metric_options(options)) rescue StandardError => e - # TODO: add telemetry logs Datadog.logger.error( "Failed to send increment stat. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Failed to send increment stat') end def gauge(stat, value = nil, options = nil, &block) @@ -138,10 +139,10 @@ def gauge(stat, value = nil, options = nil, &block) statsd.gauge(stat, value, metric_options(options)) rescue StandardError => e - # TODO: add telemetry logs Datadog.logger.error( "Failed to send gauge stat. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Failed to send gauge stat') end def time(stat, options = nil) @@ -157,10 +158,11 @@ def time(stat, options = nil) distribution(stat, ((finished - start) * 1000), options) end rescue StandardError => e - # TODO: add telemetry logs + # TODO: Likely to be redundant, since `distribution` handles its own errors. Datadog.logger.error( "Failed to send time stat. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Telemetry::Logging.report(e, description: 'Failed to send time stat') end end diff --git a/lib/datadog/core/remote/component.rb b/lib/datadog/core/remote/component.rb index c6219ccc0b4..7b65b49060a 100644 --- a/lib/datadog/core/remote/component.rb +++ b/lib/datadog/core/remote/component.rb @@ -39,7 +39,7 @@ def initialize(settings, capabilities, agent_settings) @client.sync @healthy ||= true rescue Client::SyncError => e - # Not sure + # PENDING: Not sure about sending telemetry log Datadog.logger.error do "remote worker client sync error: #{e.message} location: #{Array(e.backtrace).first}. skipping sync" end @@ -49,7 +49,7 @@ def initialize(settings, capabilities, agent_settings) # negotiation object stores error logging state that should be reset. negotiation = Negotiation.new(settings, agent_settings) - # Not sure + # PENDING: Not sure about sending telemetry log Datadog.logger.error do "remote worker error: #{e.class.name} #{e.message} location: #{Array(e.backtrace).first}. "\ 'reseting client state' diff --git a/lib/datadog/core/remote/negotiation.rb b/lib/datadog/core/remote/negotiation.rb index 2c7795c782e..a65b8128419 100644 --- a/lib/datadog/core/remote/negotiation.rb +++ b/lib/datadog/core/remote/negotiation.rb @@ -20,8 +20,7 @@ def endpoint?(path) if res.internal_error? && network_error?(res.error) unless @logged[:agent_unreachable] - # TODO: change to warning - Datadog.logger.error { "agent unreachable: cannot negotiate #{path}" } + Datadog.logger.warn { "agent unreachable: cannot negotiate #{path}" } @logged[:agent_unreachable] = true end @@ -30,8 +29,7 @@ def endpoint?(path) if res.not_found? unless @logged[:no_info_endpoint] - # TODO: change to warning - Datadog.logger.error { "agent reachable but has no /info endpoint: cannot negotiate #{path}" } + Datadog.logger.warn { "agent reachable but has no /info endpoint: cannot negotiate #{path}" } @logged[:no_info_endpoint] = true end @@ -41,8 +39,7 @@ def endpoint?(path) unless res.ok? unless @logged[:unexpected_response] # TODO: Report telemetry logs - # TODO: change to warning - Datadog.logger.error { "agent reachable but unexpected response: cannot negotiate #{path}" } + Datadog.logger.warn { "agent reachable but unexpected response: cannot negotiate #{path}" } @logged[:unexpected_response] = true end @@ -52,8 +49,7 @@ def endpoint?(path) unless res.endpoints.include?(path) unless @logged[:no_config_endpoint] # TODO: Report telemetry logs - # TODO: change to warning - Datadog.logger.error { "agent reachable but does not report #{path}" } + Datadog.logger.warn { "agent reachable but does not report #{path}" } @logged[:no_config_endpoint] = true end diff --git a/lib/datadog/core/telemetry/event.rb b/lib/datadog/core/telemetry/event.rb index 3808c475893..fabc8014207 100644 --- a/lib/datadog/core/telemetry/event.rb +++ b/lib/datadog/core/telemetry/event.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require_relative '../utils/forking' +require_relative '../utils/sequence' + module Datadog module Core module Telemetry diff --git a/lib/datadog/core/telemetry/logging.rb b/lib/datadog/core/telemetry/logging.rb index eba7e87719c..58019389bf7 100644 --- a/lib/datadog/core/telemetry/logging.rb +++ b/lib/datadog/core/telemetry/logging.rb @@ -47,7 +47,7 @@ def self.from(exception) end end - def report(exception, level:, description: nil) + def report(exception, level: :error, description: nil) # Annoymous exceptions to be logged as message = +'' message << (exception.class.name || exception.class.inspect) diff --git a/lib/datadog/opentelemetry/sdk/propagator.rb b/lib/datadog/opentelemetry/sdk/propagator.rb index dd128fa80ea..b711fba2390 100644 --- a/lib/datadog/opentelemetry/sdk/propagator.rb +++ b/lib/datadog/opentelemetry/sdk/propagator.rb @@ -15,7 +15,7 @@ def inject( setter: ::OpenTelemetry::Context::Propagation.text_map_setter ) unless setter == ::OpenTelemetry::Context::Propagation.text_map_setter - # Not to report telemetry logs for now + # PENDING: Not to report telemetry logs for now Datadog.logger.error( 'Custom setter is not supported. Please inform the `datadog` team at ' \ ' https://github.com/DataDog/dd-trace-rb of your use case so we can best support you. Using the default ' \ @@ -32,7 +32,7 @@ def extract( ) if getter != ::OpenTelemetry::Context::Propagation.text_map_getter && getter != ::OpenTelemetry::Common::Propagation.rack_env_getter - # Not to report telemetry logs for now + # PENDING: Not to report telemetry logs for now Datadog.logger.error( "Custom getter #{getter} is not supported. Please inform the `datadog` team at " \ ' https://github.com/DataDog/dd-trace-rb of your use case so we can best support you. Using the default ' \ diff --git a/lib/datadog/tracing/contrib/action_cable/instrumentation.rb b/lib/datadog/tracing/contrib/action_cable/instrumentation.rb index dcb50dece11..2baea187f23 100644 --- a/lib/datadog/tracing/contrib/action_cable/instrumentation.rb +++ b/lib/datadog/tracing/contrib/action_cable/instrumentation.rb @@ -14,22 +14,17 @@ module Instrumentation module ActionCableConnection def on_open Tracing.trace(Ext::SPAN_ON_OPEN) do |span, trace| - begin - span.resource = "#{self.class}#on_open" - span.type = Tracing::Metadata::Ext::AppTypes::TYPE_WEB + span.resource = "#{self.class}#on_open" + span.type = Tracing::Metadata::Ext::AppTypes::TYPE_WEB - span.set_tag(Ext::TAG_ACTION, 'on_open') - span.set_tag(Ext::TAG_CONNECTION, self.class.to_s) + span.set_tag(Ext::TAG_ACTION, 'on_open') + span.set_tag(Ext::TAG_CONNECTION, self.class.to_s) - span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) - span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ON_OPEN) + span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) + span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_ON_OPEN) - # Set the resource name of the trace - trace.resource = span.resource - rescue StandardError => e - # TODO: Report Telemetry logs - Datadog.logger.error("Error preparing span for ActionCable::Connection: #{e}") - end + # Set the resource name of the trace + trace.resource = span.resource super end diff --git a/lib/datadog/tracing/contrib/action_pack/action_controller/instrumentation.rb b/lib/datadog/tracing/contrib/action_pack/action_controller/instrumentation.rb index a2a6088f022..8a07371eae9 100644 --- a/lib/datadog/tracing/contrib/action_pack/action_controller/instrumentation.rb +++ b/lib/datadog/tracing/contrib/action_pack/action_controller/instrumentation.rb @@ -7,6 +7,7 @@ require_relative '../utils' require_relative '../../rack/middlewares' require_relative '../../analytics' +require_relative '../../../../core/telemetry/logging' module Datadog module Tracing @@ -42,8 +43,8 @@ def start_processing(payload) span.set_tag(Tracing::Metadata::Ext::TAG_COMPONENT, Ext::TAG_COMPONENT) span.set_tag(Tracing::Metadata::Ext::TAG_OPERATION, Ext::TAG_OPERATION_CONTROLLER) rescue StandardError => e - # TODO: Report Telemetry logs Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end def finish_processing(payload) @@ -81,8 +82,8 @@ def finish_processing(payload) span.finish end rescue StandardError => e - # TODO: Report Telemetry logs Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end # Instrumentation for ActionController::Metal diff --git a/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb b/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb index f459a57933b..095d96b4641 100644 --- a/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb +++ b/lib/datadog/tracing/contrib/active_record/configuration/resolver.rb @@ -2,6 +2,7 @@ require_relative '../../configuration/resolver' require_relative 'makara_resolver' +require_relative '../../../../core/telemetry/logging' module Datadog module Tracing @@ -73,11 +74,11 @@ def resolve(db_config) # `db_config` input may contain sensitive information such as passwords, # hence provide a succinct summary for the error logging. # - # TODO: Report Telemetry logs Datadog.logger.error( 'Failed to resolve ActiveRecord database configuration. '\ "Cause: #{e.class.name} Source: #{Array(e.backtrace).first}" ) + Core::Telemetry::Logging.report(e, description: 'Failed to resolve ActiveRecord database configuration') nil end @@ -93,11 +94,11 @@ def parse_matcher(matcher) normalized rescue => e - # TODO: Report Telemetry logs Datadog.logger.error( "Failed to resolve key #{matcher.inspect}. " \ "Cause: #{e.class.name} Source: #{Array(e.backtrace).first}" ) + Core::Telemetry::Logging.report(e, description: 'Failed to resolve key') nil end diff --git a/lib/datadog/tracing/contrib/elasticsearch/patcher.rb b/lib/datadog/tracing/contrib/elasticsearch/patcher.rb index d1d4121579d..a392d748d72 100644 --- a/lib/datadog/tracing/contrib/elasticsearch/patcher.rb +++ b/lib/datadog/tracing/contrib/elasticsearch/patcher.rb @@ -93,8 +93,9 @@ def perform_request(*args) span.resource = "#{method} #{quantized_url}" Contrib::SpanAttributeSchema.set_peer_service!(span, Ext::PEER_SERVICE_SOURCES) rescue StandardError => e - # TODO: Not to report telemetry logs now, refactor the code to avoid calling `super` in the `ensure` block + # TODO: Refactor the code to avoid calling `super` in the `ensure` block Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) ensure # the call is still executed response = super diff --git a/lib/datadog/tracing/contrib/grape/endpoint.rb b/lib/datadog/tracing/contrib/grape/endpoint.rb index 3504d91bacb..c0544174e4b 100644 --- a/lib/datadog/tracing/contrib/grape/endpoint.rb +++ b/lib/datadog/tracing/contrib/grape/endpoint.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative '../../../core' +require_relative '../../../core/telemetry/logging' require_relative '../../metadata/ext' require_relative '../analytics' require_relative '../rack/ext' @@ -63,8 +64,8 @@ def endpoint_start_process(_name, _start, _finish, _id, payload) Thread.current[KEY_RUN] = true rescue StandardError => e - # TODO: Report Telelemetry log Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end def endpoint_run(name, start, finish, id, payload) @@ -107,8 +108,8 @@ def endpoint_run(name, start, finish, id, payload) span.finish(finish) end rescue StandardError => e - # TODO: Report Telelemetry log Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end # Status code resolution is tied to the exception handling @@ -151,8 +152,8 @@ def endpoint_start_render(*) Thread.current[KEY_RENDER] = true rescue StandardError => e - # TODO: Report Telelemetry log Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end def endpoint_render(name, start, finish, id, payload) @@ -176,8 +177,8 @@ def endpoint_render(name, start, finish, id, payload) span.finish(finish) end rescue StandardError => e - # TODO: Report Telelemetry log Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end def endpoint_run_filters(name, start, finish, id, payload) @@ -215,8 +216,8 @@ def endpoint_run_filters(name, start, finish, id, payload) span.finish(finish) end rescue StandardError => e - # TODO: Report Telelemetry log Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) end private diff --git a/lib/datadog/tracing/contrib/http/instrumentation.rb b/lib/datadog/tracing/contrib/http/instrumentation.rb index 67186b57b11..2269dcc98f2 100644 --- a/lib/datadog/tracing/contrib/http/instrumentation.rb +++ b/lib/datadog/tracing/contrib/http/instrumentation.rb @@ -6,6 +6,7 @@ require_relative '../analytics' require_relative '../http_annotation_helper' require_relative '../utils/quantization/http' +require_relative '../../../core/telemetry/logging' module Datadog module Tracing @@ -42,8 +43,8 @@ def request(req, body = nil, &block) # Add additional request specific tags to the span. annotate_span_with_request!(span, req, request_options) rescue StandardError => e - # TODO: Report Telemetry logs Datadog.logger.error("error preparing span for http request: #{e}") + Datadog::Core::Telemetry::Logging.report(e) ensure response = super(req, body, &block) end diff --git a/lib/datadog/tracing/contrib/httpclient/instrumentation.rb b/lib/datadog/tracing/contrib/httpclient/instrumentation.rb index 1c4940718a7..a00701cc15a 100644 --- a/lib/datadog/tracing/contrib/httpclient/instrumentation.rb +++ b/lib/datadog/tracing/contrib/httpclient/instrumentation.rb @@ -4,6 +4,7 @@ require_relative '../http' require_relative '../analytics' require_relative '../http_annotation_helper' +require_relative '../../../core/telemetry/logging' module Datadog module Tracing @@ -36,8 +37,8 @@ def do_get_block(req, proxy, conn, &block) # Add additional request specific tags to the span. annotate_span_with_request!(span, req, request_options) rescue StandardError => e - # TODO: Report Telemetry logs - logger.error("error preparing span for httpclient request: #{e}, Source: #{e.backtrace}") + Datadog.logger.error("error preparing span for httpclient request: #{e}, Source: #{e.backtrace}") + Datadog::Core::Telemetry::Logging.report(e) ensure res = super end @@ -115,10 +116,6 @@ def analytics_enabled?(request_options) Contrib::Analytics.enabled?(request_options[:analytics_enabled]) end - def logger - Datadog.logger - end - def should_skip_distributed_tracing?(client_config) return !client_config[:distributed_tracing] if client_config && client_config.key?(:distributed_tracing) diff --git a/lib/datadog/tracing/contrib/httpclient/patcher.rb b/lib/datadog/tracing/contrib/httpclient/patcher.rb index 391089d7075..cd1779a3ce5 100644 --- a/lib/datadog/tracing/contrib/httpclient/patcher.rb +++ b/lib/datadog/tracing/contrib/httpclient/patcher.rb @@ -13,28 +13,14 @@ module Httpclient module Patcher include Contrib::Patcher - PATCH_ONLY_ONCE = Core::Utils::OnlyOnce.new - module_function - def patched? - PATCH_ONLY_ONCE.ran? - end - def target_version Integration.version end - # patch applies our patch def patch - # TODO: Refactor to remove redundancy - PATCH_ONLY_ONCE.run do - begin - ::HTTPClient.include(Instrumentation) - rescue StandardError => e - Datadog.logger.error("Unable to apply httpclient integration: #{e}") - end - end + ::HTTPClient.include(Instrumentation) end end end diff --git a/lib/datadog/tracing/contrib/httprb/instrumentation.rb b/lib/datadog/tracing/contrib/httprb/instrumentation.rb index 62de2bfdb62..e3c8e8ec6db 100644 --- a/lib/datadog/tracing/contrib/httprb/instrumentation.rb +++ b/lib/datadog/tracing/contrib/httprb/instrumentation.rb @@ -4,6 +4,7 @@ require_relative '../http' require_relative '../analytics' require_relative '../http_annotation_helper' +require_relative '../../../core/telemetry/logging' module Datadog module Tracing @@ -34,8 +35,8 @@ def perform(req, options) # Add additional request specific tags to the span. annotate_span_with_request!(span, req, request_options) rescue StandardError => e - # TODO: Report Telemetry logs logger.error("error preparing span for http.rb request: #{e}, Source: #{e.backtrace}") + Datadog::Core::Telemetry::Logging.report(e) ensure res = super(req, options) end diff --git a/lib/datadog/tracing/contrib/httprb/patcher.rb b/lib/datadog/tracing/contrib/httprb/patcher.rb index c7b390cd431..509c645a892 100644 --- a/lib/datadog/tracing/contrib/httprb/patcher.rb +++ b/lib/datadog/tracing/contrib/httprb/patcher.rb @@ -13,28 +13,14 @@ module Httprb module Patcher include Contrib::Patcher - PATCH_ONLY_ONCE = Core::Utils::OnlyOnce.new - module_function - def patched? - PATCH_ONLY_ONCE.ran? - end - def target_version Integration.version end - # patch applies our patch def patch - # TODO: Refactor to remove redundancy - PATCH_ONLY_ONCE.run do - begin - ::HTTP::Client.include(Instrumentation) - rescue StandardError => e - Datadog.logger.error("Unable to apply httprb integration: #{e}") - end - end + ::HTTP::Client.include(Instrumentation) end end end diff --git a/lib/datadog/tracing/contrib/lograge/patcher.rb b/lib/datadog/tracing/contrib/lograge/patcher.rb index 47227a1113c..0045c894930 100644 --- a/lib/datadog/tracing/contrib/lograge/patcher.rb +++ b/lib/datadog/tracing/contrib/lograge/patcher.rb @@ -24,8 +24,7 @@ def patch if defined?(::ActiveSupport::TaggedLogging::Formatter) && ::Lograge::LogSubscribers::ActionController .logger&.formatter.is_a?(::ActiveSupport::TaggedLogging::Formatter) - # TODO: Change to warn - Datadog.logger.error( + Datadog.logger.warn( 'Lograge and ActiveSupport::TaggedLogging (the default Rails log formatter) are not compatible: ' \ 'Lograge does not account for Rails log tags, creating polluted logs and breaking log formatting. ' \ 'Traces and Logs correlation may not work. ' \ diff --git a/lib/datadog/tracing/contrib/opensearch/patcher.rb b/lib/datadog/tracing/contrib/opensearch/patcher.rb index 04f8a18a835..930b60bc29f 100644 --- a/lib/datadog/tracing/contrib/opensearch/patcher.rb +++ b/lib/datadog/tracing/contrib/opensearch/patcher.rb @@ -6,6 +6,7 @@ require_relative '../ext' require_relative '../integration' require_relative '../patcher' +require_relative '../../../core/telemetry/logging' module Datadog module Tracing @@ -80,8 +81,8 @@ def perform_request(method, path, params = {}, body = nil, headers = nil) span.resource = "#{method} #{quantized_url}" Contrib::SpanAttributeSchema.set_peer_service!(span, Ext::PEER_SERVICE_SOURCES) rescue StandardError => e - # TODO: Report Telemetry logs Datadog.logger.error(e.message) + Datadog::Core::Telemetry::Logging.report(e) ensure begin response = super diff --git a/lib/datadog/tracing/contrib/patcher.rb b/lib/datadog/tracing/contrib/patcher.rb index 5dfbe854588..4d68b2101c8 100644 --- a/lib/datadog/tracing/contrib/patcher.rb +++ b/lib/datadog/tracing/contrib/patcher.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require_relative '../../core/utils/only_once' +require_relative '../../core/telemetry/logging' module Datadog module Tracing @@ -49,9 +50,8 @@ def patch # Processes patching errors. This default implementation logs the error and reports relevant metrics. # @param e [Exception] def on_patch_error(e) - # Log the error - # TODO: Report Telemetry logs Datadog.logger.error("Failed to apply #{patch_name} patch. Cause: #{e} Location: #{Array(e.backtrace).first}") + Datadog::Core::Telemetry::Logging.report(e, description: "Failed to apply #{patch_name} patch") @patch_error_result = { type: e.class.name, diff --git a/lib/datadog/tracing/contrib/presto/patcher.rb b/lib/datadog/tracing/contrib/presto/patcher.rb index dbc328a6851..4070f05a248 100644 --- a/lib/datadog/tracing/contrib/presto/patcher.rb +++ b/lib/datadog/tracing/contrib/presto/patcher.rb @@ -13,23 +13,10 @@ module Presto module Patcher include Contrib::Patcher - PATCH_ONLY_ONCE = Core::Utils::OnlyOnce.new - module_function - def patched? - PATCH_ONLY_ONCE.ran? - end - def patch - # TODO: Refactor to remove redundancy - PATCH_ONLY_ONCE.run do - begin - ::Presto::Client::Client.include(Instrumentation::Client) - rescue StandardError => e - Datadog.logger.error("Unable to apply Presto integration: #{e}") - end - end + ::Presto::Client::Client.include(Instrumentation::Client) end end end diff --git a/lib/datadog/tracing/distributed/propagation.rb b/lib/datadog/tracing/distributed/propagation.rb index a5da4093eb8..469b927c04c 100644 --- a/lib/datadog/tracing/distributed/propagation.rb +++ b/lib/datadog/tracing/distributed/propagation.rb @@ -3,6 +3,7 @@ require_relative '../configuration/ext' require_relative '../trace_digest' require_relative '../trace_operation' +require_relative '../../core/telemetry/logging' module Datadog module Tracing @@ -69,10 +70,13 @@ def inject!(digest, data) result = true rescue => e result = nil - # TODO: Report Telemetry logs, capturing propagator name ::Datadog.logger.error( "Error injecting distributed trace data. Cause: #{e} Location: #{Array(e.backtrace).first}" ) + ::Datadog::Core::Telemetry::Logging.report( + e, + description: "Error injecting distributed trace data with #{propagator.class.name}" + ) end result diff --git a/lib/datadog/tracing/sampling/rate_sampler.rb b/lib/datadog/tracing/sampling/rate_sampler.rb index 844970100c2..ea581dd70fa 100644 --- a/lib/datadog/tracing/sampling/rate_sampler.rb +++ b/lib/datadog/tracing/sampling/rate_sampler.rb @@ -20,8 +20,7 @@ def initialize(sample_rate = 1.0, decision: nil) super() unless sample_rate >= 0.0 && sample_rate <= 1.0 - # TODO: Change to warning - Datadog.logger.error('sample rate is not between 0 and 1, falling back to 1') + Datadog.logger.warn('sample rate is not between 0 and 1, falling back to 1') sample_rate = 1.0 end diff --git a/lib/datadog/tracing/sampling/rule.rb b/lib/datadog/tracing/sampling/rule.rb index e5438329d99..5e0208b66af 100644 --- a/lib/datadog/tracing/sampling/rule.rb +++ b/lib/datadog/tracing/sampling/rule.rb @@ -2,6 +2,7 @@ require_relative 'matcher' require_relative 'rate_sampler' +require_relative '../../core/telemetry/logging' module Datadog module Tracing @@ -32,10 +33,10 @@ def initialize(matcher, sampler, provenance) def match?(trace) @matcher.match?(trace) rescue => e - # TODO: Report Telemetry logs Datadog.logger.error( "Matcher failed. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Datadog::Core::Telemetry::Logging.report(e, description: 'Matcher failed') nil end diff --git a/lib/datadog/tracing/sampling/rule_sampler.rb b/lib/datadog/tracing/sampling/rule_sampler.rb index ac7bb583734..982cb737908 100644 --- a/lib/datadog/tracing/sampling/rule_sampler.rb +++ b/lib/datadog/tracing/sampling/rule_sampler.rb @@ -3,6 +3,7 @@ require_relative 'ext' require_relative 'rate_limiter' require_relative 'rule' +require_relative '../../core/telemetry/logging' module Datadog module Tracing @@ -80,10 +81,10 @@ def self.parse(rules, rate_limit, default_sample_rate) new(parsed_rules, rate_limit: rate_limit, default_sample_rate: default_sample_rate) rescue => e - # TODO: Change to warning - Datadog.logger.error do + Datadog.logger.warn do "Could not parse trace sampling rules '#{rules}': #{e.class.name} #{e.message} at #{Array(e.backtrace).first}" end + nil end @@ -138,10 +139,11 @@ def sample_trace(trace) trace.set_tag(Tracing::Metadata::Ext::Distributed::TAG_DECISION_MAKER, provenance) end rescue StandardError => e - # TODO: Report Telemetry logs Datadog.logger.error( "Rule sampling failed. Cause: #{e.class.name} #{e.message} Source: #{Array(e.backtrace).first}" ) + Datadog::Core::Telemetry::Logging.report(e, description: 'Rule sampling failed') + yield(trace) end diff --git a/lib/datadog/tracing/transport/http/client.rb b/lib/datadog/tracing/transport/http/client.rb index bb7f64d15c6..ab2c7191df8 100644 --- a/lib/datadog/tracing/transport/http/client.rb +++ b/lib/datadog/tracing/transport/http/client.rb @@ -38,7 +38,7 @@ def send_request(request, &block) if stats.consecutive_errors > 0 Datadog.logger.debug(message) else - # Not to report Telemetry logs + # Not to report telemetry logs Datadog.logger.error(message) end diff --git a/lib/datadog/tracing/transport/io/client.rb b/lib/datadog/tracing/transport/io/client.rb index f6fe0cf8aad..4288889ec76 100644 --- a/lib/datadog/tracing/transport/io/client.rb +++ b/lib/datadog/tracing/transport/io/client.rb @@ -49,7 +49,7 @@ def send_request(request) if stats.consecutive_errors > 0 Datadog.logger.debug(message) else - # Not to report Telemetry logs + # Not to report telemetry logs Datadog.logger.error(message) end diff --git a/lib/datadog/tracing/workers.rb b/lib/datadog/tracing/workers.rb index 5838a055471..030986a2204 100644 --- a/lib/datadog/tracing/workers.rb +++ b/lib/datadog/tracing/workers.rb @@ -56,9 +56,7 @@ def callback_traces # ensures that the thread will not die because of an exception. # TODO[manu]: findout the reason and reschedule the send if it's not # a fatal exception - # - # TODO: Change to warning. - Datadog.logger.error( + Datadog.logger.warn( "Error during traces flush: dropped #{traces.length} items. Cause: #{e} Location: #{Array(e.backtrace).first}" ) end diff --git a/lib/datadog/tracing/workers/trace_writer.rb b/lib/datadog/tracing/workers/trace_writer.rb index b2a717d42d6..bbb04ff219a 100644 --- a/lib/datadog/tracing/workers/trace_writer.rb +++ b/lib/datadog/tracing/workers/trace_writer.rb @@ -43,8 +43,7 @@ def write_traces(traces) traces = process_traces(traces) flush_traces(traces) rescue StandardError => e - # TODO: Change to warning. - Datadog.logger.error( + Datadog.logger.warn( "Error while writing traces: dropped #{traces.length} items. Cause: #{e} Location: #{Array(e.backtrace).first}" ) end diff --git a/sig/datadog/core/telemetry/logging.rbs b/sig/datadog/core/telemetry/logging.rbs index 57cd4ae8d37..c19818f7937 100644 --- a/sig/datadog/core/telemetry/logging.rbs +++ b/sig/datadog/core/telemetry/logging.rbs @@ -10,7 +10,7 @@ module Datadog extend Datadog::Core::Telemetry::Logging - def report: (Exception exception, level: Symbol, ?description: String?) -> void + def report: (Exception exception, ?level: Symbol, ?description: String?) -> void def error: (String description) -> void diff --git a/spec/datadog/core/environment/cgroup_spec.rb b/spec/datadog/core/environment/cgroup_spec.rb index f3fa9838e3e..18139fd73f4 100644 --- a/spec/datadog/core/environment/cgroup_spec.rb +++ b/spec/datadog/core/environment/cgroup_spec.rb @@ -10,6 +10,7 @@ context 'when the \'/proc/self/cgroup\' file is not present' do before do expect(Datadog.logger).to_not receive(:error) + expect(Datadog::Core::Telemetry::Logging).not_to receive(:report) expect(File).to receive(:exist?) .with('/proc/self/cgroup') @@ -36,6 +37,11 @@ expect(Datadog.logger).to receive(:error) do |msg| expect(msg).to match(/Error while parsing cgroup./) end + + expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + error, + description: 'Error while parsing cgroup' + ) end it do diff --git a/spec/datadog/core/environment/container_spec.rb b/spec/datadog/core/environment/container_spec.rb index fabeda42c63..89f7c30e96f 100644 --- a/spec/datadog/core/environment/container_spec.rb +++ b/spec/datadog/core/environment/container_spec.rb @@ -14,7 +14,10 @@ end shared_examples_for 'container descriptor' do - before { expect(Datadog.logger).to_not receive(:error) } + before do + expect(Datadog.logger).to_not receive(:error) + expect(Datadog::Core::Telemetry::Logging).to_not receive(:report) + end it do is_expected.to be_a_kind_of(described_class::Descriptor) @@ -119,5 +122,18 @@ let(:task_uid) { task_arn } end end + + context 'when error occurs while parsing cgroup' do + it do + allow(Datadog::Core::Environment::Cgroup).to receive(:descriptors).and_raise('Oops..') + expect(Datadog.logger).to receive(:error).with(/Error while parsing container info/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + an_instance_of(RuntimeError), + description: 'Error while parsing container info' + ) + + descriptor + end + end end end diff --git a/spec/datadog/core/metrics/client_spec.rb b/spec/datadog/core/metrics/client_spec.rb index 5f6fb218b2c..db87abccef6 100644 --- a/spec/datadog/core/metrics/client_spec.rb +++ b/spec/datadog/core/metrics/client_spec.rb @@ -13,9 +13,16 @@ it { is_expected.to have_attributes(statsd: statsd) } - shared_examples_for 'missing value arg' do - it 'logs an error without raising' do - expect(Datadog.logger).to receive(:error) + shared_examples_for 'logs an error without raising' do |action| + it do + expect(Datadog.logger).to receive(:error).with( + /Failed to send #{action} stat/ + ) + expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + a_kind_of(StandardError), + description: "Failed to send #{action} stat" + ) + expect { subject }.to_not raise_error end end @@ -383,7 +390,7 @@ context 'that does not yield args' do subject(:count) { metrics.count(stat) {} } - it_behaves_like 'missing value arg' + it_behaves_like 'logs an error without raising', 'count' end context 'that yields args' do @@ -433,12 +440,9 @@ end context 'which raises an error' do - before do - expect(statsd).to receive(:count).and_raise(StandardError) - expect(Datadog.logger).to receive(:error) - end + before { allow(statsd).to receive(:count).and_raise(StandardError) } - it { expect { count }.to_not raise_error } + it_behaves_like 'logs an error without raising', 'count' end end end @@ -464,7 +468,7 @@ context 'that does not yield args' do subject(:distribution) { metrics.distribution(stat) {} } - it_behaves_like 'missing value arg' + it_behaves_like 'logs an error without raising', 'distribution' end context 'that yields args' do @@ -514,12 +518,9 @@ end context 'which raises an error' do - before do - expect(statsd).to receive(:distribution).and_raise(StandardError) - expect(Datadog.logger).to receive(:error) - end + before { allow(statsd).to receive(:distribution).and_raise(StandardError) } - it { expect { distribution }.to_not raise_error } + it_behaves_like 'logs an error without raising', 'distribution' end end end @@ -545,7 +546,7 @@ context 'that does not yield args' do subject(:gauge) { metrics.gauge(stat) {} } - it_behaves_like 'missing value arg' + it_behaves_like 'logs an error without raising', 'gauge' end context 'that yields args' do @@ -595,12 +596,9 @@ end context 'which raises an error' do - before do - expect(statsd).to receive(:gauge).and_raise(StandardError) - expect(Datadog.logger).to receive(:error) - end + before { allow(statsd).to receive(:gauge).and_raise(StandardError) } - it { expect { gauge }.to_not raise_error } + it_behaves_like 'logs an error without raising', 'gauge' end end end @@ -622,6 +620,12 @@ context 'when #statsd is a Datadog::Statsd' do context 'and given a block' do + context 'when raise exception' do + subject(:increment) { metrics.increment(stat) { raise } } + + it_behaves_like 'logs an error without raising', 'increment' + end + context 'that yields args' do subject(:increment) { metrics.increment(stat) { stat_options } } @@ -676,12 +680,9 @@ end context 'which raises an error' do - before do - expect(statsd).to receive(:increment).and_raise(StandardError) - expect(Datadog.logger).to receive(:error) - end + before { allow(statsd).to receive(:increment).and_raise(StandardError) } - it { expect { increment }.to_not raise_error } + it_behaves_like 'logs an error without raising', 'increment' end end end @@ -711,7 +712,10 @@ let(:error) { RuntimeError.new } # Expect the given block to raise its errors through - it { expect { time }.to raise_error(error) } + it do + expect(Datadog.logger).not_to receive(:error) + expect { time }.to raise_error(error) + end end end diff --git a/spec/datadog/core/remote/negotiation_spec.rb b/spec/datadog/core/remote/negotiation_spec.rb index e34a0af2d59..71770a6ce51 100644 --- a/spec/datadog/core/remote/negotiation_spec.rb +++ b/spec/datadog/core/remote/negotiation_spec.rb @@ -50,13 +50,13 @@ end it do - expect(Datadog.logger).to_not receive(:error) + expect(Datadog.logger).to_not receive(:warn) expect(endpoint?).to be true end it do - expect(Datadog.logger).to receive(:error).and_return(nil) + expect(Datadog.logger).to receive(:warn) expect(negotiation.endpoint?('/bar')).to be false end @@ -65,7 +65,7 @@ let(:suppress_logging) { { no_config_endpoint: true } } it 'does not log an error' do - expect(Datadog.logger).to_not receive(:error) + expect(Datadog.logger).to_not receive(:warn) expect(negotiation.endpoint?('/bar')).to be false end @@ -77,7 +77,7 @@ let(:response_body) { '404 page not found' } before do - expect(Datadog.logger).to receive(:error).and_return(nil) + expect(Datadog.logger).to receive(:warn) end it { expect(endpoint?).to be false } @@ -95,7 +95,7 @@ let(:response_body) { '400 bad request' } before do - expect(Datadog.logger).to receive(:error).and_return(nil) + expect(Datadog.logger).to receive(:warn) end it { expect(endpoint?).to be false } @@ -113,7 +113,7 @@ let(:response_body) { '500 internal server error' } before do - expect(Datadog.logger).to receive(:error).and_return(nil) + expect(Datadog.logger).to receive(:warn) end it { expect(endpoint?).to be false } @@ -133,7 +133,7 @@ end before do - expect(Datadog.logger).to receive(:error).and_return(nil) + expect(Datadog.logger).to receive(:warn) end it { expect(endpoint?).to be false } @@ -150,7 +150,7 @@ let(:request_exception) { Errno::ECONNREFUSED.new } before do - expect(Datadog.logger).to receive(:error).and_return(nil) + expect(Datadog.logger).to receive(:warn) end it { expect(endpoint?).to be false } diff --git a/spec/datadog/tracing/contrib/active_record/configuration/resolver_spec.rb b/spec/datadog/tracing/contrib/active_record/configuration/resolver_spec.rb index cd17b66e46b..7bb20e661d3 100644 --- a/spec/datadog/tracing/contrib/active_record/configuration/resolver_spec.rb +++ b/spec/datadog/tracing/contrib/active_record/configuration/resolver_spec.rb @@ -106,6 +106,15 @@ let(:matcher) { 'bala boom!' } it 'does not resolves' do + expect(Datadog.logger).to receive(:error) do |message| + expect(message).to match(/failed to resolve/i) + end + + expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + an_instance_of(URI::InvalidURIError), + description: 'Failed to resolve key' + ) + add expect(resolver.configurations).to be_empty @@ -328,6 +337,11 @@ expect(message).not_to match(/password/i) end + expect(Datadog::Core::Telemetry::Logging).to receive(:report).with( + an_instance_of(URI::InvalidURIError), + description: 'Failed to resolve ActiveRecord database configuration' + ) + is_expected.to be_nil end end diff --git a/spec/datadog/tracing/contrib/grape/tracer_spec.rb b/spec/datadog/tracing/contrib/grape/tracer_spec.rb index 08be34f2eed..d6f492c3a7f 100644 --- a/spec/datadog/tracing/contrib/grape/tracer_spec.rb +++ b/spec/datadog/tracing/contrib/grape/tracer_spec.rb @@ -688,6 +688,7 @@ before do Datadog.configure { |c| c.tracing.enabled = false } expect(Datadog.logger).to_not receive(:error) + expect(Datadog::Core::Telemetry::Logging).to_not receive(:report) end it 'runs the endpoint request without tracing' do diff --git a/spec/datadog/tracing/contrib/lograge/patcher_spec.rb b/spec/datadog/tracing/contrib/lograge/patcher_spec.rb index 9aaeb0e26e8..b94b731c18c 100644 --- a/spec/datadog/tracing/contrib/lograge/patcher_spec.rb +++ b/spec/datadog/tracing/contrib/lograge/patcher_spec.rb @@ -15,7 +15,7 @@ context 'without Rails tagged logging' do it 'does not log incompatibility error' do - expect(Datadog.logger).to_not receive(:error) + expect(Datadog.logger).to_not receive(:warn) described_class.patch end @@ -26,7 +26,7 @@ logger = ActiveSupport::TaggedLogging.new(Logger.new(File::NULL)) stub_const('Lograge::LogSubscribers::ActionController', double('controller', logger: logger)) - expect(Datadog.logger).to receive(:error).with(/ActiveSupport::TaggedLogging/) + expect(Datadog.logger).to receive(:warn).with(/ActiveSupport::TaggedLogging/) described_class.patch end diff --git a/spec/datadog/tracing/contrib/patcher_spec.rb b/spec/datadog/tracing/contrib/patcher_spec.rb index f51443fb38a..b245bc7ee5b 100644 --- a/spec/datadog/tracing/contrib/patcher_spec.rb +++ b/spec/datadog/tracing/contrib/patcher_spec.rb @@ -9,10 +9,6 @@ RSpec::Mocks.space.any_instance_proxy_for(Datadog::Tracing::Contrib::Patcher::CommonMethods).unstub(:on_patch_error) end - RSpec::Matchers.define :a_patch_error do |name| - match { |actual| actual.include?("Failed to apply #{name} patch.") } - end - describe 'implemented in a class' do describe 'class behavior' do describe '#patch' do @@ -82,10 +78,6 @@ def self.target_version end context 'when patcher .patch raises an error' do - before do - allow(Datadog.logger).to receive(:error) - end - context 'and .target_version is not defined' do let(:patcher) do stub_const( @@ -101,9 +93,12 @@ def self.patch end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + expect { patch }.to_not raise_error - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError')) end @@ -128,9 +123,12 @@ def self.target_version end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + expect { patch }.to_not raise_error - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError', 'target_version:1.0')) end @@ -193,11 +191,7 @@ def self.patch subject(:on_patch_error) { patcher.on_patch_error(error) } - let(:error) { instance_double('error', class: StandardError, message: nil, backtrace: []) } - - before do - allow(Datadog.logger).to receive(:error) - end + let(:error) { StandardError.new('Oops..').tap { |e| e.set_backtrace([]) } } context 'and .target_version is not defined' do let(:patcher) do @@ -205,9 +199,11 @@ def self.patch end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + subject - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError')) end @@ -228,9 +224,12 @@ def self.target_version end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + subject - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError', 'target_version:1.0')) end @@ -280,11 +279,7 @@ def patch subject(:on_patch_error) { patcher.on_patch_error(error) } - let(:error) { instance_double('error', class: StandardError, message: nil, backtrace: []) } - - before do - allow(Datadog.logger).to receive(:error) - end + let(:error) { StandardError.new('Oops..').tap { |e| e.set_backtrace([]) } } context 'and .target_version is not defined' do let(:patcher_class) do @@ -292,9 +287,12 @@ def patch end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + subject - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher_class.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError')) end @@ -315,9 +313,12 @@ def target_version end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + subject - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher_class.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError', 'target_version:1.0')) end @@ -395,10 +396,6 @@ def self.target_version end context 'when patcher .patch raises an error' do - before do - allow(Datadog.logger).to receive(:error) - end - context 'and .target_version is not defined' do let(:patcher) do stub_const( @@ -414,9 +411,12 @@ def self.patch end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + expect { patch }.to_not raise_error - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError')) end @@ -441,9 +441,12 @@ def self.target_version end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + expect { patch }.to_not raise_error - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError', 'target_version:1.0')) end @@ -506,11 +509,7 @@ def self.patch subject(:on_patch_error) { patcher.on_patch_error(error) } - let(:error) { instance_double('error', class: StandardError, message: nil, backtrace: []) } - - before do - allow(Datadog.logger).to receive(:error) - end + let(:error) { StandardError.new('Oops..').tap { |e| e.set_backtrace([]) } } context 'and .target_version is not defined' do let(:patcher) do @@ -518,9 +517,12 @@ def self.patch end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + subject - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError')) end @@ -541,9 +543,12 @@ def self.target_version end it 'handles the error' do + expect(Datadog.logger).to receive(:error).with(/Failed to apply TestPatcher patch/) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(an_instance_of(StandardError), description: 'Failed to apply TestPatcher patch') + subject - expect(Datadog.logger).to have_received(:error) - .with(a_patch_error(patcher.name)) + expect(health_metrics).to have_received(:error_instrumentation_patch) .with(1, tags: array_including('patcher:TestPatcher', 'error:StandardError', 'target_version:1.0')) end diff --git a/spec/datadog/tracing/sampling/rate_sampler_spec.rb b/spec/datadog/tracing/sampling/rate_sampler_spec.rb index 5c26c02b47f..993efb1d0b4 100644 --- a/spec/datadog/tracing/sampling/rate_sampler_spec.rb +++ b/spec/datadog/tracing/sampling/rate_sampler_spec.rb @@ -22,6 +22,12 @@ it_behaves_like 'sampler with sample rate', 1.0 do let(:trace) { nil } end + + it do + expect(Datadog.logger).to receive(:warn).with('sample rate is not between 0 and 1, falling back to 1') + + sampler + end end context 'that is 0' do @@ -46,6 +52,12 @@ let(:sample_rate) { 1.5 } it_behaves_like 'sampler with sample rate', 1.0 + + it do + expect(Datadog.logger).to receive(:warn).with('sample rate is not between 0 and 1, falling back to 1') + + sampler + end end end end diff --git a/spec/datadog/tracing/sampling/rule_sampler_spec.rb b/spec/datadog/tracing/sampling/rule_sampler_spec.rb index 80ad835debd..c784b944f28 100644 --- a/spec/datadog/tracing/sampling/rule_sampler_spec.rb +++ b/spec/datadog/tracing/sampling/rule_sampler_spec.rb @@ -174,7 +174,7 @@ let(:rule) { { sample_rate: 'oops' } } it 'does not accept rule with a non-float sample_rate' do - expect(Datadog.logger).to receive(:error) + expect(Datadog.logger).to receive(:warn) is_expected.to be_nil end end @@ -183,7 +183,7 @@ let(:rule) { { name: 'test' } } it 'does not accept rule missing the mandatory sample_rate' do - expect(Datadog.logger).to receive(:error) + expect(Datadog.logger).to receive(:warn) is_expected.to be_nil end @@ -191,7 +191,7 @@ let(:rules) { [{ sample_rate: 0.1 }, { name: 'test' }] } it 'rejects all rules if one is missing the mandatory sample_rate' do - expect(Datadog.logger).to receive(:error) + expect(Datadog.logger).to receive(:warn) is_expected.to be_nil end end @@ -201,7 +201,7 @@ let(:rules) { 'not a json array' } it 'returns nil in case of parsing error' do - expect(Datadog.logger).to receive(:error) + expect(Datadog.logger).to receive(:warn) is_expected.to be_nil end end diff --git a/spec/datadog/tracing/sampling/rule_spec.rb b/spec/datadog/tracing/sampling/rule_spec.rb index 6451c32e51d..800a1168fde 100644 --- a/spec/datadog/tracing/sampling/rule_spec.rb +++ b/spec/datadog/tracing/sampling/rule_spec.rb @@ -53,6 +53,8 @@ expect(Datadog.logger).to receive(:error) .with(a_string_including("Matcher failed. Cause: #{error}")) + expect(Datadog::Core::Telemetry::Logging).to receive(:report) + .with(error, description: 'Matcher failed') end it { is_expected.to be nil }