Browse Source

feat: Added support for derived event attributes (#12009)

This feeds `title` and `location` into the event payload and in the process into snuba.

The UI will pick up this data in some places now instead of message.
Armin Ronacher 6 years ago
parent
commit
e0d7b7137a

+ 9 - 0
src/sentry/api/serializers/models/event.py

@@ -225,6 +225,9 @@ class EventSerializer(Serializer):
             'dist': obj.dist,
             # See GH-3248
             'message': message,
+            'title': obj.title,
+            'location': obj.location,
+            'culprit': obj.culprit,
             'user': attrs['user'],
             'contexts': attrs['contexts'],
             'crashFile': attrs['crash_file'],
@@ -296,6 +299,9 @@ class SnubaEvent(object):
         'event_id',
         'project_id',
         'message',
+        'title',
+        'location',
+        'culprit',
         'user_id',
         'username',
         'ip_address',
@@ -334,6 +340,9 @@ class SnubaEventSerializer(Serializer):
             'eventID': six.text_type(obj.event_id),
             'projectID': six.text_type(obj.project_id),
             'message': obj.message,
+            'title': obj.title,
+            'location': obj.location,
+            'culprit': obj.culprit,
             'dateCreated': obj.timestamp,
             'user': {
                 'id': obj.user_id,

+ 2 - 0
src/sentry/constants.py

@@ -173,6 +173,8 @@ CLIENT_RESERVED_ATTRS = (
 CLIENT_IGNORED_ATTRS = (
     # Internal attributes
     'hashes', 'metadata', 'type', 'key_id', 'project', 'received',
+    # Derived attributes
+    'title', 'location',
     # Deprecated attributes
     'applecrashreport', 'device', 'repos', 'query',
 )

+ 13 - 2
src/sentry/event_manager.py

@@ -852,7 +852,8 @@ class EventManager(object):
         data = self._data
 
         project = Project.objects.get_from_cache(id=project_id)
-        project._organization_cache = Organization.objects.get_from_cache(id=project.organization_id)
+        project._organization_cache = Organization.objects.get_from_cache(
+            id=project.organization_id)
 
         # Check to make sure we're not about to do a bunch of work that's
         # already been done if we've processed an event with this ID. (This
@@ -979,9 +980,19 @@ class EventManager(object):
         event_type = self.get_event_type()
         event_metadata = event_type.get_metadata()
 
+        data['hashes'] = hashes
+
+        # we want to freeze not just the metadata and type in but also the
+        # derived attributes.  The reason for this is that we push this
+        # data into kafka for snuba processing and our postprocessing
+        # picks up the data right from the snuba topic.  For most usage
+        # however the data is dynamically overriden by Event.title and
+        # Event.location (See Event.as_dict)
         data['type'] = event_type.key
         data['metadata'] = event_metadata
-        data['hashes'] = hashes
+        data['culprit'] = culprit
+        data['title'] = event_type.to_string(event_metadata)
+        data['location'] = event_type.get_location(event_metadata)
 
         # index components into ``Event.message``
         # See GH-3248

+ 1 - 1
src/sentry/eventstream/kafka/backend.py

@@ -137,7 +137,7 @@ class KafkaEventStream(EventStream):
             'message': event.message,
             'platform': event.platform,
             'datetime': event.datetime,
-            'data': dict(event.data.items()),
+            'data': event.get_raw_data(),
             'primary_hash': primary_hash,
             'retention_days': retention_days,
         }, {

+ 3 - 0
src/sentry/eventtypes/base.py

@@ -19,6 +19,9 @@ class BaseEvent(object):
     def to_string(self, metadata):
         raise NotImplementedError
 
+    def get_location(self, metadata):
+        return None
+
 
 class DefaultEvent(BaseEvent):
     key = 'default'

+ 3 - 0
src/sentry/eventtypes/error.py

@@ -53,3 +53,6 @@ class ErrorEvent(BaseEvent):
             metadata['type'],
             truncatechars(metadata['value'].splitlines()[0], 100),
         )
+
+    def get_location(self, metadata):
+        return metadata.get('filename')

+ 12 - 0
src/sentry/eventtypes/security.py

@@ -24,6 +24,9 @@ class CspEvent(BaseEvent):
     def to_string(self, metadata):
         return metadata['message']
 
+    def get_location(self, metadata):
+        return metadata.get('uri')
+
 
 class HpkpEvent(BaseEvent):
     key = 'hpkp'
@@ -42,6 +45,9 @@ class HpkpEvent(BaseEvent):
     def to_string(self, metadata):
         return metadata['message']
 
+    def get_location(self, metadata):
+        return metadata.get('origin')
+
 
 class ExpectCTEvent(BaseEvent):
     key = 'expectct'
@@ -60,6 +66,9 @@ class ExpectCTEvent(BaseEvent):
     def to_string(self, metadata):
         return metadata['message']
 
+    def get_location(self, metadata):
+        return metadata.get('origin')
+
 
 class ExpectStapleEvent(BaseEvent):
     key = 'expectstaple'
@@ -77,3 +86,6 @@ class ExpectStapleEvent(BaseEvent):
 
     def to_string(self, metadata):
         return metadata['message']
+
+    def get_location(self, metadata):
+        return metadata.get('origin')

+ 34 - 17
src/sentry/models/event.py

@@ -160,14 +160,20 @@ class Event(Model):
 
     @property
     def title(self):
+        # also see event_manager.py which inserts this for snuba
         et = eventtypes.get(self.get_event_type())(self.data)
         return et.to_string(self.get_event_metadata())
 
-    def error(self):
-        warnings.warn('Event.error is deprecated, use Event.title', DeprecationWarning)
-        return self.title
+    @property
+    def culprit(self):
+        # For a while events did not save the culprit
+        return self.data.get('culprit') or self.group.culprit
 
-    error.short_description = _('error')
+    @property
+    def location(self):
+        # also see event_manager.py which inserts this for snuba
+        et = eventtypes.get(self.get_event_type())(self.data)
+        return et.get_location(self.get_event_metadata())
 
     @property
     def real_message(self):
@@ -237,7 +243,12 @@ class Event(Model):
     def dist(self):
         return self.get_tag('sentry:dist')
 
+    def get_raw_data(self):
+        """Returns the internal raw event data dict."""
+        return dict(self.data.items())
+
     def as_dict(self):
+        """Returns the data in normalized form for external consumers."""
         # We use a OrderedDict to keep elements ordered for a potential JSON serializer
         data = OrderedDict()
         data['event_id'] = self.event_id
@@ -261,6 +272,10 @@ class Event(Model):
         if data.get('culprit') is None:
             data['culprit'] = self.group.culprit
 
+        # Override title and location with dynamically generated data
+        data['title'] = self.title
+        data['location'] = self.location
+
         return data
 
     @property
@@ -270,18 +285,19 @@ class Event(Model):
             data_len += len(repr(value))
         return data_len
 
-    # XXX(dcramer): compatibility with plugins
-    def get_level_display(self):
-        warnings.warn(
-            'Event.get_level_display is deprecated. Use Event.tags instead.', DeprecationWarning
-        )
-        return self.group.get_level_display()
-
     @property
     def level(self):
-        warnings.warn('Event.level is deprecated. Use Event.tags instead.', DeprecationWarning)
+        # we might want to move to this:
+        # return LOG_LEVELS_MAP.get(self.get_level_display()) or self.group.level
         return self.group.level
 
+    def get_level_display(self):
+        # we might want to move to this:
+        # return self.get_tag('level') or self.group.get_level_display()
+        return self.group.get_level_display()
+
+    # deprecated accessors
+
     @property
     def logger(self):
         warnings.warn('Event.logger is deprecated. Use Event.tags instead.', DeprecationWarning)
@@ -297,16 +313,17 @@ class Event(Model):
         warnings.warn('Event.server_name is deprecated. Use Event.tags instead.')
         return self.get_tag('server_name')
 
-    @property
-    def culprit(self):
-        warnings.warn('Event.culprit is deprecated. Use Group.culprit instead.')
-        return self.group.culprit
-
     @property
     def checksum(self):
         warnings.warn('Event.checksum is no longer used', DeprecationWarning)
         return ''
 
+    def error(self):
+        warnings.warn('Event.error is deprecated, use Event.title', DeprecationWarning)
+        return self.title
+
+    error.short_description = _('error')
+
     @property
     def transaction(self):
         return self.get_tag('transaction')

+ 4 - 0
src/sentry/models/group.py

@@ -422,6 +422,10 @@ class Group(Model):
         et = eventtypes.get(self.get_event_type())(self.data)
         return et.to_string(self.get_event_metadata())
 
+    def location(self):
+        et = eventtypes.get(self.get_event_type())(self.data)
+        return et.get_location(self.get_event_metadata())
+
     def error(self):
         warnings.warn('Group.error is deprecated, use Group.title', DeprecationWarning)
         return self.title

+ 1 - 1
src/sentry/plugins/bases/issue.py

@@ -70,7 +70,7 @@ class IssueTrackingPlugin(Plugin):
         return '\n'.join(output)
 
     def _get_group_title(self, request, group, event):
-        return event.error()
+        return event.title
 
     def is_configured(self, request, project, **kwargs):
         raise NotImplementedError

Some files were not shown because too many files changed in this diff