Skip to content

Commit 6a7a612

Browse files
committed
Move Grape span resource setting to beginning of request
As of #1623, the profiler records the `resource` of the root span, on top of the span id and trace id during sampling. Also, as discusssed in the description of #1623 (in the "Getting the correct `resource`" section) integrations where the `resource` is only set at the end pose an extra challenge, as the request may not be finished in time for the `resource` to be included in the profiler payload. This means that the profiler may miss the `resource` for some of the requests, depending on timing of when the sampling happens and when the request finishes. (Note that in this case, the trace id and span id will still be propagated, so the samples will always be able to be tied back to the trace that originated them; it's just the `resource` that will not be included in the profiler data). To avoid this issue for many of our customers, let's move the Grape `resource` setting to the beginning of the request, thus avoiding the issue altogether for this integration. (Note: This is similar to what we did for Rails in #1626 and Sinatra in #1628)
1 parent e391d2e commit 6a7a612

File tree

4 files changed

+98
-18
lines changed

4 files changed

+98
-18
lines changed

Gemfile

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,3 +75,6 @@ gem 'google-protobuf', ['~> 3.0', '!= 3.7.0', '!= 3.7.1'] if RUBY_PLATFORM != 'j
7575
# For type checking
7676
gem 'sorbet', '>= 0.5.6513', '< 0.6' if RUBY_VERSION >= '2.3.0'
7777
gem 'spoom', '~> 1.1' if RUBY_VERSION >= '2.4.0'
78+
79+
gem 'grape'
80+
gem 'rack-test'

lib/ddtrace/contrib/grape/endpoint.rb

Lines changed: 25 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,27 @@ def subscribe
3434
end
3535
end
3636

37-
def endpoint_start_process(*)
37+
def endpoint_start_process(_name, _start, _finish, _id, payload)
3838
return if Thread.current[KEY_RUN]
3939
return unless enabled?
4040

41+
# collect endpoint details
42+
endpoint = payload.fetch(:endpoint)
43+
api_view = api_view(endpoint.options[:for])
44+
request_method = endpoint.options.fetch(:method).first
45+
path = endpoint_expand_path(endpoint)
46+
resource = "#{api_view} #{request_method} #{path}"
47+
4148
# Store the beginning of a trace
42-
tracer.trace(
49+
span = tracer.trace(
4350
Ext::SPAN_ENDPOINT_RUN,
4451
service: service_name,
45-
span_type: Datadog::Ext::HTTP::TYPE_INBOUND
52+
span_type: Datadog::Ext::HTTP::TYPE_INBOUND,
53+
resource: resource,
4654
)
4755

56+
try_setting_rack_request_resource(payload, span.resource)
57+
4858
Thread.current[KEY_RUN] = true
4959
rescue StandardError => e
5060
Datadog.logger.error(e.message)
@@ -62,20 +72,12 @@ def endpoint_run(name, start, finish, id, payload)
6272

6373
begin
6474
# collect endpoint details
65-
api = payload[:endpoint].options[:for]
75+
endpoint = payload.fetch(:endpoint)
76+
api_view = api_view(endpoint.options[:for])
77+
request_method = endpoint.options.fetch(:method).first
78+
path = endpoint_expand_path(endpoint)
6679

67-
api_view = api_view(api)
68-
69-
request_method = payload[:endpoint].options[:method].first
70-
path = endpoint_expand_path(payload[:endpoint])
71-
resource = "#{api_view} #{request_method} #{path}"
72-
span.resource = resource
73-
74-
# set the request span resource if it's a `rack.request` span
75-
request_span = payload[:env][Datadog::Contrib::Rack::Ext::RACK_ENV_REQUEST_SPAN]
76-
if !request_span.nil? && request_span.name == Datadog::Contrib::Rack::Ext::SPAN_REQUEST
77-
request_span.resource = resource
78-
end
80+
try_setting_rack_request_resource(payload, span.resource)
7981

8082
# Set analytics sample rate
8183
Contrib::Analytics.set_sample_rate(span, analytics_sample_rate) if analytics_enabled?
@@ -230,6 +232,13 @@ def enabled?
230232
def datadog_configuration
231233
Datadog.configuration[:grape]
232234
end
235+
236+
def try_setting_rack_request_resource(payload, resource)
237+
request_span = payload[:env][Datadog::Contrib::Rack::Ext::RACK_ENV_REQUEST_SPAN]
238+
if !request_span.nil? && request_span.name == Datadog::Contrib::Rack::Ext::SPAN_REQUEST
239+
request_span.resource = resource
240+
end
241+
end
233242
end
234243
end
235244
end

lib/ddtrace/contrib/grape/instrumentation.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def generate_api_method(*params, &block)
2424
# InstanceMethods - implementing instrumentation
2525
module InstanceMethods
2626
def run(*args)
27-
::ActiveSupport::Notifications.instrument('endpoint_run.grape.start_process')
27+
::ActiveSupport::Notifications.instrument('endpoint_run.grape.start_process', endpoint: self, env: env)
2828
super
2929
end
3030
end

spec/ddtrace/contrib/grape/tracer_spec.rb

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,11 @@
1616
let(:run_span) { spans.find { |x| x.name == Datadog::Contrib::Grape::Ext::SPAN_ENDPOINT_RUN } }
1717
let(:run_filter_span) { spans.find { |x| x.name == Datadog::Contrib::Grape::Ext::SPAN_ENDPOINT_RUN_FILTERS } }
1818
let(:span) { spans.last }
19+
let(:observed) { {} }
1920

2021
let(:testing_api) do
22+
observed = self.observed
23+
2124
# patch Grape before the application
2225
Datadog::Contrib::Grape::Patcher.patch
2326

@@ -80,10 +83,20 @@
8083
end
8184
end
8285
end
86+
87+
resource :span_resource do
88+
get :span_resource do
89+
root_span = Datadog.tracer.active_root_span
90+
observed[:root_span] = { name: root_span.name, resource: root_span.resource }
91+
'OK'
92+
end
93+
end
8394
end)
8495
end
8596

