Merge pull request #1812 from ruby-agent/dev

Merge to release for 5.6.0

.travis.yml

@@ -9,7 +9,9 @@ language: ruby
 sudo: required
 
 before_install:
-  - sudo apt-get update && sudo apt-get install --only-upgrade openssl && sudo apt-get install --only-upgrade libssl-dev
+  # RUBY-2072 Prevent Travis setup failure before our test even starts
+  - sudo rm -f /etc/apt/sources.list.d/travis_ci_zeromq3.list
+
   - gem --version
   - gem update --system
   - ./test/script/before_install/gemstash_mirror.sh
@@ -19,7 +21,11 @@ install: bundle install
 
 addons:
   apt:
+    update: true
     packages:
+      - openssl
+      - libssl-dev
+      - build-essential
       - haveged
 
 before_script: ./test/script/before_script/install_mongodb.sh
@@ -40,6 +46,7 @@ notifications:
 rvm:
   # Run slowest builds first to try and optimize overall cycle time.
   - jruby-9.1.13.0
+  - 2.6.0-preview3
   - 2.5.0
   - 2.4.2
   - 2.3.5
@@ -88,6 +95,24 @@ matrix:
   fast_finish: true
   exclude:
     # Unsupported Rails/Ruby combinations
+    # 2.6
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails21
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails22
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails23
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails30
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails31
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails32
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails40
+    - rvm: 2.6.0-preview3
+      env: TYPE=UNIT ENVIRONMENT=rails41
+
     # 2.5
     - rvm: 2.5.0
       env: TYPE=UNIT ENVIRONMENT=rails21

CHANGELOG.md

@@ -1,5 +1,55 @@
 # New Relic Ruby Agent Release Notes #
 
