fix: Actually run tests for fast_serialize (getsentry#521)

untitaker · web-flow · commit fb15e130aa3a · 2019-10-02T13:44:33.000+02:00
* fix: Fix typo in conftest

* fix: Fix tests

* fix: Fix some more tests

* fix: Fix celery tests

* fix: Fix subprocess test

* fix: Fix more tests
diff --git a/sentry_sdk/hub.py b/sentry_sdk/hub.py
@@ -301,6 +301,8 @@ def bind_client(
         """Binds a new client to the hub."""
         top = self._stack[-1]
         self._stack[-1] = (new, top[1])
+        if not new or new.options["_experiments"].get("fast_serialize", False):
+            top[1].clear_breadcrumbs()
 
     def capture_event(
         self,
diff --git a/sentry_sdk/integrations/_wsgi_common.py b/sentry_sdk/integrations/_wsgi_common.py
@@ -73,7 +73,9 @@ def extract_into_event(self, event):
         if data is not None:
             request_info["data"] = data
 
-        event["request"] = partial_serialize(client, request_info)
+        event["request"] = partial_serialize(
+            client, request_info, should_repr_strings=False
+        )
 
     def content_length(self):
         # type: () -> int
diff --git a/sentry_sdk/integrations/celery.py b/sentry_sdk/integrations/celery.py
@@ -161,10 +161,12 @@ def _make_event_processor(task, uuid, args, kwargs, request=None):
     # type: (Any, Any, Any, Any, Optional[Any]) -> EventProcessor
     def event_processor(event, hint):
         # type: (Event, Hint) -> Optional[Event]
+        client = Hub.current.client
+
         with capture_internal_exceptions():
             extra = event.setdefault("extra", {})
             extra["celery-job"] = partial_serialize(
-                Hub.current.client,
+                client,
                 {"task_name": task.name, "args": args, "kwargs": kwargs},
                 should_repr_strings=False,
             )
@@ -175,7 +177,11 @@ def event_processor(event, hint):
                     event["fingerprint"] = [
                         "celery",
                         "SoftTimeLimitExceeded",
-                        getattr(task, "name", task),
+                        partial_serialize(
+                            client,
+                            getattr(task, "name", task),
+                            should_repr_strings=False,
+                        ),
                     ]
 
         return event
diff --git a/sentry_sdk/integrations/logging.py b/sentry_sdk/integrations/logging.py
@@ -202,11 +202,16 @@ def _emit(self, record):
 
         hint["log_record"] = record
 
+        client = Hub.current.client
+
         event["level"] = _logging_to_event_level(record.levelname)
         event["logger"] = record.name
-        event["logentry"] = {"message": to_string(record.msg), "params": record.args}
+        event["logentry"] = {
+            "message": to_string(record.msg),
+            "params": partial_serialize(client, record.args, should_repr_strings=False),
+        }
         event["extra"] = partial_serialize(
-            Hub.current.client, _extra_from_record(record), should_repr_strings=False
+            client, _extra_from_record(record), should_repr_strings=False
         )
 
         hub.capture_event(event, hint=hint)
diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py
@@ -108,7 +108,6 @@ def getresponse(self, *args, **kwargs):
             rv = real_getresponse(self, *args, **kwargs)
 
             if data_dict is not None:
-                data_dict["httplib_response"] = rv
                 data_dict["status_code"] = rv.status
                 data_dict["reason"] = rv.reason
         except TypeError:
@@ -200,7 +199,8 @@ def sentry_patched_popen_init(self, *a, **kw):
             env["SUBPROCESS_" + k.upper().replace("-", "_")] = v
 
         with hub.start_span(op="subprocess", description=description) as span:
-            span.set_data("subprocess.cwd", cwd)
+            if cwd:
+                span.set_data("subprocess.cwd", cwd)
 
             rv = old_popen_init(self, *a, **kw)  # type: ignore
 
diff --git a/sentry_sdk/integrations/wsgi.py b/sentry_sdk/integrations/wsgi.py
@@ -278,7 +278,8 @@ def event_processor(event, hint):
 
             if _should_send_default_pii():
                 user_info = event.setdefault("user", {})
-                user_info["ip_address"] = client_ip
+                if client_ip:
+                    user_info["ip_address"] = client_ip
 
             request_info["url"] = request_url
             request_info["query_string"] = query_string
diff --git a/sentry_sdk/tracing.py b/sentry_sdk/tracing.py
@@ -338,13 +338,12 @@ def finish(self, hub=None):
         )
 
     def to_json(self, client):
-        # type: (Optional[sentry_sdk.Client]) -> Any
+        # type: (Optional[sentry_sdk.Client]) -> Dict[str, Any]
         rv = {
             "trace_id": self.trace_id,
             "span_id": self.span_id,
             "parent_span_id": self.parent_span_id,
             "same_process_as_parent": self.same_process_as_parent,
-            "transaction": self.transaction,
             "op": self.op,
             "description": self.description,
             "start_timestamp": partial_serialize(
@@ -356,9 +355,19 @@ def to_json(self, client):
             "timestamp": partial_serialize(
                 client, self.timestamp, is_databag=False, should_repr_strings=False
             ),
-            "tags": self._tags,
-            "data": self._data,
-        }
+        }  # type: Dict[str, Any]
+
+        transaction = self.transaction
+        if transaction:
+            rv["transaction"] = transaction
+
+        tags = self._tags
+        if tags:
+            rv["tags"] = tags
+
+        data = self._data
+        if data:
+            rv["data"] = data
 
         return rv
 
@@ -460,17 +469,11 @@ def _maybe_create_breadcrumbs_from_span(hub, span):
             message=span.description, type="redis", category="redis", data=span._tags
         )
     elif span.op == "http" and span.is_success():
-        hub.add_breadcrumb(
-            type="http",
-            category="httplib",
-            data=span._data,
-            hint={"httplib_response": span._data.pop("httplib_response", None)},
-        )
+        hub.add_breadcrumb(type="http", category="httplib", data=span._data)
     elif span.op == "subprocess":
         hub.add_breadcrumb(
             type="subprocess",
             category="subprocess",
             message=span.description,
             data=span._data,
-            hint={"popen_instance": span._data.pop("popen_instance", None)},
         )
diff --git a/tests/conftest.py b/tests/conftest.py
@@ -178,12 +178,17 @@ def inner(event):
     return inner
 
 
-@pytest.fixture(params=[True, False], ids=["fast_serializer", "default_serializer"])
-def sentry_init(monkeypatch_test_transport, request):
+@pytest.fixture(params=[True, False], ids=["fast_serialize", "default_serialize"])
+def fast_serialize(request):
+    return request.param
+
+
+@pytest.fixture
+def sentry_init(monkeypatch_test_transport, fast_serialize):
     def inner(*a, **kw):
         hub = sentry_sdk.Hub.current
         client = sentry_sdk.Client(*a, **kw)
-        client.options["_experiments"]["fast_serializer"] = request.param
+        client.options["_experiments"]["fast_serialize"] = fast_serialize
         hub.bind_client(client)
         monkeypatch_test_transport(sentry_sdk.Hub.current.client)
 
diff --git a/tests/integrations/bottle/test_bottle.py b/tests/integrations/bottle/test_bottle.py
@@ -117,7 +117,9 @@ def index():
     assert event["exception"]["values"][0]["mechanism"]["handled"] is False
 
 
-def test_large_json_request(sentry_init, capture_events, app, get_client):
+def test_large_json_request(
+    sentry_init, capture_events, app, get_client, fast_serialize
+):
     sentry_init(integrations=[bottle_sentry.BottleIntegration()])
 
     data = {"foo": {"bar": "a" * 2000}}
@@ -140,10 +142,10 @@ def index():
     assert response[1] == "200 OK"
 
     event, = events
-    # __import__("pdb").set_trace()
-    assert event["_meta"]["request"]["data"]["foo"]["bar"] == {
-        "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"]["foo"]["bar"] == {
+            "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
+        }
     assert len(event["request"]["data"]["foo"]["bar"]) == 512
 
 
@@ -171,7 +173,9 @@ def index():
     assert event["request"]["data"] == data
 
 
-def test_medium_formdata_request(sentry_init, capture_events, app, get_client):
+def test_medium_formdata_request(
+    sentry_init, capture_events, app, get_client, fast_serialize
+):
     sentry_init(integrations=[bottle_sentry.BottleIntegration()])
 
     data = {"foo": "a" * 2000}
@@ -191,15 +195,16 @@ def index():
     assert response[1] == "200 OK"
 
     event, = events
-    assert event["_meta"]["request"]["data"]["foo"] == {
-        "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"]["foo"] == {
+            "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
+        }
     assert len(event["request"]["data"]["foo"]) == 512
 
 
 @pytest.mark.parametrize("input_char", [u"a", b"a"])
 def test_too_large_raw_request(
-    sentry_init, input_char, capture_events, app, get_client
+    sentry_init, input_char, capture_events, app, get_client, fast_serialize
 ):
     sentry_init(
         integrations=[bottle_sentry.BottleIntegration()], request_bodies="small"
@@ -226,13 +231,14 @@ def index():
     assert response[1] == "200 OK"
 
     event, = events
-    assert event["_meta"]["request"]["data"] == {
-        "": {"len": 2000, "rem": [["!config", "x", 0, 2000]]}
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"] == {
+            "": {"len": 2000, "rem": [["!config", "x", 0, 2000]]}
+        }
     assert not event["request"]["data"]
 
 
-def test_files_and_form(sentry_init, capture_events, app, get_client):
+def test_files_and_form(sentry_init, capture_events, app, get_client, fast_serialize):
     sentry_init(
         integrations=[bottle_sentry.BottleIntegration()], request_bodies="always"
     )
@@ -256,17 +262,19 @@ def index():
     assert response[1] == "200 OK"
 
     event, = events
-    assert event["_meta"]["request"]["data"]["foo"] == {
-        "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"]["foo"] == {
+            "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
+        }
     assert len(event["request"]["data"]["foo"]) == 512
 
-    assert event["_meta"]["request"]["data"]["file"] == {
-        "": {
-            "len": -1,
-            "rem": [["!raw", "x", 0, -1]],
-        }  # bottle default content-length is -1
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"]["file"] == {
+            "": {
+                "len": -1,
+                "rem": [["!raw", "x", 0, -1]],
+            }  # bottle default content-length is -1
+        }
     assert not event["request"]["data"]["file"]
 
 
diff --git a/tests/integrations/celery/test_celery.py b/tests/integrations/celery/test_celery.py
@@ -131,14 +131,12 @@ def dummy_task(x, y):
     assert execution_event["spans"] == []
     assert submission_event["spans"] == [
         {
-            u"data": {},
             u"description": u"dummy_task",
             u"op": "celery.submit",
             u"parent_span_id": submission_event["contexts"]["trace"]["span_id"],
             u"same_process_as_parent": True,
             u"span_id": submission_event["spans"][0]["span_id"],
             u"start_timestamp": submission_event["spans"][0]["start_timestamp"],
-            u"tags": {},
             u"timestamp": submission_event["spans"][0]["timestamp"],
             u"trace_id": text_type(span.trace_id),
         }
diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py
@@ -341,7 +341,7 @@ def test_transaction_style(
     assert event["transaction"] == expected_transaction
 
 
-def test_request_body(sentry_init, client, capture_events):
+def test_request_body(sentry_init, client, capture_events, fast_serialize):
     sentry_init(integrations=[DjangoIntegration()])
     events = capture_events()
     content, status, headers = client.post(
@@ -354,10 +354,11 @@ def test_request_body(sentry_init, client, capture_events):
 
     assert event["message"] == "hi"
     assert event["request"]["data"] == ""
-    assert event["_meta"]["request"]["data"][""] == {
-        "len": 6,
-        "rem": [["!raw", "x", 0, 6]],
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"][""] == {
+            "len": 6,
+            "rem": [["!raw", "x", 0, 6]],
+        }
 
     del events[:]
 
diff --git a/tests/integrations/falcon/test_falcon.py b/tests/integrations/falcon/test_falcon.py
@@ -98,7 +98,7 @@ def on_get(self, req, resp):
     assert event["exception"]["values"][0]["mechanism"]["type"] == "falcon"
 
 
-def test_falcon_large_json_request(sentry_init, capture_events):
+def test_falcon_large_json_request(sentry_init, capture_events, fast_serialize):
     sentry_init(integrations=[FalconIntegration()])
 
     data = {"foo": {"bar": "a" * 2000}}
@@ -119,9 +119,10 @@ def on_post(self, req, resp):
     assert response.status == falcon.HTTP_200
 
     event, = events
-    assert event["_meta"]["request"]["data"]["foo"]["bar"] == {
-        "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
-    }
+    if not fast_serialize:
+        assert event["_meta"]["request"]["data"]["foo"]["bar"] == {
+            "": {"len": 2000, "rem": [["!limit", "x", 509, 512]]}
+        }
     assert len(event["request"]["data"]["foo"]["bar"]) == 512
 
 
diff --git a/tests/integrations/flask/test_flask.py b/tests/integrations/flask/test_flask.py
diff --git a/tests/integrations/pyramid/test_pyramid.py b/tests/integrations/pyramid/test_pyramid.py
diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py
diff --git a/tests/integrations/stdlib/test_subprocess.py b/tests/integrations/stdlib/test_subprocess.py
diff --git a/tests/test_tracing.py b/tests/test_tracing.py
