Add iterable logic to notice_error (#216)

* Add iterable behavior to notice_error

* Remove extra logic from record_exception.

* Add testing and fix outside_transaction tests

* Correct missing app_name parameter.

* Fix missing status code in test.

Author: TimPansino

newrelic/api/application.py

@@ -13,6 +13,7 @@
 # limitations under the License.
 
 import threading
+import warnings
 
 import newrelic.core.config
 import newrelic.core.agent
@@ -113,13 +114,13 @@ def link_to_application(self, name):
 
     def record_exception(self, exc=None, value=None, tb=None, params={},
             ignore_errors=[]):
-        # Deprecated, but deprecation warning are handled by underlying function calls
+        # Deprecation Warning
+        warnings.warn((
+            'The record_exception function is deprecated. Please use the '
+            'new api named notice_error instead.'
+        ), DeprecationWarning)
 
-        if not self.active:
-            return
-
-        self._agent.record_exception(self._name, exc, value, tb, params,
-                ignore_errors)
+        self.notice_error(error=(exc, value, tb), attributes=params, ignore=ignore_errors)
 
     def notice_error(self, error=None, attributes={}, expected=None, ignore=None, status_code=None):
         if not self.active:

newrelic/api/time_trace.py

@@ -216,16 +216,16 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
 
         # If no exception details provided, use current exception.
 
-        if exc_info and None not in exc_info:
-            exc, value, tb = exc_info
-        else:
-            exc, value, tb = sys.exc_info()
-
-        # Has to be an error to be logged.
+        # Pull from sys.exc_info if no exception is passed
+        if not exc_info or None in exc_info:
+            exc_info = sys.exc_info()
 
-        if exc is None or value is None or tb is None:
+        # If no exception to report, exit
+        if not exc_info or None in exc_info:
             return
 
+        exc, value, tb = exc_info
+
         module, name, fullnames, message = parse_exc_info((exc, value, tb))
         fullname = fullnames[0]
 
@@ -246,7 +246,8 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
         # 1. function parameter override as bool
         # 2. function parameter callable
         # 3. callable on transaction
-        # 4. default rule matching from settings
+        # 4. function parameter iterable of class names
+        # 5. default rule matching from settings
 
         should_ignore = None
         is_expected = None
@@ -270,9 +271,17 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
             if should_ignore:
                 return
 
+        # List of class names
+        if should_ignore is None and ignore is not None and not callable(ignore):
+            # Do not set should_ignore to False
+            # This should cascade into default settings rule matching
+            for name in fullnames:
+                if name in ignore:
+                    return
+
         # Default rule matching
         if should_ignore is None:
-            should_ignore = should_ignore_error((exc, value, tb), status_code=status_code)
+            should_ignore = should_ignore_error(exc_info, status_code=status_code)
             if should_ignore:
                 return
 
@@ -285,13 +294,18 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
         if is_expected is None and callable(expected):
             is_expected = expected(exc, value, tb)
 
-        # Callable on transaction
-        if is_expected is None and hasattr(transaction, '_expected_errors'):
-            is_expected = transaction._expected_errors(exc, value, tb)
+
+        # List of class names
+        if is_expected is None and expected is not None and not callable(expected):
+            # Do not set is_expected to False
+            # This should cascade into default settings rule matching
+            for name in fullnames:
+                if name in expected:
+                    is_expected = True
 
         # Default rule matching
         if is_expected is None:
-            is_expected = is_expected_error((exc, value, tb), status_code=status_code)
+            is_expected = is_expected_error(exc_info, status_code=status_code)
 
         # Record a supportability metric if error attributes are being
         # overiden.
@@ -343,49 +357,14 @@ def notice_error(self, error=None, attributes={}, expected=None, ignore=None, st
                     settings, fullname, message, is_expected, custom_params, self.guid, tb)
 
 
-    def record_exception(self, exc_info=None,
-                         params={}, ignore_errors=[]):
+    def record_exception(self, exc_info=None, params={}, ignore_errors=[]):
         # Deprecation Warning
         warnings.warn((
             'The record_exception function is deprecated. Please use the '
             'new api named notice_error instead.'
         ), DeprecationWarning)
 
-        transaction = self.transaction
-
-        # Pull from sys.exc_info if no exception is passed
-        if not exc_info or None in exc_info:
-            exc_info = sys.exc_info()
-
-        # If no exception to report, exit
-        if not exc_info or None in exc_info:
-            return
-
-        exc, value, tb = exc_info
-
-        # Check ignore_errors callables
-        # We check these here separatly from the notice_error implementation
-        # to preserve previous functionality in precedence
-        should_ignore = None
-        
-        if hasattr(transaction, '_ignore_errors'):
-            should_ignore = transaction._ignore_errors(exc, value, tb)
-            if should_ignore:
-                return
-
-        if callable(ignore_errors):
-            should_ignore = ignore_errors(exc, value, tb)
-            if should_ignore:
-                return
-
-        # Check ignore_errors iterables
-        if should_ignore is None and not callable(ignore_errors):
-            _, _, fullnames, _ = parse_exc_info(exc_info)
-            for fullname in fullnames:
-                if fullname in ignore_errors:
-                    return
-
-        self.notice_error(error=exc_info, attributes=params, ignore=should_ignore)
+        self.notice_error(error=exc_info, attributes=params, ignore=ignore_errors)
 
     def _add_agent_attribute(self, key, value):
         self.agent_attributes[key] = value
@@ -590,19 +569,16 @@ def get_linking_metadata():
 
 def record_exception(exc=None, value=None, tb=None, params={},
         ignore_errors=[], application=None):
-    # Deprecated, but deprecation warning are handled by underlying function calls
-    if application is None:
-        trace = current_trace()
-        if trace:
-            trace.record_exception((exc, value, tb), params,
-                    ignore_errors)
-    else:
-        if application.enabled:
-            application.record_exception(exc, value, tb, params,
-                    ignore_errors)
+        # Deprecation Warning
+        warnings.warn((
+            'The record_exception function is deprecated. Please use the '
+            'new api named notice_error instead.'
+        ), DeprecationWarning)
+
+        notice_error(error=(exc, value, tb), attributes=params, ignore=ignore_errors, application=application)
 
 
-def notice_error(error=None, attributes={}, expected=None, ignore=None, application=None, status_code=None):
+def notice_error(error=None, attributes={}, expected=None, ignore=None, status_code=None, application=None):
     if application is None:
         trace = current_trace()
         if trace:

newrelic/api/transaction.py

@@ -1479,26 +1479,14 @@ def set_transaction_name(self, name, group=None, priority=None):
         self._group = group
         self._name = name
 
-    def record_exception(self, exc=None, value=None, tb=None,
-                         params={}, ignore_errors=[]):
-        # Deprecated, but deprecation warning are handled by underlying function calls
-        settings = self._settings
-
-        if not settings:
-            return
-
-        if not settings.error_collector.enabled:
-            return
-
-        if not settings.collect_errors and not settings.collect_error_events:
-            return
+    def record_exception(self, exc=None, value=None, tb=None, params={}, ignore_errors=[]):
+        # Deprecation Warning
+        warnings.warn((
+            'The record_exception function is deprecated. Please use the '
+            'new api named notice_error instead.'
+        ), DeprecationWarning)
 
-        current_span = trace_cache().current_trace()
-        if current_span:
-            current_span.record_exception(
-                    (exc, value, tb),
-                    params=params,
-                    ignore_errors=ignore_errors)
+        self.notice_error(error=(exc, value, tb), attributes=params, ignore=ignore_errors)
 
     def notice_error(self, error=None, attributes={}, expected=None, ignore=None, status_code=None):
         settings = self._settings

newrelic/core/agent.py

@@ -26,6 +26,7 @@
 import threading
 import atexit
 import traceback
+import warnings
 
 import newrelic
 import newrelic.core.config
@@ -503,15 +504,14 @@ def remove_thread_utilization(self):
         from newrelic.core.thread_utilization import _utilization_trackers
         _utilization_trackers.clear()
 
-    def record_exception(self, app_name, exc=None, value=None, tb=None,
-            params={}, ignore_errors=[]):
-        # Deprecated, but deprecation warning are handled by underlying function calls
+    def record_exception(self, app_name, exc=None, value=None, tb=None, params={}, ignore_errors=[]):
+        # Deprecation Warning
+        warnings.warn((
+            'The record_exception function is deprecated. Please use the '
+            'new api named notice_error instead.'
+        ), DeprecationWarning)
 
-        application = self._applications.get(app_name, None)
-        if application is None or not application.active:
-            return
-
-        application.record_exception(exc, value, tb, params, ignore_errors)
+        self.notice_error(app_name, error=(exc, value, tb), attributes=params, ignore=ignore_errors)
 
     def notice_error(self, app_name, error=None, attributes={}, expected=None, ignore=None, status_code=None):
         application = self._applications.get(app_name, None)

newrelic/core/application.py

@@ -24,6 +24,7 @@
 import os
 import traceback
 import imp
+import warnings
 
 from functools import partial
 
@@ -711,26 +712,18 @@ def remove_data_source(self, name):
 
                 self._data_samplers.remove(data_sampler)
 
-    def record_exception(self, exc=None, value=None, tb=None, params={},
-            ignore_errors=[]):
+    def record_exception(self, exc=None, value=None, tb=None, params={}, ignore_errors=[]):
         """Record a global exception against the application independent
         of a specific transaction.
 
-        Deprecated, but deprecation warning are handled by underlying function calls
         """
+        # Deprecation Warning
+        warnings.warn((
+            'The record_exception function is deprecated. Please use the '
+            'new api named notice_error instead.'
+        ), DeprecationWarning)
 
-        if not self._active_session:
-            return
-
-        with self._stats_lock:
-            # It may still actually be rejected if no exception
-            # supplied or if was in the ignored list. For now
-            # always attempt anyway and also increment the events
-            # count still so that short harvest is extended.
-
-            self._global_events_account += 1
-            self._stats_engine.record_exception(exc, value, tb,
-                    params, ignore_errors)
+        self.notice_error(error=(exc, value, tb), attributes=params, ignore=ignore_errors)
 
     def notice_error(self, error=None, attributes={}, expected=None, ignore=None, status_code=None):
         """Record a global exception against the application independent

newrelic/core/stats_engine.py

@@ -573,32 +573,7 @@ def record_exception(self, exc=None, value=None, tb=None, params={},
             'new api named notice_error instead.'
         ), DeprecationWarning)
 
-        # Pull from sys.exc_info if no exception is passed
-        if None in (exc, value, tb):
-            exc, value, tb = sys.exc_info()
-
-        # If no exception to report, exit
-        if None in (exc, value, tb):
-            return
-
-        # Check ignore_errors callables
-        # We check these here separatly from the notice_error implementation
-        # to preserve previous functionality in precedence
-        should_ignore = None
-
-        if callable(ignore_errors):
-            should_ignore = ignore_errors(exc, value, tb)
-            if should_ignore:
-                return
-
-        # Check ignore_errors iterables
-        if should_ignore is None and not callable(ignore_errors):
-            _, _, fullnames, _ = parse_exc_info((exc, value, tb))
-            for fullname in fullnames:
-                if fullname in ignore_errors:
-                    return
-
-        self.notice_error(error=(exc, value, tb), attributes=params, ignore=should_ignore)
+        self.notice_error(error=(exc, value, tb), attributes=params, ignore=ignore_errors)
 
     def notice_error(self, error=None, attributes={}, expected=None, ignore=None, status_code=None):
         settings = self.__settings
@@ -624,17 +599,7 @@ def notice_error(self, error=None, attributes={}, expected=None, ignore=None, st
 
         exc, value, tb = error
 
-        # If no exception details provided, use current exception.
-
-        if exc is None and value is None and tb is None:
-            exc, value, tb = sys.exc_info()
-
-        # Has to be an error to be logged.
-
-        if exc is None or value is None or tb is None:
-            return
-
-        module, name, fullnames, message = parse_exc_info((exc, value, tb))
+        module, name, fullnames, message = parse_exc_info(error)
         fullname = fullnames[0]
 
         # Check to see if we need to strip the message before recording it.
@@ -653,7 +618,8 @@ def notice_error(self, error=None, attributes={}, expected=None, ignore=None, st
         # Precedence: 
         # 1. function parameter override as bool
         # 2. function parameter callable
-        # 3. default rule matching from settings
+        # 3. function parameter iterable of class names
+        # 4. default rule matching from settings
 
         should_ignore = None
         is_expected = None
@@ -671,9 +637,17 @@ def notice_error(self, error=None, attributes={}, expected=None, ignore=None, st
             if should_ignore:
                 return
 
+        # List of class names
+        if should_ignore is None and ignore is not None and not callable(ignore):
+            # Do not set should_ignore to False
+            # This should cascade into default settings rule matching
+            for name in fullnames:
+                if name in ignore:
+                    return
+
         # Default rule matching
         if should_ignore is None:
-            should_ignore = should_ignore_error((exc, value, tb), status_code=status_code)
+            should_ignore = should_ignore_error(error, status_code=status_code)
             if should_ignore:
                 return
 
@@ -686,9 +660,18 @@ def notice_error(self, error=None, attributes={}, expected=None, ignore=None, st
         if is_expected is None and callable(expected):
             is_expected = expected(exc, value, tb)
 
+
+        # List of class names
+        if is_expected is None and expected is not None and not callable(expected):
+            # Do not set is_expected to False
+            # This should cascade into default settings rule matching
+            for name in fullnames:
+                if name in expected:
+                    is_expected = True
+
         # Default rule matching
         if is_expected is None:
-            is_expected = is_expected_error((exc, value, tb), status_code=status_code)
+            is_expected = is_expected_error(error, status_code=status_code)
 
         # Only add attributes if High Security Mode is off.