Skip to content

refactor: unify rack middleware_args #1660

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

refactor: unify rack middleware_args #1660

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11584fd
refactor: unify rack middleware_args
sergiofbsilva Sep 5, 2025
dabdb87
Merge branch 'main' into fix-rack-middleware-args
sergiofbsilva Sep 15, 2025
2797f5f
use only middleware_args in sinatra tests
sergiofbsilva Sep 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/railtie.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,9 @@ module ActionPack
class Railtie < ::Rails::Railtie
config.before_initialize do |app|
OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.install({})

stability_opt_in = ENV.fetch('OTEL_SEMCONV_STABILITY_OPT_IN', '')
values = stability_opt_in.split(',').map(&:strip)

rack_middleware_args = if values.include?('http/dup')
OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args_dup
elsif values.include?('http')
OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args_stable
else
OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args
end

app.middleware.insert_before(
0,
*rack_middleware_args
*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args
)
end
end
Expand Down
9 changes: 6 additions & 3 deletions instrumentation/rack/lib/opentelemetry/instrumentation/rack/instrumentation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
# This option is only valid for applications using Rack 2.0 or greater
option :use_rack_events, default: true, validate: :boolean

def middleware_args
patch_type = determine_semconv
send(:"middleware_args_#{patch_type}")
end

# Temporary Helper for Sinatra and ActionPack middleware to use during installation
#
# @example Default usage
Expand All @@ -40,16 +45,14 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
# run lambda { |_arg| [200, { 'Content-Type' => 'text/plain' }, body] }
# end
# @return [Array] consisting of a middleware and arguments used in rack builders
def middleware_args
def middleware_args_old
if config.fetch(:use_rack_events, false) == true && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Old::EventHandler)
[::Rack::Events, [OpenTelemetry::Instrumentation::Rack::Middlewares::Old::EventHandler.new]]
else
[OpenTelemetry::Instrumentation::Rack::Middlewares::Old::TracerMiddleware]
end
end

alias middleware_args_old middleware_args

def middleware_args_dup
if config.fetch(:use_rack_events, false) == true && defined?(OpenTelemetry::Instrumentation::Rack::Middlewares::Dup::EventHandler)
[::Rack::Events, [OpenTelemetry::Instrumentation::Rack::Middlewares::Dup::EventHandler.new]]
Expand Down
14 changes: 1 addition & 13 deletions instrumentation/sinatra/lib/opentelemetry/instrumentation/sinatra/instrumentation.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,7 @@ class Instrumentation < OpenTelemetry::Instrumentation::Base
end

def install_middleware(app)
if config[:install_rack]
stability_opt_in = ENV.fetch('OTEL_SEMCONV_STABILITY_OPT_IN', '')
values = stability_opt_in.split(',').map(&:strip)

if values.include?('http/dup')
app.use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args_dup)
elsif values.include?('http')
app.use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args_stable)
else
app.use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args)
end
end

app.use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args) if config[:install_rack]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks so great. Thank you for working on this.

Two Sinatra tests are using the direct stable/dup methods. It would be great to update those tests to better match the changes you've made by replacing middleware_args_stable and middleware_args_dup with middleware_args.

The files are:

  • sinatra/test/opentelemetry/instrumentation/sinatra_dup_http_test
  • sinatra/test/opentelemetry/instrumentation/sinatra_stable_http_test

app.use(Middlewares::TracerMiddleware)
end
end
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/sinatra/test/opentelemetry/instrumentation/sinatra_dup_http_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ class CustomError < StandardError; end
let(:app) do
apps_to_build = apps
Rack::Builder.new do
use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args_dup)
use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args)

apps_to_build.each do |root, app|
map root do
Expand Down
2 changes: 1 addition & 1 deletion instrumentation/sinatra/test/opentelemetry/instrumentation/sinatra_stable_http_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class CustomError < StandardError; end
let(:app) do
apps_to_build = apps
Rack::Builder.new do
use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args_stable)
use(*OpenTelemetry::Instrumentation::Rack::Instrumentation.instance.middleware_args)

apps_to_build.each do |root, app|
map root do
Expand Down
Loading