ref(spans): Modify segment attributes during conversion

relay-event-normalization/src/eap/mod.rs

@@ -800,26 +800,6 @@ fn normalize_http_attributes(
     }
 }
 
-/// Makes sure web vital spans are not identified with segments.
-///
-/// This was ported from the legacy pipeline for behavior parity.
-/// At some point in the future, web vital spans will become metrics,
-/// and this will become academic.
-pub fn normalize_web_vital_span_segment(span: &mut SpanV2) {
-    let Some(attributes) = span.attributes.value_mut() else {
-        return;
-    };
-
-    if let Some(op) = attributes.get_value(SENTRY__OP)
-        && let Some(op_name) = op.as_str()
-        && (op_name.starts_with("ui.interaction.") || op_name.starts_with("ui.webvital."))
-    {
-        span.is_segment = None.into();
-        span.parent_span_id = None.into();
-        attributes.remove(SENTRY__SEGMENT__ID);
-    }
-}
-
 /// Double writes sentry conventions attributes into legacy attributes.
 ///
 /// This achieves backwards compatibility as it allows products to continue using legacy attributes

relay-server/src/processing/legacy_spans/normalize.rs

@@ -13,7 +13,7 @@ use relay_event_normalization::{
 use relay_event_schema::processor::{ProcessingState, process_value};
 use relay_event_schema::protocol::{BrowserContext, EventId, IpAddr, Span, SpanData};
 use relay_metrics::UnixTimestamp;
-use relay_protocol::{Annotated, Empty, Value};
+use relay_protocol::{Annotated, Value};
 use relay_sampling::DynamicSamplingContext;
 
 /// Config needed to normalize a standalone span.
@@ -62,33 +62,7 @@ pub struct NormalizeSpanConfig<'a> {
 
 fn set_segment_attributes(span: &mut Annotated<Span>) {
     let Some(span) = span.value_mut() else { return };
-
-    // Identify INP spans or other WebVital spans and make sure they are not wrapped in a segment.
-    if let Some(span_op) = span.op.value()
-        && (span_op.starts_with("ui.interaction.") || span_op.starts_with("ui.webvital."))
-    {
-        span.is_segment = None.into();
-        span.parent_span_id = None.into();
-        span.segment_id = None.into();
-        return;
-    }
-
-    let Some(span_id) = span.span_id.value() else {
-        return;
-    };
-
-    if let Some(segment_id) = span.segment_id.value() {
-        // The span is a segment if and only if the segment_id matches the span_id.
-        span.is_segment = (segment_id == span_id).into();
-    } else if span.parent_span_id.is_empty() {
-        // If the span has no parent, it is automatically a segment:
-        span.is_segment = true.into();
-    }
-
-    // If the span is a segment, always set the segment_id to the current span_id:
-    if span.is_segment.value() == Some(&true) {
-        span.segment_id = span.span_id.clone();
-    }
+    relay_spans::set_segment_attributes(span);
 }
 
 /// Normalizes a standalone span.

relay-server/src/processing/spans/process.rs

@@ -167,7 +167,12 @@ fn expand_legacy_span(item: &Item) -> Result<WithHeader<SpanV2>> {
         // We can't enable `infer_name` here:
         // These spans haven't been PII scrubbed yet, so inferring a name would risk
         // leaking PII.
-        .map_value(|span| relay_spans::span_v1_to_span_v2(span, false));
+        .map_value(|mut span| {
+            // Set some segment-related attributes before converting to V2.
+            // This exists for parity with the legacy standalone span pipeline.
+            relay_spans::set_segment_attributes(&mut span);
+            relay_spans::span_v1_to_span_v2(span, false)
+        });
 
     Ok(WithHeader::new(span))
 }
@@ -234,7 +239,6 @@ fn normalize_span(
         // because category derivation depends on having the sentry.op attribute
         // available.
         eap::normalize_sentry_op(&mut span.attributes);
-        eap::normalize_web_vital_span_segment(span);
         eap::normalize_span_category(&mut span.attributes);
         eap::normalize_received(&mut span.attributes, meta.received_at());
         eap::normalize_client_address(&mut span.attributes, meta.client_addr());

relay-spans/src/lib.rs

@@ -10,6 +10,7 @@ pub use crate::description::derive_description_for_v2_span;
 pub use crate::name::name_for_attributes;
 pub use crate::op::derive_op_for_v2_span;
 pub use crate::otel_to_sentry_v2::otel_to_sentry_span as otel_to_sentry_span_v2;
+pub use crate::segment::set_segment_attributes;
 pub use crate::v1_to_v2::span_v1_to_span_v2;
 
 pub use opentelemetry_proto::tonic::trace::v1 as otel_trace;
@@ -18,4 +19,5 @@ mod description;
 mod name;
 mod op;
 mod otel_to_sentry_v2;
+mod segment;
 mod v1_to_v2;

relay-spans/src/segment.rs

@@ -0,0 +1,37 @@
+use relay_event_schema::protocol::Span;
+use relay_protocol::Empty;
+
+/// Normalizes various segment-related fields on a span:
+///
+/// * Web vital spans have `is_segment`, `parent_span_id`, and `segment_id` erased.
+/// * Spans whose `span_id` is equal to the `segment_id` or which don't have a `parent_span_id` have
+///   `is_segment` set to `true`.
+/// * Finally, segment spans have the `segment_id` set to their own `span_id`.
+pub fn set_segment_attributes(span: &mut Span) {
+    // Identify INP spans or other WebVital spans and make sure they are not wrapped in a segment.
+    if let Some(span_op) = span.op.value()
+        && (span_op.starts_with("ui.interaction.") || span_op.starts_with("ui.webvital."))
+    {
+        span.is_segment = None.into();
+        span.parent_span_id = None.into();
+        span.segment_id = None.into();
+        return;
+    }
+
+    let Some(span_id) = span.span_id.value() else {
+        return;
+    };
+
+    if let Some(segment_id) = span.segment_id.value() {
+        // The span is a segment if and only if the segment_id matches the span_id.
+        span.is_segment = (segment_id == span_id).into();
+    } else if span.parent_span_id.is_empty() {
+        // If the span has no parent, it is automatically a segment:
+        span.is_segment = true.into();
+    }
+
+    // If the span is a segment, always set the segment_id to the current span_id:
+    if span.is_segment.value() == Some(&true) {
+        span.segment_id = span.span_id.clone();
+    }
+}

tests/integration/test_spans_standalone.py

@@ -1,9 +1,12 @@
 from datetime import datetime, timezone
 
 from sentry_sdk.envelope import Envelope, Item, PayloadRef
+from sentry_relay.consts import DataCategory
 
 from .asserts import time_within_delta, time_within
 
+from .test_dynamic_sampling import add_sampling_config
+
 import pytest
 
 # Some profiles from Sentry
@@ -1192,5 +1195,137 @@ def test_name_inference(
     }
 
 
+@pytest.mark.parametrize("sampled", [False, True])
+def test_inp_span_is_segment_ds(
+    mini_sentry,
+    relay,
+    relay_with_processing,
+    metrics_consumer,
+    outcomes_consumer,
+    sampled,
+):
+    """
+    Verifies that the `is_segment` tag on metrics is correctly unset
+    regardless of whether the span is sampled.
+
+    This is specifically related to https://github.com/getsentry/relay/pull/6042.
+    and https://github.com/getsentry/relay/pull/6126.
+    If we ever decide to get rid of the functionality introduced in those PRs, this
+    test becomes obsolete.
+    """
+    metrics_consumer = metrics_consumer()
+    outcomes_consumer = outcomes_consumer()
+
+    project_id = 42
+    project_config = mini_sentry.add_full_project_config(project_id)
+    project_config["config"]["performanceScore"] = {
+        "profiles": performance_score_profiles
+    }
+    project_config["config"].setdefault("features", []).append(
+        "organizations:relay-generate-billing-outcome"
+    )
+    project_config["config"].setdefault("features", []).append(
+        "projects:span-v2-experimental-processing"
+    )
+
+    add_sampling_config(
+        project_config, sample_rate=1.0 if sampled else 0.0, rule_type="project"
+    )
+
+    relay = relay_with_processing()
+
+    ts = datetime.now(timezone.utc)
+
+    envelope = envelope_with_spans(
+        {
+            "data": {
+                "sentry.op": "ui.interaction.click",
+                "release": "frontend@488531b11e6401fa530ac25554d44426e6ef0f0b",
+                "environment": "prod",
+                "replay_id": "3d76a6311de149b9b3f560827ea0ecf9",
+                "transaction": "/insights/projects/",
+                "user_agent.original": "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Firefox/42.0",
+                "client.address": "{{auto}}",
+                "sentry.exclusive_time": 104,
+            },
+            "description": "<unknown>",
+            "op": "ui.interaction.click",
+            "parent_span_id": "8a6626cc9bdd5d9b",
+            "span_id": "a6f029fbe0e2389a",
+            "start_timestamp": ts.timestamp() - 0.5,
+            "timestamp": ts.timestamp(),
+            "trace_id": "d3d20f000885466b8c8f947c9b92b8d3",
+            "origin": "auto.http.browser.inp",
+            "exclusive_time": 104,
+            "measurements": {"inp": {"value": 104, "unit": "millisecond"}},
+            "segment_id": "8a6626cc9bdd5d9b",
+            "is_segment": True,
+        },
+        trace_info={
+            "trace_id": "d3d20f000885466b8c8f947c9b92b8d3",
+            "public_key": project_config["publicKeys"][0]["publicKey"],
+            "transaction": "/insights/projects/",
+        },
+    )
+
+    relay.send_envelope(project_id, envelope)
+
+    assert metrics_consumer.get_metrics(with_headers=False) == [
+        {
+            "org_id": 1,
+            "project_id": 42,
+            "name": "c:spans/count_per_root_project@none",
+            "type": "c",
+            "value": 1.0,
+            "timestamp": time_within_delta(ts),
+            "tags": {
+                "decision": "keep" if sampled else "drop",
+                "is_segment": "false",
+                "target_project_id": "42",
+                "transaction": "/insights/projects/",
+            },
+            "retention_days": 90,
+            "received_at": time_within(ts, precision="s"),
+        },
+        {
+            "org_id": 1,
+            "project_id": 42,
+            "name": "c:spans/usage@none",
+            "type": "c",
+            "value": 1.0,
+            "timestamp": time_within_delta(ts),
+            "tags": {"is_segment": "false", "billing_outcome_emitted": "true"},
+            "retention_days": 90,
+            "received_at": time_within(ts, precision="s"),
+        },
+    ]
+
+    expected_outcomes = [
+        {
+            "category": DataCategory.SPAN.value,
+            "key_id": 123,
+            "org_id": 1,
+            "outcome": 0,
+            "project_id": 42,
+            "quantity": 1,
+        }
+    ]
+
+    if not sampled:
+        expected_outcomes.append(
+            {
+                "category": DataCategory.SPAN_INDEXED.value,
+                "key_id": 123,
+                "org_id": 1,
+                "outcome": 1,
+                "project_id": 42,
+                "quantity": 1,
+                "reason": "Sampled:0",
+            }
+        )
+
+    assert outcomes_consumer.get_aggregated_outcomes() == expected_outcomes
+
+
 def _if_dict(cond, then):
     return then if cond else {}

tests/integration/test_spansv2.py

@@ -470,6 +470,7 @@ def test_spansv2_ds_drop(mini_sentry, relay, span, rule_type):
                         "timestamp": ts.timestamp() + 0.5,
                         "trace_id": "5b8efff798038103d269b633813fc60c",
                         "span_id": "eee19b7ec3c1b176",
+                        "parent_span_id": "eee19b7ec3c1b177",
                         "op": "some op",
                         "description": "some description",
                         "data": {"foo": "bar"},