Skip to content

Commit

Permalink
wip
Browse files Browse the repository at this point in the history
  • Loading branch information
@marcotc
marcotc committed Feb 13, 2024
1 parent 0058848 commit e19b10b
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 55 deletions.
4 changes: 0 additions & 4 deletions Gemfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,3 @@ end
gem 'docile', '~> 1.3.5' if RUBY_VERSION < '2.5'
gem 'ffi', '~> 1.12.2' if RUBY_VERSION < '2.3'
gem 'msgpack', '~> 1.3.3' if RUBY_VERSION < '2.4'


gem 'grape'
gem 'rack-test'
50 changes: 30 additions & 20 deletions lib/datadog/tracing/contrib/grape/endpoint.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -92,23 +92,7 @@ def endpoint_run(name, start, finish, id, payload)
# Measure service stats
Contrib::Analytics.set_measured(span)

# catch thrown exceptions
handle_error(span, payload[:exception_object]) if payload[:exception_object]

if (exception_object = payload[:exception_object])
span.set_error(exception_object)
if exception_object.respond_to?(:status)
span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, exception_object.status)
end
else
# Status code is unreliable at this point if there was an exception.
# Only after `Grape::Middleware::Error#run_rescue_handler` that an error status code is resolved,
# but this handler is called much further down the grape middleware stack.
# In exception cases, the rack span will be the most reliable source for status quote.
# DEV: As a corollary, instrumenting Grape without Rack will provide incomplete
# DEV: status quote information.
span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, endpoint.status)
end
handle_error_and_status_code(span, endpoint, payload)

integration_route = endpoint.env['grape.routing_args'][:route_info].pattern.origin

Expand All @@ -130,6 +114,30 @@ def endpoint_run(name, start, finish, id, payload)
Datadog.logger.error(e.message)
end

# Status code resolution is tied to the exception handling
def handle_error_and_status_code(span, endpoint, payload)
status = nil

# Handle exceptions and status code
if (exception_object = payload[:exception_object])
# If the exception is not an internal Grape error, we won't have a status code at this point.
status = exception_object.status if exception_object.respond_to?(:status)

handle_error(span, exception_object, status)
else
# Status code is unreliable in `endpoint_run.grape` if there was an exception.
# Only after `Grape::Middleware::Error#run_rescue_handler` that the error status code of a request with a
# Ruby exception error is resolved. But that handler is called further down the Grape middleware stack.
# Rack span will then be the most reliable source for status codes.
# DEV: As a corollary, instrumenting Grape without Rack will provide incomplete
# DEV: status quote information.
status = endpoint.status
span.set_error(endpoint) if error_status_codes.include?(status)
end

span.set_tag(Tracing::Metadata::Ext::HTTP::TAG_STATUS_CODE, status) if status
end

def endpoint_start_render(*)
return if Thread.current[KEY_RENDER]
return unless enabled?
Expand Down Expand Up @@ -213,9 +221,10 @@ def endpoint_run_filters(name, start, finish, id, payload)

private

def handle_error(span, exception)
if exception.respond_to?('status')
span.set_error(exception) if error_status_codes.include?(exception.status)
def handle_error(span, exception, status = nil)
status ||= (exception.status if exception.respond_to?(:status))
if status
span.set_error(exception) if error_status_codes.include?(status)
else
on_error.call(span, exception)
end
Expand Down Expand Up @@ -270,6 +279,7 @@ def exception_is_error?(exception)
def error_status?(status)
matcher = datadog_configuration[:error_statuses]
return true unless matcher

matcher.include?(status) if matcher
end

Expand Down
53 changes: 22 additions & 31 deletions spec/datadog/tracing/contrib/grape/tracer_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,8 @@
expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')

expect(run_span.get_tag('http.status_code')).to eq('200')
end
end

Expand Down Expand Up @@ -261,6 +263,8 @@
expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')

expect(run_span.get_tag('http.status_code')).to eq('200')
end
end
end
Expand All @@ -276,6 +280,7 @@
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
expect(span.get_tag('http.status_code')).to eq('405')
end

context 'and error_responses' do
Expand All @@ -290,6 +295,7 @@
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
expect(span.get_tag('http.status_code')).to eq('405')
end
end
end
Expand All @@ -311,7 +317,7 @@
end
end

