Skip to content

Commit 1005755

Browse files
committed
Fix NoMethodError in sinatra integration when Tracer middleware is missing
This fixes a regression introduced in version 0.52.0 by #1628. When a Sinatra modular application being traced is missing the `Datadog::Contrib::Sinatra::Tracer` middleware (as documented in <https://docs.datadoghq.com/tracing/setup_overview/setup/ruby/#sinatra>), no Sinatra request span is ever created (only a route span). Because we didn't properly account for this in `#datadog_span`, the following `NoMethodError` was triggered: ``` NoMethodError: undefined method `[]' for nil:NilClass dd-trace-rb/lib/ddtrace/contrib/sinatra/env.rb:13:in `datadog_span' dd-trace-rb/lib/ddtrace/contrib/sinatra/tracer.rb:122:in `block in route_eval' dd-trace-rb/lib/ddtrace/tracer.rb:283:in `trace' dd-trace-rb/lib/ddtrace/contrib/sinatra/tracer.rb:105:in `route_eval' ruby-2.7.4/gems/sinatra-2.1.0/lib/sinatra/base.rb:1013:in `block (2 levels) in route!' ``` I suspect this is the same issue as was reported in #1643. Because this configuration is actually not desirable -- spans reported will be missing crucial request information, as discussed in #1643 -- I've also added a warning to hopefully steer users in the right direction.
1 parent f7431ce commit 1005755

File tree

3 files changed

+46
-3
lines changed

3 files changed

+46
-3
lines changed

lib/ddtrace/contrib/sinatra/env.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ module Env
1010
module_function
1111

1212
def datadog_span(env, app)
13-
env[Ext::RACK_ENV_REQUEST_SPAN][app]
13+
request_span = env[Ext::RACK_ENV_REQUEST_SPAN]
14+
request_span && request_span[app]
1415
end
1516

1617
def set_datadog_span(env, app, span)

lib/ddtrace/contrib/sinatra/tracer.rb

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
require 'ddtrace/ext/errors'
66
require 'ddtrace/ext/http'
77
require 'ddtrace/propagation/http_propagator'
8-
8+
require 'ddtrace/utils/only_once'
99
require 'ddtrace/contrib/sinatra/ext'
1010
require 'ddtrace/contrib/sinatra/tracer_middleware'
1111
require 'ddtrace/contrib/sinatra/env'
@@ -77,6 +77,9 @@ def self.registered(app)
7777

7878
# Method overrides for Sinatra::Base
7979
module Base
80+
MISSING_REQUEST_SPAN_ONLY_ONCE = Datadog::Utils::OnlyOnce.new
81+
private_constant :MISSING_REQUEST_SPAN_ONLY_ONCE
82+
8083
def render(engine, data, *)
8184
tracer = Datadog.configuration[:sinatra][:tracer]
8285
return super unless tracer.enabled
@@ -121,8 +124,18 @@ def route_eval
121124
else
122125
Sinatra::Env.datadog_span(env, self.class)
123126
end
124-
if sinatra_request_span # DEV: Is it possible for sinatra_request_span to ever be nil here?
127+
if sinatra_request_span
125128
sinatra_request_span.resource = span.resource
129+
else
130+
MISSING_REQUEST_SPAN_ONLY_ONCE.run do
131+
Datadog.logger.warn do
132+
'Sinatra integration is misconfigured, reported traces will be missing request metadata ' \
133+
'such as path and HTTP status code. ' \
134+
'Did you forget to add `register Datadog::Contrib::Sinatra::Tracer` to your ' \
135+
'`Sinatra::Base` subclass? ' \
136+
'See <https://docs.datadoghq.com/tracing/setup_overview/setup/ruby/#sinatra> for more details.'
137+
end
138+
end
126139
end
127140

128141
Contrib::Analytics.set_measured(span)

spec/ddtrace/contrib/sinatra/tracer_spec.rb

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -609,6 +609,35 @@
609609
end
610610
end
611611
end
612+
613+
context 'when modular app does not register the Datadog::Contrib::Sinatra::Tracer middleware' do
614+
let(:sinatra_app) do
615+
sinatra_routes = self.sinatra_routes
616+
stub_const('App', Class.new(Sinatra::Base) do
617+
instance_exec(&sinatra_routes)
618+
end)
619+
end
620+
621+
subject(:response) { get url }
622+
623+
before do
624+
allow(Datadog.logger).to receive(:warn)
625+
Datadog::Contrib::Sinatra::Tracer::Base
626+
.const_get('MISSING_REQUEST_SPAN_ONLY_ONCE').send(:reset_ran_once_state_for_tests)
627+
end
628+
629+
it { is_expected.to be_ok }
630+
631+
it 'logs a warning' do
632+
expect(Datadog.logger).to receive(:warn) do |&message|
633+
expect(message.call).to include 'Sinatra integration is misconfigured'
634+
end
635+
636+
# NOTE: We actually need to check that the request finished ok, as sinatra may otherwise "swallow" an RSpec
637+
# exception and thus the test will appear to pass because the RSpec exception doesn't propagate out
638+
is_expected.to be_ok
639+
end
640+
end
612641
end
613642

614643
RSpec::Matchers.define :be_request_span do |opts = {}|

0 commit comments

Comments
 (0)