From 548c7ab0bb9cb1dc737a42b0f985956f10c0e678 Mon Sep 17 00:00:00 2001 From: Daniel Colson Date: Thu, 20 Jun 2024 12:26:11 -0400 Subject: [PATCH] Improve compatibility with Faraday v1 Fixes https://github.com/open-telemetry/opentelemetry-ruby-contrib/issues/1031 Faraday >= 1 changes where the #adapter method gets called, affecting the RackBuilder patch in this gem. Without this change, the tracer middleware gets added to the beginning of the stack instead of the end, and explicitly adding the middleware would cause it to get added twice. This change patches `Connection#initialize` for Faraday >= 1 to preserve the behavior we had with Faraday < 1. This is similar to the dd-trace-rb change made in https://github.com/DataDog/dd-trace-rb/pull/906. There is some followup work here to explore moving the middleware to the beginning of the stack instead of the end, but we can work on that separately. --- .../faraday/instrumentation.rb | 7 ++++- .../faraday/patches/connection.rb | 27 +++++++++++++++++++ .../middlewares/tracer_middleware_test.rb | 13 +++++++++ 3 files changed, 46 insertions(+), 1 deletion(-) create mode 100644 instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/connection.rb diff --git a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb index abed74af45..17d4826e8d 100644 --- a/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/instrumentation.rb @@ -27,6 +27,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base def require_dependencies require_relative 'middlewares/tracer_middleware' + require_relative 'patches/connection' require_relative 'patches/rack_builder' end @@ -37,7 +38,11 @@ def register_tracer_middleware end def use_middleware_by_default - ::Faraday::RackBuilder.prepend(Patches::RackBuilder) + if Gem::Version.new(::Faraday::VERSION) >= Gem::Version.new('1') + ::Faraday::Connection.prepend(Patches::Connection) + else + ::Faraday::RackBuilder.prepend(Patches::RackBuilder) + end 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 new file mode 100644 index 0000000000..51d8ad0b53 --- /dev/null +++ b/instrumentation/faraday/lib/opentelemetry/instrumentation/faraday/patches/connection.rb @@ -0,0 +1,27 @@ +# 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 Connection + # Wraps Faraday::Connection#initialize: + # https://github.com/lostisland/faraday/blob/ff9dc1d1219a1bbdba95a9a4cf5d135b97247ee2/lib/faraday/connection.rb#L62-L92 + def initialize(*args) + super.tap do + use(:open_telemetry) unless builder.handlers.any? do |handler| + handler.klass == Middlewares::TracerMiddleware + end + end + end + end + end + end + end +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 acedc19d23..8a0def73cc 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 @@ -198,5 +198,18 @@ _(span.status.code).must_equal OpenTelemetry::Trace::Status::ERROR end end + + describe 'when explicitly adding the tracer middleware' do + let(:client) do + Faraday.new do |builder| + builder.use :open_telemetry + end + end + + it 'only adds the middleware once' do + tracers = client.builder.handlers.count(OpenTelemetry::Instrumentation::Faraday::Middlewares::TracerMiddleware) + _(tracers).must_equal 1 + end + end end end