Skip to content

Commit

Permalink
[fix] rack middleware close trace via body proxy
Browse files Browse the repository at this point in the history
  • Loading branch information
skcc321 committed Nov 19, 2021
1 parent 8b97105 commit 513c157
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 5 deletions.
4 changes: 2 additions & 2 deletions lib/ddtrace/contrib/delayed_job/plugin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ module DelayedJob
# DelayedJob plugin that instruments invoke_job hook
class Plugin < Delayed::Plugin
def self.instrument_invoke(job, &block)
return block.call(job) unless tracer && tracer.enabled
return yield(job) unless tracer && tracer.enabled

tracer.trace(Ext::SPAN_JOB, service: configuration[:service_name], resource: job_name(job),
on_error: configuration[:error_handler]) do |span|
Expand All @@ -29,7 +29,7 @@ def self.instrument_invoke(job, &block)
end

def self.instrument_enqueue(job, &block)
return block.call(job) unless tracer && tracer.enabled
return yield(job) unless tracer && tracer.enabled

tracer.trace(Ext::SPAN_ENQUEUE, service: configuration[:client_service_name], resource: job_name(job)) do |span|
set_sample_rate(span)
Expand Down
24 changes: 21 additions & 3 deletions lib/ddtrace/contrib/rack/middlewares.rb
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,21 @@ def call(env)

# call the rest of the stack
status, headers, response = @app.call(env)
[status, headers, response]

# some middleware like Rack::CommonLogger wraps body via Rack::BodyProxy
# Rack::BodyProxy is a correct mechanism to finish span after all middleware call
# so Rack::CommonLogger will have trace_id & span_id tags

body = if defined?(::Rack::BodyProxy)
::Rack::BodyProxy.new(response) do
close_trace(request_span, frontend_span, env, status, headers, response, original_env, tracer)
end
else
close_trace(request_span, frontend_span, env, status, headers, response, original_env, tracer)
response
end

[status, headers, body]

# rubocop:disable Lint/RescueException
# Here we really want to catch *any* exception, not only StandardError,
Expand All @@ -98,9 +112,13 @@ def call(env)
# catch exceptions that may be raised in the middleware chain
# Note: if a middleware catches an Exception without re raising,
# the Exception cannot be recorded here.
request_span.set_error(e) unless request_span.nil?
request_span.set_error(e) if request_span

close_trace(request_span, frontend_span, env, status, headers, response, original_env, tracer)
raise e
ensure
end

def close_trace(request_span, frontend_span, env, status, headers, response, original_env, tracer)
if request_span
# Rack is a really low level interface and it doesn't provide any
# advanced functionality like routers. Because of that, we assume that
Expand Down
1 change: 1 addition & 0 deletions lib/ddtrace/contrib/rack/patcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ def target_version

def patch
# Patch middleware
require 'rack'
require_relative 'middlewares'
end
end
Expand Down
1 change: 1 addition & 0 deletions sorbet/rbi/todo.rbi

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 513c157

Please sign in to comment.