feat!: Remove retain_middleware_names Rack Option

This configuration option sets a rack environment variable that isn't used in the instrumentation or its dependencies.

dd-trace-rb used this option to set the span name to the Rack application Name:

https://github.com/DataDog/dd-trace-rb/blob/d76de947d66fe50f5ac6c7a6016ebbfa5c0de986/lib/datadog/tracing/contrib/rack/middlewares.rb#L158

It looks like some of the functionality was ported over but applying it to the span name was never completed:

open-telemetry/opentelemetry-ruby#166

Author: arielvalentin

instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb

@@ -12,10 +12,9 @@ module Rack
       # The Instrumentation class contains logic to detect and install the Rack
       # instrumentation
       class Instrumentation < OpenTelemetry::Instrumentation::Base
-        install do |config|
+        install do |_config|
+          # TODO: move logic that configures allow lists here
           require_dependencies
-
-          retain_middleware_names if config[:retain_middleware_names]
         end
 
         present do
@@ -26,7 +25,6 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
         option :allowed_response_headers, default: [],    validate: :array
         option :application,              default: nil,   validate: :callable
         option :record_frontend_span,     default: false, validate: :boolean
-        option :retain_middleware_names,  default: false, validate: :boolean
         option :untraced_endpoints,       default: [],    validate: :array
         option :url_quantization,         default: nil,   validate: :callable
         option :untraced_requests,        default: nil,   validate: :callable
@@ -37,30 +35,6 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
         def require_dependencies
           require_relative 'middlewares/tracer_middleware'
         end
-
-        MissingApplicationError = Class.new(StandardError)
-
-        # intercept all middleware-compatible calls, retain class name
-        def retain_middleware_names
-          next_middleware = config[:application]
-          raise MissingApplicationError unless next_middleware
-
-          while next_middleware
-            if next_middleware.respond_to?(:call)
-              next_middleware.singleton_class.class_eval do
-                alias_method :__call, :call
-
-                def call(env)
-                  env['RESPONSE_MIDDLEWARE'] = self.class.to_s
-                  __call(env)
-                end
-              end
-            end
-
-            next_middleware = next_middleware.instance_variable_defined?('@app') &&
-                              next_middleware.instance_variable_get('@app')
-          end
-        end
       end
     end
   end

instrumentation/rack/opentelemetry-instrumentation-rack.gemspec

@@ -38,6 +38,7 @@ Gem::Specification.new do |spec|
   spec.add_development_dependency 'rack', '~> 2.0.8'
   spec.add_development_dependency 'rack-test', '~> 1.1.0'
   spec.add_development_dependency 'rake', '~> 13.0'
+  spec.add_development_dependency 'rspec-mocks'
   spec.add_development_dependency 'rubocop', '~> 1.41.1'
   spec.add_development_dependency 'simplecov', '~> 0.17.1'
   spec.add_development_dependency 'webmock', '~> 3.7.6'

instrumentation/rack/test/opentelemetry/instrumentation/rack/instrumentation_test.rb

@@ -11,64 +11,45 @@
   let(:instrumentation) { instrumentation_class.instance }
   let(:config) { {} }
 
-  after do
+  before do
     # simulate a fresh install:
     instrumentation.instance_variable_set('@installed', false)
-    instrumentation.install({})
   end
 
-  describe 'config[:retain_middleware_names]' do
-    let(:config) { Hash(retain_middleware_names: true) }
-
-    describe 'without config[:application]' do
-      it 'raises error' do
-        # allow for re-installation with new config:
-        instrumentation.instance_variable_set('@installed', false)
-
-        assert_raises instrumentation_class::MissingApplicationError do
-          instrumentation.install(config)
-        end
-      end
+  describe 'given default config options' do
+    before do
+      instrumentation.install(config)
     end
 
-    describe 'default' do
-      class MyAppClass
-        attr_reader :env
-
-        def use(*); end
-
-        def call(env)
-          @env = env
-          [200, { 'Content-Type' => 'text/plain' }, ['OK']]
-        end
-      end
-
-      let(:app) { MyAppClass.new }
-
-      describe 'without config' do
-        it 'does not set RESPONSE_MIDDLEWARE' do
-          app.call({})
-
-          _(app.env['RESPONSE_MIDDLEWARE']).must_be_nil
-        end
-      end
-
-      describe 'with config[:application]' do
-        let(:config) do
-          { retain_middleware_names: true,
-            application: app }
-        end
-
-        it 'retains RESPONSE_MIDDLEWARE after .call' do
-          # allow for re-installation with new config:
-          instrumentation.instance_variable_set('@installed', false)
+    it 'is installed with default settings' do
+      _(instrumentation).must_be :installed?
+      _(instrumentation.config[:allowed_request_headers]).must_be_empty
+      _(instrumentation.config[:allowed_response_headers]).must_be_empty
+      _(instrumentation.config[:application]).must_be_nil
+      _(instrumentation.config[:record_frontend_span]).must_equal false
+      _(instrumentation.config[:untraced_endpoints]).must_be_empty
+      _(instrumentation.config[:url_quantization]).must_be_nil
+      _(instrumentation.config[:untraced_requests]).must_be_nil
+      _(instrumentation.config[:response_propagators]).must_be_empty
+    end
+  end
 
-          instrumentation.install(config)
-          app.call({})
+  describe 'when rack gem does not exist' do
+    before do
+      hide_const('Rack')
+      instrumentation.install(config)
+    end
 
-          _(app.env['RESPONSE_MIDDLEWARE']).must_equal 'MyAppClass'
-        end
-      end
+    it 'skips installation' do
+      _(instrumentation).wont_be :installed?
+      _(instrumentation.config[:allowed_request_headers]).must_be_empty
+      _(instrumentation.config[:allowed_response_headers]).must_be_empty
+      _(instrumentation.config[:application]).must_be_nil
+      _(instrumentation.config[:record_frontend_span]).must_equal false
+      _(instrumentation.config[:untraced_endpoints]).must_be_empty
+      _(instrumentation.config[:url_quantization]).must_be_nil
+      _(instrumentation.config[:untraced_requests]).must_be_nil
+      _(instrumentation.config[:response_propagators]).must_be_empty
     end
   end
 end

instrumentation/rack/test/test_helper.rb

@@ -8,6 +8,7 @@
 Bundler.require(:default, :development, :test)
 
 require 'minitest/autorun'
+require 'rspec/mocks/minitest_integration'
 require 'webmock/minitest'
 
 require 'opentelemetry-instrumentation-rack'