8697
let(:rack_testing_api) do
98+
observed = self.observed
99+
87100
# patch Grape before the application
88101
Datadog::Contrib::Grape::Patcher.patch
89102

@@ -97,6 +110,22 @@
97110
get :hard_failure do
98111
raise StandardError, 'Ouch!'
99112
end
113+
114+
resource :span_resource_rack do
115+
get :span_resource do
116+
endpoint_span = Datadog.tracer.active_span.parent
117+
observed[:endpoint_span] = { name: endpoint_span.name, resource: endpoint_span.resource }
118+
root_span = Datadog.tracer.active_root_span
119+
observed[:root_span] = { name: root_span.name, resource: root_span.resource }
120+
'OK'
121+
end
122+
123+
get :custom_span_resource do
124+
endpoint_span = Datadog.tracer.active_span.parent
125+
endpoint_span.resource = 'CustomSpanResource'
126+
'OK'
127+
end
128+
end
100129
end)
101130

102131
# create a custom Rack application with the Rack middleware and a Grape API
@@ -498,6 +527,19 @@
498527
end
499528
end
500529
end
530+
531+
describe 'span resource' do
532+
subject(:response) { get '/span_resource/span_resource' }
533+
534+
before do
535+
is_expected.to be_ok
536+
end
537+
538+
it 'sets the request (root) span resource before calling the endpoint' do
539+
expect(observed[:root_span])
540+
.to eq(name: 'grape.endpoint_run', resource: 'TestingAPI GET /span_resource/span_resource')
541+
end
542+
end
501543
end
502544

503545
context 'with rack' do
@@ -519,7 +561,7 @@
519561
let(:analytics_sample_rate_var) { Datadog::Contrib::Grape::Ext::ENV_ANALYTICS_SAMPLE_RATE }
520562
end
521563

522-
it 'intergrates with the Rack integration' do
564+
it 'integrates with the Rack integration' do
523565
is_expected.to be_ok
524566
expect(response.body).to eq('OK')
525567
expect(spans.length).to eq(3)
@@ -634,5 +676,31 @@
634676
expect(rack_span.parent).to be_nil
635677
end
636678
end
679+
680+
describe 'span resource' do
681+
subject(:response) { get '/api/span_resource_rack/span_resource' }
682+
683+
before do
684+
is_expected.to be_ok
685+
end
686+
687+
it 'sets the request (grape) span resource before calling the endpoint' do
688+
expect(observed[:endpoint_span])
689+
.to eq(name: 'grape.endpoint_run', resource: 'RackTestingAPI GET /span_resource_rack/span_resource')
690+
end
691+
692+
it 'sets the rack (root) span resource before calling the endpoint' do
693+
expect(observed[:root_span])
694+
.to eq(name: 'rack.request', resource: 'RackTestingAPI GET /span_resource_rack/span_resource')
695+
end
696+
697+
context 'when a custom request span resource is applied' do
698+
subject(:response) { get '/api/span_resource_rack/custom_span_resource' }
699+
700+
it 'propagates the custom request span resource to the rack (root) span resource' do
701+
expect(spans.last.resource).to eq('CustomSpanResource')
702+
end
703+
end
704+
end
637705
end
638706
end

0 commit comments

Comments
 (0)