feat!: Add a connect span to excon (#712)

* Add a connect span to excon and add more span attributes to the tracer middleware

* Skip matching on the error message as it varies by platform

* Rescue the IOError that can occur on accept

* Switch to must_be_empty assertion instead of size

* Move allowing and disallowing connect to setup and after respectively.

* use dig when getting hostname and port of proxy

* Call untraced when we hit an untraced host.

* Switch to using recording? to test whether we should finish a span.

* Switch to handle_error instead of debug logging.

* Record the exception on error

* Perform the next step in the middleware stack in the context of the current span.

* Add assertions on http spans to connect tests.

* Fix rubocop lints

* Remove interpolation from status message on span now that we capture exceptions as an event.

* Switch to attach and detach instead of with_span

* Include untraced context into untraced? check for the middleware and patch.

* Add test for untraced.

* Add a module that centralizes the untraced hosts concern.

* Expand doc comment.

* Move module to the excon gem.

* Add doc comment for untraced? in the concern

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

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

---------

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

instrumentation/excon/lib/opentelemetry/instrumentation/concerns/untraced_hosts.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module OpenTelemetry
+  module Instrumentation
+    module Concerns
+      # The untraced hosts concerns allows instrumentation to skip traces on hostnames in an exclusion list.
+      # If the current OpenTelemetry context is untraced, all hosts will be treated as untraced.
+      # When included in a class that extends OpenTelemetry::Instrumentation::Base, this module defines an option named :untraced_hosts.
+      module UntracedHosts
+        def self.included(klass)
+          klass.instance_eval do
+            # untraced_hosts: if a request's address matches any of the `String`
+            #   or `Regexp` in this array, the instrumentation will not record a
+            #   `kind = :client` representing the request and will not propagate
+            #   context in the request.
+            option :untraced_hosts, default: [], validate: :array
+          end
+        end
+
+        # Checks whether the given host should be treated as untraced.
+        # If the current OpenTelemetry context is untraced, all hosts will be treated as untraced.
+        # The given host must be a String.
+        def untraced?(host)
+          OpenTelemetry::Common::Utilities.untraced? || untraced_host?(host)
+        end
+
+        private
+
+        def untraced_host?(host)
+          config[:untraced_hosts].any? do |rule|
+            rule.is_a?(Regexp) ? rule.match?(host) : rule == host
+          end
+        end
+      end
+    end
+  end
+end

instrumentation/excon/lib/opentelemetry/instrumentation/excon/instrumentation.rb

@@ -4,15 +4,20 @@
 #
 # SPDX-License-Identifier: Apache-2.0
 
+require_relative '../concerns/untraced_hosts'
+
 module OpenTelemetry
   module Instrumentation
     module Excon
       # The Instrumentation class contains logic to detect and install the Excon
       # instrumentation
       class Instrumentation < OpenTelemetry::Instrumentation::Base
+        include OpenTelemetry::Instrumentation::Concerns::UntracedHosts
+
         install do |_config|
           require_dependencies
           add_middleware
+          patch
         end
 
         present do
@@ -25,11 +30,15 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
 
         def require_dependencies
           require_relative 'middlewares/tracer_middleware'
+          require_relative 'patches/socket'
         end
 
         def add_middleware
-          ::Excon.defaults[:middlewares] =
-            Middlewares::TracerMiddleware.around_default_stack
+          ::Excon.defaults[:middlewares] = Middlewares::TracerMiddleware.around_default_stack
+        end
+
+        def patch
+          ::Excon::Socket.prepend(Patches::Socket)
         end
       end
     end

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

@@ -22,24 +22,30 @@ class TracerMiddleware < ::Excon::Middleware::Base
           end.freeze
 
           def request_call(datum)
-            begin
-              unless datum.key?(:otel_span)
-                http_method = HTTP_METHODS_TO_UPPERCASE[datum[:method]]
-                attributes = span_creation_attributes(datum, http_method)
-                tracer.start_span(
-                  HTTP_METHODS_TO_SPAN_NAMES[http_method],
-                  attributes: attributes,
-                  kind: :client
-                ).tap do |span|
-                  datum[:otel_span] = span
-                  OpenTelemetry::Trace.with_span(span) do
-                    OpenTelemetry.propagation.inject(datum[:headers])
-                  end
-                end
-              end
-            rescue StandardError => e
-              OpenTelemetry.logger.debug(e.message)
-            end
+            return @stack.request_call(datum) if untraced?(datum)
+
+            http_method = HTTP_METHODS_TO_UPPERCASE[datum[:method]]
+
+            attributes = {
+              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::NET_PEER_NAME => datum[:hostname],
+              OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => datum[:port]
+            }
+
+            peer_service = Excon::Instrumentation.instance.config[:peer_service]
+            attributes[OpenTelemetry::SemanticConventions::Trace::PEER_SERVICE] = peer_service if peer_service
+            attributes.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes)
+
+            span = tracer.start_span(HTTP_METHODS_TO_SPAN_NAMES[http_method], attributes: attributes, kind: :client)
+            ctx = OpenTelemetry::Trace.context_with_span(span)
+
+            datum[:otel_span] = span
+            datum[:otel_token] = OpenTelemetry::Context.attach(ctx)
+
+            OpenTelemetry.propagation.inject(datum[:headers])
 
             @stack.request_call(datum)
           end
