fix: Omit nil net.peer.name attributes
#1280
Conversation
Faraday uses a base URL to derive the host name which is used by the tracer middleware as the span attribute value for `net.peer.name`. Although it seems incorrect, Faraday does not require a base URL when it is initialized and is a commonly used when using Faraday test stubs. When the base URL is absent, the `net.peer.name` is set to `nil` triggering and invalid attribute error report and creates a bit of noise for test cases. This change omits the `net.peer.name` attribute when it is `nil` to suppress error reporting when the host is absent.
arielvalentin
requested review from
ahayworth,
dazuma,
ericmustin,
fbogsany,
mwear,
plantfansam and
robertlaurin
as code owners
|
Going to add additional tests where |
plantfansam
left a comment
plantfansam
left a comment
There was a problem hiding this comment.
Looks great! Very excited about the tests for when url is an IP address 😄
| 'net.peer.name' => url.host | ||
| 'http.url' => url.to_s | ||
| } | ||
| instrumentation_attrs['net.peer.name'] = url.host unless url.host.nil? |
There was a problem hiding this comment.
IDK which Rubocop prefers but I believe we can do= url.host if url.host
| _(span.attributes['http.method']).must_equal 'GET' | ||
| _(span.attributes['http.status_code']).must_equal 200 | ||
| _(span.attributes['http.url']).must_equal 'http:/success' | ||
| _(span.attributes).wont_include('net.peer.name') |
| span_processor = OpenTelemetry::SDK::Trace::Export::SimpleSpanProcessor.new(EXPORTER) | ||
|
|
||
| OpenTelemetry::SDK.configure do |c| | ||
| c.error_handler = ->(exception:, message:) { raise(exception || message) } |
|
Turns out Ruby stdlib URI will return the IP Addr as the host value so it won't be as simple as I thought. I am going to table this PR and come back to it later. |
|
Hello, and thank you for your contribution! We recently split Ruby instrumentation out into the This PR is related to instrumentation, so we'll need you to re-open it against To do that, you can:
Sorry again for the inconvenience, and thank you for contributing! |
3 participants
Faraday uses a base URL to derive the host name which is used by the tracer middleware as the span attribute value for
net.peer.name.Although it seems incorrect, Faraday does not require a base URL when it is initialized and is a commonly used when using Faraday test stubs.
When the base URL is absent, the
net.peer.nameis set toniltriggering and invalid attribute error report and creates a bit of noise for test cases.This change omits the
net.peer.nameattribute when it isnilto suppress error reporting when the host is absent.