Skip to content

Commit 604dd53

Browse files
prettycoderprettycoder
committed
make sure we preserve custome headers
fix bug in RestClient add tests for all other instrumented outbound methods
1 parent 28b038d commit 604dd53

File tree

8 files changed

+165
-5
lines changed

8 files changed

+165
-5
lines changed

‎lib/appoptics_apm/base.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,9 @@ def tracing_layer?(layer)
135135
#
136136
def tracing_layer_op?(operation)
137137
if operation.is_a?(Array)
138-
return operation.include?(AppOpticsAPM.layer_op)
138+
operation.include?(AppOpticsAPM.layer_op)
139139
else
140-
return AppOpticsAPM.layer_op == operation.to_sym
140+
AppOpticsAPM.layer_op == operation.to_sym
141141
end
142142
end
143143

‎lib/appoptics_apm/inst/rest-client.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ def execute_with_appoptics(&block)
1818

1919
unless AppOpticsAPM.tracing?
2020
xtrace = AppOpticsAPM::Context.toString
21-
@processed_headers = make_headers('X-Trace' => xtrace) if AppOpticsAPM::XTrace.valid?(xtrace) && !blacklisted
21+
@processed_headers['X-Trace'] = AppOpticsAPM::Context.toString if AppOpticsAPM::XTrace.valid?(xtrace) && !blacklisted
2222
return execute_without_appoptics(&block)
2323
end
2424

@@ -27,7 +27,7 @@ def execute_with_appoptics(&block)
2727
kvs[:Backtrace] = AppOpticsAPM::API.backtrace if AppOpticsAPM::Config[:rest_client][:collect_backtraces]
2828
AppOpticsAPM::API.log_entry('rest-client', kvs)
2929

30-
@processed_headers = make_headers('X-Trace' => AppOpticsAPM::Context.toString) unless blacklisted
30+
@processed_headers['X-Trace'] = AppOpticsAPM::Context.toString unless blacklisted
3131

3232
# The core rest-client call
3333
execute_without_appoptics(&block)

‎test/mocked/curb_mocked_test.rb‎

Lines changed: 73 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,12 +109,14 @@ def test_multi_get_tracing
109109
url_confs.each do |conf|
110110
headers = conf[:headers] || {}
111111
assert headers['X-Trace']
112+
assert headers['Custom']
113+
assert_match /specialvalue/, headers['Custom']
112114
assert sampled?(headers['X-Trace'])
113115
end
114116
true
115117
end
116118

117-
easy_options = {:follow_location => true}
119+
easy_options = {:follow_location => true, :headers => { 'Custom' => 'specialvalue' }}
118120
multi_options = {:pipeline => false}
119121

120122
urls = []
@@ -192,13 +194,15 @@ def test_multi_perform_tracing
192194
urls.each do |url|
193195
cu = Curl::Easy.new(url) do |curl|
194196
curl.follow_location = true
197+
curl.headers = { 'Custom' => 'specialvalue' }
195198
end
196199
m.add cu
197200
end
198201

