From 6ce31865d7b79b039acac78fe5d4f546fff35fc7 Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 7 Mar 2018 13:12:43 -0500 Subject: [PATCH 1/2] Refactored: :datadog_rack_request_span to 'datadog_rack_request_span' in Rack. --- lib/ddtrace/contrib/grape/endpoint.rb | 2 +- lib/ddtrace/contrib/rack/middlewares.rb | 8 +++++++- test/contrib/rack/helpers.rb | 6 +++--- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lib/ddtrace/contrib/grape/endpoint.rb b/lib/ddtrace/contrib/grape/endpoint.rb index 3d0d173c9c..25c7578d6c 100644 --- a/lib/ddtrace/contrib/grape/endpoint.rb +++ b/lib/ddtrace/contrib/grape/endpoint.rb @@ -71,7 +71,7 @@ def self.endpoint_run(name, start, finish, id, payload) span.resource = resource # set the request span resource if it's a `rack.request` span - request_span = payload[:env][:datadog_rack_request_span] + request_span = payload[:env][Datadog::Contrib::Rack::TraceMiddleware::RACK_REQUEST_SPAN] if !request_span.nil? && request_span.name == 'rack.request' request_span.resource = resource end diff --git a/lib/ddtrace/contrib/rack/middlewares.rb b/lib/ddtrace/contrib/rack/middlewares.rb index 0818e3c114..7363f70fa5 100644 --- a/lib/ddtrace/contrib/rack/middlewares.rb +++ b/lib/ddtrace/contrib/rack/middlewares.rb @@ -13,6 +13,8 @@ module Rack # application. If request tags are not set by the app, they will be set using # information available at the Rack level. class TraceMiddleware + RACK_REQUEST_SPAN = 'datadog.rack_request_span'.freeze + def initialize(app) @app = app end @@ -35,7 +37,11 @@ def call(env) # start a new request span and attach it to the current Rack environment; # we must ensure that the span `resource` is set later request_span = tracer.trace('rack.request', trace_options) - env[:datadog_rack_request_span] = request_span + env[RACK_REQUEST_SPAN] = request_span + + # TODO: For backwards compatibility; this attribute is deprecated. + # Will be removed in version 0.13.0. + env[:datadog_rack_request_span] = env[RACK_REQUEST_SPAN] # Copy the original env, before the rest of the stack executes. # Values may change; we want values before that happens. diff --git a/test/contrib/rack/helpers.rb b/test/contrib/rack/helpers.rb index 1a7f856777..5d80300568 100644 --- a/test/contrib/rack/helpers.rb +++ b/test/contrib/rack/helpers.rb @@ -40,7 +40,7 @@ def app run(proc do |env| # this should be considered a web framework that can alter # the request span after routing / controller processing - request_span = env[:datadog_rack_request_span] + request_span = env[Datadog::Contrib::Rack::TraceMiddleware::RACK_REQUEST_SPAN] request_span.resource = 'GET /app/' request_span.set_tag('http.method', 'GET_V2') request_span.set_tag('http.status_code', 201) @@ -54,7 +54,7 @@ def app run(proc do |env| # this should be considered a web framework that can alter # the request span after routing / controller processing - request_span = env[:datadog_rack_request_span] + request_span = env[Datadog::Contrib::Rack::TraceMiddleware::RACK_REQUEST_SPAN] request_span.status = 1 request_span.set_tag('error.stack', 'Handled exception') @@ -66,7 +66,7 @@ def app run(proc do |env| # this should be considered a web framework that can alter # the request span after routing / controller processing - request_span = env[:datadog_rack_request_span] + request_span = env[Datadog::Contrib::Rack::TraceMiddleware::RACK_REQUEST_SPAN] request_span.set_tag('error.stack', 'Handled exception') [500, { 'Content-Type' => 'text/html' }, 'OK'] From 7edd4583842202bd30b647e0fa1e8787799548de Mon Sep 17 00:00:00 2001 From: David Elner Date: Wed, 7 Mar 2018 13:13:08 -0500 Subject: [PATCH 2/2] Added: Deprecation warning for :datadog_rack_request_span. --- lib/ddtrace/contrib/rack/middlewares.rb | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/ddtrace/contrib/rack/middlewares.rb b/lib/ddtrace/contrib/rack/middlewares.rb index 7363f70fa5..824e9c2fab 100644 --- a/lib/ddtrace/contrib/rack/middlewares.rb +++ b/lib/ddtrace/contrib/rack/middlewares.rb @@ -43,6 +43,9 @@ def call(env) # Will be removed in version 0.13.0. env[:datadog_rack_request_span] = env[RACK_REQUEST_SPAN] + # Add deprecation warnings + add_deprecation_warnings(env) + # Copy the original env, before the rest of the stack executes. # Values may change; we want values before that happens. original_env = env.dup @@ -145,6 +148,34 @@ def get_request_id(headers, env) headers ||= {} headers['X-Request-Id'] || headers['X-Request-ID'] || env['HTTP_X_REQUEST_ID'] end + + private + + REQUEST_SPAN_DEPRECATION_WARNING = %( + :datadog_rack_request_span is considered an internal symbol in the Rack env, + and has been been DEPRECATED. Public support for its usage is discontinued. + If you need the Rack request span, try using `Datadog.tracer.active_span`. + This key will be removed in version 0.13.0).freeze + + def add_deprecation_warnings(env) + env.instance_eval do + def [](key) + if key == :datadog_rack_request_span && !@request_span_warning_issued + Datadog::Tracer.log.warn(REQUEST_SPAN_DEPRECATION_WARNING) + @request_span_warning_issued = true + end + super + end + + def []=(key, value) + if key == :datadog_rack_request_span && !@request_span_warning_issued + Datadog::Tracer.log.warn(REQUEST_SPAN_DEPRECATION_WARNING) + @request_span_warning_issued = true + end + super + end + end + end end end end