Skip to content

Commit

Permalink
Add Datadog::Error value-object
Browse files Browse the repository at this point in the history
This ensures all error-related data is properly encoded.
  • Loading branch information
p-lambert committed Aug 29, 2017
1 parent 3ccbf91 commit 51ff1c3
Show file tree
Hide file tree
Showing 12 changed files with 116 additions and 47 deletions.
1 change: 1 addition & 0 deletions lib/ddtrace.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'ddtrace/monkey'
require 'ddtrace/pin'
require 'ddtrace/tracer'
require 'ddtrace/error'

# \Datadog global namespace that includes all tracing functionality for Tracer and Span classes.
module Datadog
Expand Down
6 changes: 1 addition & 5 deletions lib/ddtrace/contrib/rails/action_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,7 @@ def self.process_action(_name, start, finish, _id, payload)
else
status = '500'
end
if status.starts_with?('5')
span.status = 1
span.set_tag(Datadog::Ext::Errors::TYPE, error[0])
span.set_tag(Datadog::Ext::Errors::MSG, error[1])
end
span.set_error(error) if status.starts_with?('5')
end
ensure
span.start_time = start
Expand Down
16 changes: 2 additions & 14 deletions lib/ddtrace/contrib/rails/action_view.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,13 +74,7 @@ def self.render_template(_name, start, finish, _id, payload)
template_name = Datadog::Contrib::Rails::Utils.normalize_template_name(payload.fetch(:identifier))
span.set_tag('rails.template_name', template_name)
span.set_tag('rails.layout', payload.fetch(:layout))

if payload[:exception]
error = payload[:exception]
span.status = 1
span.set_tag(Datadog::Ext::Errors::TYPE, error[0])
span.set_tag(Datadog::Ext::Errors::MSG, error[1])
end
span.set_error(payload[:exception]) if payload[:exception]
ensure
span.start_time = start
span.finish(finish)
Expand All @@ -102,13 +96,7 @@ def self.render_partial(_name, start, finish, _id, payload)
begin
template_name = Datadog::Contrib::Rails::Utils.normalize_template_name(payload.fetch(:identifier))
span.set_tag('rails.template_name', template_name)

if payload[:exception]
error = payload[:exception]
span.status = 1
span.set_tag(Datadog::Ext::Errors::TYPE, error[0])
span.set_tag(Datadog::Ext::Errors::MSG, error[1])
end
span.set_error(payload[:exception]) if payload[:exception]
ensure
span.start_time = start
span.finish(finish)
Expand Down
8 changes: 1 addition & 7 deletions lib/ddtrace/contrib/rails/active_support.rb
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,7 @@ def self.trace_cache(resource, _name, start, finish, _id, payload)
store, = *Array.wrap(::Rails.configuration.cache_store).flatten
span.set_tag('rails.cache.backend', store)
span.set_tag('rails.cache.key', payload.fetch(:key))

if payload[:exception]
error = payload[:exception]
span.status = 1
span.set_tag(Datadog::Ext::Errors::TYPE, error[0])
span.set_tag(Datadog::Ext::Errors::MSG, error[1])
end
span.set_error(payload[:exception]) if payload[:exception]
ensure
span.start_time = start
span.finish(finish)
Expand Down
12 changes: 1 addition & 11 deletions lib/ddtrace/contrib/sinatra/tracer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,17 +151,7 @@ def render(engine, data, *)
span.resource = "#{request.request_method} #{@datadog_route}"
span.set_tag('sinatra.route.path', @datadog_route)
span.set_tag(Datadog::Ext::HTTP::STATUS_CODE, response.status)

if response.server_error?
span.status = 1

err = env['sinatra.error']
if err
span.set_tag(Datadog::Ext::Errors::TYPE, err.class)
span.set_tag(Datadog::Ext::Errors::MSG, err.message)
end
end

span.set_error(env['sinatra.error']) if response.server_error?
span.finish()
ensure
@datadog_request_span = nil
Expand Down
38 changes: 38 additions & 0 deletions lib/ddtrace/error.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Datadog global namespace
module Datadog
# Error is a value-object responsible for sanitizing/encapsulating error data
class Error
attr_reader :type, :message, :backtrace

def self.build_from(value)
case value
when Error then value
when Array then new(*value)
when Exception then new(value.class, value.message, value.backtrace)
when ContainsMessage then new(value.class, value.message)
else BlankError
end
end

def initialize(type, message, backtrace = [])
backtrace = Array(backtrace).join("\n")

@type = sanitize(type)
@message = sanitize(message)
@backtrace = sanitize(backtrace)
end

private

def sanitize(value)
value = value.to_s

return value if value.encoding == ::Encoding::UTF_8

value.encode(::Encoding::UTF_8)
end

BlankError = Error.new(nil, nil)
ContainsMessage = ->(v) { v.respond_to?(:message) }
end
end
1 change: 1 addition & 0 deletions lib/ddtrace/ext/errors.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
module Datadog
module Ext
module Errors
STATUS = 1
MSG = 'error.msg'.freeze
TYPE = 'error.type'.freeze
STACK = 'error.stack'.freeze
Expand Down
13 changes: 8 additions & 5 deletions lib/ddtrace/span.rb
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,14 @@ def get_metric(key)

