From 0a8151a7db253a543902c55a75fab37ba5ef1ec0 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Sun, 24 Nov 2024 14:01:33 -0600 Subject: [PATCH 1/2] fix: Share Faraday Attrs with Adapter Spans The Faraday gem has additional http attributes that are sometimes missing or difficult to derive in Adapter instrumentations. --- instrumentation/faraday/Appraisals | 14 +++++----- .../faraday/middlewares/tracer_middleware.rb | 27 ++++++++++--------- .../middlewares/tracer_middleware_test.rb | 19 ++++++++++--- 3 files changed, 36 insertions(+), 24 deletions(-) diff --git a/instrumentation/faraday/Appraisals b/instrumentation/faraday/Appraisals index 6d5e0f26e..0c19136a4 100644 --- a/instrumentation/faraday/Appraisals +++ b/instrumentation/faraday/Appraisals @@ -4,14 +4,12 @@ # # SPDX-License-Identifier: Apache-2.0 -appraise 'faraday-0.17' do - gem 'faraday', '~> 0.17.6' +%w[0.17.6 1.0 2.0].each do |version| + appraise "faraday-#{version}" do + gem 'faraday', "~> #{version}" + end end -appraise 'faraday-1.x' do - gem 'faraday', '~> 1.0' -end - -appraise 'faraday-2.x' do - gem 'faraday', '~> 2.0' +appraise 'faraday-latest' do + gem 'faraday' end diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb index 981e44e26..26904ee8d 100644 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware.rb @@ -33,31 +33,34 @@ def call(env) attributes = span_creation_attributes( http_method: http_method, url: env.url, config: config ) - tracer.in_span( - "HTTP #{http_method}", attributes: attributes, kind: config.fetch(:span_kind) - ) do |span| - OpenTelemetry.propagation.inject(env.request_headers) - app.call(env).on_complete { |resp| trace_response(span, resp.status) } - rescue ::Faraday::Error => e - trace_response(span, e.response[:status]) if e.response + OpenTelemetry::Common::HTTP::ClientContext.with_attributes(attributes) do |attrs, _| + tracer.in_span( + "HTTP #{http_method}", attributes: attrs, kind: config.fetch(:span_kind) + ) do |span| + OpenTelemetry.propagation.inject(env.request_headers) - raise + app.call(env).on_complete { |resp| trace_response(span, resp.status) } + rescue ::Faraday::Error => e + trace_response(span, e.response[:status]) if e.response + + raise + end end end private def span_creation_attributes(http_method:, url:, config:) - instrumentation_attrs = { + attrs = { 'http.method' => http_method, 'http.url' => OpenTelemetry::Common::Utilities.cleanse_url(url.to_s), 'faraday.adapter.name' => app.class.name } - instrumentation_attrs['net.peer.name'] = url.host if url.host - instrumentation_attrs['peer.service'] = config[:peer_service] if config[:peer_service] + attrs['net.peer.name'] = url.host if url.host + attrs['peer.service'] = config[:peer_service] if config[:peer_service] - instrumentation_attrs.merge!( + attrs.merge!( OpenTelemetry::Common::HTTP::ClientContext.attributes ) end diff --git a/instrumentation/faraday/test/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware_test.rb b/instrumentation/faraday/test/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware_test.rb index 8a0def73c..67aab661f 100644 --- a/instrumentation/faraday/test/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware_test.rb +++ b/instrumentation/faraday/test/opentelemetry/instrumentation/faraday/middlewares/tracer_middleware_test.rb @@ -20,6 +20,7 @@ stub.get('/success') { |_| [200, {}, 'OK'] } stub.get('/failure') { |_| [500, {}, 'OK'] } stub.get('/not_found') { |_| [404, {}, 'OK'] } + stub.get('/show-shared-attributes') { |_| [200, {}, OpenTelemetry::Common::HTTP::ClientContext.attributes.to_json] } end end end @@ -31,6 +32,8 @@ @orig_propagation = OpenTelemetry.propagation propagator = OpenTelemetry::Trace::Propagation::TraceContext.text_map_propagator OpenTelemetry.propagation = propagator + instrumentation.instance_variable_set(:@installed, false) + instrumentation.install end after do @@ -38,10 +41,6 @@ end describe 'first span' do - before do - instrumentation.install - end - describe 'given a client with a base url' do it 'has http 200 attributes' do response = client.get('/success') @@ -101,6 +100,18 @@ ) end + it 'ammends attributes to client context' do + response = client.get('/show-shared-attributes') + shared_attributes = JSON.parse(response.body) + expected_attributes = { + 'http.method' => 'GET', 'http.url' => 'http://example.com/show-shared-attributes', + 'faraday.adapter.name' => 'Faraday::Adapter::Test', + 'net.peer.name' => 'example.com' + } + + _(shared_attributes).must_equal expected_attributes + end + it 'accepts peer service name from config' do instrumentation.instance_variable_set(:@installed, false) instrumentation.install(peer_service: 'example:faraday') From 2d56edf33d774c57a26b92bcb3b9897bdfed0b34 Mon Sep 17 00:00:00 2001 From: Ariel Valentin Date: Sun, 24 Nov 2024 14:19:12 -0600 Subject: [PATCH 2/2] feat: Faraday Minimum v1.0 Faraday pre-1.0 is no longer receiving updates. --- instrumentation/faraday/Appraisals | 2 +- .../faraday/instrumentation.rb | 17 ++++++++----- .../faraday/patches/connection.rb | 2 +- .../faraday/patches/rack_builder.rb | 25 ------------------- 4 files changed, 13 insertions(+), 33 deletions(-) delete mode 100644 instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/rack_builder.rb diff --git a/instrumentation/faraday/Appraisals b/instrumentation/faraday/Appraisals index 0c19136a4..4ed32e342 100644 --- a/instrumentation/faraday/Appraisals +++ b/instrumentation/faraday/Appraisals @@ -4,7 +4,7 @@ # # SPDX-License-Identifier: Apache-2.0 -%w[0.17.6 1.0 2.0].each do |version| +%w[1.0 2.0].each do |version| appraise "faraday-#{version}" do gem 'faraday', "~> #{version}" end diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb index 17d4826e8..a194d709e 100644 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb @@ -10,12 +10,18 @@ module Faraday # The Instrumentation class contains logic to detect and install the Faraday # instrumentation class Instrumentation < OpenTelemetry::Instrumentation::Base + MINIMUM_VERSION = Gem::Version.new('1.0') + install do |_config| require_dependencies register_tracer_middleware use_middleware_by_default end + compatible do + gem_version >= MINIMUM_VERSION + end + present do defined?(::Faraday) end @@ -25,10 +31,13 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base private + def gem_version + Gem::Version.new(::Faraday::VERSION) + end + def require_dependencies require_relative 'middlewares/tracer_middleware' require_relative 'patches/connection' - require_relative 'patches/rack_builder' end def register_tracer_middleware @@ -38,11 +47,7 @@ def register_tracer_middleware end def use_middleware_by_default - if Gem::Version.new(::Faraday::VERSION) >= Gem::Version.new('1') - ::Faraday::Connection.prepend(Patches::Connection) - else - ::Faraday::RackBuilder.prepend(Patches::RackBuilder) - end + ::Faraday::Connection.prepend(Patches::Connection) end end end diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/connection.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/connection.rb index 51d8ad0b5..1819488ad 100644 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/connection.rb +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/connection.rb @@ -13,7 +13,7 @@ module Patches module Connection # Wraps Faraday::Connection#initialize: # https://github.com/lostisland/faraday/blob/ff9dc1d1219a1bbdba95a9a4cf5d135b97247ee2/lib/faraday/connection.rb#L62-L92 - def initialize(*args) + def initialize(...) super.tap do use(:open_telemetry) unless builder.handlers.any? do |handler| handler.klass == Middlewares::TracerMiddleware diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/rack_builder.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/rack_builder.rb deleted file mode 100644 index 1b32a00da..000000000 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/rack_builder.rb +++ /dev/null @@ -1,25 +0,0 @@ -# frozen_string_literal: true - -# Copyright The OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - module Instrumentation - module Faraday - module Patches - # Module to be prepended to force Faraday to use the middleware by - # default so the user doesn't have to call `use` for every connection. - module RackBuilder - def adapter(*args) - use(:open_telemetry) unless @handlers.any? do |handler| - handler.klass == Faraday::Middlewares::TracerMiddleware - end - - super - end - end - end - end - end -end