diff --git a/relay-server/src/services/processor.rs b/relay-server/src/services/processor.rs index 9fa3747689..6ae0c27ce2 100644 --- a/relay-server/src/services/processor.rs +++ b/relay-server/src/services/processor.rs @@ -313,20 +313,21 @@ impl ProcessingGroup { )) } - if project_info.has_feature(Feature::SpanV2ExperimentalProcessing) { - let span_v2_items = envelope.take_items_by(|item| { - matches!( - item.integration(), - Some(Integration::Spans(SpansIntegration::OtelV1 { .. })) - ) || ItemContainer::::is_container(item) - }); + let span_v2_items = envelope.take_items_by(|item| { + let exp_feature = project_info.has_feature(Feature::SpanV2ExperimentalProcessing); + let is_supported_integration = matches!( + item.integration(), + Some(Integration::Spans(SpansIntegration::OtelV1 { .. })) + ); - if !span_v2_items.is_empty() { - grouped_envelopes.push(( - ProcessingGroup::SpanV2, - Envelope::from_parts(headers.clone(), span_v2_items), - )) - } + ItemContainer::::is_container(item) || (exp_feature && is_supported_integration) + }); + + if !span_v2_items.is_empty() { + grouped_envelopes.push(( + ProcessingGroup::SpanV2, + Envelope::from_parts(headers.clone(), span_v2_items), + )) } // Extract spans. @@ -2122,7 +2123,6 @@ impl EnvelopeProcessorService { ) -> Result, ProcessingError> { let mut extracted_metrics = ProcessingExtractedMetrics::new(); - span::expand_v2_spans(managed_envelope)?; span::filter(managed_envelope, ctx.config, ctx.project_info); span::convert_otel_traces_data(managed_envelope); diff --git a/relay-server/src/services/processor/span.rs b/relay-server/src/services/processor/span.rs index f3ca3554a5..a50dd5f3a8 100644 --- a/relay-server/src/services/processor/span.rs +++ b/relay-server/src/services/processor/span.rs @@ -3,17 +3,16 @@ use prost::Message; use relay_dynamic_config::Feature; use relay_event_normalization::span::tag_extraction; -use relay_event_schema::protocol::{Event, Span, SpanV2}; +use relay_event_schema::protocol::{Event, Span}; use relay_protocol::Annotated; use relay_quotas::DataCategory; use relay_spans::otel_trace::TracesData; -use crate::envelope::{ContentType, Item, ItemContainer, ItemType}; +use crate::envelope::{ContentType, Item, ItemType}; use crate::integrations::{Integration, OtelFormat, SpansIntegration}; use crate::managed::{ItemAction, TypedEnvelope}; use crate::services::outcome::{DiscardReason, Outcome}; use crate::services::processor::{SpanGroup, should_filter}; -use crate::statsd::RelayTimers; #[cfg(feature = "processing")] mod processing; @@ -22,8 +21,6 @@ use crate::services::projects::project::ProjectInfo; pub use processing::*; use relay_config::Config; -use super::ProcessingError; - pub fn filter( managed_envelope: &mut TypedEnvelope, config: &Config, @@ -48,70 +45,6 @@ pub fn filter( }); } -/// Expands V2 spans to V1 spans. -/// -/// This expands one item (contanining multiple V2 spans) into several -/// (containing one V1 span each). -pub fn expand_v2_spans( - managed_envelope: &mut TypedEnvelope, -) -> Result<(), ProcessingError> { - let span_v2_items = managed_envelope - .envelope_mut() - .take_items_by(ItemContainer::::is_container); - - // V2 spans must always be sent as an `ItemContainer`, currently it is not allowed to - // send multiple containers for V2 spans. - // - // This restriction may be lifted in the future, this is why this validation only happens - // when processing is enabled, allowing it to be changed easily in the future. - // - // This limit mostly exists to incentivise SDKs to batch multiple spans into a single container, - // technically it can be removed without issues. - if span_v2_items.len() > 1 { - return Err(ProcessingError::DuplicateItem(ItemType::Span)); - } - - if span_v2_items.is_empty() { - return Ok(()); - } - - let now = std::time::Instant::now(); - - for span_v2_item in span_v2_items { - let spans_v2 = match ItemContainer::parse(&span_v2_item) { - Ok(spans_v2) => spans_v2, - Err(err) => { - relay_log::debug!("failed to parse V2 spans: {err}"); - track_invalid( - managed_envelope, - DiscardReason::InvalidSpan, - span_v2_item.item_count().unwrap_or(1) as usize, - ); - continue; - } - }; - - for span_v2 in spans_v2.into_items() { - let span_v1 = span_v2.value.map_value(relay_spans::span_v2_to_span_v1); - match span_v1.to_json() { - Ok(payload) => { - let mut new_item = Item::new(ItemType::Span); - new_item.set_payload(ContentType::Json, payload); - managed_envelope.envelope_mut().add_item(new_item); - } - Err(err) => { - relay_log::debug!("failed to serialize span: {}", err); - track_invalid(managed_envelope, DiscardReason::Internal, 1); - } - } - } - } - - relay_statsd::metric!(timer(RelayTimers::SpanV2Expansion) = now.elapsed()); - - Ok(()) -} - pub fn convert_otel_traces_data(managed_envelope: &mut TypedEnvelope) { let envelope = managed_envelope.envelope_mut(); diff --git a/relay-server/src/statsd.rs b/relay-server/src/statsd.rs index 1b153e3394..e9631cd38a 100644 --- a/relay-server/src/statsd.rs +++ b/relay-server/src/statsd.rs @@ -591,8 +591,6 @@ pub enum RelayTimers { BodyReadDuration, /// Timing in milliseconds to count spans in a serialized transaction payload. CheckNestedSpans, - /// The time in milliseconds it takes to expand a Span V2 container into Spans V1. - SpanV2Expansion, /// The time it needs to create a signature. Includes both the signature used for /// trusted relays and for register challenges. SignatureCreationDuration, @@ -649,7 +647,6 @@ impl TimerMetric for RelayTimers { RelayTimers::BufferEnvelopeDecompression => "buffer.envelopes_decompression", RelayTimers::BodyReadDuration => "requests.body_read.duration", RelayTimers::CheckNestedSpans => "envelope.check_nested_spans", - RelayTimers::SpanV2Expansion => "envelope.span_v2_expansion", RelayTimers::SignatureCreationDuration => "signature.create.duration", } } diff --git a/tests/integration/test_spans.py b/tests/integration/test_spans.py index 337bfd1c17..3729bbc336 100644 --- a/tests/integration/test_spans.py +++ b/tests/integration/test_spans.py @@ -469,91 +469,6 @@ def envelope_with_spans( ) ) - envelope.add_item( - Item( - type="span", - headers={"metrics_extracted": metrics_extracted, "item_count": 2}, - content_type="application/vnd.sentry.items.span.v2+json", - payload=PayloadRef( - json={ - "items": [ - { - "trace_id": "89143b0763095bd9c9955e8175d1fb23", - "span_id": "a342abb1214ca182", - "name": "my 1st V2 span", - "start_timestamp": start.timestamp(), - "end_timestamp": end.timestamp(), - "attributes": { - "sentry.category": { - "type": "string", - "value": "db", - }, - "sentry.exclusive_time": { - "type": "double", - "value": int((end - start).total_seconds() * 1e3), - }, - }, - "links": [ - { - "trace_id": "89143b0763095bd9c9955e8175d1fb24", - "span_id": "e342abb1214ca183", - "sampled": False, - "attributes": { - "link_double_key": { - "type": "double", - "value": 1.23, - }, - }, - }, - ], - }, - { - "trace_id": "ff62a8b040f340bda5d830223def1d81", - "span_id": "b0429c44b67a3eb2", - "name": "resource.script", - "status": "ok", - "start_timestamp": start.timestamp(), - "end_timestamp": end.timestamp() + 1, - "links": [ - { - "trace_id": "99143b0763095bd9c9955e8175d1fb25", - "span_id": "e342abb1214ca183", - "sampled": True, - "attributes": { - "link_bool_key": { - "type": "boolean", - "value": True, - }, - }, - }, - ], - "attributes": { - "browser.name": {"type": "string", "value": "Chrome"}, - "sentry.description": { - "type": "string", - "value": "https://example.com/p/blah.js", - }, - "sentry.op": { - "type": "string", - "value": "resource.script", - }, - "sentry.exclusive_time": { - "type": "double", - "value": 161.0, - }, - # Span with the same `span_id` and `segment_id`, to make sure it is classified as `is_segment`. - "sentry.segment.id": { - "type": "string", - "value": "b0429c44b67a3eb2", - }, - }, - }, - ] - } - ), - ) - ) - return envelope @@ -737,7 +652,7 @@ def test_span_ingestion( headers={"Content-Type": "application/x-protobuf"}, ) - spans = spans_consumer.get_spans(timeout=10.0, n=7) + spans = spans_consumer.get_spans(timeout=10.0, n=5) for span in spans: span.pop("received", None) @@ -746,47 +661,6 @@ def test_span_ingestion( spans.sort(key=lambda msg: msg["span_id"]) assert spans == [ - { - "organization_id": 1, - "project_id": 42, - "key_id": 123, - "retention_days": 90, - "downsampled_retention_days": 90, - "attributes": { - "browser.name": {"type": "string", "value": "Chrome"}, - "client.address": {"type": "string", "value": "127.0.0.1"}, - "sentry.browser.name": {"type": "string", "value": "Chrome"}, - "sentry.category": {"type": "string", "value": "db"}, - "sentry.description": {"type": "string", "value": "my 1st V2 span"}, - "sentry.exclusive_time": {"type": "double", "value": 500.0}, - "sentry.is_segment": {"type": "boolean", "value": True}, - "sentry.name": {"type": "string", "value": "my 1st V2 span"}, - "sentry.op": {"type": "string", "value": "default"}, - "sentry.segment.id": {"type": "string", "value": "a342abb1214ca182"}, - "sentry.status": {"type": "string", "value": "unknown"}, - "user_agent.original": { - "type": "string", - "value": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/111.0.0.0 Safari/537.36", - }, - }, - "end_timestamp": end.timestamp(), - "is_remote": False, - "links": [ - { - "trace_id": "89143b0763095bd9c9955e8175d1fb24", - "span_id": "e342abb1214ca183", - "sampled": False, - "attributes": { - "link_double_key": {"type": "double", "value": 1.23} - }, - } - ], - "name": "my 1st V2 span", - "span_id": "a342abb1214ca182", - "start_timestamp": start.timestamp(), - "status": "error", - "trace_id": "89143b0763095bd9c9955e8175d1fb23", - }, { "organization_id": 1, "project_id": 42, @@ -836,55 +710,6 @@ def test_span_ingestion( "status": "ok", "trace_id": "ff62a8b040f340bda5d830223def1d81", }, - { - "organization_id": 1, - "project_id": 42, - "key_id": 123, - "retention_days": 90, - "downsampled_retention_days": 90, - "attributes": { - "browser.name": {"type": "string", "value": "Chrome"}, - "client.address": {"type": "string", "value": "127.0.0.1"}, - "sentry.browser.name": {"type": "string", "value": "Chrome"}, - "sentry.category": {"type": "string", "value": "resource"}, - "sentry.description": { - "type": "string", - "value": "https://example.com/p/blah.js", - }, - "sentry.domain": {"type": "string", "value": "example.com"}, - "sentry.exclusive_time": {"type": "double", "value": 161.0}, - "sentry.file_extension": {"type": "string", "value": "js"}, - "sentry.group": {"type": "string", "value": "8a97a9e43588e2bd"}, - "sentry.is_segment": {"type": "boolean", "value": True}, - "sentry.name": {"type": "string", "value": "resource.script"}, - "sentry.normalized_description": { - "type": "string", - "value": "https://example.com/*/blah.js", - }, - "sentry.op": {"type": "string", "value": "resource.script"}, - "sentry.segment.id": {"type": "string", "value": "b0429c44b67a3eb2"}, - "sentry.status": {"type": "string", "value": "ok"}, - "user_agent.original": { - "type": "string", - "value": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/111.0.0.0 Safari/537.36", - }, - }, - "end_timestamp": end.timestamp() + 1, - "is_remote": False, - "links": [ - { - "trace_id": "99143b0763095bd9c9955e8175d1fb25", - "span_id": "e342abb1214ca183", - "sampled": True, - "attributes": {"link_bool_key": {"type": "boolean", "value": True}}, - } - ], - "name": "resource.script", - "span_id": "b0429c44b67a3eb2", - "start_timestamp": start.timestamp(), - "status": "ok", - "trace_id": "ff62a8b040f340bda5d830223def1d81", - }, { "organization_id": 1, "project_id": 42, @@ -1053,7 +878,7 @@ def test_span_ingestion( "tags": {"decision": "keep", "target_project_id": "42"}, "timestamp": expected_timestamp, "type": "c", - "value": 3.0, + "value": 2.0, }, { "name": "c:spans/count_per_root_project@none", @@ -1064,7 +889,7 @@ def test_span_ingestion( "tags": {"decision": "keep", "target_project_id": "42"}, "timestamp": expected_timestamp + 1, "type": "c", - "value": 4.0, + "value": 3.0, }, { "name": "c:spans/usage@none", @@ -1074,7 +899,7 @@ def test_span_ingestion( "tags": {}, "timestamp": expected_timestamp, "type": "c", - "value": 3.0, + "value": 2.0, "received_at": time_after(now_timestamp), }, { @@ -1085,7 +910,7 @@ def test_span_ingestion( "tags": {}, "timestamp": expected_timestamp + 1, "type": "c", - "value": 4.0, + "value": 3.0, "received_at": time_after(now_timestamp), }, ] @@ -1099,112 +924,6 @@ def test_span_ingestion( metrics_consumer.assert_empty() -def test_standalone_span_ingestion_metric_extraction( - mini_sentry, - relay_with_processing, - spans_consumer, - metrics_consumer, -): - relay = relay_with_processing( - options={ - "aggregator": { - "bucket_interval": 1, - "initial_delay": 0, - "max_secs_in_past": 2**64 - 1, - "shift_key": "none", - } - } - ) - metrics_consumer = metrics_consumer() - - project_id = 42 - project_config = mini_sentry.add_full_project_config(project_id) - project_config["config"]["features"] = [ - "organizations:standalone-span-ingestion", - ] - - duration = timedelta(milliseconds=500) - now = datetime.now(timezone.utc) - end = now - timedelta(seconds=1) - start = end - duration - - envelope = Envelope() - - envelope.add_item( - Item( - type="span", - headers={"metrics_extracted": True, "item_count": 1}, - content_type="application/vnd.sentry.items.span.v2+json", - payload=PayloadRef( - json={ - "items": [ - { - "trace_id": "89143b0763095bd9c9955e8175d1fb23", - "span_id": "a342abb1214ca182", - "name": "SELECT from users", - "start_timestamp": start.timestamp(), - "end_timestamp": end.timestamp(), - "attributes": { - "db.system": { - "type": "string", - "value": "mysql", - }, - }, - }, - ] - } - ), - ) - ) - - relay.send_envelope( - project_id, - envelope, - ) - - metrics = [metric for (metric, _headers) in metrics_consumer.get_metrics()] - - metrics.sort(key=lambda m: (m["name"], sorted(m["tags"].items()), m["timestamp"])) - - for metric in metrics: - try: - metric["value"].sort() - except AttributeError: - pass - - expected_timestamp = int(end.timestamp()) - expected_received = time_after(int(now.timestamp())) - - expected_metrics = [ - { - "name": "c:spans/count_per_root_project@none", - "org_id": 1, - "project_id": 42, - "received_at": expected_received, - "retention_days": 90, - "tags": {"decision": "keep", "target_project_id": "42"}, - "timestamp": expected_timestamp, - "type": "c", - "value": 1.0, - }, - { - "name": "c:spans/usage@none", - "org_id": 1, - "project_id": 42, - "received_at": expected_received, - "retention_days": 90, - "tags": {}, - "timestamp": expected_timestamp, - "type": "c", - "value": 1.0, - }, - ] - - assert metrics == expected_metrics - - metrics_consumer.assert_empty() - - def test_otel_endpoint_disabled(mini_sentry, relay): relay = relay( mini_sentry, @@ -1459,19 +1178,19 @@ def summarize_outcomes(): # First batch passes relay.send_envelope(project_id, envelope) - spans = spans_consumer.get_spans(n=5, timeout=10) - assert len(spans) == 5 - assert summarize_outcomes() == {(16, 0): 5} # SpanIndexed, Accepted + spans = spans_consumer.get_spans(n=3, timeout=10) + assert len(spans) == 3 + assert summarize_outcomes() == {(16, 0): 3} # SpanIndexed, Accepted # Second batch is limited relay.send_envelope(project_id, envelope) - assert summarize_outcomes() == {(16, 2): 5} # SpanIndexed, RateLimited + assert summarize_outcomes() == {(16, 2): 3} # SpanIndexed, RateLimited spans_consumer.assert_empty() outcomes_consumer.assert_empty() -@pytest.mark.parametrize("category", ["span", "span_indexed"]) +@pytest.mark.parametrize("category", ["span"]) def test_rate_limit_consistent_extracted( category, mini_sentry, @@ -1584,7 +1303,7 @@ def test_rate_limit_spans_in_envelope( project_config["config"]["quotas"] = [ { "categories": ["span"], - "limit": 3, + "limit": 2, "window": int(datetime.now(UTC).timestamp()), "id": uuid.uuid4(), "reasonCode": "total_exceeded", @@ -1608,7 +1327,7 @@ def summarize_outcomes(): relay.send_envelope(project_id, envelope) - assert summarize_outcomes() == {(12, 2): 5, (16, 2): 5} + assert summarize_outcomes() == {(12, 2): 3, (16, 2): 3} # We emit transaction metrics from spans for legacy reasons. These are not rate limited. # (could be a bug) @@ -1887,14 +1606,14 @@ def summarize_outcomes(outcomes): return counter if sample_rate == 1.0: - spans = spans_consumer.get_spans(timeout=10, n=5) - assert len(spans) == 5 - outcomes = outcomes_consumer.get_outcomes(timeout=10, n=5) - assert summarize_outcomes(outcomes) == {(16, 0): 5} # SpanIndexed, Accepted + spans = spans_consumer.get_spans(timeout=10, n=3) + assert len(spans) == 3 + outcomes = outcomes_consumer.get_outcomes(timeout=10, n=3) + assert summarize_outcomes(outcomes) == {(16, 0): 3} # SpanIndexed, Accepted else: outcomes = outcomes_consumer.get_outcomes(timeout=10, n=1) assert summarize_outcomes(outcomes) == { - (16, 1): 5, # SpanIndexed, Filtered + (16, 1): 3, # SpanIndexed, Filtered } assert {o["reason"] for o in outcomes} == { "Sampled:3000", @@ -1980,77 +1699,3 @@ def test_scrubs_ip_addresses( assert parent_span["attributes"]["sentry.user.ip"]["value"] == "127.0.0.1" spans_consumer.assert_empty() - - -def test_spans_v2_multiple_containers_not_allowed( - mini_sentry, - relay_with_processing, - spans_consumer, - outcomes_consumer, -): - spans_consumer = spans_consumer() - outcomes_consumer = outcomes_consumer() - project_id = 42 - project_config = mini_sentry.add_full_project_config(project_id) - project_config["config"]["features"] = [ - "organizations:standalone-span-ingestion", - ] - - relay = relay_with_processing(options=TEST_CONFIG) - start = datetime.now(timezone.utc) - envelope = Envelope() - - payload = { - "start_timestamp": start.timestamp(), - "end_timestamp": start.timestamp() + 0.500, - "trace_id": "5b8efff798038103d269b633813fc60c", - "span_id": "eee19b7ec3c1b175", - "name": "some op", - } - envelope.add_item( - Item( - type="span", - payload=PayloadRef(json={"items": [payload]}), - content_type="application/vnd.sentry.items.span.v2+json", - headers={"item_count": 1}, - ) - ) - envelope.add_item( - Item( - type="span", - payload=PayloadRef(json={"items": [payload, payload]}), - content_type="application/vnd.sentry.items.span.v2+json", - headers={"item_count": 2}, - ) - ) - - relay.send_envelope(project_id, envelope) - - spans_consumer.assert_empty() - - outcomes = outcomes_consumer.get_outcomes() - - outcomes.sort(key=lambda o: sorted(o.items())) - - assert outcomes == [ - { - "category": DataCategory.SPAN.value, - "timestamp": time_within_delta(), - "key_id": 123, - "org_id": 1, - "outcome": 3, # Invalid - "project_id": 42, - "quantity": 3, - "reason": "duplicate_item", - }, - { - "category": DataCategory.SPAN_INDEXED.value, - "timestamp": time_within_delta(), - "key_id": 123, - "org_id": 1, - "outcome": 3, # Invalid - "project_id": 42, - "quantity": 3, - "reason": "duplicate_item", - }, - ]