@@ -71,43 +77,35 @@ def self.around_default_stack
           private
 
           def handle_response(datum)
-            if datum.key?(:otel_span)
-              datum[:otel_span].tap do |span|
-                return span if span.end_timestamp
+            datum.delete(:otel_span)&.tap do |span|
+              return unless span.recording?
 
-                if datum.key?(:response)
-                  response = datum[:response]
-                  span.set_attribute('http.status_code', response[:status])
-                  span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(response[:status].to_i)
-                end
-
-                span.status = OpenTelemetry::Trace::Status.error("Request has failed: #{datum[:error]}") if datum.key?(:error)
+              if datum.key?(:response)
+                response = datum[:response]
+                span.set_attribute(OpenTelemetry::SemanticConventions::Trace::HTTP_STATUS_CODE, response[:status])
+                span.status = OpenTelemetry::Trace::Status.error unless (100..399).cover?(response[:status].to_i)
+              end
 
-                span.finish
-                datum.delete(:otel_span)
+              if datum.key?(:error)
+                span.status = OpenTelemetry::Trace::Status.error('Request has failed')
+                span.record_exception(datum[:error])
               end
+
+              span.finish
+
+              OpenTelemetry::Context.detach(datum.delete(:otel_token)) if datum.include?(:otel_token)
             end
           rescue StandardError => e
-            OpenTelemetry.logger.debug(e.message)
-          end
-
-          def span_creation_attributes(datum, http_method)
-            instrumentation_attrs = {
-              'http.host' => datum[:host],
-              'http.method' => http_method,
-              'http.scheme' => datum[:scheme],
-              'http.target' => datum[:path]
-            }
-            config = Excon::Instrumentation.instance.config
-            instrumentation_attrs['peer.service'] = config[:peer_service] if config[:peer_service]
-            instrumentation_attrs.merge!(
-              OpenTelemetry::Common::HTTP::ClientContext.attributes
-            )
+            OpenTelemetry.handle_error(e)
           end
 
           def tracer
             Excon::Instrumentation.instance.tracer
           end
+
+          def untraced?(datum)
+            datum.key?(:otel_span) || Excon::Instrumentation.instance.untraced?(datum[:host])
+          end
         end
       end
     end

instrumentation/excon/lib/opentelemetry/instrumentation/excon/patches/socket.rb

@@ -0,0 +1,58 @@
+# frozen_string_literal: true
+
+# Copyright The OpenTelemetry Authors
+#
+# SPDX-License-Identifier: Apache-2.0
+
+module OpenTelemetry
+  module Instrumentation
+    module Excon
+      module Patches
+        # Module to prepend to an Excon Socket for instrumentation
+        module Socket
+          private
+
+          def connect
+            return super if untraced?
+
+            if @data[:proxy]
+              conn_address = @data.dig(:proxy, :hostname)
+              conn_port = @data.dig(:proxy, :port)
+            else
+              conn_address = @data[:hostname]
+              conn_port = @port
+            end
+
+            attributes = { OpenTelemetry::SemanticConventions::Trace::NET_PEER_NAME => conn_address, OpenTelemetry::SemanticConventions::Trace::NET_PEER_PORT => conn_port }.merge!(OpenTelemetry::Common::HTTP::ClientContext.attributes)
+
+            if is_a?(::Excon::SSLSocket) && @data[:proxy]
+              span_name = 'HTTP CONNECT'
+              span_kind = :client
+            else
+              span_name = 'connect'
+              span_kind = :internal
+            end
+
+            tracer.in_span(span_name, attributes: attributes, kind: span_kind) do
+              super
+            end
+          end
+
+          def tracer
+            Excon::Instrumentation.instance.tracer
+          end
+
+          def untraced?
+            address = if @data[:proxy]
+                        @data.dig(:proxy, :hostname)
+                      else
+                        @data[:hostname]
+                      end
+
+            Excon::Instrumentation.instance.untraced?(address)
+          end
+        end
+      end
+    end
+  end
+end