Browse Source

Accept events with invalid tag key-value pairs
Store event processing errors, only working with tags at this time

David Burke 4 years ago
parent
commit
bc22dfb8d1
5 changed files with 136 additions and 8 deletions
  1. 49 0
      events/fields.py
  2. 23 0
      events/migrations/0002_auto_20210122_1836.py
  3. 8 1
      events/models.py
  4. 35 7
      events/serializers.py
  5. 21 0
      events/tests/tests.py

+ 49 - 0
events/fields.py

@@ -0,0 +1,49 @@
+from collections import OrderedDict
+from typing import List
+from rest_framework import serializers
+from rest_framework.exceptions import ValidationError, ErrorDetail
+
+
+class ErrorValueDetail(ErrorDetail):
+    """Extended ErrorDetail with validation value"""
+
+    value = None
+
+    def __new__(cls, string, code=None, value=None):
+        self = super().__new__(cls, string, code)
+        self.value = value
+        return self
+
+    def __repr__(self):
+        return "ErrorDetail(string=%r, code=%r, value=%r)" % (
+            str(self),
+            self.code,
+            self.value,
+        )
+
+
+class GenericField(serializers.Field):
+    def to_internal_value(self, data):
+        return data
+
+
+class ForgivingHStoreField(serializers.HStoreField):
+    def run_child_validation(self, data):
+        result = {}
+        errors = OrderedDict()
+
+        for key, value in data.items():
+            key = str(key)
+
+            try:
+                result[key] = self.child.run_validation(value)
+            except ValidationError as e:
+                details: List[ErrorValueDetail] = []
+                for detail in e.detail:
+                    details.append(ErrorValueDetail(str(detail), detail.code, value))
+                errors[key] = details
+
+        if errors:
+            handled_errors = self.context.get("handled_errors", {})
+            self.context["handled_errors"] = handled_errors | {self.field_name: errors}
+        return result

+ 23 - 0
events/migrations/0002_auto_20210122_1836.py

@@ -0,0 +1,23 @@
+# Generated by Django 3.1.5 on 2021-01-22 18:36
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('events', '0001_squashed_0003_auto_20210116_2110'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='event',
+            name='errors',
+            field=models.JSONField(blank=True, help_text='Event processing errors from event intake, including validation errors', null=True),
+        ),
+        migrations.AlterField(
+            model_name='event',
+            name='data',
+            field=models.JSONField(help_text='General event data that is searchable'),
+        ),
+    ]

+ 8 - 1
events/models.py

@@ -38,7 +38,12 @@ class Event(models.Model):
     )
 
     created = models.DateTimeField(auto_now_add=True, db_index=True)
-    data = models.JSONField()
+    data = models.JSONField(help_text="General event data that is searchable")
+    errors = models.JSONField(
+        null=True,
+        blank=True,
+        help_text="Event processing errors from event intake, including validation errors",
+    )
     tags = HStoreField(default=dict)
     release = models.ForeignKey(
         "releases.Release", blank=True, null=True, on_delete=models.SET_NULL
@@ -69,6 +74,8 @@ class Event(models.Model):
         event["event_id"] = self.event_id_hex
         event["project"] = self.issue.project_id
         event["tags"] = self.tags.items()
+        if self.errors:
+            event["errors"] = self.errors
         if self.timestamp:
             event["datetime"] = self.timestamp.isoformat().replace("+00:00", "Z")
         if self.release:

+ 35 - 7
events/serializers.py

@@ -1,6 +1,6 @@
 from typing import Dict, List, Tuple, Union
 from urllib.parse import urlparse
-from django.db import transaction, connection
+from django.db import transaction
 from django.db.utils import IntegrityError
 from ipware import get_client_ip
 from anonymizeip import anonymize_ip
@@ -14,6 +14,7 @@ from environments.models import Environment
 from releases.models import Release
 from glitchtip.serializers import FlexibleDateTimeField
 from .models import Event
+from .fields import GenericField, ForgivingHStoreField
 from .event_tag_processors import TAG_PROCESSORS
 from .event_context_processors import EVENT_CONTEXT_PROCESSORS
 
@@ -49,6 +50,21 @@ def sanitize_bad_postgres_json(data: Union[str, dict, list]):
     return data
 
 
+# class ForgivingSerializer(serializers.Serializer):
+#     """ Allows handled validation errors and tracks them """
+
+#     handled_errors = []
+
+#     def is_valid(self, *args, **kwargs):
+#         valid = super().is_valid(*args, **kwargs)
+#         if valid:
+#             import ipdb
+
+#             ipdb.set_trace()
+#             print("do this stuff")
+#         return valid
+
+
 class RequestSerializer(serializers.Serializer):
     env = serializers.DictField(
         child=serializers.CharField(allow_blank=True), required=False
@@ -60,11 +76,6 @@ class RequestSerializer(serializers.Serializer):
     query_string = serializers.CharField(required=False, allow_blank=True)
 
 
-class GenericField(serializers.Field):
-    def to_internal_value(self, data):
-        return data
-
-
 class BreadcrumbsSerializer(BaseBreadcrumbsSerializer):
     timestamp = GenericField(required=False)
 
@@ -92,7 +103,7 @@ class SentrySDKEventSerializer(BaseSerializer):
     """ Represents events coming from a OSS sentry SDK client """
 
     breadcrumbs = serializers.JSONField(required=False)
-    tags = serializers.DictField(child=serializers.CharField(), required=False)
+    tags = ForgivingHStoreField(required=False)
     event_id = serializers.UUIDField()
     extra = serializers.JSONField(required=False)
     request = RequestSerializer(required=False)
@@ -274,10 +285,27 @@ class StoreDefaultSerializer(SentrySDKEventSerializer):
             if user:
                 json_data["user"] = user
 
+            errors = None
+            handled_errors = self.context.get("handled_errors")
+            if handled_errors:
+                errors = []
+                for field_name, field_error in handled_errors.items():
+                    for field_errors in field_error.values():
+                        for error in field_errors:
+                            errors.append(
+                                {
+                                    "reason": str(error),
+                                    "type": error.code,
+                                    "name": field_name,
+                                    "value": error.value,
+                                }
+                            )
+
             params = {
                 "event_id": data["event_id"],
                 "issue": issue,
                 "tags": tags,
+                "errors": errors,
                 "timestamp": data.get("timestamp"),
                 "data": sanitize_bad_postgres_json(json_data),
                 "release": release,

+ 21 - 0
events/tests/tests.py

@@ -231,3 +231,24 @@ class EventStoreTestCase(APITestCase):
             "the value", tuple(event_json.get("tags"))[1],
         )
 
+    def test_client_tags_invalid(self):
+        """ Invalid tags should not be saved. But should not error. """
+        with open("events/test_data/py_hi_event.json") as json_file:
+            data = json.load(json_file)
+        data["tags"] = {
+            "value": "valid value",
+            "my invalid tag key": {"oh": "this is invalid"},
+        }
+        res = self.client.post(self.url, data, format="json")
+        event = Event.objects.first()
+        self.assertEqual(res.status_code, 200)
+        self.assertTrue(event)
+        event_json = event.event_json()
+        tags = tuple(event_json.get("tags"))
+        self.assertIn(
+            "valid value", tags[0],
+        )
+        for tag in tags:
+            self.assertNotIn("this is invalid", tag)
+        self.assertEqual(len(event_json.get("errors")), 1)
+