it 'handles exceptions' do
it 'handles request with exception' do
expect { subject }.to raise_error(StandardError, 'Ouch!')

expect(spans.length).to eq(2)
Expand Down Expand Up @@ -345,6 +351,8 @@
expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
# Status code has not been set yet at this instrumentation point
expect(run_span.get_tag('http.status_code')).to be_nil
end
end

Expand Down Expand Up @@ -406,13 +414,7 @@
expect(response.body).to eq('405 Not Allowed')

expect(span.name).to eq('grape.endpoint_run')
expect(span.status).to eq(1)
expect(span.get_tag('error.stack')).to_not be_nil
expect(span.get_tag('error.type')).to_not be_nil
expect(span.get_tag('error.message')).to_not be_nil,
"DEV: 🚧 Flaky test! Please send the maintainers a link for this CI failure. Thank you! 🚧\n" \
"response=#{response.inspect}\n" \
"spans=#{spans.inspect}\n"
expect(span).to_not have_error
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
Expand All @@ -428,13 +430,11 @@
expect(response.body).to eq('405 Not Allowed')

expect(span.name).to eq('grape.endpoint_run')
expect(span.status).to eq(1)
expect(span.get_tag('error.stack')).to_not be_nil
expect(span.get_tag('error.type')).to_not be_nil
expect(span.get_tag('error.message')).to_not be_nil
expect(span).to_not have_error
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
expect(span.get_tag('http.status_code')).to eq('405')
end
end

Expand All @@ -447,13 +447,11 @@
expect(response.body).to eq('405 Not Allowed')

expect(span.name).to eq('grape.endpoint_run')
expect(span.status).to eq(1)
expect(span.get_tag('error.stack')).to_not be_nil
expect(span.get_tag('error.type')).to_not be_nil
expect(span.get_tag('error.message')).to_not be_nil
expect(span).to_not have_error
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
expect(span.get_tag('http.status_code')).to eq('405')
end
end

Expand All @@ -467,12 +465,10 @@

expect(span.name).to eq('grape.endpoint_run')
expect(span).to_not have_error
expect(span.get_tag('error.stack')).to be_nil
expect(span.get_tag('error.type')).to be_nil
expect(span.get_tag('error.message')).to be_nil
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
expect(span.get_tag('http.status_code')).to eq('405')
end
end

Expand All @@ -485,13 +481,11 @@
expect(response.body).to eq('405 Not Allowed')

expect(span.name).to eq('grape.endpoint_run')
expect(span.status).to eq(0)
expect(span.get_tag('error.stack')).to be_nil
expect(span.get_tag('error.type')).to be_nil
expect(span.get_tag('error.message')).to be_nil
expect(span.get_tag('http.status_code')).to eq('405')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_run')
expect(span.get_tag('http.status_code')).to eq('405')
end
end
end
Expand All @@ -513,30 +507,27 @@
end
end

it 'handles exceptions' do
it 'handles request' do
subject

expect(spans.length).to eq(2)

expect(render_span.name).to eq('grape.endpoint_render')
expect(render_span.span_type).to eq('template')
expect(render_span.type).to eq('template')
expect(render_span.service).to eq(tracer.default_service)
expect(render_span.resource).to eq('grape.endpoint_render')
expect(render_span.parent_id).to eq(run_span.span_id)
expect(render_span.parent_id).to eq(run_span.id)

expect(render_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
expect(render_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_OPERATION))
.to eq('endpoint_render')

expect(run_span.name).to eq('grape.endpoint_run')
expect(run_span.span_type).to eq('web')
expect(run_span.type).to eq('web')
expect(run_span.service).to eq(tracer.default_service)
expect(run_span.resource).to eq('TestingAPI GET /base/soft_failure')
expect(run_span).to have_error # TODO: Should this be an error span?
expect(run_span).to have_error

expect(run_span).to have_error_type('StandardError')
expect(run_span).to have_error_message('Ouch!')
expect(run_span.get_tag('error.stack')).to include('grape/tracer_spec.rb')
expect(run_span).to be_root_span

expect(run_span.get_tag(Datadog::Tracing::Metadata::Ext::TAG_COMPONENT)).to eq('grape')
Expand Down

0 comments on commit e19b10b

Please sign in to comment.