Skip to content
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.

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

ViewComponent instrumentation #2367

Merged
merged 10 commits into from Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
Expand Up @@ -2,7 +2,12 @@

## v9.7.0

Version 9.7.0 changes the endpoint used to access the cluster name for Elasticsearch instrumentation, adds support for Falcon, and removes the creation of the Ruby/Thread and Ruby/Fiber spans.

Version 9.7.0 introduces ViewComponent instrumentation, changes the endpoint used to access the cluster name for Elasticsearch instrumentation, removes the creation of the Ruby/Thread and Ruby/Fiber spans, and adds support for Falcon.

- **Feature: ViewComponent instrumentation**

[ViewComponent](https://viewcomponent.org/) is a now an instrumented framework. The agent currently supports Roda versions 2.0.0+. [PR#2367](https://github.com/newrelic/newrelic-ruby-agent/pull/2367)

- **Feature: Use root path to access Elasticsearch cluster name**

Expand Down
10 changes: 10 additions & 0 deletions lib/new_relic/agent/configuration/default_source.rb
Expand Up @@ -313,6 +313,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
'webpacker:compile'
].join(',').freeze

# rubocop:disable Metrics/CollectionLiteralLength
DEFAULTS = {
# Critical
:agent_enabled => {
Expand Down Expand Up @@ -1657,6 +1658,14 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
:allowed_from_server => false,
:description => 'Controls auto-instrumentation of Stripe at startup. May be one of: `enabled`, `disabled`.'
},
:'instrumentation.view_component' => {
:default => 'auto',
:public => true,
:type => String,
:dynamic_name => true,
:allowed_from_server => false,
:description => 'Controls auto-instrumentation of ViewComponent at startup. May be one of: `auto`, `prepend`, `chain`, `disabled`.'
},
:'stripe.user_data.include' => {
default: NewRelic::EMPTY_ARRAY,
public: true,
Expand Down Expand Up @@ -2405,6 +2414,7 @@ def self.enforce_fallback(allowed_values: nil, fallback: nil)
:description => 'This value represents the total amount of memory available to the host (not the process), in mebibytes (1024 squared or 1,048,576 bytes).'
}
}.freeze
# rubocop:enable Metrics/CollectionLiteralLength
end
end
end
26 changes: 26 additions & 0 deletions lib/new_relic/agent/instrumentation/view_component.rb
@@ -0,0 +1,26 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

require_relative 'view_component/instrumentation'
require_relative 'view_component/chain'
require_relative 'view_component/prepend'

DependencyDetection.defer do
named :view_component

depends_on do
defined?(ViewComponent) &&
ViewComponent::Base.method_defined?(:render_in)
end

executes do
NewRelic::Agent.logger.info('Installing ViewComponent instrumentation')

if use_prepend?
prepend_instrument ViewComponent::Base, NewRelic::Agent::Instrumentation::ViewComponent::Prepend
else
chain_instrument NewRelic::Agent::Instrumentation::ViewComponent::Chain
end
end
end
21 changes: 21 additions & 0 deletions lib/new_relic/agent/instrumentation/view_component/chain.rb
@@ -0,0 +1,21 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

module NewRelic::Agent::Instrumentation
module ViewComponent::Chain
def self.instrument!
::ViewComponent::Base.class_eval do
include NewRelic::Agent::Instrumentation::ViewComponent

alias_method(:render_in_without_tracing, :render_in)

def render_in(*args)
render_in_with_tracing(*args) do
render_in_without_tracing(*args)
end
end
end
end
end
end
@@ -0,0 +1,38 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

module NewRelic::Agent::Instrumentation
module ViewComponent
INSTRUMENTATION_NAME = NewRelic::Agent.base_name(name)

def render_in_with_tracing(*args)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused, i'm probably missing something obvious (sorry!).
But this method doesn't seem to take a block or yield or anything, so i'm curious what the segment thats being created is measuring?

NewRelic::Agent.record_instrumentation_invocation(INSTRUMENTATION_NAME)

