diff --git a/instrumentation/excon/lib/opentelemetry/instrumentation/concerns/untraced_hosts.rb b/instrumentation/excon/lib/opentelemetry/instrumentation/concerns/untraced_hosts.rb new file mode 100644 index 000000000..81acdd6a3 --- /dev/null +++ b/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 diff --git a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/instrumentation.rb b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/instrumentation.rb index 1a8a4d334..e20ca9b2d 100644 --- a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/instrumentation.rb +++ b/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 diff --git a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb index 856a52114..ca0b50d7f 100644 --- a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware.rb +++ b/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 diff --git a/instrumentation/excon/lib/opentelemetry/instrumentation/excon/patches/socket.rb b/instrumentation/excon/lib/opentelemetry/instrumentation/excon/patches/socket.rb new file mode 100644 index 000000000..b96ed8833 --- /dev/null +++ b/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 diff --git a/instrumentation/excon/test/opentelemetry/instrumentation/excon/instrumentation_test.rb b/instrumentation/excon/test/opentelemetry/instrumentation/excon/instrumentation_test.rb index 2e1ba7bea..fa166e1e6 100644 --- a/instrumentation/excon/test/opentelemetry/instrumentation/excon/instrumentation_test.rb +++ b/instrumentation/excon/test/opentelemetry/instrumentation/excon/instrumentation_test.rb @@ -8,6 +8,7 @@ require_relative '../../../../lib/opentelemetry/instrumentation/excon' require_relative '../../../../lib/opentelemetry/instrumentation/excon/middlewares/tracer_middleware' +require_relative '../../../../lib/opentelemetry/instrumentation/excon/patches/socket' describe OpenTelemetry::Instrumentation::Excon::Instrumentation do let(:instrumentation) { OpenTelemetry::Instrumentation::Excon::Instrumentation.instance } @@ -39,7 +40,7 @@ end it 'before request' do - _(exporter.finished_spans.size).must_equal 0 + _(exporter.finished_spans).must_be_empty end it 'after request with success code' do @@ -96,14 +97,16 @@ _(span.status.code).must_equal( OpenTelemetry::Trace::Status::ERROR ) - _(span.status.description).must_equal( - 'Request has failed: Excon::Error::Timeout' - ) + _(span.status.description).must_equal('Request has failed') assert_requested( :get, 'http://example.com/timeout', headers: { 'Traceparent' => "00-#{span.hex_trace_id}-#{span.hex_span_id}-01" } ) + + exception_event = span.events.first + _(exception_event.attributes['exception.type']).must_equal('Excon::Error::Timeout') + _(exception_event.attributes['exception.message']).must_equal('Excon::Error::Timeout') end it 'merges HTTP client context' do @@ -150,4 +153,164 @@ _(span.attributes['peer.service']).must_equal 'example:custom' end end + + describe 'untraced?' do + before do + instrumentation.install(untraced_hosts: ['foobar.com', /bazqux\.com/]) + + stub_request(:get, 'http://example.com/body').to_return(status: 200) + stub_request(:get, 'http://foobar.com/body').to_return(status: 200) + stub_request(:get, 'http://bazqux.com/body').to_return(status: 200) + end + + it 'does not create a span when request ignored using a string' do + Excon.get('http://foobar.com/body') + _(exporter.finished_spans).must_be_empty + end + + it 'does not create a span when request ignored using a regexp' do + Excon.get('http://bazqux.com/body') + _(exporter.finished_spans).must_be_empty + end + + it 'does not create a span on connect when request ignored using a regexp' do + uri = URI.parse('http://bazqux.com') + + Excon::Socket.new(hostname: uri.host, port: uri.port) + + _(exporter.finished_spans).must_be_empty + end + + it 'creates a span for a non-ignored request' do + Excon.get('http://example.com/body') + + _(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' + end + + it 'creates a span on connect for a non-ignored request' do + uri = URI.parse('http://example.com') + + Excon::Socket.new(hostname: uri.host, port: uri.port) + + _(exporter.finished_spans.size).must_equal 1 + _(span.name).must_equal('connect') + _(span.kind).must_equal(:internal) + _(span.attributes['net.peer.name']).must_equal('example.com') + _(span.attributes['net.peer.port']).must_equal(80) + end + end + + # NOTE: WebMock introduces an extra HTTP request span due to the way the mocking is implemented. + describe '#connect' do + before do + instrumentation.install + WebMock.allow_net_connect! + end + + after do + WebMock.disable_net_connect! + end + + it 'emits span on connect' do + TCPServer.open('localhost', 0) do |server| + Thread.start do + server.accept + rescue IOError + nil + end + + port = server.addr[1] + + _(-> { Excon.get("http://localhost:#{port}/example", read_timeout: 0) }).must_raise(Excon::Error::Timeout) + end + + _(exporter.finished_spans.size).must_equal(3) + _(span.name).must_equal 'connect' + _(span.attributes['net.peer.name']).must_equal('localhost') + _(span.attributes['net.peer.port']).wont_be_nil + + assert_http_spans(target: '/example', exception: 'Excon::Error::Timeout') + end + + it 'captures errors' do + _(-> { Excon.get('http://invalid.com:99999/example') }).must_raise + + _(exporter.finished_spans.size).must_equal(3) + _(span.name).must_equal 'connect' + _(span.attributes['net.peer.name']).must_equal('invalid.com') + _(span.attributes['net.peer.port']).must_equal(99_999) + + span_event = span.events.first + + _(span_event.name).must_equal 'exception' + _(span_event.attributes['exception.type']).must_equal(SocketError.name) + + assert_http_spans(host: 'invalid.com', target: '/example') + end + + it '[BUG] fails to emit an HTTP CONNECT span when connecting through an SSL proxy for an HTTP service' do + _(-> { Excon.get('http://localhost/', proxy: 'https://proxy_user:proxy_pass@localhost') }).must_raise(Excon::Error::Socket) + + _(exporter.finished_spans.size).must_equal(3) + _(span.name).must_equal 'connect' + _(span.kind).must_equal(:internal) + _(span.attributes['net.peer.name']).must_equal('localhost') + _(span.attributes['net.peer.port']).must_equal(443) + + assert_http_spans + end + + it 'emits an HTTP CONNECT span when connecting through an SSL proxy' do + _(-> { Excon.get('https://localhost/', proxy: 'https://proxy_user:proxy_pass@localhost') }).must_raise(Excon::Error::Socket) + + _(exporter.finished_spans.size).must_equal(3) + _(span.name).must_equal 'HTTP CONNECT' + _(span.kind).must_equal(:client) + _(span.attributes['net.peer.name']).must_equal('localhost') + _(span.attributes['net.peer.port']).must_equal(443) + + assert_http_spans(scheme: 'https') + end + + it 'emits a "connect" span when connecting through an non-ssl proxy' do + _(-> { Excon.get('http://localhost', proxy: 'https://proxy_user:proxy_pass@localhost') }).must_raise(Excon::Error::Socket) + + _(exporter.finished_spans.size).must_equal(3) + _(span.name).must_equal 'connect' + _(span.kind).must_equal(:internal) + _(span.attributes['net.peer.name']).must_equal('localhost') + _(span.attributes['net.peer.port']).must_equal(443) + + assert_http_spans(exception: 'Excon::Error::Socket') + end + + it 'emits no spans when untraced' do + OpenTelemetry::Common::Utilities.untraced do + _(-> { Excon.get('http://localhost', proxy: 'https://proxy_user:proxy_pass@localhost') }).must_raise(Excon::Error::Socket) + + _(exporter.finished_spans.size).must_equal(0) + end + end + end + + def assert_http_spans(scheme: 'http', host: 'localhost', target: '/', exception: nil) + exporter.finished_spans[1..].each do |http_span| + _(http_span.name).must_equal 'HTTP GET' + _(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.status.code).must_equal( + OpenTelemetry::Trace::Status::ERROR + ) + + if exception + exception_event = http_span.events.first + _(exception_event.attributes['exception.type']).must_equal(exception) + end + end + end end