diff --git a/instrumentation/action_pack/README.md b/instrumentation/action_pack/README.md index 877e0ec4b..8bbcd69bf 100644 --- a/instrumentation/action_pack/README.md +++ b/instrumentation/action_pack/README.md @@ -30,6 +30,25 @@ OpenTelemetry::SDK.configure do |c| end ``` +## Active Support Instrumentation + +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 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: + +| Event Name | Subscribe? | Creates Span? | Notes | +| - | - | - | - | +| `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 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. + ## 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 89225fe71..0a8130efb 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..1e902d4e3 --- /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 Action Controller notifications + # @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 + end + end + end + end +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 new file mode 100644 index 000000000..d7b79b64d --- /dev/null +++ b/instrumentation/action_pack/lib/opentelemetry/instrumentation/action_pack/handlers/action_controller.rb @@ -0,0 +1,59 @@ +# 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 Active Support + 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] 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 + + request = payload[:request] + + 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] = request.filtered_path if request.filtered_path != 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] 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 + 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 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 4b547a7e9..cad8a14a4 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 53% 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..0da17ab15 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,19 +6,30 @@ 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(: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' @@ -75,18 +86,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 +146,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..3d2419b03 --- /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