Skip to content

Commit

Permalink
Curb instrumentation method_with_tracing fix
Browse files Browse the repository at this point in the history
To address an issue apparently dating back to when the Curb
instrumentation was first split into separate "prepend" and "chain"
approaches, be sure to pass a verb argument to `method_with_tracing`,
which requires it.

Thank you to @knarewski for bringing this issue to our attention,
for providing a means of reproducing an error, and for providing a fix.

resolves #1033
  • Loading branch information
fallwith committed Mar 24, 2022
1 parent 8e45382 commit b5f349f
Show file tree
Hide file tree
Showing 5 changed files with 13 additions and 3 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## v8.6.0

* **Bugfix: Curb - satify method_with_tracing's verb argument requirement**

When Curb instrumentation is used (either via prepend or chain), be sure to always pass the verb argument over to `method_with_tracing` which requires it. Thank you to @knarewski for bringing this issue to our attention, for providing a means of reproducing an error, and for providing a fix. That fix has been replicated by the agent team with permission. See [Issue 1033](https://github.com/newrelic/newrelic-ruby-agent/issues/1033) for more details.

* **Improve the usage of the 'hostname' executable and other executables**

In all places where a call to an executable binary is made (currently this is done only for the 'hostname' and 'uname' binaries), leverage a new helper method when making the call. This new helper will a) not attempt to execute the binary if it cannot be found, and b) prevent STDERR/STDOUT content from appearing anywhere except New Relic's own logs if the New Relic logger is set to the 'debug' level. When calling 'hostname', fall back to `Socket.gethostname` if the 'hostname' binary cannot be found. When calling 'uname', fall back on using a value of 'unknown' if the 'uname' command fails. Many thanks to @metaskills and @brcarp for letting us know that Ruby AWS Lambda functions can't invoke 'hostname' and for providing ideas and feedback with [Issue #697](https://github.com/newrelic/newrelic-ruby-agent/issues/697).
Expand All @@ -14,6 +18,7 @@

Previously, unit tests would fail with unexpected invocation errors when `NEW_RELIC_LICENSE_KEY` and `NEW_RELIC_HOST` environment variables were present. Now, tests will discard these environment variables before running.


## v8.5.0

* **AWS: Support IMDSv2 by using a token with metadata API calls**
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/instrumentation/curb/chain.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def perform_with_newrelic

# Record the HTTP verb for future #perform calls
def method_with_newrelic verb
method_with_tracing { method_without_newrelic(verb) }
method_with_tracing(verb) { method_without_newrelic(verb) }
end

alias_method :method_without_newrelic, :method
Expand Down
2 changes: 1 addition & 1 deletion lib/new_relic/agent/instrumentation/curb/prepend.rb
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def perform
end

def method verb
method_with_tracing { super }
method_with_tracing(verb) { super }
end

def header_str
Expand Down
2 changes: 1 addition & 1 deletion test/multiverse/suites/curb/Envfile
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ end
# https://github.com/taf2/curb#version-compatibility-chart
def curb_gem_version
if curl_version.between? 7.58, Float::INFINITY
'~> 0.9.8'
'~> 1.0.0'
elsif curl_version.between? 7.56, 7.60
'0.9.7'
elsif curl_version.between? 7.51, 7.59
Expand Down
5 changes: 5 additions & 0 deletions test/multiverse/suites/curb/curb_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,11 @@ def test_block_passed_to_multi_perform_should_be_called
assert_equal "External/Multiple/Curb::Multi/perform", last_node.metric_name
end

# https://github.com/newrelic/newrelic-ruby-agent/issues/1033
def test_method_with_tracing_passes_the_verb_downstream
assert Curl::Easy.new.method(:to_s).call.is_a?(String), 'Failed to create #to_s method'
end

#
# Helper functions
#
Expand Down

0 comments on commit b5f349f

Please sign in to comment.