Skip to content

Commit

Permalink
Merge pull request #184 from DataDog/pedro/ensure-clean-context
Browse files Browse the repository at this point in the history
Fix trace leak across multiple requests
  • Loading branch information
p-lambert authored Aug 31, 2017
2 parents 5a85f94 + faa9e3a commit a5b0476
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 1 deletion.
11 changes: 10 additions & 1 deletion lib/ddtrace/contrib/rack/middlewares.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def initialize(app, options = {})

def configure
# ensure that the configuration is executed only once
return if @tracer && @service
return clean_context if @tracer && @service

# retrieve the current tracer and service
@tracer = @options.fetch(:tracer)
Expand Down Expand Up @@ -135,6 +135,15 @@ def call(env)

[status, headers, response]
end

private

# TODO: Remove this once we change how context propagation works. This
# ensures we clean thread-local variables on each HTTP request avoiding
# memory leaks.
def clean_context
@tracer.provider.context = Datadog::Context.new
end
end
end
end
Expand Down
13 changes: 13 additions & 0 deletions test/contrib/rack/helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ class RackBaseTest < Minitest::Test
def app
tracer = @tracer

# rubocop:disable Metrics/BlockLength
Rack::Builder.new do
use Datadog::Contrib::Rack::TraceMiddleware, tracer: tracer

Expand Down Expand Up @@ -73,6 +74,18 @@ def app
[500, { 'Content-Type' => 'text/html' }, 'OK']
end)
end

map '/leak/' do
handler = proc do
tracer.trace('leaky-span-1')
tracer.trace('leaky-span-2')
tracer.trace('leaky-span-3')

[200, { 'Content-Type' => 'text/html' }, 'OK']
end

run(handler)
end
end.to_app
end

Expand Down
8 changes: 8 additions & 0 deletions test/contrib/rack/middleware_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,14 @@ def test_request_middleware_non_standard_error
assert_equal(1, span.status)
assert_nil(span.parent)
end

def test_middleware_context_cleaning
get '/leak'
get '/success'

assert_equal(0, @tracer.provider.context.trace.length)
assert_equal(1, @tracer.writer.spans.length)
end
end

class CustomTracerTest < RackBaseTest
Expand Down

0 comments on commit a5b0476

Please sign in to comment.