Errors Inbox Improvements (#791)

* Errors inbox attributes and tests (#778) * Initial errors inbox commit Co-authored-by: Timothy Pansino <[email protected]> Co-authored-by: Hannah Stepanek <[email protected]> Co-authored-by: Uma Annamalai <[email protected]> * Add enduser.id field * Move validate_error_trace_attributes into validators directory * Add error callback attributes test * Add tests for enduser.id & error.group.name Co-authored-by: Timothy Pansino <[email protected]> * Uncomment code_coverage * Drop commented out line --------- Co-authored-by: Timothy Pansino <[email protected]> Co-authored-by: Hannah Stepanek <[email protected]> Co-authored-by: Uma Annamalai <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Timothy Pansino <[email protected]> * Error Group Callback API (#785) * Error group initial implementation * Rewrite error callback to pass map of info * Fixed incorrect validators causing errors Co-authored-by: Uma Annamalai <[email protected]> Co-authored-by: Hannah Stepanek <[email protected]> * Fix validation of error trace attributes * Expanded error callback test * Add incorrect type to error callback testing * Change error group callback to private setting * Add testing for error group callback inputs * Separate error group callback tests * Add explicit testing for the set API * Ensure error group is string * Fix python 2 type validation --------- Co-authored-by: Uma Annamalai <[email protected]> Co-authored-by: Hannah Stepanek <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> * User Tracking for Errors Inbox (#789) * Add user tracking feature for errors inbox. * Address review comments, * Add high_security test. * Cleanup invalid tests test. * Update user_id string check. * Remove set_id outside txn test. --------- Co-authored-by: Timothy Pansino <[email protected]> --------- Co-authored-by: Lalleh Rafeei <[email protected]> Co-authored-by: Timothy Pansino <[email protected]> Co-authored-by: Hannah Stepanek <[email protected]> Co-authored-by: Uma Annamalai <[email protected]> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Uma Annamalai <[email protected]>
newrelic · Mar 30, 2023 · 107c0a6 · 107c0a6
1 parent 637879a
commit 107c0a6
Show file tree

Hide file tree

Showing 15 changed files with 778 additions and 261 deletions.
diff --git a/newrelic/agent.py b/newrelic/agent.py
@@ -155,7 +155,9 @@ def __asgi_application(*args, **kwargs):
 from newrelic.api.profile_trace import ProfileTraceWrapper as __ProfileTraceWrapper
 from newrelic.api.profile_trace import profile_trace as __profile_trace
 from newrelic.api.profile_trace import wrap_profile_trace as __wrap_profile_trace
+from newrelic.api.settings import set_error_group_callback as __set_error_group_callback
 from newrelic.api.supportability import wrap_api_call as __wrap_api_call
+from newrelic.api.transaction import set_user_id as __set_user_id
 from newrelic.api.transaction_name import (
     TransactionNameWrapper as __TransactionNameWrapper,
 )
@@ -223,6 +225,8 @@ def __asgi_application(*args, **kwargs):
 get_linking_metadata = __wrap_api_call(__get_linking_metadata, "get_linking_metadata")
 add_custom_span_attribute = __wrap_api_call(__add_custom_span_attribute, "add_custom_span_attribute")
 current_transaction = __wrap_api_call(__current_transaction, "current_transaction")
+set_user_id = __wrap_api_call(__set_user_id, "set_user_id")
+set_error_group_callback = __wrap_api_call(__set_error_group_callback, "set_error_group_callback")
 set_transaction_name = __wrap_api_call(__set_transaction_name, "set_transaction_name")
 end_of_transaction = __wrap_api_call(__end_of_transaction, "end_of_transaction")
 set_background_task = __wrap_api_call(__set_background_task, "set_background_task")

diff --git a/newrelic/api/settings.py b/newrelic/api/settings.py
@@ -12,16 +12,42 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import logging
+
 import newrelic.core.config
 
 settings = newrelic.core.config.global_settings
 
+_logger = logging.getLogger(__name__)
+
+
 RECORDSQL_OFF = 'off'
 RECORDSQL_RAW = 'raw'
 RECORDSQL_OBFUSCATED = 'obfuscated'
 
 COMPRESSED_CONTENT_ENCODING_DEFLATE = 'deflate'
 COMPRESSED_CONTENT_ENCODING_GZIP = 'gzip'
 
-STRIP_EXCEPTION_MESSAGE = ("Message removed by New Relic "
-        "'strip_exception_messages' setting")
+STRIP_EXCEPTION_MESSAGE = ("Message removed by New Relic 'strip_exception_messages' setting")
+
+
+def set_error_group_callback(callback, application=None):
+    """Set the current callback to be used to determine error groups."""
+    from newrelic.api.application import application_instance
+
+    if callback is not None and not callable(callback):
+        _logger.error("Error group callback must be a callable, or None to unset this setting.")
+        return
+
+    # Check for activated application if it exists and was not given.
+    application = application_instance(activate=False) if application is None else application
+
+    # Get application settings if it exists, or fallback to global settings object
+    _settings = application.settings if application is not None else settings()
+
+    if _settings is None:
+        _logger.error("Failed to set error_group_callback in application settings. Report this issue to New Relic support.")
+        return
+
+    if _settings.error_collector:
+        _settings.error_collector._error_group_callback = callback
diff --git a/newrelic/api/time_trace.py b/newrelic/api/time_trace.py
@@ -30,6 +30,8 @@
 from newrelic.core.config import is_expected_error, should_ignore_error
 from newrelic.core.trace_cache import trace_cache
 
+from newrelic.packages import six
+
 _logger = logging.getLogger(__name__)
 
 
@@ -255,13 +257,15 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
         if getattr(value, "_nr_ignored", None):
             return
 
-        module, name, fullnames, message = parse_exc_info((exc, value, tb))
+        module, name, fullnames, message_raw = parse_exc_info((exc, value, tb))
         fullname = fullnames[0]
 
         # Check to see if we need to strip the message before recording it.
 
         if settings.strip_exception_messages.enabled and fullname not in settings.strip_exception_messages.allowlist:
             message = STRIP_EXCEPTION_MESSAGE
+        else:
+            message = message_raw
 
         # Where expected or ignore are a callable they should return a
         # tri-state variable with the following behavior.
@@ -344,7 +348,7 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
             is_expected = is_expected_error(exc_info, status_code=status_code, settings=settings)
 
         # Record a supportability metric if error attributes are being
-        # overiden.
+        # overridden.
         if "error.class" in self.agent_attributes:
             transaction._record_supportability("Supportability/SpanEvent/Errors/Dropped")
 
@@ -353,19 +357,31 @@ def _observe_exception(self, exc_info=None, ignore=None, expected=None, status_c
         self._add_agent_attribute("error.message", message)
         self._add_agent_attribute("error.expected", is_expected)
 
-        return fullname, message, tb, is_expected
+        return fullname, message, message_raw, tb, is_expected
 
     def notice_error(self, error=None, attributes=None, expected=None, ignore=None, status_code=None):
         attributes = attributes if attributes is not None else {}
 
+        # If no exception details provided, use current exception.
+
+        # Pull from sys.exc_info if no exception is passed
+        if not error or None in error:
+            error = sys.exc_info()
+
+            # If no exception to report, exit
+            if not error or None in error:
+                return
+
+        exc, value, tb = error
+
         recorded = self._observe_exception(
             error,
             ignore=ignore,
             expected=expected,
             status_code=status_code,
         )
         if recorded:
-            fullname, message, tb, is_expected = recorded
+            fullname, message, message_raw, tb, is_expected = recorded
             transaction = self.transaction
             settings = transaction and transaction.settings
 
@@ -392,16 +408,45 @@ def notice_error(self, error=None, attributes=None, expected=None, ignore=None,
                     )
                     custom_params = {}
 
-            if settings and settings.code_level_metrics and settings.code_level_metrics.enabled:
-                source = extract_code_from_traceback(tb)
-            else:
-                source = None
+            # Extract additional details about the exception
+
+            source = None
+            error_group_name = None
+            if settings:
+                if settings.code_level_metrics and settings.code_level_metrics.enabled:
+                    source = extract_code_from_traceback(tb)
+
+                if settings.error_collector and settings.error_collector.error_group_callback is not None:
+                    try:
+                        # Call callback to obtain error group name
+                        input_attributes = {}
+                        input_attributes.update(transaction._custom_params)
+                        input_attributes.update(attributes)
+                        error_group_name_raw = settings.error_collector.error_group_callback(value, {
+                            "traceback": tb,
+                            "error.class": exc,
+                            "error.message": message_raw,
+                            "error.expected": is_expected,
+                            "custom_params": input_attributes,
+                            "transactionName": getattr(transaction, "name", None),
+                            "response.status": getattr(transaction, "_response_code", None),
+                            "request.method": getattr(transaction, "_request_method", None),
+                            "request.uri": getattr(transaction, "_request_uri", None),
+                        })
+                        if error_group_name_raw:
+                            _, error_group_name = process_user_attribute("error.group.name", error_group_name_raw)
+                            if error_group_name is None or not isinstance(error_group_name, six.string_types):
+                                raise ValueError("Invalid attribute value for error.group.name. Expected string, got: %s" % repr(error_group_name_raw))
+                    except Exception:
+                        _logger.error("Encountered error when calling error group callback:\n%s", "".join(traceback.format_exception(*sys.exc_info())))
+                        error_group_name = None
 
             transaction._create_error_node(
                 settings,
                 fullname,
                 message,
                 is_expected,
+                error_group_name,
                 custom_params,
                 self.guid,
                 tb,
@@ -634,6 +679,7 @@ def get_service_linking_metadata(application=None, settings=None):
     if not settings:
         if application is None:
             from newrelic.api.application import application_instance
+
             application = application_instance(activate=False)
 
         if application is not None:

diff --git a/newrelic/api/transaction.py b/newrelic/api/transaction.py
@@ -46,6 +46,7 @@
     obfuscate,
 )
 from newrelic.core.attribute import (
+    MAX_ATTRIBUTE_LENGTH,
     MAX_LOG_MESSAGE_LENGTH,
     MAX_NUM_USER_ATTRIBUTES,
     create_agent_attributes,
@@ -1547,7 +1548,9 @@ def notice_error(self, error=None, attributes=None, expected=None, ignore=None,
                 status_code=status_code,
             )
 
-    def _create_error_node(self, settings, fullname, message, expected, custom_params, span_id, tb, source):
+    def _create_error_node(
+        self, settings, fullname, message, expected, error_group_name, custom_params, span_id, tb, source
+    ):
         # Only remember up to limit of what can be caught for a
         # single transaction. This could be trimmed further
         # later if there are already recorded errors and would
@@ -1576,9 +1579,8 @@ def _create_error_node(self, settings, fullname, message, expected, custom_param
             span_id=span_id,
             stack_trace=exception_stack(tb),
             custom_params=custom_params,
-            file_name=None,
-            line_number=None,
             source=source,
+            error_group_name=error_group_name,
         )
 
         # TODO: Errors are recorded in time order. If
@@ -1812,6 +1814,21 @@ def add_custom_parameters(items):
     return add_custom_attributes(items)
 
 
+def set_user_id(user_id):
+    transaction = current_transaction()
+
+    if not user_id or not transaction:
+        return
+
+    if not isinstance(user_id, six.string_types):
+        _logger.warning("The set_user_id API requires a string-based user ID.")
+        return
+
+    user_id = truncate(user_id, MAX_ATTRIBUTE_LENGTH)
+
+    transaction._add_agent_attribute("enduser.id", user_id)
+
+
 def add_framework_info(name, version=None):
     transaction = current_transaction()
     if transaction:

diff --git a/newrelic/core/attribute.py b/newrelic/core/attribute.py
@@ -42,46 +42,48 @@
 
 _TRANSACTION_EVENT_DEFAULT_ATTRIBUTES = set(
     (
-        "host.displayName",
-        "request.method",
-        "request.headers.contentType",
-        "request.headers.contentLength",
-        "request.uri",
-        "response.status",
-        "request.headers.accept",
-        "response.headers.contentLength",
-        "response.headers.contentType",
-        "request.headers.host",
-        "request.headers.userAgent",
-        "message.queueName",
-        "message.routingKey",
-        "http.url",
-        "http.statusCode",
-        "aws.requestId",
-        "aws.operation",
         "aws.lambda.arn",
         "aws.lambda.coldStart",
         "aws.lambda.eventSource.arn",
+        "aws.operation",
+        "aws.requestId",
+        "code.filepath",
+        "code.function",
+        "code.lineno",
+        "code.namespace",
         "db.collection",
         "db.instance",
         "db.operation",
         "db.statement",
+        "enduser.id",
         "error.class",
-        "error.message",
         "error.expected",
-        "peer.hostname",
-        "peer.address",
+        "error.message",
+        "error.group.name",
         "graphql.field.name",
         "graphql.field.parentType",
         "graphql.field.path",
         "graphql.field.returnType",
         "graphql.operation.name",
-        "graphql.operation.type",
         "graphql.operation.query",
-        "code.filepath",
-        "code.function",
-        "code.lineno",
-        "code.namespace",
+        "graphql.operation.type",
+        "host.displayName",
+        "http.statusCode",
+        "http.url",
+        "message.queueName",
+        "message.routingKey",
+        "peer.address",
+        "peer.hostname",
+        "request.headers.accept",
+        "request.headers.contentLength",
+        "request.headers.contentType",
+        "request.headers.host",
+        "request.headers.userAgent",
+        "request.method",
+        "request.uri",
+        "response.headers.contentLength",
+        "response.headers.contentType",
+        "response.status",
     )
 )
 

diff --git a/newrelic/core/config.py b/newrelic/core/config.py
@@ -138,7 +138,9 @@ class TransactionTracerAttributesSettings(Settings):
 
 
 class ErrorCollectorSettings(Settings):
-    pass
+    @property
+    def error_group_callback(self):
+        return self._error_group_callback
 
 
 class ErrorCollectorAttributesSettings(Settings):
@@ -698,6 +700,7 @@ def default_host(license_key):
 _settings.error_collector.ignore_status_codes = _parse_status_codes("100-102 200-208 226 300-308 404", set())
 _settings.error_collector.expected_classes = []
 _settings.error_collector.expected_status_codes = set()
+_settings.error_collector._error_group_callback = None
 _settings.error_collector.attributes.enabled = True
 _settings.error_collector.attributes.exclude = []
 _settings.error_collector.attributes.include = []

diff --git a/newrelic/core/error_node.py b/newrelic/core/error_node.py
@@ -24,8 +24,7 @@
         "span_id",
         "stack_trace",
         "custom_params",
-        "file_name",
-        "line_number",
         "source",
+        "error_group_name",
     ],
 )