Skip to content

Commit

Permalink
Merge branch 'main' into create-sql-instrumentation-helpers
Browse files Browse the repository at this point in the history
  • Loading branch information
arielvalentin authored Dec 11, 2023
2 parents a9ff319 + 73fa809 commit 36686ef
Show file tree
Hide file tree
Showing 65 changed files with 285 additions and 110 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/stale.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@ jobs:
runs-on: ubuntu-latest

steps:
- uses: actions/stale@v8
- uses: actions/stale@v9
name: Clean up stale issues and PRs
with:
repo-token: ${{ secrets.GITHUB_TOKEN }}
stale-issue-message: "👋 This issue has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the `keep` label to hold stale off permanently, or do nothing. If you do nothing this issue will be closed eventually by the stale bot."
stale-issue-label: "stale"
exempt-issue-labels: "keep"
days-before-issue-stale: 30
days-before-issue-close: -1
days-before-issue-close: 30
stale-pr-message: "👋 This pull request has been marked as stale because it has been open with no activity. You can: comment on the issue or remove the stale label to hold stale off for a while, add the `keep` label to hold stale off permanently, or do nothing. If you do nothing this pull request will be closed eventually by the stale bot"
stale-pr-label: "stale"
exempt-pr-labels: "keep"
Expand Down
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,5 +7,5 @@
source 'https://rubygems.org'

gem 'rake', '~> 13.0'
gem 'rubocop', '~> 1.57.1'
gem 'rubocop', '~> 1.58.0'
gem 'rubocop-performance', '~> 1.19.1'
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ The Ruby special interest group (SIG) meets regularly. See the OpenTelemetry

Approvers ([@open-telemetry/ruby-contrib-approvers](https://github.com/orgs/open-telemetry/teams/ruby-contrib-approvers)):

- (Could _your_ name appear here?)
- [Josef Šimánek](https://github.com/simi)
- [Kayla Reopelle](https://github.com/kaylareopelle), New Relic

*Find more about the approver role in [community repository](https://github.com/open-telemetry/community/blob/master/community-membership.md#approver).*

Expand Down
19 changes: 19 additions & 0 deletions instrumentation/action_pack/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions instrumentation/action_pack/example/trace_demonstration.ru
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ require 'action_controller/railtie'
class TraceRequestApp < Rails::Application
config.root = __dir__
config.hosts << 'example.org'
secrets.secret_key_base = 'secret_key_base'
credentials.secret_key_base = 'secret_key_base'
config.eager_load = false
config.logger = Logger.new($stdout)
Rails.logger = config.logger
Expand All @@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ Gem::Specification.new do |spec|
spec.add_development_dependency 'opentelemetry-test-helpers', '~> 0.3'
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 'rubocop', '~> 1.58.0'
spec.add_development_dependency 'rubocop-performance', '~> 1.19.1'
spec.add_development_dependency 'simplecov', '~> 0.17.1'
spec.add_development_dependency 'webmock', '~> 3.19'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand All @@ -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

Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ require 'action_view/railtie'
class TraceRequestApp < Rails::Application
config.root = __dir__
config.hosts << 'example.org'
secrets.secret_key_base = 'secret_key_base'
credentials.secret_key_base = 'secret_key_base'

config.eager_load = false

Expand Down
Loading

0 comments on commit 36686ef

Please sign in to comment.