Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(spans): Copy transaction tags to segment #3386

Merged
merged 7 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- Pass `retention_days` in the Kafka profile messages. ([#3362](https://github.com/getsentry/relay/pull/3362))
- Support and expose namespaces for metric rate limit propagation via the `x-sentry-rate-limits` header. ([#3347](https://github.com/getsentry/relay/pull/3347))
- Tag span duration metric by group for all ops supporting description scrubbing. ([#3370](https://github.com/getsentry/relay/pull/3370))
- Copy transaction tags to segment. ([#3386](https://github.com/getsentry/relay/pull/3386))
- Route spans according to trace_id. ([#3387](https://github.com/getsentry/relay/pull/3387))

## 24.3.0
Expand Down
7 changes: 4 additions & 3 deletions relay-event-schema/src/protocol/span/convert.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ macro_rules! map_fields {
fn from(event: &Event) -> Self {
Self {
$(
$span_field: event.$event_field.clone(),
$span_field: event.$event_field.clone().map_value(Into::into),
)*
$(
$(
Expand All @@ -82,7 +82,7 @@ macro_rules! map_fields {
fn from(span: &Span) -> Self {
Self {
$(
$event_field: span.$span_field.clone(),
$event_field: span.$span_field.clone().map_value(Into::into),
)*
$(
$fixed_event_field: $fixed_event_value.into(),
Expand Down Expand Up @@ -118,6 +118,7 @@ map_fields!(
span.platform <=> event.platform,
span.received <=> event.received,
span.start_timestamp <=> event.start_timestamp,
span.tags <=> event.tags,
span.timestamp <=> event.timestamp
}
contexts {
Expand All @@ -136,7 +137,7 @@ map_fields!(
}
fixed_for_span {
// A transaction event corresponds to a segment span.
span.is_segment <= Some(true),
span.is_segment <= true,
span.was_transaction <= true
}
fixed_for_event {
Expand Down
28 changes: 26 additions & 2 deletions relay-event-schema/src/protocol/tags.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#[cfg(feature = "jsonschema")]
use relay_jsonschema_derive::JsonSchema;
use relay_protocol::{Annotated, Array, Empty, FromValue, IntoValue, Value};
use relay_protocol::{Annotated, Array, Empty, FromValue, IntoValue, Object, Value};

use crate::processor::ProcessValue;
use crate::protocol::{AsPair, LenientString, PairList};
use crate::protocol::{AsPair, JsonLenientString, LenientString, PairList};

#[derive(Clone, Debug, Default, PartialEq, Empty, IntoValue, ProcessValue)]
#[cfg_attr(feature = "jsonschema", derive(JsonSchema))]
Expand Down Expand Up @@ -76,6 +76,30 @@ impl std::ops::DerefMut for Tags {
}
}

impl<T: Into<String>> From<Object<T>> for Tags {
fn from(value: Object<T>) -> Self {
Self(PairList(
value
.into_iter()
.map(|(k, v)| TagEntry(k.into(), v.map_value(|s| s.into())))
.map(Annotated::new)
.collect(),
))
}
}

impl From<Tags> for Object<JsonLenientString> {
fn from(value: Tags) -> Self {
value
.0
.0
.into_iter()
.flat_map(Annotated::into_value)
.flat_map(|p| Some((p.0.into_value()?, p.1.map_value(Into::into))))
.collect()
}
}

#[cfg(test)]
mod tests {
use similar_asserts::assert_eq;
Expand Down
6 changes: 6 additions & 0 deletions relay-event-schema/src/protocol/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -886,6 +886,12 @@ impl std::ops::DerefMut for JsonLenientString {
}
}

impl From<JsonLenientString> for String {
fn from(value: JsonLenientString) -> Self {
value.0
}
}

impl FromValue for JsonLenientString {
fn from_value(value: Annotated<Value>) -> Annotated<Self> {
match value {
Expand Down
10 changes: 8 additions & 2 deletions relay-server/src/services/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1350,7 +1350,6 @@ struct SpanKafkaMessage<'a> {
start_timestamp: f64,
#[serde(rename(deserialize = "timestamp"), skip_serializing)]
end_timestamp: f64,

#[serde(default, skip_serializing_if = "Option::is_none")]
description: Option<&'a RawValue>,
#[serde(default)]
Expand Down Expand Up @@ -1385,14 +1384,21 @@ struct SpanKafkaMessage<'a> {
span_id: &'a str,
#[serde(default)]
start_timestamp_ms: u64,
#[serde(default, skip_serializing_if = "Option::is_none")]
#[serde(default, skip_serializing_if = "none_or_empty_object")]
tags: Option<&'a RawValue>,
trace_id: EventId,

#[serde(borrow, default, skip_serializing)]
platform: Cow<'a, str>, // We only use this for logging for now
}

fn none_or_empty_object(value: &Option<&RawValue>) -> bool {
match value {
None => true,
Some(raw) => raw.get() == "{}",
}
}

#[derive(Debug, Deserialize)]
struct SpanMetricsSummary {
#[serde(default)]
Expand Down
66 changes: 63 additions & 3 deletions tests/integration/test_spans.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,8 @@ def test_span_ingestion(
for span in spans:
span.pop("received", None)

spans.sort(key=lambda msg: msg["span_id"]) # endpoint might overtake envelope
# endpoint might overtake envelope
spans.sort(key=lambda msg: msg["span_id"])

assert spans == [
{
Expand Down Expand Up @@ -1041,7 +1042,8 @@ def test_span_ingestion_with_performance_scores(
for span in spans:
span.pop("received", None)

spans.sort(key=lambda msg: msg["span_id"]) # endpoint might overtake envelope
# endpoint might overtake envelope
spans.sort(key=lambda msg: msg["span_id"])

assert spans == [
{
Expand Down Expand Up @@ -1210,7 +1212,8 @@ def summarize_outcomes():
# First send should be accepted.
relay.send_event(project_id, event)
spans = list(spans_consumer.get_spans(max_attempts=2, timeout=10))
assert len(spans) == 2 # one for the transaction, one for the contained span
# one for the transaction, one for the contained span
assert len(spans) == 2
assert summarize_outcomes() == {(16, 0): 2} # SpanIndexed, Accepted

# Second send should be rejected immediately.
Expand Down Expand Up @@ -1287,3 +1290,60 @@ def summarize_outcomes():

spans_consumer.assert_empty()
outcomes_consumer.assert_empty()


@pytest.mark.parametrize(
"tags, expected_tags",
[
(
{
"some": "tag",
"other": "value",
},
{
"some": "tag",
"other": "value",
},
),
(
{
"some": 1,
"other": True,
},
{
"some": "1",
"other": "True",
},
),
],
)
def test_span_extraction_with_tags(
mini_sentry,
relay_with_processing,
spans_consumer,
tags,
expected_tags,
):
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",
]

event = make_transaction(
{
"event_id": "e022a2da91e9495d944c291fe065972d",
"tags": tags,
}
)

relay.send_event(project_id, event)

transaction_span = spans_consumer.get_span()

assert transaction_span["tags"] == expected_tags

spans_consumer.assert_empty()
Loading