begin
segment = NewRelic::Agent::Tracer.start_segment(
name: metric_name(self.class.identifier, self.class.name)
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Normally I would expect a yield after this line, which would call super/render_in_without_tracing from the do blocks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great catch @tannalynn. This wasn't yielding and preventing views from showing up 😱 Added the yield, tested, and we get both the NR metric and app view now.

yield
rescue => e
::NewRelic::Agent.logger.debug('Error capturing ViewComponent segment', e)
Copy link
Contributor

Choose a reason for hiding this comment

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

Late to the party, but... is suppressing errors here the intended behavior?

ViewComponent/view_component#1981

ensure
segment&.finish
end
end

def metric_name(identifier, component)
"View/#{metric_path(identifier)}/#{component}"
end

def metric_path(identifier)
return 'component' unless identifier

if (parts = identifier.split('/')).size > 1
parts[-2..-1].join('/') # Get filepath by assuming the Rails' structure: app/components/home/example_component.rb
else
NewRelic::Agent::UNKNOWN_METRIC
end
end
end
end
13 changes: 13 additions & 0 deletions lib/new_relic/agent/instrumentation/view_component/prepend.rb
@@ -0,0 +1,13 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

module NewRelic::Agent::Instrumentation
module ViewComponent::Prepend
include NewRelic::Agent::Instrumentation::ViewComponent

def render_in(*args)
render_in_with_tracing(*args) { super }
end
end
end
2 changes: 2 additions & 0 deletions test/multiverse/suites/rails/rails3_app/my_app.rb
Expand Up @@ -94,6 +94,8 @@ class MyApp < Rails::Application

post '/parameter_capture', :to => 'parameter_capture#create'

get '/view_components', :to => 'view_component#index' # This app and route is used in ViewComponent tests

get '/:controller(/:action(/:id))'
end

Expand Down
21 changes: 21 additions & 0 deletions test/multiverse/suites/view_component/Envfile
@@ -0,0 +1,21 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

instrumentation_methods :chain, :prepend

VIEW_COMPONENT_VERSIONS = [
[nil, 2.7],
['2.53.0', 2.4]
]

def gem_list(view_component_version = nil)
<<~RB
gem 'rails'
gem 'view_component'#{view_component_version}
gem 'rack-test'
gem 'loofah', '~> 2.20.0' if RUBY_VERSION >= '2.4.0' && RUBY_VERSION < '2.5.0'
RB
end

create_gemfiles(VIEW_COMPONENT_VERSIONS)
19 changes: 19 additions & 0 deletions test/multiverse/suites/view_component/config/newrelic.yml
@@ -0,0 +1,19 @@
---
development:
error_collector:
enabled: true
apdex_t: 0.5
monitor_mode: true
license_key: bootstrap_newrelic_admin_license_key_000
instrumentation:
view_component: <%= $instrumentation_method %>
app_name: test
log_level: debug
host: 127.0.0.1
api_host: 127.0.0.1
transaction_trace:
record_sql: obfuscated
enabled: true
stack_trace_threshold: 0.5
transaction_threshold: 1.0
capture_params: false
@@ -0,0 +1,54 @@
# This file is distributed under New Relic's license terms.
# See https://github.com/newrelic/newrelic-ruby-agent/blob/main/LICENSE for complete details.
# frozen_string_literal: true

require_relative '../rails/app'

class ExampleComponent < ViewComponent::Base
<<~ERB
<%= @title %>
ERB

def initialize(title:)
@title = title
end
end

class ViewComponentController < ActionController::Base
def index
render(ExampleComponent.new(title: 'Hello World'))
end
end

class DummyViewComponentInstrumentationClass
include NewRelic::Agent::Instrumentation::ViewComponent
end

class ViewComponentInstrumentationTest < ActionDispatch::IntegrationTest
include MultiverseHelpers
setup_and_teardown_agent

FAKE_CLASS = DummyViewComponentInstrumentationClass.new

def test_metric_recorded
get('/view_components')

assert_metrics_recorded('View/view_component/view_component_instrumentation_test.rb/ExampleComponent')
Copy link
Contributor

Choose a reason for hiding this comment

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

The metric name looked odd to me with the .rb file path, but it does match the convention we've long had in place for View/ based metrics:

'View/model/index.html.erb/Rendering'
'View/model/_list.html.erb/Partial'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree! I don't love the look and thought it was a bit weird, but matches 🤷‍♀️ It is also how ViewComponent itself has chosen to do their instrumentation (self.class.identifier is the full path including .rb)

end

def test_records_nothing_if_tracing_disabled
NewRelic::Agent.disable_all_tracing do
get('/view_components')
end

assert_metrics_not_recorded('View/view_component/view_component_instrumentation_test.rb/ExampleComponent')
end

def test_metric_path_falsey
assert(FAKE_CLASS.metric_path(nil), 'component')
end

def test_metric_path_unknown_file_pattern
assert(FAKE_CLASS.metric_path('nothing_to_see_here'), 'unknown')
end
end