Merge pull request #1064 from newrelic/drop-none-attrs

Drop attributes with a value of None

Author: hmstepanek

newrelic/api/transaction.py

@@ -52,12 +52,12 @@
     MAX_NUM_USER_ATTRIBUTES,
     create_agent_attributes,
     create_attributes,
-    create_user_attributes,
     process_user_attribute,
     resolve_logging_context_attributes,
     truncate,
 )
 from newrelic.core.attribute_filter import (
+    DST_ALL,
     DST_ERROR_COLLECTOR,
     DST_NONE,
     DST_TRANSACTION_TRACER,
@@ -1004,7 +1004,7 @@ def _update_agent_attributes(self):
 
     @property
     def user_attributes(self):
-        return create_user_attributes(self._custom_params, self.attribute_filter)
+        return create_attributes(self._custom_params, DST_ALL, self.attribute_filter)
 
     def _compute_sampled_and_priority(self):
         if self._priority is None:

newrelic/core/attribute.py

@@ -110,6 +110,10 @@ class CastingFailureException(Exception):
     pass
 
 
+class NullValueException(ValueError):
+    pass
+
+
 class Attribute(_Attribute):
     def __repr__(self):
         return "Attribute(name=%r, value=%r, destinations=%r)" % (self.name, self.value, bin(self.destinations))
@@ -126,6 +130,13 @@ def create_attributes(attr_dict, destinations, attribute_filter):
 
 
 def create_agent_attributes(attr_dict, attribute_filter):
+    """
+    Returns a dictionary of Attribute objects with appropriate destinations.
+
+    If the attribute's key is in the known list of event attributes, it is assigned
+    to _DESTINATIONS_WITH_EVENTS, otherwise it is assigned to _DESTINATIONS.
+    Note attributes with a value of None are filtered out.
+    """
     attributes = []
 
     for k, v in attr_dict.items():
@@ -143,12 +154,15 @@ def create_agent_attributes(attr_dict, attribute_filter):
 
 
 def resolve_user_attributes(attr_dict, attribute_filter, target_destination, attr_class=dict):
+    """
+    Returns an attr_class of key value attributes filtered to the target_destination.
+
+    process_user_attribute MUST be called before this function to filter out invalid
+    attributes.
+    """
     u_attrs = attr_class()
 
     for attr_name, attr_value in attr_dict.items():
-        if attr_value is None:
-            continue
-
         dest = attribute_filter.apply(attr_name, DST_ALL)
 
         if dest & target_destination:
@@ -201,11 +215,6 @@ def resolve_logging_context_attributes(attr_dict, attribute_filter, attr_prefix,
     return c_attrs
 
 
-def create_user_attributes(attr_dict, attribute_filter):
-    destinations = DST_ALL
-    return create_attributes(attr_dict, destinations, attribute_filter)
-
-
 def truncate(text, maxsize=MAX_ATTRIBUTE_LENGTH, encoding="utf-8", ending=None):
     # Truncate text so that its byte representation
     # is no longer than maxsize bytes.
@@ -285,6 +294,15 @@ def process_user_attribute(name, value, max_length=MAX_ATTRIBUTE_LENGTH, ending=
         _logger.debug("Attribute value cannot be cast to a string. Dropping attribute: %r=%r", name, value)
         return FAILED_RESULT
 
+    except NullValueException:
+        _logger.debug(
+            "Attribute value is None. There is no difference between omitting the key "
+            "and sending None. Dropping attribute: %r=%r",
+            name,
+            value,
+        )
+        return FAILED_RESULT
+
     else:
         # Check length after casting
 
@@ -311,9 +329,18 @@ def sanitize(value):
     Insights. Otherwise, convert value to a string.
 
     Raise CastingFailureException, if str(value) somehow fails.
+    Raise NullValueException, if value is None (null values SHOULD NOT be reported).
     """
 
     valid_value_types = (six.text_type, six.binary_type, bool, float, six.integer_types)
+    # According to the agent spec, agents should not report None attribute values.
+    # There is no difference between omitting the key and sending a None, so we can
+    # reduce the payload size by not sending None values.
+    if value is None:
+        raise NullValueException(
+            "Attribute value is of type: None. Omitting value since there is "
+            "no difference between omitting the key and sending None."
+        )
 
     # When working with numpy, note that numpy has its own `int`s, `str`s,
     # et cetera. `numpy.str_` and `numpy.float_` inherit from Python's native

newrelic/core/node_mixin.py

@@ -13,141 +13,118 @@
 # limitations under the License.
 
 import newrelic.core.attribute as attribute
-
-from newrelic.core.attribute_filter import (DST_SPAN_EVENTS,
-        DST_TRANSACTION_SEGMENTS)
+from newrelic.core.attribute_filter import DST_SPAN_EVENTS, DST_TRANSACTION_SEGMENTS
 
 
 class GenericNodeMixin(object):
     @property
     def processed_user_attributes(self):
-        if hasattr(self, '_processed_user_attributes'):
+        if hasattr(self, "_processed_user_attributes"):
             return self._processed_user_attributes
 
         self._processed_user_attributes = u_attrs = {}
-        user_attributes = getattr(self, 'user_attributes', u_attrs)
+        user_attributes = getattr(self, "user_attributes", u_attrs)
         for k, v in user_attributes.items():
             k, v = attribute.process_user_attribute(k, v)
-            u_attrs[k] = v
+            # Only record the attribute if it passes processing.
+            # Failures return (None, None).
+            if k:
+                u_attrs[k] = v
         return u_attrs
 
     def get_trace_segment_params(self, settings, params=None):
         _params = attribute.resolve_agent_attributes(
-                self.agent_attributes,
-                settings.attribute_filter,
-                DST_TRANSACTION_SEGMENTS)
+            self.agent_attributes, settings.attribute_filter, DST_TRANSACTION_SEGMENTS
+        )
 
         if params:
             _params.update(params)
 
-        _params.update(attribute.resolve_user_attributes(
-                self.processed_user_attributes,
-                settings.attribute_filter,
-                DST_TRANSACTION_SEGMENTS))
+        _params.update(
+            attribute.resolve_user_attributes(
+                self.processed_user_attributes, settings.attribute_filter, DST_TRANSACTION_SEGMENTS
+            )
+        )
 
-        _params['exclusive_duration_millis'] = 1000.0 * self.exclusive
+        _params["exclusive_duration_millis"] = 1000.0 * self.exclusive
         return _params
 
-    def span_event(
-                self,
-                settings,
-                base_attrs=None,
-                parent_guid=None,
-                attr_class=dict):
+    def span_event(self, settings, base_attrs=None, parent_guid=None, attr_class=dict):
         i_attrs = base_attrs and base_attrs.copy() or attr_class()
-        i_attrs['type'] = 'Span'
-        i_attrs['name'] = self.name
-        i_attrs['guid'] = self.guid
-        i_attrs['timestamp'] = int(self.start_time * 1000)
-        i_attrs['duration'] = self.duration
-        i_attrs['category'] = 'generic'
+        i_attrs["type"] = "Span"
+        i_attrs["name"] = self.name
+        i_attrs["guid"] = self.guid
+        i_attrs["timestamp"] = int(self.start_time * 1000)
+        i_attrs["duration"] = self.duration
+        i_attrs["category"] = "generic"
 
         if parent_guid:
-            i_attrs['parentId'] = parent_guid
+            i_attrs["parentId"] = parent_guid
 
         a_attrs = attribute.resolve_agent_attributes(
-                self.agent_attributes,
-                settings.attribute_filter,
-                DST_SPAN_EVENTS,
-                attr_class=attr_class)
+            self.agent_attributes, settings.attribute_filter, DST_SPAN_EVENTS, attr_class=attr_class
+        )
 
         u_attrs = attribute.resolve_user_attributes(
-                self.processed_user_attributes,
-                settings.attribute_filter,
-                DST_SPAN_EVENTS,
-                attr_class=attr_class)
+            self.processed_user_attributes, settings.attribute_filter, DST_SPAN_EVENTS, attr_class=attr_class
+        )
 
         # intrinsics, user attrs, agent attrs
         return [i_attrs, u_attrs, a_attrs]
 
-    def span_events(self,
-            settings, base_attrs=None, parent_guid=None, attr_class=dict):
-
-        yield self.span_event(
-                settings,
-                base_attrs=base_attrs,
-                parent_guid=parent_guid,
-                attr_class=attr_class)
+    def span_events(self, settings, base_attrs=None, parent_guid=None, attr_class=dict):
+        yield self.span_event(settings, base_attrs=base_attrs, parent_guid=parent_guid, attr_class=attr_class)
 
         for child in self.children:
             for event in child.span_events(
-                    settings,
-                    base_attrs=base_attrs,
-                    parent_guid=self.guid,
-                    attr_class=attr_class):
+                settings, base_attrs=base_attrs, parent_guid=self.guid, attr_class=attr_class
+            ):
                 yield event
 
 
 class DatastoreNodeMixin(GenericNodeMixin):
-
     @property
     def name(self):
         product = self.product
         target = self.target
-        operation = self.operation or 'other'
+        operation = self.operation or "other"
 
         if target:
-            name = 'Datastore/statement/%s/%s/%s' % (product, target,
-                    operation)
+            name = "Datastore/statement/%s/%s/%s" % (product, target, operation)
         else:
-            name = 'Datastore/operation/%s/%s' % (product, operation)
+            name = "Datastore/operation/%s/%s" % (product, operation)
 
         return name
 
     @property
     def db_instance(self):
-        if hasattr(self, '_db_instance'):
+        if hasattr(self, "_db_instance"):
             return self._db_instance
 
         db_instance_attr = None
         if self.database_name:
-            _, db_instance_attr = attribute.process_user_attribute(
-                    'db.instance', self.database_name)
+            _, db_instance_attr = attribute.process_user_attribute("db.instance", self.database_name)
 
         self._db_instance = db_instance_attr
         return db_instance_attr
 
     def span_event(self, *args, **kwargs):
-        self.agent_attributes['db.instance'] = self.db_instance
+        self.agent_attributes["db.instance"] = self.db_instance
         attrs = super(DatastoreNodeMixin, self).span_event(*args, **kwargs)
         i_attrs = attrs[0]
         a_attrs = attrs[2]
 
-        i_attrs['category'] = 'datastore'
-        i_attrs['component'] = self.product
-        i_attrs['span.kind'] = 'client'
+        i_attrs["category"] = "datastore"
+        i_attrs["component"] = self.product
+        i_attrs["span.kind"] = "client"
 
         if self.instance_hostname:
-            _, a_attrs['peer.hostname'] = attribute.process_user_attribute(
-                    'peer.hostname', self.instance_hostname)
+            _, a_attrs["peer.hostname"] = attribute.process_user_attribute("peer.hostname", self.instance_hostname)
         else:
-            a_attrs['peer.hostname'] = 'Unknown'
+            a_attrs["peer.hostname"] = "Unknown"
 
-        peer_address = '%s:%s' % (
-                self.instance_hostname or 'Unknown',
-                self.port_path_or_id or 'Unknown')
+        peer_address = "%s:%s" % (self.instance_hostname or "Unknown", self.port_path_or_id or "Unknown")
 
-        _, a_attrs['peer.address'] = attribute.process_user_attribute(
-                'peer.address', peer_address)
+        _, a_attrs["peer.address"] = attribute.process_user_attribute("peer.address", peer_address)
 
         return attrs

newrelic/core/root_node.py

@@ -16,30 +16,39 @@
 
 import newrelic.core.trace_node
 from newrelic.core.node_mixin import GenericNodeMixin
-from newrelic.core.attribute import resolve_user_attributes
 
-from newrelic.packages import six
-
-_RootNode = namedtuple('_RootNode',
-        ['name', 'children', 'start_time', 'end_time', 'exclusive',
-        'duration', 'guid', 'agent_attributes', 'user_attributes',
-        'path', 'trusted_parent_span', 'tracing_vendors',])
+_RootNode = namedtuple(
+    "_RootNode",
+    [
+        "name",
+        "children",
+        "start_time",
+        "end_time",
+        "exclusive",
+        "duration",
+        "guid",
+        "agent_attributes",
+        "user_attributes",
+        "path",
+        "trusted_parent_span",
+        "tracing_vendors",
+    ],
+)
 
 
 class RootNode(_RootNode, GenericNodeMixin):
     def span_event(self, *args, **kwargs):
         span = super(RootNode, self).span_event(*args, **kwargs)
         i_attrs = span[0]
-        i_attrs['transaction.name'] = self.path
-        i_attrs['nr.entryPoint'] = True
+        i_attrs["transaction.name"] = self.path
+        i_attrs["nr.entryPoint"] = True
         if self.trusted_parent_span:
-            i_attrs['trustedParentId'] = self.trusted_parent_span
+            i_attrs["trustedParentId"] = self.trusted_parent_span
         if self.tracing_vendors:
-            i_attrs['tracingVendors'] = self.tracing_vendors
+            i_attrs["tracingVendors"] = self.tracing_vendors
         return span
 
     def trace_node(self, stats, root, connections):
-
         name = self.path
 
         start_time = newrelic.core.trace_node.node_start_time(root, self)
@@ -57,9 +66,5 @@ def trace_node(self, stats, root, connections):
         params = self.get_trace_segment_params(root.settings)
 
         return newrelic.core.trace_node.TraceNode(
-                start_time=start_time,
-                end_time=end_time,
-                name=name,
-                params=params,
-                children=children,
-                label=None)
+            start_time=start_time, end_time=end_time, name=name, params=params, children=children, label=None
+        )

newrelic/core/stats_engine.py

@@ -41,12 +41,12 @@
 from newrelic.core.attribute import (
     MAX_LOG_MESSAGE_LENGTH,
     create_agent_attributes,
-    create_user_attributes,
+    create_attributes,
     process_user_attribute,
     resolve_logging_context_attributes,
     truncate,
 )
-from newrelic.core.attribute_filter import DST_ERROR_COLLECTOR
+from newrelic.core.attribute_filter import DST_ALL, DST_ERROR_COLLECTOR
 from newrelic.core.code_level_metrics import extract_code_from_traceback
 from newrelic.core.config import is_expected_error, should_ignore_error
 from newrelic.core.database_utils import explain_plan
@@ -824,7 +824,7 @@ def notice_error(self, error=None, attributes=None, expected=None, ignore=None,
                 )
                 custom_attributes = {}
 
-            user_attributes = create_user_attributes(custom_attributes, settings.attribute_filter)
+            user_attributes = create_attributes(custom_attributes, DST_ALL, settings.attribute_filter)
 
         # Extract additional details about the exception as agent attributes
         agent_attributes = {}