fix: issue with sending traces to IPv6 endpoints #1935
Conversation
chen-anders
requested review from
ahayworth,
arielvalentin,
dazuma,
ericmustin,
fbogsany,
kaylareopelle,
mwear,
plantfansam,
robbkidd and
robertlaurin
as code owners
fbogsany
changed the title
|
|
||
| def http_connection(uri, ssl_verify_mode, certificate_file, client_certificate_file, client_key_file) | ||
| http = Net::HTTP.new(uri.host, uri.port) | ||
| http = Net::HTTP.new(uri.hostname, uri.port) |
There was a problem hiding this comment.
Thank you for opening this PR, @chen-anders! I appreciate the Reviewer's note. The tests look good to me, but I think we should add them to the other libraries getting the IPv6 compatible hostname too. Could you update the tests in otlp-http, otlp-logs and otlp-metrics too?
There was a problem hiding this comment.
I went ahead and added the tests requested for the other exporters.
arielvalentin
changed the title
|
👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the |
| stub_request(:post, 'http://[::1]:4318/v1/traces').to_return(status: 200) | ||
| exp = OpenTelemetry::Exporter::OTLP::Exporter.new(endpoint: 'http://[::1]:4318/v1/traces') | ||
| span_data = OpenTelemetry::TestHelpers.create_span_data | ||
| result = exp.export([span_data]) |
There was a problem hiding this comment.
Hi @chen-anders thank you for pushing this along.
Overall looks good, but might need to consider the performance of the numerous test additions which use exp.export(...), which could lead to test pipeline performance slowdown.
Also I believe that the of actual exporting of span data is covered by other tests, so these new tests should only be concerned with parser validation such as:
http = exp.instance_variable_get(:@http)
_(http.address).must_equal '::1'
_(http.port).must_equal 4318
Do you agree @kaylareopelle ?
4 participants
Resolves: #1721
This PR resolves the issue around using the opentelemetry-ruby SDK for apps that send traces to an IPv6 endpoint using the suggested fix in the original issue. We also add some tests for verifying the expected value of the collector hostname based on a passed-in value.
Reviewer's Note: Tests were written with significant AI assistance.