From b312a86adabf04efee4f7d3a12e6fc0150f35728 Mon Sep 17 00:00:00 2001 From: Alex Vandiver Date: Thu, 20 Apr 2023 22:07:13 +0000 Subject: [PATCH] sentry: Add the observed user's IP address before forwarding. The default for Javascript reporting is that Sentry sets the IP address of the user to the IP address that the report was observed to come from[^1]. Since all reports come through the Zulip server, this results in all reports being "from" one IP address, thus undercounting the number of affected unauthenticated users, and making it difficult to correlate Sentry reports with server logs. Consume the Sentry Envelope format[^2] to inject the submitting client's observed IP address, when possible. This ensures that Sentry reports contain the same IP address that Zulip's server logs do. [^1]: https://docs.sentry.io/platforms/python/guides/logging/enriching-events/identify-user/ [^2]: https://develop.sentry.dev/sdk/envelopes/ --- zerver/views/sentry.py | 50 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/zerver/views/sentry.py b/zerver/views/sentry.py index 0a66c683b6..a0c5b31cab 100644 --- a/zerver/views/sentry.py +++ b/zerver/views/sentry.py @@ -1,5 +1,7 @@ import urllib +from contextlib import suppress +import orjson from django.conf import settings from django.http import HttpRequest, HttpResponse from django.utils.translation import gettext as _ @@ -23,9 +25,9 @@ def sentry_tunnel( request: HttpRequest, ) -> HttpResponse: try: - envelope = request.body - header = to_wild_value("envelope", envelope.split(b"\n")[0].decode("utf-8")) - dsn = urllib.parse.urlparse(header["dsn"].tame(check_url)) + envelope_header_line, envelope_items = request.body.split(b"\n", 1) + envelope_header = to_wild_value("envelope_header", envelope_header_line.decode("utf-8")) + dsn = urllib.parse.urlparse(envelope_header["dsn"].tame(check_url)) except Exception: raise JsonableError(_("Invalid request format")) @@ -35,7 +37,47 @@ def sentry_tunnel( assert dsn.hostname project_id = dsn.path.strip("/") url = dsn._replace(netloc=dsn.hostname, path=f"/api/{project_id}/envelope/").geturl() + + # Adjust the payload to explicitly contain the IP address of the + # user we see. If left blank, Sentry will assume the IP it + # received the request from, which is Zulip's, which can make + # debugging more complicated. + updated_body = request.body + # If we fail to update the body for any reason, leave it as-is; it + # is better to mis-report the IP than to drop the report entirely. + with suppress(Exception): + # This parses the Sentry ingestion format, known as an + # Envelope. See https://develop.sentry.dev/sdk/envelopes/ for + # spec. + parts = [envelope_header_line, b"\n"] + while envelope_items != b"": + item_header_line, rest = envelope_items.split(b"\n", 1) + parts.append(item_header_line) + parts.append(b"\n") + item_header = orjson.loads(item_header_line.decode("utf-8")) + length = item_header.get("length") + if length is None: + item_body, envelope_items = ([*rest.split(b"\n", 1), b""])[:2] + else: + item_body, envelope_items = rest[0:length], rest[length:] + if item_header.get("type") in ("transaction", "event"): + # Event schema: + # https://develop.sentry.dev/sdk/event-payloads/#core-interfaces + # https://develop.sentry.dev/sdk/event-payloads/user/ + # + # Transaction schema: + # https://develop.sentry.dev/sdk/event-payloads/transaction/#anatomy + # Note that "Transactions are Events enriched with Span data." + payload_data = orjson.loads(item_body) + if "user" in payload_data: + payload_data["user"]["ip_address"] = request.META.get("REMOTE_ADDR") + item_body = orjson.dumps(payload_data) + parts.append(item_body) + if length is None: + parts.append(b"\n") + updated_body = b"".join(parts) + SentryTunnelSession().post( - url=url, data=envelope, headers={"Content-Type": "application/x-sentry-envelope"} + url=url, data=updated_body, headers={"Content-Type": "application/x-sentry-envelope"} ).raise_for_status() return HttpResponse(status=200)