Merge pull request #2417 from newrelic/dev

Release 9.7.1

CHANGELOG.md

@@ -1,13 +1,25 @@
 # New Relic Ruby Agent Release Notes
 
-## v9.7.0
 
+## v9.7.1
+
+Version 9.7.1 fixes a ViewComponent instrumentation bug and enforces maximum size limits for custom event attributes.
+
+- **Bugfix: Stop suppressing ViewComponent errors**
+
+  Previously, the agent suppressed ViewComponent render errors. The agent now reports these errors and allows them to raise. Thank you [@mjacobus](https://github.com/mjacobus) for reporting this bug and providing a fix! [PR#2410](https://github.com/newrelic/newrelic-ruby-agent/pull/2410)
+
+- **Bugfix: Enforce maximum size limits for custom event attributes**
+
+  Previously, the agent would allow custom event attributes to be any size. This would lead to the New Relic backend dropping attributes larger than the maximum size. Now, the agent will truncate custom event attribute values to 4095 characters, attribute names to 255 characters, and the total count of attributes to 64. [PR#2401](https://github.com/newrelic/newrelic-ruby-agent/pull/2401)
+
+## v9.7.0
 
 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) 
+  [ViewComponent](https://viewcomponent.org/) is a now an instrumented library. [PR#2367](https://github.com/newrelic/newrelic-ruby-agent/pull/2367) 
 
 - **Feature: Use root path to access Elasticsearch cluster name**
 

lib/new_relic/agent/custom_event_aggregator.rb

@@ -14,6 +14,9 @@ class CustomEventAggregator < EventAggregator
       TIMESTAMP = 'timestamp'.freeze
       PRIORITY = 'priority'.freeze
       EVENT_TYPE_REGEX = /^[a-zA-Z0-9:_ ]+$/.freeze
+      MAX_ATTRIBUTE_COUNT = 64
+      MAX_ATTRIBUTE_SIZE = 4095
+      MAX_NAME_SIZE = 255
 
       named :CustomEventAggregator
       capacity_key :'custom_insights_events.max_samples_stored'
@@ -49,10 +52,33 @@ def create_event(type, priority, attributes)
           {TYPE => type,
            TIMESTAMP => Process.clock_gettime(Process::CLOCK_REALTIME).to_i,
            PRIORITY => priority},
-          AttributeProcessing.flatten_and_coerce(attributes)
+          create_custom_event_attributes(attributes)
         ]
       end
 
+      def create_custom_event_attributes(attributes)
+        result = AttributeProcessing.flatten_and_coerce(attributes)
+
+        if result.size > MAX_ATTRIBUTE_COUNT
+          NewRelic::Agent.logger.warn("Custom event attributes are limited to #{MAX_ATTRIBUTE_COUNT}. Discarding #{result.size - MAX_ATTRIBUTE_COUNT} attributes")
+          result = result.first(MAX_ATTRIBUTE_COUNT)
+        end
+
+        result.each_with_object({}) do |(key, val), new_result|
+          # name is limited to 255
+          if key.is_a?(String) && key.length > MAX_NAME_SIZE
+            key = key[0, MAX_NAME_SIZE]
+          end
+
+          # value is limited to 4095
+          if val.is_a?(String) && val.length > MAX_ATTRIBUTE_SIZE
+            val = val[0, MAX_ATTRIBUTE_SIZE]
+          end
+
+          new_result[key] = val
+        end
+      end
+
       def after_initialize
         @type_strings = Hash.new { |hash, key| hash[key] = key.to_s.freeze }
       end

lib/new_relic/agent/instrumentation/async_http.rb

@@ -10,7 +10,9 @@
   named :async_http
 
   depends_on do
-    defined?(Async::HTTP) && Gem::Version.new(Async::HTTP::VERSION) >= Gem::Version.new('0.59.0')
+    defined?(Async::HTTP) &&
+      Gem::Version.new(Async::HTTP::VERSION) >= Gem::Version.new('0.59.0') &&
+      !defined?(Traces::Backend::NewRelic) # defined in the traces-backend-newrelic gem
   end
 
   executes do

lib/new_relic/agent/instrumentation/grpc_server.rb

@@ -14,7 +14,7 @@
   end
 
   executes do
-    supportability_name = NewRelic::Agent::Instrumentation::GRPC::Client::INSTRUMENTATION_NAME
+    supportability_name = NewRelic::Agent::Instrumentation::GRPC::Server::INSTRUMENTATION_NAME
     if use_prepend?
       prepend_instrument GRPC::RpcServer, NewRelic::Agent::Instrumentation::GRPC::Server::RpcServerPrepend, supportability_name
       prepend_instrument GRPC::RpcDesc, NewRelic::Agent::Instrumentation::GRPC::Server::RpcDescPrepend, supportability_name

lib/new_relic/agent/instrumentation/view_component/instrumentation.rb

@@ -15,7 +15,8 @@ def render_in_with_tracing(*args)
         )
         yield
       rescue => e
-        ::NewRelic::Agent.logger.debug('Error capturing ViewComponent segment', e)
+        NewRelic::Agent.notice_error(e)
+        raise
       ensure
         segment&.finish
       end

lib/new_relic/version.rb

@@ -7,7 +7,7 @@ module NewRelic
   module VERSION # :nodoc:
     MAJOR = 9
     MINOR = 7
-    TINY = 0
+    TINY = 1
 
     STRING = "#{MAJOR}.#{MINOR}.#{TINY}"
   end

