fix(spans): Null user.ip in sentry_tags (#4370)

Fixes [RELAY-2P1C](https://sentry.my.sentry.io/organizations/sentry/issues/2217203/). --------- Co-authored-by: Joris Bayer <[email protected]>
getsentry · Dec 11, 2024 · 2f3b713 · 2f3b713
1 parent 182d5e5
commit 2f3b713
Show file tree

Hide file tree

Showing 3 changed files with 173 additions and 3 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,7 +12,7 @@
 - Accept incoming requests even if there was an error fetching their project config. ([#4140](https://github.com/getsentry/relay/pull/4140))
 - Rate limit profiles when transaction was sampled. ([#4195](https://github.com/getsentry/relay/pull/4195))
 - Fix scrubbing user paths in minidump debug module names. ([#4351](https://github.com/getsentry/relay/pull/4351))
-- Scrub user fields in span.sentry_tags. ([#4364](https://github.com/getsentry/relay/pull/4364))
+- Scrub user fields in span.sentry_tags. ([#4364](https://github.com/getsentry/relay/pull/4364)), ([#4370](https://github.com/getsentry/relay/pull/4370))
 
 **Features**:
 

diff --git a/relay-server/src/services/store.rs b/relay-server/src/services/store.rs
@@ -3,6 +3,7 @@
 
 use anyhow::Context;
 use relay_base_schema::organization::OrganizationId;
+use serde::ser::SerializeMap;
 use std::borrow::Cow;
 use std::collections::BTreeMap;
 use std::error::Error;
@@ -1024,6 +1025,25 @@ where
         .serialize(serializer)
 }
 
+pub fn serialize_btreemap_skip_nulls<S>(
+    map: &Option<BTreeMap<&str, Option<String>>>,
+    serializer: S,
+) -> Result<S::Ok, S::Error>
+where
+    S: serde::Serializer,
+{
+    let Some(map) = map else {
+        return serializer.serialize_none();
+    };
+    let mut m = serializer.serialize_map(Some(map.len()))?;
+    for (key, value) in map.iter() {
+        if let Some(value) = value {
+            m.serialize_entry(key, value)?;
+        }
+    }
+    m.end()
+}
+
 /// Container payload for event messages.
 #[derive(Debug, Serialize)]
 struct EventKafkaMessage {
@@ -1249,8 +1269,12 @@ struct SpanKafkaMessage<'a> {
     retention_days: u16,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     segment_id: Option<&'a str>,
-    #[serde(default, skip_serializing_if = "Option::is_none")]
-    sentry_tags: Option<BTreeMap<&'a str, String>>,
+    #[serde(
+        default,
+        skip_serializing_if = "Option::is_none",
+        serialize_with = "serialize_btreemap_skip_nulls"
+    )]
+    sentry_tags: Option<BTreeMap<&'a str, Option<String>>>,
     span_id: &'a str,
     #[serde(default, skip_serializing_if = "none_or_empty_object")]
     tags: Option<&'a RawValue>,

diff --git a/tests/integration/test_spans.py b/tests/integration/test_spans.py
@@ -1873,3 +1873,149 @@ def test_ingest_in_eap(
         spans_consumer.get_span()
 
     spans_consumer.assert_empty()
+
+
+@pytest.mark.parametrize("scrub_ip_addresses", [False, True])
+def test_scrubs_ip_addresses(
+    mini_sentry,
+    relay_with_processing,
+    spans_consumer,
+    scrub_ip_addresses,
+):
+    spans_consumer = spans_consumer()
+
+    relay = relay_with_processing()
+
+    project_id = 42
+    project_config = mini_sentry.add_full_project_config(project_id)
+    project_config["config"]["features"] = [
+        "projects:span-metrics-extraction",
+        "organizations:indexed-spans-extraction",
+    ]
+    project_config["config"].setdefault("datascrubbingSettings", {})[
+        "scrubIpAddresses"
+    ] = scrub_ip_addresses
+
+    event = make_transaction({"event_id": "cbf6960622e14a45abc1f03b2055b186"})
+    event["user"] = {
+        "id": "unique_id",
+        "username": "my_user",
+        "email": "[email protected]",
+        "ip_address": "127.0.0.1",
+        "subscription": "basic",
+    }
+    end = datetime.now(timezone.utc) - timedelta(seconds=1)
+    duration = timedelta(milliseconds=500)
+    start = end - duration
+    event["spans"] = [
+        {
+            "description": "GET /api/0/organizations/?member=1",
+            "op": "http",
+            "origin": "manual",
+            "parent_span_id": "aaaaaaaaaaaaaaaa",
+            "span_id": "bbbbbbbbbbbbbbbb",
+            "start_timestamp": start.isoformat(),
+            "status": "success",
+            "timestamp": end.isoformat(),
+            "trace_id": "ff62a8b040f340bda5d830223def1d81",
+        },
+    ]
+
+    relay.send_event(project_id, event)
+
+    child_span = spans_consumer.get_span()
+    del child_span["received"]
+
+    expected = {
+        "description": "GET /api/0/organizations/?member=1",
+        "duration_ms": int(duration.total_seconds() * 1e3),
+        "event_id": "cbf6960622e14a45abc1f03b2055b186",
+        "exclusive_time_ms": 500.0,
+        "is_segment": False,
+        "organization_id": 1,
+        "origin": "manual",
+        "parent_span_id": "aaaaaaaaaaaaaaaa",
+        "project_id": 42,
+        "retention_days": 90,
+        "segment_id": "968cff94913ebb07",
+        "sentry_tags": {
+            "category": "http",
+            "description": "GET *",
+            "group": "37e3d9fab1ae9162",
+            "op": "http",
+            "platform": "other",
+            "sdk.name": "raven-node",
+            "sdk.version": "2.6.3",
+            "status": "ok",
+            "trace.status": "unknown",
+            "transaction": "hi",
+            "transaction.op": "hi",
+            "user": "id:unique_id",
+            "user.email": "[email]",
+            "user.id": "unique_id",
+            "user.ip": "127.0.0.1",
+            "user.username": "my_user",
+        },
+        "span_id": "bbbbbbbbbbbbbbbb",
+        "start_timestamp_ms": int(start.timestamp() * 1e3),
+        "start_timestamp_precise": start.timestamp(),
+        "end_timestamp_precise": start.timestamp() + duration.total_seconds(),
+        "trace_id": "ff62a8b040f340bda5d830223def1d81",
+    }
+    if scrub_ip_addresses:
+        del expected["sentry_tags"]["user.ip"]
+    assert child_span == expected
+
+    start_timestamp = datetime.fromisoformat(event["start_timestamp"]).replace(
+        tzinfo=timezone.utc
+    )
+    end_timestamp = datetime.fromisoformat(event["timestamp"]).replace(
+        tzinfo=timezone.utc
+    )
+    duration = (end_timestamp - start_timestamp).total_seconds()
+    duration_ms = int(duration * 1e3)
+
+    child_span = spans_consumer.get_span()
+    del child_span["received"]
+
+    expected = {
+        "data": {
+            "sentry.sdk.name": "raven-node",
+            "sentry.sdk.version": "2.6.3",
+            "sentry.segment.name": "hi",
+        },
+        "description": "hi",
+        "duration_ms": duration_ms,
+        "event_id": "cbf6960622e14a45abc1f03b2055b186",
+        "exclusive_time_ms": 2000.0,
+        "is_segment": True,
+        "organization_id": 1,
+        "project_id": 42,
+        "retention_days": 90,
+        "segment_id": "968cff94913ebb07",
+        "sentry_tags": {
+            "op": "hi",
+            "platform": "other",
+            "sdk.name": "raven-node",
+            "sdk.version": "2.6.3",
+            "status": "unknown",
+            "trace.status": "unknown",
+            "transaction": "hi",
+            "transaction.op": "hi",
+            "user": "id:unique_id",
+            "user.email": "[email]",
+            "user.id": "unique_id",
+            "user.ip": "127.0.0.1",
+            "user.username": "my_user",
+        },
+        "span_id": "968cff94913ebb07",
+        "start_timestamp_ms": int(start_timestamp.timestamp() * 1e3),
+        "start_timestamp_precise": start_timestamp.timestamp(),
+        "end_timestamp_precise": start_timestamp.timestamp() + duration,
+        "trace_id": "a0fa8803753e40fd8124b21eeb2986b5",
+    }
+    if scrub_ip_addresses:
+        del expected["sentry_tags"]["user.ip"]
+    assert child_span == expected
+
+    spans_consumer.assert_empty()