-
Notifications
You must be signed in to change notification settings - Fork 600
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
Changes from 9 commits
318683c
b101b88
427ad20
d57e3e9
d746f68
732d23f
d115441
70c3c77
a0e188c
5ebc4ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) | ||
NewRelic::Agent.record_instrumentation_invocation(INSTRUMENTATION_NAME) | ||
|
||
begin | ||
segment = NewRelic::Agent::Tracer.start_segment( | ||
name: metric_name(self.class.identifier, self.class.name) | ||
) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normally I would expect a yield after this line, which would call There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Late to the party, but... is suppressing errors here the intended behavior? |
||
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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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/model/index.html.erb/Rendering'
'View/model/_list.html.erb/Partial' There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( |
||
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 |
There was a problem hiding this comment.
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?