lib/tasks/instrumentation_generator/instrumentation.thor

@@ -82,7 +82,7 @@ class Instrumentation < Thor
     insert_into_file(
       DEFAULT_SOURCE_LOCATION,
       config_block(name.downcase),
-      after: ":description => 'Controls auto-instrumentation of bunny at start-up.  May be one of [auto|prepend|chain|disabled].'
+      after: ":description => 'Controls auto-instrumentation of bunny at start-up. May be one of: `auto`, `prepend`, `chain`, `disabled`.'
         },\n"
     )
   end

lib/tasks/instrumentation_generator/templates/chain.tt

@@ -9,7 +9,6 @@ module NewRelic::Agent::Instrumentation
         include NewRelic::Agent::Instrumentation::<%= @class_name %>
 
         alias_method(:<%= @method.downcase %>_without_new_relic, :<%= @method.downcase %>)
-        alias_method(:<%= @method.downcase %>, :<%= @method.downcase %>_with_new_relic)
 
         def <%= @method.downcase %><%= "(#{@args})" unless @args.empty? %>
           <%= @method.downcase %>_with_new_relic<%= "(#{@args})" unless @args.empty? %> do

lib/tasks/instrumentation_generator/templates/chain_method.tt

@@ -1,5 +1,4 @@
 alias_method(:<%= @method.downcase %>_without_new_relic, :<%= @method.downcase %>)
-alias_method(:<%= @method.downcase %>, :<%= @method.downcase %>_with_new_relic)
 
 def <%= @method.downcase %><%= "(#{@args})" unless @args.empty? %>
   <%= @method.downcase %>_with_new_relic<%= "(#{@args})" unless @args.empty? %> do

test/multiverse/suites/deferred_instrumentation/Envfile

@@ -5,7 +5,8 @@
 instrumentation_methods :chain, :prepend
 
 SINATRA_VERSIONS = [
-  [nil, 2.4],
+  [nil, 2.7],
+  ['3.2.0', 2.6],
   ['2.1.0', 2.4],
   ['1.4.8']
 ]
@@ -14,9 +15,11 @@ def gem_list(sinatra_version = nil)
   <<~RB
     gem 'sinatra'#{sinatra_version}, :require => false
     gem 'rack-test', '>= 0.8.0', :require => 'rack/test'
+    #{"gem 'rackup'" if sinatra_version.nil? || sinatra_version > '3.2.0'}
 
   RB
 end
 
+
 create_gemfiles(SINATRA_VERSIONS)
 

test/multiverse/suites/padrino/Envfile

@@ -9,7 +9,7 @@ gemfile <<~RB
   gem 'activesupport'
   gem 'padrino'
   gem 'rack-test'
-  gem 'sinatra'
+  gem 'sinatra', '< 4'
 RB
 
 # Sinatra <3

test/multiverse/suites/resque/Envfile

@@ -10,7 +10,9 @@ RESQUE_VERSIONS = [
 
 def gem_list(resque_version = nil)
   <<~RB
-  gem 'resque'#{resque_version}
+    gem 'resque'#{resque_version}
+    #{"gem 'rackup'" if RUBY_VERSION >= '2.7.8'}
+
 
   RB
 end

test/multiverse/suites/sinatra/Envfile

@@ -5,14 +5,15 @@
 instrumentation_methods :chain, :prepend
 
 SINATRA_VERSIONS = [
-  [nil, 2.4],
+  [nil, 2.7],
+  ['3.2.0', 2.6],
   ['2.0.0', 2.4]
 ]
 
 def gem_list(sinatra_version = nil)
   <<~RB
     gem 'sinatra'#{sinatra_version}
-    gem 'rack', '~> 2.2'
+    gem 'rack'#{", '~> 2.2'" if !sinatra_version.nil? && sinatra_version < '4.0.0'}
     gem 'rack-test', '>= 0.8.0', :require => 'rack/test'
 
   RB

test/multiverse/suites/view_component/view_component_instrumentation_test.rb

@@ -51,4 +51,10 @@ def test_metric_path_falsey
   def test_metric_path_unknown_file_pattern
     assert(FAKE_CLASS.metric_path('nothing_to_see_here'), 'unknown')
   end
+
+  def test_error_raised
+    NewRelic::Agent::Tracer.stub(:start_segment, proc { |_args| raise 'kaboom' }) do
+      assert_equal(500, get('/view_components'))
+    end
+  end
 end

test/new_relic/agent/custom_event_aggregator_test.rb

@@ -69,6 +69,34 @@ def test_record_by_default_limit
       assert_equal(max_samples, results.size)
     end
 
+    def test_max_attribute_count
+      attributes = {}
+      70.times do |i|
+        attributes["key#{i}"] = "value#{i}"
+      end
+      @aggregator.record(:footype, attributes)
+
+      event = @aggregator.harvest![1].first
+
+      assert_equal(64, event[1].size)
+    end
+
+    def test_truncates_attributes
+      params = {
+        "#{'b' * 300}" => 'a' * 5000
+      }
+
+      expected = {
+        "#{'b' * 255}" => 'a' * 4095
+      }
+
+      @aggregator.record(:footype, params)
+
+      actual = @aggregator.harvest![1].first[1]
+
+      assert_equal(expected, actual)
+    end
+
     def test_lowering_limit_truncates_buffer
       orig_max_samples = NewRelic::Agent.config[:'custom_insights_events.max_samples_stored']