fix: Add http.url to Excon instrumentation (#1020)

* Sort attributes alphabetically

* Verify port equivalence

* Add `http.url` to Excon instrumentation

---------

Co-authored-by: Ariel Valentin <[email protected]>

Author: tt

instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb

@@ -27,10 +27,11 @@ def request_call(datum)
             http_method = HTTP_METHODS_TO_UPPERCASE[datum[:method]]
 
             attributes = {
+              OpenTelemetry::SemanticConventions::Trace::HTTP_HOST => datum[:host],
               OpenTelemetry::SemanticConventions::Trace::HTTP_METHOD => http_method,
               OpenTelemetry::SemanticConventions::Trace::HTTP_SCHEME => datum[:scheme],
               OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET => datum[:path],
-              OpenTelemetry::SemanticConventions::Trace::HTTP_HOST => datum[:host],
+              OpenTelemetry::SemanticConventions::Trace::HTTP_URL => OpenTelemetry::Common::Utilities.cleanse_url(::Excon::Utils.request_uri(datum)),
               OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => datum[:hostname],
               OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => datum[:port]
             }

instrumentation/excon/test/opentelemetry/instrumentation/excon/instrumentation_test.rb

@@ -48,11 +48,12 @@
 
       _(exporter.finished_spans.size).must_equal 1
       _(span.name).must_equal 'HTTP GET'
+      _(span.attributes['http.host']).must_equal 'example.com'
       _(span.attributes['http.method']).must_equal 'GET'
-      _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.scheme']).must_equal 'http'
-      _(span.attributes['http.host']).must_equal 'example.com'
+      _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
+      _(span.attributes['http.url']).must_equal 'http://example.com/success'
       assert_requested(
         :get,
         'http://example.com/success',
@@ -71,11 +72,12 @@
 
       _(exporter.finished_spans.size).must_equal 1
       _(span.name).must_equal 'HTTP GET'
+      _(span.attributes['http.host']).must_equal 'example.com'
       _(span.attributes['http.method']).must_equal 'GET'
-      _(span.attributes['http.status_code']).must_equal 500
       _(span.attributes['http.scheme']).must_equal 'http'
-      _(span.attributes['http.host']).must_equal 'example.com'
+      _(span.attributes['http.status_code']).must_equal 500
       _(span.attributes['http.target']).must_equal '/failure'
+      _(span.attributes['http.url']).must_equal 'http://example.com/failure'
       assert_requested(
         :get,
         'http://example.com/failure',
@@ -90,10 +92,11 @@
 
       _(exporter.finished_spans.size).must_equal 1
       _(span.name).must_equal 'HTTP GET'
+      _(span.attributes['http.host']).must_equal 'example.com'
       _(span.attributes['http.method']).must_equal 'GET'
       _(span.attributes['http.scheme']).must_equal 'http'
-      _(span.attributes['http.host']).must_equal 'example.com'
       _(span.attributes['http.target']).must_equal '/timeout'
+      _(span.attributes['http.url']).must_equal 'http://example.com/timeout'
       _(span.status.code).must_equal(
         OpenTelemetry::Trace::Status::ERROR
       )
@@ -119,11 +122,12 @@
 
       _(exporter.finished_spans.size).must_equal 1
       _(span.name).must_equal 'HTTP GET'
+      _(span.attributes['http.host']).must_equal 'example.com'
       _(span.attributes['http.method']).must_equal 'OVERRIDE'
-      _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.scheme']).must_equal 'http'
-      _(span.attributes['http.host']).must_equal 'example.com'
+      _(span.attributes['http.status_code']).must_equal 200
       _(span.attributes['http.target']).must_equal '/success'
+      _(span.attributes['http.url']).must_equal 'http://example.com/success'
       _(span.attributes['test.attribute']).must_equal 'test.value'
       assert_requested(
         :get,
@@ -186,8 +190,8 @@
 
       _(exporter.finished_spans.size).must_equal 1
       _(span.name).must_equal 'HTTP GET'
-      _(span.attributes['http.method']).must_equal 'GET'
       _(span.attributes['http.host']).must_equal 'example.com'
+      _(span.attributes['http.method']).must_equal 'GET'
     end
 
     it 'creates a span on connect for a non-ignored request' do
@@ -215,6 +219,8 @@
     end
 
     it 'emits span on connect' do
+      port = nil
+
       TCPServer.open('localhost', 0) do |server|
         Thread.start do
           server.accept
@@ -231,8 +237,9 @@
       _(span.name).must_equal 'connect'
       _(span.attributes['net.peer.name']).must_equal('localhost')
       _(span.attributes['net.peer.port']).wont_be_nil
+      _(span.attributes['net.peer.port']).must_equal(port)
 
-      assert_http_spans(target: '/example', exception: 'Excon::Error::Timeout')
+      assert_http_spans(port: port, target: '/example', exception: 'Excon::Error::Timeout')
     end
 
     it 'captures errors' do
@@ -248,7 +255,7 @@
       # Depending on the Ruby and Excon Version this will be a SocketError, Socket::ResolutionError or Resolv::ResolvError
       _(span_event.attributes['exception.type']).must_match(/(Socket|Resolv)/)
 
-      assert_http_spans(host: 'invalid.com', target: '/example')
+      assert_http_spans(host: 'invalid.com', port: 99_999, target: '/example')
     end
 
     it '[BUG] fails to emit an HTTP CONNECT span when connecting through an SSL proxy for an HTTP service' do
@@ -296,13 +303,14 @@
     end
   end
 
-  def assert_http_spans(scheme: 'http', host: 'localhost', target: '/', exception: nil)
+  def assert_http_spans(scheme: 'http', host: 'localhost', port: nil, target: '/', exception: nil)
     exporter.finished_spans[1..].each do |http_span|
       _(http_span.name).must_equal 'HTTP GET'
+      _(http_span.attributes['http.host']).must_equal host
       _(http_span.attributes['http.method']).must_equal 'GET'
       _(http_span.attributes['http.scheme']).must_equal scheme
-      _(http_span.attributes['http.host']).must_equal host
       _(http_span.attributes['http.target']).must_equal target
+      _(http_span.attributes['http.url']).must_equal "#{scheme}://#{host}#{port&.to_s&.prepend(':')}#{target}"
       _(http_span.status.code).must_equal(
         OpenTelemetry::Trace::Status::ERROR
       )