+## v5.6.0
+
+  * Bugfix for transactions with `ActionController::Live`
+
+    Previously, transactions containing `ActionController::Live` resulted in
+    incorrect calculations of capacity analysis as well as error backtraces
+    appearing in agent logs in agent versions 5.4 and later. The agent now
+    correctly calculates capacity for transactions with `ActionController::Live`.
+
+  * Add ability to exclude attributes from span events and transaction
+    segments
+
+    Agent versions 5.5 and lower could selectively exclude attributes
+    from page views, error traces, transaction traces, and
+    transaction events.  With agent version 5.6 and higher, you can
+    also exclude attributes from span events (via the
+    `span_events.include/exclude` options) and from transaction
+    segments (via the `transaction_segments.include/exclude` options).
+
+    As with other attribute destinations, these new options will
+    inherit values from the top-level `attributes.include/exclude`
+    settings. See the
+    [documentation](https://docs.newrelic.com/docs/agents/ruby-agent/attributes/enabling-disabling-attributes-ruby)
+    for more information.
+
+  * Increasing backoff sequence on failing to connect to New Relic
+
+    If the agent cannot reach New Relic, it will now wait for an
+    increasing amount of time after each failed attempt.  We are also
+    starting with a shorter delay initially, which will help customer
+    apps bounce back more quickly from transient network errors.
+
+  * Truncation of long stack traces
+
+    Previous versions of the agent would truncate long stack traces to
+    50 frames.  To give customers more flexibility, we have added the
+    `error_collector.max_backtrace_frames` configuration option.
+    Thanks to Patrick Tulskie for the contribution!
+
+  * Update link in documentation
+
+    The community forum link in `README.md` now goes to the updated
+    location.  Thanks to Sam Killgallon for the contribution!
+
+  * Active Storage instrumentation
+
+    The agent now provides instrumentation for Active Storage, introduced in
+    Rails 5.2. Customers will see Active Storage operations represented as
+    segments within transaction traces.
+
 ## v5.5.0
 
   * Bugfix for `perform` instrumentation with curb gem

README.md

@@ -127,7 +127,7 @@ You can find more detailed documentation [on our website](http://newrelic.com/do
 and specifically in the [Ruby category](http://newrelic.com/docs/ruby).
 
 If you can't find what you're looking for there, reach out to us on our [support
-site](http://support.newrelic.com/) or our [community forum](http://forum.newrelic.com)
+site](http://support.newrelic.com/) or our [community forum](https://discuss.newrelic.com/)
 and we'll be happy to help you.
 
 Find a bug? Contact us via [support.newrelic.com](http://support.newrelic.com/),

config.dot

@@ -18,6 +18,7 @@ digraph AgentEnabled {
   "[error_collector.capture_source]"
   "[error_collector.enabled]"
   "[error_collector.ignore_errors]"
+  "[error_collector.max_backtrace_frames]"
   "[browser_monitoring.auto_instrument]"
   "[license_key]"
   "[verify_certificate]"

lib/new_relic/agent/agent.rb

@@ -727,12 +727,14 @@ def should_connect?(force=false)
             force || (!connected? && !disconnected?)
           end
 
-          # Retry period is a minute for each failed attempt that
-          # we've made. This should probably do some sort of sane TCP
-          # backoff to prevent hammering the server, but a minute for
-          # each attempt seems to work reasonably well.
+          # Per the spec at
+          # /agents/agent-specs/Collector-Response-Handling.md, retry
+          # connections after a specific backoff sequence to prevent
+          # hammering the server.
+          CONNECT_RETRY_PERIODS = [15, 15, 30, 60, 120, 300]
+
           def connect_retry_period
-            [600, connect_attempts * 60].min
+            CONNECT_RETRY_PERIODS[connect_attempts] || 300
           end
 
           def note_connect_failure

lib/new_relic/agent/attribute_filter.rb

@@ -64,52 +64,91 @@ module Agent
     class AttributeFilter
       DST_NONE = 0x0
 
-      DST_TRANSACTION_EVENTS = 1 << 0
-      DST_TRANSACTION_TRACER = 1 << 1
-      DST_ERROR_COLLECTOR    = 1 << 2
-      DST_BROWSER_MONITORING = 1 << 3
+      DST_TRANSACTION_EVENTS   = 1 << 0
+      DST_TRANSACTION_TRACER   = 1 << 1
+      DST_ERROR_COLLECTOR      = 1 << 2
+      DST_BROWSER_MONITORING   = 1 << 3
+      DST_SPAN_EVENTS          = 1 << 4
+      DST_TRANSACTION_SEGMENTS = 1 << 5
 
-      DST_ALL = 0xF
+      DST_ALL                  = 0x3f
 
       attr_reader :rules
 
       def initialize(config)
         @enabled_destinations = DST_NONE
 
-        @enabled_destinations |= DST_TRANSACTION_TRACER if config[:'transaction_tracer.attributes.enabled']
-        @enabled_destinations |= DST_TRANSACTION_EVENTS if config[:'transaction_events.attributes.enabled']
-        @enabled_destinations |= DST_ERROR_COLLECTOR    if config[:'error_collector.attributes.enabled']
-        @enabled_destinations |= DST_BROWSER_MONITORING if config[:'browser_monitoring.attributes.enabled']
+        @enabled_destinations |= DST_TRANSACTION_TRACER   if config[:'transaction_tracer.attributes.enabled']
+        @enabled_destinations |= DST_TRANSACTION_EVENTS   if config[:'transaction_events.attributes.enabled']
+        @enabled_destinations |= DST_ERROR_COLLECTOR      if config[:'error_collector.attributes.enabled']
+        @enabled_destinations |= DST_BROWSER_MONITORING   if config[:'browser_monitoring.attributes.enabled']
+        @enabled_destinations |= DST_SPAN_EVENTS          if config[:'span_events.attributes.enabled']
+        @enabled_destinations |= DST_TRANSACTION_SEGMENTS if config[:'transaction_segments.attributes.enabled']
 
         @enabled_destinations = DST_NONE unless config[:'attributes.enabled']
 
         @rules = []
 
         build_rule(config[:'attributes.exclude'], DST_ALL, false)
-        build_rule(config[:'transaction_tracer.attributes.exclude'], DST_TRANSACTION_TRACER, false)
-        build_rule(config[:'transaction_events.attributes.exclude'], DST_TRANSACTION_EVENTS, false)
-        build_rule(config[:'error_collector.attributes.exclude'],    DST_ERROR_COLLECTOR,    false)
-        build_rule(config[:'browser_monitoring.attributes.exclude'], DST_BROWSER_MONITORING, false)
+        build_rule(config[:'transaction_tracer.attributes.exclude'],   DST_TRANSACTION_TRACER,   false)
+        build_rule(config[:'transaction_events.attributes.exclude'],   DST_TRANSACTION_EVENTS,   false)
+        build_rule(config[:'error_collector.attributes.exclude'],      DST_ERROR_COLLECTOR,      false)
+        build_rule(config[:'browser_monitoring.attributes.exclude'],   DST_BROWSER_MONITORING,   false)
+        build_rule(config[:'span_events.attributes.exclude'],          DST_SPAN_EVENTS,          false)
+        build_rule(config[:'transaction_segments.attributes.exclude'], DST_TRANSACTION_SEGMENTS, false)
 
         build_rule(['request.parameters.*'], include_destinations_for_capture_params(config[:capture_params]), true)
         build_rule(['job.resque.args.*'],    include_destinations_for_capture_params(config[:'resque.capture_params']), true)
         build_rule(['job.sidekiq.args.*'],   include_destinations_for_capture_params(config[:'sidekiq.capture_params']), true)
 
+        build_rule(['host', 'port_path_or_id'], DST_TRANSACTION_SEGMENTS, config[:'datastore_tracer.instance_reporting.enabled'])
+        build_rule(['database_name'],           DST_TRANSACTION_SEGMENTS, config[:'datastore_tracer.database_name_reporting.enabled'])
+
         build_rule(config[:'attributes.include'], DST_ALL, true)
-        build_rule(config[:'transaction_tracer.attributes.include'], DST_TRANSACTION_TRACER, true)
-        build_rule(config[:'transaction_events.attributes.include'], DST_TRANSACTION_EVENTS, true)
-        build_rule(config[:'error_collector.attributes.include'],    DST_ERROR_COLLECTOR,    true)
-        build_rule(config[:'browser_monitoring.attributes.include'], DST_BROWSER_MONITORING, true)
+        build_rule(config[:'transaction_tracer.attributes.include'],   DST_TRANSACTION_TRACER,   true)
+        build_rule(config[:'transaction_events.attributes.include'],   DST_TRANSACTION_EVENTS,   true)
+        build_rule(config[:'error_collector.attributes.include'],      DST_ERROR_COLLECTOR,      true)
+        build_rule(config[:'browser_monitoring.attributes.include'],   DST_BROWSER_MONITORING,   true)
+        build_rule(config[:'span_events.attributes.include'],          DST_SPAN_EVENTS,          true)
+        build_rule(config[:'transaction_segments.attributes.include'], DST_TRANSACTION_SEGMENTS, true)
+
+        build_uri_rule(config[:'attributes.exclude'])
 
         @rules.sort!
 
         # We're ok to cache high security for fast lookup because the attribute
         # filter is re-generated on any significant config change.
         @high_security = config[:high_security]
 
+        setup_key_cache
         cache_prefix_blacklist
       end
 
+      # Note the key_cache is a global cache, accessible by multiple threads,
+      # but is intentionally left unsynchronized for liveness. Writes will always
+      # involve writing the same boolean value for each key, so there is no
+      # worry of one value clobbering another. For reads, if a value hasn't been
+      # written to the cache yet, the worst that will happen is that it will run
+      # through the filter rules again. Both reads and writes will become
+      # eventually consistent.
+
+      def setup_key_cache
+        destinations = [
+          DST_TRANSACTION_EVENTS,
+          DST_TRANSACTION_TRACER,
+          DST_ERROR_COLLECTOR,
+          DST_BROWSER_MONITORING,
+          DST_SPAN_EVENTS,
+          DST_TRANSACTION_SEGMENTS,
+          DST_ALL
+        ]
+
+        @key_cache = destinations.inject({}) do |memo, destination|
+          memo[destination] = {}
+          memo
+        end
+      end
+
       def include_destinations_for_capture_params(capturing)
         if capturing
           DST_TRANSACTION_TRACER | DST_ERROR_COLLECTOR
@@ -125,6 +164,14 @@ def build_rule(attribute_names, destinations, is_include)
         end
       end
 
+      def build_uri_rule(excluded_attributes)
+        uri_aliases = %w(uri url request_uri request.uri http.url)
+
+        if (excluded_attributes & uri_aliases).size > 0
+          build_rule(uri_aliases - excluded_attributes, DST_ALL, false)
+        end
+      end
+
       def apply(attribute_name, default_destinations)
         return DST_NONE if @enabled_destinations == DST_NONE
 
@@ -148,6 +195,19 @@ def allows?(allowed_destinations, requested_destination)
         allowed_destinations & requested_destination == requested_destination
       end
 
+      def allows_key?(key, destination)
+        return false unless destination & @enabled_destinations == destination
+
+        value = @key_cache[destination][key]
+
+        if value.nil?
+          allowed_destinations = apply(key, destination)
+          @key_cache[destination][key] = allows?(allowed_destinations, destination)
+        else
+          value
+        end
+      end
+
       def high_security?
         @high_security
       end

lib/new_relic/agent/configuration/default_source.rb

@@ -719,6 +719,14 @@ def self.convert_to_constant_list(raw_value)
           :allowed_from_server => false,
           :description => 'If <code>true</code>, disables Action Cable instrumentation.'
         },
+        :disable_active_storage => {
+          :default => false,
+          :public => true,
+          :type => Boolean,
+          :dynamic_name => true,
+          :allowed_from_server => false,
+          :description => 'If <code>true</code>, disables ActiveStorage instrumentation.'
+        },
         :disable_memcached => {
           :default => value_of(:disable_memcache_instrumentation),
           :public => true,
@@ -981,6 +989,13 @@ def self.convert_to_constant_list(raw_value)
           :allowed_from_server => true,
           :description => 'Specify a comma-delimited list of error classes that the agent should ignore.'
         },
+        :'error_collector.max_backtrace_frames' => {
+          :default => 50,
+          :public => true,
+          :type => Integer,
+          :allowed_from_server => false,
+          :description => 'Defines the maximum number of frames in an error backtrace. Backtraces over this amount are truncated at the beginning and end.'
+        },
         :'error_collector.capture_events' => {
           :default => value_of(:'error_collector.enabled'),
           :public => true,
@@ -1505,6 +1520,20 @@ def self.convert_to_constant_list(raw_value)
           :allowed_from_server => false,
           :description => 'If <code>true</code>, the agent captures attributes from browser monitoring.'
         },
+        :'span_events.attributes.enabled' => {
+          :default     => true,
+          :public      => true,
+          :type        => Boolean,
+          :allowed_from_server => false,
+          :description => 'If <code>true</code>, the agent captures attributes on span events.'
+        },
+        :'transaction_segments.attributes.enabled' => {
+          :default     => true,
+          :public      => true,
+          :type        => Boolean,
+          :allowed_from_server => false,
+          :description => 'If <code>true</code>, the agent captures attributes on transaction segments.'
+        },
         :'attributes.exclude' => {
           :default     => [],
           :public      => true,
@@ -1545,6 +1574,22 @@ def self.convert_to_constant_list(raw_value)
           :transform    => DefaultSource.method(:convert_to_list),
           :description => 'Prefix of attributes to exclude from browser monitoring. Allows <code>*</code> as wildcard at end.'
         },
+        :'span_events.attributes.exclude' => {
+          :default     => [],
+          :public      => true,
+          :type        => Array,
+          :allowed_from_server => false,
+          :transform   => DefaultSource.method(:convert_to_list),
+          :description => 'Prefix of attributes to exclude from span events. Allows <code>*</code> as wildcard at end.'
+        },
+        :'transaction_segments.attributes.exclude' => {
+          :default     => [],
+          :public      => true,
+          :type        => Array,
+          :allowed_from_server => false,
+          :transform   => DefaultSource.method(:convert_to_list),
+          :description => 'Prefix of attributes to exclude from transaction segments. Allows <code>*</code> as wildcard at end.'
+        },
         :'attributes.include' => {
           :default     => [],
           :public      => true,
@@ -1585,6 +1630,22 @@ def self.convert_to_constant_list(raw_value)
           :transform    => DefaultSource.method(:convert_to_list),
           :description => 'Prefix of attributes to include in browser monitoring. Allows <code>*</code> as wildcard at end.'
         },
+        :'span_events.attributes.include' => {
+          :default     => [],
+          :public      => true,
+          :type        => Array,
+          :allowed_from_server => false,
+          :transform    => DefaultSource.method(:convert_to_list),
+          :description => 'Prefix of attributes to include on span events. Allows <code>*</code> as wildcard at end.'
+        },
+        :'transaction_segments.attributes.include' => {
+          :default     => [],
+          :public      => true,
+          :type        => Array,
+          :allowed_from_server => false,
+          :transform    => DefaultSource.method(:convert_to_list),
+          :description => 'Prefix of attributes to include on transaction segments. Allows <code>*</code> as wildcard at end.'
+        },
         :'custom_attributes.enabled' => {
           :default     => true,
           :public      => true,