From f0d83a240f9861ff3d86d442ab6e65582542148b Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Thu, 19 Oct 2023 12:22:36 -0400 Subject: [PATCH 01/15] feat!(action_pack): Use ActiveSupport instead of patches --- instrumentation/action_pack/README.md | 12 ++++ .../example/trace_demonstration.ru | 2 +- .../instrumentation/action_pack/handlers.rb | 38 +++++++++++ .../action_pack/handlers/action_controller.rb | 65 +++++++++++++++++++ .../action_pack/instrumentation.rb | 4 +- .../patches/action_controller/metal.rb | 40 ------------ .../action_controller_test.rb} | 59 ++++++++++++++++- .../action_pack/handlers_test.rb | 34 ++++++++++ 8 files changed, 208 insertions(+), 46 deletions(-) create mode 100644 instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb create mode 100644 instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb delete mode 100644 instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb rename instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/{patches/action_controller/metal_test.rb => handlers/action_controller_test.rb} (56%) create mode 100644 instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb diff --git a/instrumentation/action_pack/README.md b/instrumentation/action_pack/README.md index 877e0ec4b..142a99bf4 100644 --- a/instrumentation/action_pack/README.md +++ b/instrumentation/action_pack/README.md @@ -30,6 +30,18 @@ OpenTelemetry::SDK.configure do |c| end ``` +## Active Support Instrumentation + +Earlier versions of this instrumentation relied on patching custom `dispatch` hooks from rails [action_controller](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/metal.rb#L224) in order to extract request information. + +This instrumentation now relies on `ActiveSupport::Notifications` and registers a custom Subscriber that listens to relevant events to modify rack span. + +See the table below for details of what [Rails Framework Hook Events](https://guides.rubyonrails.org/active_support_instrumentation.html#action-controller) are recorded by this instrumentation: + +| Event Name | Subscribe? | Creates Span? | Notes | +| - | - | - | - | +| `process_action.action_controller` | :white_check_mark: | :x: | It modifies the existing Rack span | + ## Examples Example usage can be seen in the `./example/trace_demonstration.rb` file [here](https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/instrumentation/action_pack/example/trace_demonstration.ru) diff --git a/instrumentation/action_pack/example/trace_demonstration.ru b/instrumentation/action_pack/example/trace_demonstration.ru index 6e9f08c94..c9c65422f 100644 --- a/instrumentation/action_pack/example/trace_demonstration.ru +++ b/instrumentation/action_pack/example/trace_demonstration.ru @@ -39,6 +39,6 @@ Rails.application.initialize! run Rails.application # To run this example run the `rackup` command with this file -# Example: rackup trace_request_demonstration.ru +# Example: rackup trace_demonstration.ru # Navigate to http://localhost:9292/ # Spans for the requests will appear in the console diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb new file mode 100644 index 000000000..c7f50081d --- /dev/null +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require_relative 'handlers/action_controller' + +module OpenTelemetry + module Instrumentation + module ActionPack + # Module that contains custom event handlers, which are used to generate spans per event + module Handlers + module_function + + def subscribe + return unless Array(@subscriptions).empty? + + config = ActionPack::Instrumentation.instance.config + handlers_by_pattern = { + 'process_action.action_controller' => Handlers::ActionController.new(config) + } + + @subscriptions = handlers_by_pattern.map do |key, handler| + ::ActiveSupport::Notifications.subscribe(key, handler) + end + end + + # Removes Event Handler Subscriptions for ActionController notifications + # @note this method is not thread safe and sholud not be used in a multi-threaded context + def unsubscribe + @subscriptions&.each { |subscriber| ::ActiveSupport::Notifications.unsubscribe(subscriber) } + @subscriptions = nil + end + end + end + end +end \ No newline at end of file diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb new file mode 100644 index 000000000..f90907cdb --- /dev/null +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +module OpenTelemetry + module Instrumentation + module ActionPack + module Handlers + # Action controller handler to handle the notification from ActiveSupport + class ActionController + # @param config [Hash] of instrumentation options + def initialize(config) + @config = config + end + + # Invoked by ActiveSupport::Notifications at the start of the instrumentation block + # + # @param _name [String] of the Event (unused) + # @param _id [String] of the event (unused) + # @param payload [Hash] containing job run information + # @return [Hash] the payload passed as a method argument + def start(_name, _id, payload) + + rack_span = OpenTelemetry::Instrumentation::Rack.current_span + + rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless payload[:request]&.env['action_dispatch.exception'] + + attributes_to_append = { + OpenTelemetry::SemanticConventions::Trace::CODE_NAMESPACE => String(payload[:controller]), + OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => String(payload[:action]) + } + + attributes_to_append[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = payload[:request]&.filtered_path if payload[:request]&.filtered_path != payload[:request]&.fullpath + rack_span.add_attributes(attributes_to_append) + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + + # Invoked by ActiveSupport::Notifications at the end of the instrumentation block + # + # @param _name [String] of the Event (unused) + # @param _id [String] of the event (unused) + # @param payload [Hash] containing job run information + # @return [Hash] the payload passed as a method argument + def finish(_name, _id, payload) + + # payload in finish: {:controller=>"UsersController", :action=>"new", :request=>#, + # :params=>{"controller"=>"users", "action"=>"new"}, :headers=>#>, + # :format=>"*/*", :method=>"GET", :path=>"/users/new", :view_runtime=>nil, :db_runtime=>0.23200000578071922, + # :exception=>["NoMethodError", "undefined method `asdfsf' for #"], + # :exception_object=>#>} + # error will appear here + + rack_span = OpenTelemetry::Instrumentation::Rack.current_span + rack_span.record_exception(payload[:exception_object]) if payload[:exception_object] + rescue StandardError => e + OpenTelemetry.handle_error(exception: e) + end + end + end + end + end +end \ No newline at end of file diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb index 52252ff24..896bf4621 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb @@ -32,11 +32,11 @@ def gem_version end def patch - ::ActionController::Metal.prepend(Patches::ActionController::Metal) + Handlers.subscribe end def require_dependencies - require_relative 'patches/action_controller/metal' + require_relative 'handlers' end def require_railtie diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb deleted file mode 100644 index 6e3b17c9e..000000000 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -# Copyright The OpenTelemetry Authors -# -# SPDX-License-Identifier: Apache-2.0 - -module OpenTelemetry - module Instrumentation - module ActionPack - module Patches - module ActionController - # Module to prepend to ActionController::Metal for instrumentation - module Metal - def dispatch(name, request, response) - rack_span = OpenTelemetry::Instrumentation::Rack.current_span - if rack_span.recording? - rack_span.name = "#{self.class.name}##{name}" unless request.env['action_dispatch.exception'] - - attributes_to_append = { - OpenTelemetry::SemanticConventions::Trace::CODE_NAMESPACE => self.class.name, - OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => String(name) - } - attributes_to_append[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = request.filtered_path if request.filtered_path != request.fullpath - rack_span.add_attributes(attributes_to_append) - end - - super(name, request, response) - end - - private - - def instrumentation_config - ActionPack::Instrumentation.instance.config - end - end - end - end - end - end -end diff --git a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/patches/action_controller/metal_test.rb b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb similarity index 56% rename from instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/patches/action_controller/metal_test.rb rename to instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb index fe488d31c..c8b5e5867 100644 --- a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/patches/action_controller/metal_test.rb +++ b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb @@ -6,10 +6,10 @@ require 'test_helper' -require_relative '../../../../../../lib/opentelemetry/instrumentation/action_pack' -require_relative '../../../../../../lib/opentelemetry/instrumentation/action_pack/patches/action_controller/metal' +require_relative '../../../../../lib/opentelemetry/instrumentation/action_pack' +require_relative '../../../../../lib/opentelemetry/instrumentation/action_pack/handlers' -describe OpenTelemetry::Instrumentation::ActionPack::Patches::ActionController::Metal do +describe OpenTelemetry::Instrumentation::ActionPack::Handlers::ActionController do include Rack::Test::Methods let(:exporter) { EXPORTER } @@ -75,18 +75,57 @@ get 'internal_server_error' _(span.name).must_equal 'ExampleController#internal_server_error' + _(span.kind).must_equal :server + _(span.status.ok?).must_equal false + + _(span.instrumentation_library.name).must_equal 'OpenTelemetry::Instrumentation::Rack' + _(span.instrumentation_library.version).must_equal OpenTelemetry::Instrumentation::Rack::VERSION + + _(span.attributes['http.method']).must_equal 'GET' + _(span.attributes['http.host']).must_equal 'example.org' + _(span.attributes['http.scheme']).must_equal 'http' + _(span.attributes['http.target']).must_equal '/internal_server_error' + _(span.attributes['http.status_code']).must_equal 500 + _(span.attributes['http.user_agent']).must_be_nil + _(span.attributes['code.namespace']).must_equal 'ExampleController' + _(span.attributes['code.function']).must_equal 'internal_server_error' end it 'does not set the span name when an exception is raised in middleware' do get '/ok?raise_in_middleware' _(span.name).must_equal 'HTTP GET' + _(span.kind).must_equal :server + _(span.status.ok?).must_equal false + + _(span.instrumentation_library.name).must_equal 'OpenTelemetry::Instrumentation::Rack' + _(span.instrumentation_library.version).must_equal OpenTelemetry::Instrumentation::Rack::VERSION + + _(span.attributes['http.method']).must_equal 'GET' + _(span.attributes['http.host']).must_equal 'example.org' + _(span.attributes['http.scheme']).must_equal 'http' + _(span.attributes['http.target']).must_equal '/ok?raise_in_middleware' + _(span.attributes['http.status_code']).must_equal 500 + _(span.attributes['http.user_agent']).must_be_nil + _(span.attributes['code.namespace']).must_be_nil + _(span.attributes['code.function']).must_be_nil end it 'does not set the span name when the request is redirected in middleware' do get '/ok?redirect_in_middleware' _(span.name).must_equal 'HTTP GET' + _(span.kind).must_equal :server + _(span.status.ok?).must_equal true + + _(span.attributes['http.method']).must_equal 'GET' + _(span.attributes['http.host']).must_equal 'example.org' + _(span.attributes['http.scheme']).must_equal 'http' + _(span.attributes['http.target']).must_equal '/ok?redirect_in_middleware' + _(span.attributes['http.status_code']).must_equal 307 + _(span.attributes['http.user_agent']).must_be_nil + _(span.attributes['code.namespace']).must_be_nil + _(span.attributes['code.function']).must_be_nil end describe 'when the application has exceptions_app configured' do @@ -96,6 +135,20 @@ get 'internal_server_error' _(span.name).must_equal 'ExampleController#internal_server_error' + _(span.kind).must_equal :server + _(span.status.ok?).must_equal false + + _(span.instrumentation_library.name).must_equal 'OpenTelemetry::Instrumentation::Rack' + _(span.instrumentation_library.version).must_equal OpenTelemetry::Instrumentation::Rack::VERSION + + _(span.attributes['http.method']).must_equal 'GET' + _(span.attributes['http.host']).must_equal 'example.org' + _(span.attributes['http.scheme']).must_equal 'http' + _(span.attributes['http.target']).must_equal '/internal_server_error' + _(span.attributes['http.status_code']).must_equal 500 + _(span.attributes['http.user_agent']).must_be_nil + _(span.attributes['code.namespace']).must_equal 'ExceptionsController' + _(span.attributes['code.function']).must_equal 'show' end end diff --git a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb new file mode 100644 index 000000000..c4818f416 --- /dev/null +++ b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +# Copyright The OpenTelemetry Authors +# +# SPDX-License-Identifier: Apache-2.0 + +require 'test_helper' + +require_relative '../../../../lib/opentelemetry/instrumentation/action_pack' + +describe 'OpenTelemetry::Instrumentation::ActionPack::Handlers' do + let(:instrumentation) { OpenTelemetry::Instrumentation::ActionPack::Instrumentation.instance } + let(:config) { {} } + + before do + OpenTelemetry::Instrumentation::ActionPack::Handlers.unsubscribe + instrumentation.instance_variable_set(:@config, config) + instrumentation.instance_variable_set(:@installed, false) + + instrumentation.install(config) + end + + it 'success subscribe the notification' do + subscriptions = OpenTelemetry::Instrumentation::ActionPack::Handlers.instance_variable_get(:@subscriptions) + _(subscriptions.count).must_equal 1 + _(subscriptions[0].pattern).must_equal 'process_action.action_controller' + end + + it 'success unsubscribe the notification' do + OpenTelemetry::Instrumentation::ActionPack::Handlers.unsubscribe + subscriptions = OpenTelemetry::Instrumentation::ActionPack::Handlers.instance_variable_get(:@subscriptions) + _(subscriptions).must_be_nil + end +end \ No newline at end of file From 48080b07321e093a4a936fa83fe12065cbf26e0a Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Thu, 19 Oct 2023 12:29:41 -0400 Subject: [PATCH 02/15] feat!(action_pack): remove comments --- .../action_pack/handlers/action_controller.rb | 7 ------- 1 file changed, 7 deletions(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index f90907cdb..589e2ff00 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -46,13 +46,6 @@ def start(_name, _id, payload) # @return [Hash] the payload passed as a method argument def finish(_name, _id, payload) - # payload in finish: {:controller=>"UsersController", :action=>"new", :request=>#, - # :params=>{"controller"=>"users", "action"=>"new"}, :headers=>#>, - # :format=>"*/*", :method=>"GET", :path=>"/users/new", :view_runtime=>nil, :db_runtime=>0.23200000578071922, - # :exception=>["NoMethodError", "undefined method `asdfsf' for #"], - # :exception_object=>#>} - # error will appear here - rack_span = OpenTelemetry::Instrumentation::Rack.current_span rack_span.record_exception(payload[:exception_object]) if payload[:exception_object] rescue StandardError => e From 68a9404717e13325d1fbe7b2a1a561d23bf00493 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Thu, 19 Oct 2023 16:00:52 -0400 Subject: [PATCH 03/15] feat: drop 6.0 on action_pack to verify the test case --- instrumentation/action_pack/Appraisals | 8 ++++---- .../instrumentation/action_pack/handlers.rb | 2 +- .../action_pack/handlers/action_controller.rb | 10 +++++----- .../instrumentation/action_pack/instrumentation.rb | 2 +- ...pentelemetry-instrumentation-action_pack.gemspec | 2 +- .../action_pack/handlers/action_controller_test.rb | 13 ++++++++++++- .../instrumentation/action_pack/handlers_test.rb | 2 +- .../action_pack/test/test_helpers/app_config.rb | 9 --------- 8 files changed, 25 insertions(+), 23 deletions(-) diff --git a/instrumentation/action_pack/Appraisals b/instrumentation/action_pack/Appraisals index 03f29edc9..018ce75b2 100644 --- a/instrumentation/action_pack/Appraisals +++ b/instrumentation/action_pack/Appraisals @@ -4,10 +4,6 @@ # # SPDX-License-Identifier: Apache-2.0 -appraise 'rails-6.0' do - gem 'rails', '~> 6.0.0' -end - appraise 'rails-6.1' do gem 'rails', '~> 6.1.0' end @@ -15,3 +11,7 @@ end appraise 'rails-7.0' do gem 'rails', '~> 7.0.0' end + +appraise 'rails-7.1' do + gem 'rails', '~> 7.1.0' +end diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb index c7f50081d..ce250fe22 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb @@ -35,4 +35,4 @@ def unsubscribe end end end -end \ No newline at end of file +end diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index 589e2ff00..dc5a8a308 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -22,17 +22,18 @@ def initialize(config) # @param payload [Hash] containing job run information # @return [Hash] the payload passed as a method argument def start(_name, _id, payload) - rack_span = OpenTelemetry::Instrumentation::Rack.current_span - rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless payload[:request]&.env['action_dispatch.exception'] + # from rails 6.1, the request is added to payload + rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless payload[:request].env['action_dispatch.exception'] attributes_to_append = { OpenTelemetry::SemanticConventions::Trace::CODE_NAMESPACE => String(payload[:controller]), OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => String(payload[:action]) } - attributes_to_append[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = payload[:request]&.filtered_path if payload[:request]&.filtered_path != payload[:request]&.fullpath + attributes_to_append[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = payload[:request].filtered_path if payload[:request].filtered_path != payload[:request].fullpath + rack_span.add_attributes(attributes_to_append) rescue StandardError => e OpenTelemetry.handle_error(exception: e) @@ -45,7 +46,6 @@ def start(_name, _id, payload) # @param payload [Hash] containing job run information # @return [Hash] the payload passed as a method argument def finish(_name, _id, payload) - rack_span = OpenTelemetry::Instrumentation::Rack.current_span rack_span.record_exception(payload[:exception_object]) if payload[:exception_object] rescue StandardError => e @@ -55,4 +55,4 @@ def finish(_name, _id, payload) end end end -end \ No newline at end of file +end diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb index 896bf4621..cad8a14a4 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb @@ -9,7 +9,7 @@ module Instrumentation module ActionPack # The Instrumentation class contains logic to detect and install the ActionPack instrumentation class Instrumentation < OpenTelemetry::Instrumentation::Base - MINIMUM_VERSION = Gem::Version.new('6.0.0') + MINIMUM_VERSION = Gem::Version.new('6.1.0') install do |_config| require_railtie diff --git a/instrumentation/action_pack/opentelemetry-instrumentation-action_pack.gemspec b/instrumentation/action_pack/opentelemetry-instrumentation-action_pack.gemspec index f12531d06..2e23f7fef 100644 --- a/instrumentation/action_pack/opentelemetry-instrumentation-action_pack.gemspec +++ b/instrumentation/action_pack/opentelemetry-instrumentation-action_pack.gemspec @@ -34,7 +34,7 @@ Gem::Specification.new do |spec| spec.add_development_dependency 'minitest', '~> 5.0' spec.add_development_dependency 'opentelemetry-sdk', '~> 1.1' spec.add_development_dependency 'opentelemetry-test-helpers', '~> 0.3' - spec.add_development_dependency 'rails', '>= 6' + spec.add_development_dependency 'rails', '>= 6.1' spec.add_development_dependency 'rake', '~> 13.0' spec.add_development_dependency 'rubocop', '~> 1.56.1' spec.add_development_dependency 'simplecov', '~> 0.17.1' diff --git a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb index c8b5e5867..0da17ab15 100644 --- a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb +++ b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers/action_controller_test.rb @@ -12,13 +12,24 @@ describe OpenTelemetry::Instrumentation::ActionPack::Handlers::ActionController do include Rack::Test::Methods + let(:instrumentation) { OpenTelemetry::Instrumentation::ActionPack::Instrumentation.instance } let(:exporter) { EXPORTER } let(:spans) { exporter.finished_spans } let(:span) { exporter.finished_spans.last } let(:rails_app) { DEFAULT_RAILS_APP } + let(:config) { {} } # Clear captured spans - before { exporter.reset } + before do + OpenTelemetry::Instrumentation::ActionPack::Handlers.unsubscribe + + instrumentation.instance_variable_set(:@config, config) + instrumentation.instance_variable_set(:@installed, false) + + instrumentation.install(config) + + exporter.reset + end it 'sets the span name to the format: ControllerName#action' do get '/ok' diff --git a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb index c4818f416..3d2419b03 100644 --- a/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb +++ b/instrumentation/action_pack/test/opentelemetry/instrumentation/action_pack/handlers_test.rb @@ -31,4 +31,4 @@ subscriptions = OpenTelemetry::Instrumentation::ActionPack::Handlers.instance_variable_get(:@subscriptions) _(subscriptions).must_be_nil end -end \ No newline at end of file +end diff --git a/instrumentation/action_pack/test/test_helpers/app_config.rb b/instrumentation/action_pack/test/test_helpers/app_config.rb index b62d198de..8874d61e2 100644 --- a/instrumentation/action_pack/test/test_helpers/app_config.rb +++ b/instrumentation/action_pack/test/test_helpers/app_config.rb @@ -28,8 +28,6 @@ def initialize_app(use_exceptions_app: false, remove_rack_tracer_middleware: fal new_app.config.filter_parameters = [:param_to_be_filtered] case Rails.version - when /^6\.0/ - apply_rails_6_0_configs(new_app) when /^6\.1/ apply_rails_6_1_configs(new_app) when /^7\./ @@ -73,13 +71,6 @@ def add_middlewares(application) ) end - def apply_rails_6_0_configs(application) - # Required in Rails 6 - application.config.hosts << 'example.org' - # Creates a lot of deprecation warnings on subsequent app initializations if not explicitly set. - application.config.action_view.finalize_compiled_template_methods = ActionView::Railtie::NULL_OPTION - end - def apply_rails_6_1_configs(application) # Required in Rails 6 application.config.hosts << 'example.org' From 6dc91848be827b9f63f4b0303b722951239ce799 Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Fri, 20 Oct 2023 10:05:36 -0400 Subject: [PATCH 04/15] feat: add rails 6.0 back --- instrumentation/action_pack/Appraisals | 4 ++++ .../action_pack/handlers/action_controller.rb | 9 ++++++--- .../instrumentation/action_pack/instrumentation.rb | 2 +- .../action_pack/test/test_helpers/app_config.rb | 9 +++++++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/instrumentation/action_pack/Appraisals b/instrumentation/action_pack/Appraisals index 018ce75b2..895a185d4 100644 --- a/instrumentation/action_pack/Appraisals +++ b/instrumentation/action_pack/Appraisals @@ -4,6 +4,10 @@ # # SPDX-License-Identifier: Apache-2.0 +appraise 'rails-6.0' do + gem 'rails', '~> 6.0.0' +end + appraise 'rails-6.1' do gem 'rails', '~> 6.1.0' end diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index dc5a8a308..15d54a5bb 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -24,15 +24,18 @@ def initialize(config) def start(_name, _id, payload) rack_span = OpenTelemetry::Instrumentation::Rack.current_span - # from rails 6.1, the request is added to payload - rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless payload[:request].env['action_dispatch.exception'] + # from rails 6.1, the request will be added to payload + request = payload[:request] + request = payload[:headers].instance_variable_get(:@req) if ::ActionPack.version < Gem::Version.new('6.1.0') + + rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless request.env['action_dispatch.exception'] attributes_to_append = { OpenTelemetry::SemanticConventions::Trace::CODE_NAMESPACE => String(payload[:controller]), OpenTelemetry::SemanticConventions::Trace::CODE_FUNCTION => String(payload[:action]) } - attributes_to_append[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = payload[:request].filtered_path if payload[:request].filtered_path != payload[:request].fullpath + attributes_to_append[OpenTelemetry::SemanticConventions::Trace::HTTP_TARGET] = request.filtered_path if request.filtered_path != request.fullpath rack_span.add_attributes(attributes_to_append) rescue StandardError => e diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb index cad8a14a4..896bf4621 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/instrumentation.rb @@ -9,7 +9,7 @@ module Instrumentation module ActionPack # The Instrumentation class contains logic to detect and install the ActionPack instrumentation class Instrumentation < OpenTelemetry::Instrumentation::Base - MINIMUM_VERSION = Gem::Version.new('6.1.0') + MINIMUM_VERSION = Gem::Version.new('6.0.0') install do |_config| require_railtie diff --git a/instrumentation/action_pack/test/test_helpers/app_config.rb b/instrumentation/action_pack/test/test_helpers/app_config.rb index 8874d61e2..b62d198de 100644 --- a/instrumentation/action_pack/test/test_helpers/app_config.rb +++ b/instrumentation/action_pack/test/test_helpers/app_config.rb @@ -28,6 +28,8 @@ def initialize_app(use_exceptions_app: false, remove_rack_tracer_middleware: fal new_app.config.filter_parameters = [:param_to_be_filtered] case Rails.version + when /^6\.0/ + apply_rails_6_0_configs(new_app) when /^6\.1/ apply_rails_6_1_configs(new_app) when /^7\./ @@ -71,6 +73,13 @@ def add_middlewares(application) ) end + def apply_rails_6_0_configs(application) + # Required in Rails 6 + application.config.hosts << 'example.org' + # Creates a lot of deprecation warnings on subsequent app initializations if not explicitly set. + application.config.action_view.finalize_compiled_template_methods = ActionView::Railtie::NULL_OPTION + end + def apply_rails_6_1_configs(application) # Required in Rails 6 application.config.hosts << 'example.org' From 5555980e873a7936b54be23e725c0eb5a8358fda Mon Sep 17 00:00:00 2001 From: xuan-cao-swi Date: Thu, 23 Nov 2023 10:50:32 -0500 Subject: [PATCH 05/15] feat!(action_pack): remove rails 6.0 support + update readme on error handling --- instrumentation/action_pack/README.md | 7 +++++++ .../action_pack/handlers/action_controller.rb | 2 -- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/instrumentation/action_pack/README.md b/instrumentation/action_pack/README.md index 142a99bf4..fd5c50178 100644 --- a/instrumentation/action_pack/README.md +++ b/instrumentation/action_pack/README.md @@ -42,6 +42,13 @@ See the table below for details of what [Rails Framework Hook Events](https://gu | - | - | - | - | | `process_action.action_controller` | :white_check_mark: | :x: | It modifies the existing Rack span | + +### Error Handling for Action Controller + +If an error is triggered by the action controller (such as a 500 internal server error), actionpack will typically employ the default `ActionDispatch::PublicExceptions.new(Rails.public_path)` as the `exceptions_app`, as detailed in the [documentation](https://guides.rubyonrails.org/configuring.html#config-exceptions-app). + +The error object will be retained within `payload[:exception_object]`. Additionally, its storage in `request.env['action_dispatch.exception']` is contingent upon the configuration of `action_dispatch.show_exceptions` in Rails. + ## Examples Example usage can be seen in the `./example/trace_demonstration.rb` file [here](https://github.com/open-telemetry/opentelemetry-ruby-contrib/blob/main/instrumentation/action_pack/example/trace_demonstration.ru) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index 15d54a5bb..fc4f93a9f 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -24,9 +24,7 @@ def initialize(config) def start(_name, _id, payload) rack_span = OpenTelemetry::Instrumentation::Rack.current_span - # from rails 6.1, the request will be added to payload request = payload[:request] - request = payload[:headers].instance_variable_get(:@req) if ::ActionPack.version < Gem::Version.new('6.1.0') rack_span.name = "#{payload[:controller]}##{payload[:action]}" unless request.env['action_dispatch.exception'] From 5ca405388f79e48ff7eff70198ce3b086a2388d8 Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Tue, 5 Dec 2023 12:53:06 -0500 Subject: [PATCH 06/15] Update instrumentation/action_pack/README.md Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- instrumentation/action_pack/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/README.md b/instrumentation/action_pack/README.md index fd5c50178..e369808b3 100644 --- a/instrumentation/action_pack/README.md +++ b/instrumentation/action_pack/README.md @@ -32,7 +32,7 @@ end ## Active Support Instrumentation -Earlier versions of this instrumentation relied on patching custom `dispatch` hooks from rails [action_controller](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/metal.rb#L224) in order to extract request information. +Earlier versions of this instrumentation relied on patching custom `dispatch` hooks from Rails's [Action Controller](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/metal.rb#L224) to extract request information. This instrumentation now relies on `ActiveSupport::Notifications` and registers a custom Subscriber that listens to relevant events to modify rack span. From 988194ce44261782800dae51e39144c8dde1a569 Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Tue, 5 Dec 2023 12:53:15 -0500 Subject: [PATCH 07/15] Update instrumentation/action_pack/README.md Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- instrumentation/action_pack/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/README.md b/instrumentation/action_pack/README.md index e369808b3..ee1557ccb 100644 --- a/instrumentation/action_pack/README.md +++ b/instrumentation/action_pack/README.md @@ -34,7 +34,7 @@ end Earlier versions of this instrumentation relied on patching custom `dispatch` hooks from Rails's [Action Controller](https://github.com/rails/rails/blob/main/actionpack/lib/action_controller/metal.rb#L224) to extract request information. -This instrumentation now relies on `ActiveSupport::Notifications` and registers a custom Subscriber that listens to relevant events to modify rack span. +This instrumentation now relies on `ActiveSupport::Notifications` and registers a custom Subscriber that listens to relevant events to modify the Rack span. See the table below for details of what [Rails Framework Hook Events](https://guides.rubyonrails.org/active_support_instrumentation.html#action-controller) are recorded by this instrumentation: From 7ceac0348a5263ffc742271ed3c0c6eeb2e07c51 Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Tue, 5 Dec 2023 12:53:21 -0500 Subject: [PATCH 08/15] Update instrumentation/action_pack/README.md Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- instrumentation/action_pack/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/README.md b/instrumentation/action_pack/README.md index ee1557ccb..8bbcd69bf 100644 --- a/instrumentation/action_pack/README.md +++ b/instrumentation/action_pack/README.md @@ -45,7 +45,7 @@ See the table below for details of what [Rails Framework Hook Events](https://gu ### Error Handling for Action Controller -If an error is triggered by the action controller (such as a 500 internal server error), actionpack will typically employ the default `ActionDispatch::PublicExceptions.new(Rails.public_path)` as the `exceptions_app`, as detailed in the [documentation](https://guides.rubyonrails.org/configuring.html#config-exceptions-app). +If an error is triggered by Action Controller (such as a 500 internal server error), Action Pack will typically employ the default `ActionDispatch::PublicExceptions.new(Rails.public_path)` as the `exceptions_app`, as detailed in the [documentation](https://guides.rubyonrails.org/configuring.html#config-exceptions-app). The error object will be retained within `payload[:exception_object]`. Additionally, its storage in `request.env['action_dispatch.exception']` is contingent upon the configuration of `action_dispatch.show_exceptions` in Rails. From 297555d97e205e3b1623458db227fd2d211962b0 Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Tue, 5 Dec 2023 12:53:28 -0500 Subject: [PATCH 09/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../instrumentation/action_pack/handlers/action_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index fc4f93a9f..2514397f1 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -8,7 +8,7 @@ module OpenTelemetry module Instrumentation module ActionPack module Handlers - # Action controller handler to handle the notification from ActiveSupport + # Action Controller handler to handle the notification from Active Support class ActionController # @param config [Hash] of instrumentation options def initialize(config) From 8f827af4e6f69acd1820940d68935c9a736a1fb8 Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Tue, 5 Dec 2023 12:53:34 -0500 Subject: [PATCH 10/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../lib/opentelemetry/instrumentation/action_pack/handlers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb index ce250fe22..29bede7b6 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb @@ -26,7 +26,7 @@ def subscribe end end - # Removes Event Handler Subscriptions for ActionController notifications + # Removes Event Handler Subscriptions for Action Controller notifications # @note this method is not thread safe and sholud not be used in a multi-threaded context def unsubscribe @subscriptions&.each { |subscriber| ::ActiveSupport::Notifications.unsubscribe(subscriber) } From 46775d5abf8316fdf57dc35ca074e014904bacfa Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Tue, 5 Dec 2023 12:53:44 -0500 Subject: [PATCH 11/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../instrumentation/action_pack/handlers/action_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index 2514397f1..0dbe0b44a 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -19,7 +19,7 @@ def initialize(config) # # @param _name [String] of the Event (unused) # @param _id [String] of the event (unused) - # @param payload [Hash] containing job run information + # @param payload [Hash] the payload passed as a method argument # @return [Hash] the payload passed as a method argument def start(_name, _id, payload) rack_span = OpenTelemetry::Instrumentation::Rack.current_span From 3bbe073903cb0328de4a935ddc450ba815a88abc Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Wed, 6 Dec 2023 16:09:04 -0500 Subject: [PATCH 12/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../instrumentation/action_pack/handlers/action_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index 0dbe0b44a..eeed9e251 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -44,7 +44,7 @@ def start(_name, _id, payload) # # @param _name [String] of the Event (unused) # @param _id [String] of the event (unused) - # @param payload [Hash] containing job run information + # @param payload [Hash] the payload passed as a method argument # @return [Hash] the payload passed as a method argument def finish(_name, _id, payload) rack_span = OpenTelemetry::Instrumentation::Rack.current_span From 5f8f76f148e901e9702f9e90c996c82601390f52 Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Wed, 6 Dec 2023 16:09:11 -0500 Subject: [PATCH 13/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../instrumentation/action_pack/handlers/action_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index eeed9e251..9ddbd0216 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -42,7 +42,7 @@ def start(_name, _id, payload) # Invoked by ActiveSupport::Notifications at the end of the instrumentation block # - # @param _name [String] of the Event (unused) + # @param _name [String] of the event (unused) # @param _id [String] of the event (unused) # @param payload [Hash] the payload passed as a method argument # @return [Hash] the payload passed as a method argument From 59401f749787dfc62ddbafd8a0969518b553d21e Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Wed, 6 Dec 2023 16:09:17 -0500 Subject: [PATCH 14/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../instrumentation/action_pack/handlers/action_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb index 9ddbd0216..d7b79b64d 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -17,7 +17,7 @@ def initialize(config) # Invoked by ActiveSupport::Notifications at the start of the instrumentation block # - # @param _name [String] of the Event (unused) + # @param _name [String] of the event (unused) # @param _id [String] of the event (unused) # @param payload [Hash] the payload passed as a method argument # @return [Hash] the payload passed as a method argument From bb720fc3c637971ee49c5d9b618ea12579ddd69b Mon Sep 17 00:00:00 2001 From: Xuan <112967240+xuan-cao-swi@users.noreply.github.com> Date: Wed, 6 Dec 2023 16:09:54 -0500 Subject: [PATCH 15/15] Update instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb Co-authored-by: Kayla Reopelle (she/her) <87386821+kaylareopelle@users.noreply.github.com> --- .../lib/opentelemetry/instrumentation/action_pack/handlers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb index 29bede7b6..1e902d4e3 100644 --- a/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers.rb @@ -27,7 +27,7 @@ def subscribe end # Removes Event Handler Subscriptions for Action Controller notifications - # @note this method is not thread safe and sholud not be used in a multi-threaded context + # @note this method is not thread-safe and should not be used in a multi-threaded context def unsubscribe @subscriptions&.each { |subscriber| ::ActiveSupport::Notifications.unsubscribe(subscriber) } @subscriptions = nil