Skip to content
Open
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 @@ -12,6 +12,7 @@
- Correctly handle minidump objecstore upload failures. ([#6033](https://github.com/getsentry/relay/pull/6033))
- Add `client.address` attribute to known IP fields. ([#6058](https://github.com/getsentry/relay/pull/6058))
- Fix a bug in mobile attribute normalization. ([#6065](https://github.com/getsentry/relay/pull/6065))
- Don't infer names during tag extraction in transaction processing. ([#6080](https://github.com/getsentry/relay/pull/6080))

**Internal**:

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ expression: event.to_json_pretty().unwrap()
"cache.hit": "true",
"cache.key": "[\"my_key\"]",
"thread.name": "Thread-4 (process_request_thread)",
"thread.id": "6286962688",
"name": "cache.get_item"
"thread.id": "6286962688"
},
"measurements": {
"cache.item_size": {
Expand Down Expand Up @@ -74,8 +73,7 @@ expression: event.to_json_pretty().unwrap()
"cache.hit": "false",
"cache.key": "[\"my_key\",\"my_key_2\"]",
"thread.name": "Thread-4 (process_request_thread)",
"thread.id": "6286962688",
"name": "cache.get_item"
"thread.id": "6286962688"
},
"measurements": {
"cache.item_size": {
Expand Down Expand Up @@ -114,8 +112,7 @@ expression: event.to_json_pretty().unwrap()
"cache.hit": "false",
"cache.key": "[\"my_key_2\"]",
"thread.name": "Thread-4 (process_request_thread)",
"thread.id": "6286962688",
"name": "cache.get"
"thread.id": "6286962688"
},
"measurements": {
"cache.item_size": {
Expand Down
37 changes: 1 addition & 36 deletions relay-event-normalization/src/normalize/span/tag_extraction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ use relay_event_schema::protocol::{
TraceContext,
};
use relay_protocol::{Annotated, Empty, FiniteF64, Value};
use relay_spans::name_for_span;
use sqlparser::ast::{ObjectName, Visitor};
use sqlparser::ast::{ObjectNamePart, Visit};
use sqlparser::ast::{ObjectName, ObjectNamePart, Visit, Visitor};
use url::Url;

use crate::GeoIpLookup;
Expand Down Expand Up @@ -1142,8 +1140,6 @@ pub fn extract_tags(
&& !name.is_empty()
{
span_tags.name = name.to_owned().into();
} else if let Some(name) = name_for_span(span) {
span_tags.name = name.into();
Comment on lines -1145 to -1146

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the operative change in this PR.

}

span_tags
Expand Down Expand Up @@ -3485,35 +3481,4 @@ LIMIT 1

assert_eq!(tags.name.value(), Some(&"my name".to_owned()));
}

#[test]
fn generate_name_from_attributes() {
let span: Span = Annotated::<Span>::from_json(
r#"{
"start_timestamp": 0,
"timestamp": 1,
"span_id": "922dda2462ea4ac2",
"data": {
"db.query.summary": "SELECT users"
},
"op": "db"
}"#,
)
.unwrap()
.into_value()
.unwrap();

let tags = extract_tags(
&span,
200,
None,
None,
false,
None,
&[],
&GeoIpLookup::empty(),
);

assert_eq!(tags.name.value(), Some(&"SELECT users".to_owned()));
}
}
1 change: 0 additions & 1 deletion relay-spans/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
html_favicon_url = "https://raw.githubusercontent.com/getsentry/relay/master/artwork/relay-icon.png"
)]

pub use crate::name::name_for_span;
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::v1_to_v2::span_v1_to_span_v2;
Expand Down
110 changes: 3 additions & 107 deletions relay-spans/src/name.rs
Original file line number Diff line number Diff line change
@@ -1,51 +1,14 @@
use relay_conventions::attributes::SENTRY__OP;
use relay_conventions::name_for_op_and_attributes;
use relay_event_schema::protocol::{Attributes, Span};
use relay_protocol::{Getter, GetterIter, Val};

/// Constructs a name attribute for a span, following the rules defined in sentry-conventions.
pub fn name_for_span(span: &Span) -> Option<String> {
let op = span.op.value()?;

let Some(data) = span.data.value() else {
return Some(name_for_op_and_attributes(op, &EmptyGetter {}));
};

Some(name_for_op_and_attributes(
op,
// SpanData's Getter impl treats dots in attribute names as object traversals.
// They have to be escaped in order for an attribute name with dots to be treated as a root
// attribute.
&EscapedGetter(data),
))
}
use relay_event_schema::protocol::Attributes;
use relay_protocol::{Getter, Val};

/// Constructs a name attribute for a span, following the rules defined in sentry-conventions.
pub fn name_for_attributes(attributes: &Attributes) -> Option<String> {
let op = attributes.get_value(SENTRY__OP)?.as_str()?;
Some(name_for_op_and_attributes(op, &AttributeGetter(attributes)))
}

struct EmptyGetter {}

impl Getter for EmptyGetter {
fn get_value(&self, _path: &str) -> Option<Val<'_>> {
None
}
}

struct EscapedGetter<'a, T: Getter>(&'a T);

impl<'a, T: Getter> Getter for EscapedGetter<'a, T> {
fn get_value(&self, path: &str) -> Option<Val<'_>> {
self.0.get_value(&path.replace(".", "\\."))
}

fn get_iter(&self, path: &str) -> Option<GetterIter<'_>> {
self.0.get_iter(&path.replace(".", "\\."))
}
}

/// A custom getter for [`Attributes`] which only resolves values based on the attribute name.
///
/// This [`Getter`] does not implement nested traversals, which is the behaviour required for
Expand All @@ -60,20 +23,10 @@ impl<'a> Getter for AttributeGetter<'a> {

#[cfg(test)]
mod tests {
use relay_event_schema::protocol::SpanData;
use relay_protocol::{Annotated, Object, Value};
use relay_protocol::Annotated;

use super::*;

#[test]
fn test_span_falls_back_to_op_when_no_templates_defined() {
let span = Span {
op: Annotated::new("foo".to_owned()),
..Default::default()
};
assert_eq!(name_for_span(&span), Some("foo".to_owned()));
}

#[test]
fn test_attributes_falls_back_to_op_when_no_templates_defined() {
let attributes = Attributes::from([(
Expand All @@ -84,32 +37,6 @@ mod tests {
assert_eq!(name_for_attributes(&attributes), Some("foo".to_owned()));
}

#[test]
fn test_span_uses_the_first_matching_template() {
let span = Span {
op: Annotated::new("db".to_owned()),
data: Annotated::new(SpanData {
other: Object::from([
(
"db.query.summary".to_owned(),
Value::String("SELECT users".to_owned()).into(),
),
(
"db.operation.name".to_owned(),
Value::String("INSERT".to_owned()).into(),
),
(
"db.collection.name".to_owned(),
Value::String("widgets".to_owned()).into(),
),
]),
..Default::default()
}),
..Default::default()
};
assert_eq!(name_for_span(&span), Some("SELECT users".to_owned()));
}

#[test]
fn test_attributes_uses_the_first_matching_template() {
let attributes = Attributes::from([
Expand Down Expand Up @@ -137,28 +64,6 @@ mod tests {
);
}

#[test]
fn test_span_uses_fallback_templates_when_data_is_missing() {
let span = Span {
op: Annotated::new("db".to_owned()),
data: Annotated::new(SpanData {
other: Object::from([
(
"db.operation.name".to_owned(),
Value::String("INSERT".to_owned()).into(),
),
(
"db.collection.name".to_owned(),
Value::String("widgets".to_owned()).into(),
),
]),
..Default::default()
}),
..Default::default()
};
assert_eq!(name_for_span(&span), Some("INSERT widgets".to_owned()));
}

#[test]
fn test_attributes_uses_fallback_templates_when_data_is_missing() {
let attributes = Attributes::from([
Expand All @@ -182,15 +87,6 @@ mod tests {
);
}

#[test]
fn test_span_falls_back_to_hardcoded_name_when_nothing_matches() {
let span = Span {
op: Annotated::new("db".to_owned()),
..Default::default()
};
assert_eq!(name_for_span(&span), Some("Database operation".to_owned()));
}

#[test]
fn test_attributes_falls_back_to_hardcoded_name_when_nothing_matches() {
let attributes = Attributes::from([(
Expand Down
Loading
Loading