199202
m.perform do
200203
m.requests.each do |request|
201204
assert request.headers['X-Trace']
205+
assert request.headers['Custom']
202206
assert sampled?(request.headers['X-Trace'])
203207
end
204208
end
@@ -234,6 +238,74 @@ def test_multi_perform_tracing_not_sampling
234238
end
235239
end
236240
end
241+
242+
# preserve custom headers
243+
#
244+
# this calls Curl::Easy.http
245+
def test_preserves_custom_headers_on_get
246+
stub_request(:get, "http://127.0.0.6:8101/").to_return(status: 200, body: "", headers: {})
247+
248+
AppOpticsAPM::API.start_trace('curb_tests') do
249+
Curl.get("http://127.0.0.6:8101/") do |curl|
250+
curl.headers = { 'Custom' => 'specialvalue' }
251+
end
252+
end
253+
254+
assert_requested :get, "http://127.0.0.6:8101/", headers: {'Custom'=>'specialvalue'}, times: 1
255+
end
256+
257+
# The following test can't use WebMock because it interferes with our instrumentation
258+
def test_preserves_custom_headers_on_http_put
259+
WebMock.disable!
260+
261+
curl = Curl::Easy.new("http://127.0.0.1:8101/")
262+
curl.headers = { 'Custom' => 'specialvalue4' }
263+
264+
AppOpticsAPM::API.start_trace('curb_tests') do
265+
curl.http_put nil
266+
end
267+
268+
assert curl.headers
269+
assert curl.headers['X-Trace']
270+
assert curl.headers['Custom']
271+
assert_match /^2B[0-9,A-F]*01$/, curl.headers['X-Trace']
272+
assert_match /specialvalue4/, curl.headers['Custom']
273+
end
274+
275+
def test_preserves_custom_headers_on_http_post
276+
WebMock.disable!
277+
278+
curl = Curl::Easy.new("http://127.0.0.1:8101/")
279+
curl.headers = { 'Custom' => 'specialvalue4' }
280+
281+
AppOpticsAPM::API.start_trace('curb_tests') do
282+
curl.http_post
283+
end
284+
285+
assert curl.headers
286+
assert curl.headers['X-Trace']
287+
assert curl.headers['Custom']
288+
assert_match /^2B[0-9,A-F]*01$/, curl.headers['X-Trace']
289+
assert_match /specialvalue4/, curl.headers['Custom']
290+
end
291+
292+
def test_preserves_custom_headers_on_perform
293+
WebMock.disable!
294+
295+
curl = Curl::Easy.new("http://127.0.0.1:8101/")
296+
curl.headers = { 'Custom' => 'specialvalue4' }
297+
298+
AppOpticsAPM::API.start_trace('curb_tests') do
299+
curl.perform
300+
end
301+
302+
assert curl.headers
303+
assert curl.headers['X-Trace']
304+
assert curl.headers['Custom']
305+
assert_match /^2B[0-9,A-F]*01$/, curl.headers['X-Trace']
306+
assert_match /specialvalue4/, curl.headers['Custom']
307+
end
308+
237309
end
238310
end
239311

‎test/mocked/excon_mocked_test.rb‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,16 @@ def test_xtrace_pipelined_tracing_blacklisted
151151
assert_not_requested :put, "http://127.9.0.8:8101/", headers: {'X-Trace'=>/^.*$/}
152152
end
153153

154+
# ========== excon make sure headers are preserved =============================
155+
def test_preserves_custom_headers
156+
stub_request(:get, "http://127.0.0.6:8101/").to_return(status: 200, body: "", headers: {})
157+
158+
AppOpticsAPM::API.start_trace('excon_tests') do
159+
Excon.get('http://127.0.0.6:8101', headers: { 'Custom' => 'specialvalue' })
160+
end
161+
162+
assert_requested :get, "http://127.0.0.6:8101/", headers: {'Custom'=>'specialvalue'}, times: 1
163+
end
154164
end
155165
end
156166

‎test/mocked/http_mocked_test.rb‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,5 +108,22 @@ def test_not_sampling_blacklisted
108108
end
109109
end
110110
end
111+
112+
# ========== make sure headers are preserved =============================
113+
def test_preserves_custom_headers
114+
Net::HTTP.any_instance.expects(:request_without_appoptics).with do |req, _|
115+
assert req.to_hash['custom'], "Custom header missing"
116+
assert_match /specialvalue/, req.to_hash['custom'].first
117+
end.returns(MockResponse.new)
118+
119+
AppOpticsAPM::API.start_trace('Net::HTTP_tests') do
120+
uri = URI('http://127.0.0.1:8101/?q=1')
121+
Net::HTTP.start(uri.host, uri.port) do |http|
122+
request = Net::HTTP::Get.new(uri)
123+
request['Custom'] = 'specialvalue'
124+
http.request(request) # Net::HTTPResponse object
125+
end
126+
end
127+
end
111128
end
112129
end