# Mark the span with the given error.
def set_error(e)
return if e.nil?
@status = 1
@meta[Datadog::Ext::Errors::MSG] = e.message if e.respond_to?(:message) && e.message
@meta[Datadog::Ext::Errors::TYPE] = e.class.to_s
@meta[Datadog::Ext::Errors::STACK] = e.backtrace.join("\n") if e.respond_to?(:backtrace) && e.backtrace
e = Error.build_from(e)

return unless e

@status = Ext::Errors::STATUS
set_tag(Ext::Errors::TYPE, e.type)
set_tag(Ext::Errors::MSG, e.message)
set_tag(Ext::Errors::STACK, e.backtrace)
end

# Mark the span finished at the current time and submit it.
Expand Down
2 changes: 1 addition & 1 deletion test/contrib/rails/controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ class TracingControllerTest < ActionController::TestCase
assert_equal(1, span.status, 'span should be flagged as an error')
assert_equal('ZeroDivisionError', span.get_tag('error.type'), 'type should contain the class name of the error')
assert_equal('divided by 0', span.get_tag('error.msg'), 'msg should state we tried to divided by 0')
assert_nil(span.get_tag('error.stack'))
assert_empty(span.get_tag('error.stack'))
end

test 'http error code should be trapped and reported as such, even with no exception' do
Expand Down
4 changes: 2 additions & 2 deletions test/contrib/rails/rack_middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ class FullStackTest < ActionDispatch::IntegrationTest
assert_equal(controller_span.status, 1)
assert_equal(controller_span.get_tag('error.type'), 'ZeroDivisionError')
assert_equal(controller_span.get_tag('error.msg'), 'divided by 0')
assert_nil(controller_span.get_tag('error.stack')) # error stack is in rack span
assert_empty(controller_span.get_tag('error.stack')) # error stack is in rack span

assert_equal('rack.request', request_span.name)
assert_equal(request_span.span_type, 'http')
Expand Down Expand Up @@ -141,7 +141,7 @@ class FullStackTest < ActionDispatch::IntegrationTest
assert_equal(controller_span.status, 1)
assert_equal(controller_span.get_tag('error.type'), 'ZeroDivisionError')
assert_equal(controller_span.get_tag('error.msg'), 'divided by 0')
assert_nil(controller_span.get_tag('error.stack')) # error stack is in rack span
assert_empty(controller_span.get_tag('error.stack')) # error stack is in rack span

assert_equal('rack.request', request_span.name)
assert_equal(request_span.span_type, 'http')
Expand Down
4 changes: 2 additions & 2 deletions test/contrib/sinatra/tracer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,8 @@ def test_error
assert_equal('GET /error', span.resource)
assert_equal('GET', span.get_tag(Datadog::Ext::HTTP::METHOD))
assert_equal('/error', span.get_tag(Datadog::Ext::HTTP::URL))
assert_nil(span.get_tag(Datadog::Ext::Errors::TYPE))
assert_nil(span.get_tag(Datadog::Ext::Errors::MSG))
assert_empty(span.get_tag(Datadog::Ext::Errors::TYPE))
assert_empty(span.get_tag(Datadog::Ext::Errors::MSG))
assert_equal(Datadog::Ext::HTTP::TYPE, span.span_type)
assert_equal(1, span.status)
assert_nil(span.parent)
Expand Down
58 changes: 58 additions & 0 deletions test/error_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
require 'helper'
require 'ddtrace/error'

module Datadog
class ErrorTest < Minitest::Test
def setup
@error = Error.new('StandardError', 'message', %w[x y z])
end

def test_type
assert_equal('StandardError', @error.type)
end

def test_message
assert_equal('message', @error.message)
end

def test_backtrace
assert_equal("x\ny\nz", @error.backtrace)
end

def test_default_values
error = Error.new(nil, nil)

assert_equal('', error.type)
assert_equal('', error.message)
assert_equal('', error.backtrace)
end

# Empty strings were being interpreted as ASCII strings breaking `msgpack`
# decoding on the agent-side.
def test_enconding
error = Datadog::Error.new(nil, nil)

assert_equal(::Encoding::UTF_8, error.type.encoding)
assert_equal(::Encoding::UTF_8, error.message.encoding)
assert_equal(::Encoding::UTF_8, error.backtrace.encoding)
end

def test_array_coercion
error_payload = ['ZeroDivisionError', 'divided by 0']
error = Error.build_from(error_payload)

assert_equal('ZeroDivisionError', error.type)
assert_equal('divided by 0', error.message)
assert_equal('', error.backtrace)
end

def test_exception_coercion
exception = ZeroDivisionError.new('divided by 0')
error = Error.build_from(exception)

assert_equal('ZeroDivisionError', error.type)
assert_equal('divided by 0', error.message)
assert_equal('', error.backtrace)
end
end
end

0 comments on commit 51ff1c3

Please sign in to comment.