‎test/mocked/httpclient_mocked_test.rb‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -213,5 +213,33 @@ def test_async_not_sampling_blacklisted
213213
end
214214
end
215215
end
216+
217+
# ========== make sure headers are preserved =============================
218+
def test_preserves_custom_headers
219+
stub_request(:get, "http://127.0.0.6:8101/").to_return(status: 200, body: "", headers: {})
220+
221+
AppOpticsAPM::API.start_trace('httpclient_tests') do
222+
clnt = HTTPClient.new
223+
clnt.get('http://127.0.0.6:8101/', nil, [['Custom', 'specialvalue'], ['some_header2', 'some_value2']])
224+
end
225+
226+
assert_requested :get, "http://127.0.0.6:8101/", headers: {'Custom'=>'specialvalue'}, times: 1
227+
end
228+
229+
def test_async_preserves_custom_headers
230+
WebMock.disable!
231+
232+
Thread.expects(:new).yields # continue without forking off a thread
233+
234+
HTTPClient.any_instance.expects(:do_get_stream_without_appoptics).with do |req, _, _|
235+
assert req.headers['Custom'], "Custom header missing"
236+
assert_match(/^specialvalue$/, req.headers['Custom'] )
237+
end
238+
239+
AppOpticsAPM::API.start_trace('httpclient_tests') do
240+
clnt = HTTPClient.new
241+
clnt.get_async('http://127.0.0.6:8101/', nil, [['Custom', 'specialvalue'], ['some_header2', 'some_value2']])
242+
end
243+
end
216244
end
217245
end

‎test/mocked/rest_client_mocked_test.rb‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,5 +89,15 @@ def test_not_sampling_blacklisted
8989
assert_not_requested :get, "http://127.0.0.5:8101/", headers: {'X-Trace'=>/^.*$/}
9090
end
9191

92+
def test_preserves_custom_headers
93+
stub_request(:get, "http://127.0.0.6:8101/").to_return(status: 200, body: "", headers: {})
94+
95+
AppOpticsAPM::API.start_trace('rest_client_tests') do
96+
RestClient::Resource.new('http://127.0.0.6:8101', headers: { 'Custom' => 'specialvalue' }).get
97+
end
98+
99+
assert_requested :get, "http://127.0.0.6:8101/", headers: {'Custom'=>'specialvalue'}, times: 1
100+
end
101+
92102
end
93103
end

‎test/mocked/typhoeus_mocked_test.rb‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ def test_not_sampling_blacklisted
7878
end
7979
end
8080

81+
def test_preserves_custom_headers
82+
AppOpticsAPM::API.start_trace('typhoeus_tests') do
83+
request = Typhoeus::Request.new('http://127.0.0.6:8101', headers: { 'Custom' => 'specialvalue' }, :method => :get)
84+
request.run
85+
86+
assert request.options[:headers]['Custom']
87+
assert_match /specialvalue/, request.options[:headers]['Custom']
88+
end
89+
end
90+
8191

8292
############# Typhoeus::Hydra ##############################################
8393

@@ -165,5 +175,18 @@ def test_hydra_not_sampling_blacklisted
165175
end
166176
end
167177

178+
179+
def test_hydra_preserves_custom_headers
180+
AppOpticsAPM::API.start_trace('typhoeus_tests') do
181+
hydra = Typhoeus::Hydra.hydra
182+
request = Typhoeus::Request.new('http://127.0.0.6:8101', headers: { 'Custom' => 'specialvalue' }, :method => :get)
183+
hydra.queue(request)
184+
hydra.run
185+
186+
assert request.options[:headers]['Custom']
187+
assert_match /specialvalue/, request.options[:headers]['Custom']
188+
end
189+
end
190+
168191
end
169192
end

0 commit comments

Comments
 (0)