Skip to content

Commit

Permalink
feat(spans): Enable spans needed for the mobile Starfish module (#2570)
Browse files Browse the repository at this point in the history
  • Loading branch information
@phacops
phacops authored Oct 5, 2023
1 parent bd15ccf commit 5517346
Show file tree
Hide file tree
Showing 6 changed files with 183 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
**Features**:

- Scrub span descriptions with encoded data images. ([#2560](https://github.com/getsentry/relay/pull/2560))
- Accept spans needed for the mobile Starfish module. ([#2570](https://github.com/getsentry/relay/pull/2570))

**Bug Fixes**:

Expand Down
6 changes: 5 additions & 1 deletion relay-dynamic-config/src/defaults.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ use crate::project::ProjectConfig;
/// A list of `span.op` patterns that indicate databases that should be skipped.
const DISABLED_DATABASES: &[&str] = &["*clickhouse*", "*mongodb*", "*redis*", "*compiler*"];

/// A list of span.op` patterns we want to enable for mobile.
const MOBILE_OPS: &[&str] = &["app.*", "ui.load*"];

/// Adds configuration for extracting metrics from spans.
///
/// This configuration is temporarily hard-coded here. It will later be provided by the upstream.
Expand Down Expand Up @@ -41,7 +44,8 @@ pub fn add_span_metrics(project_config: &mut ProjectConfig) {
let condition = RuleCondition::eq("span.op", "http.client")
| (RuleCondition::glob("span.op", "db*")
& !RuleCondition::glob("span.op", DISABLED_DATABASES)
& !(RuleCondition::eq("span.op", "db.sql.query") & is_mongo));
& !(RuleCondition::eq("span.op", "db.sql.query") & is_mongo))
| RuleCondition::glob("span.op", MOBILE_OPS);

Some(condition)
};
Expand Down
43 changes: 43 additions & 0 deletions relay-event-normalization/src/normalize/span/description/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub use sql::parse_query;

use std::borrow::Cow;
use std::collections::BTreeMap;
use std::path::Path;

use itertools::Itertools;
use relay_event_schema::processor::{self, ProcessingResult};
Expand Down Expand Up @@ -58,6 +59,13 @@ pub(crate) fn scrub_span_description(span: &mut Span, rules: &Vec<SpanDescriptio
// be low-risk to start adding the description.
Some(description.to_owned())
}
("app", _) => {
// `app.*` has static descriptions, like `Cold Start`
// or `Pre Runtime Init`.
// They are low-cardinality.
Some(description.to_owned())
}
("file", _) => scrub_file(description),
_ => None,
});

Expand Down Expand Up @@ -111,6 +119,20 @@ fn scrub_http(string: &str) -> Option<String> {
Some(scrubbed)
}

fn scrub_file(description: &str) -> Option<String> {
let filename = match description.split_once(' ') {
Some((filename, _)) => filename,
_ => description,
};
match Path::new(filename).extension() {
Some(extension) => {
let ext = extension.to_str()?;
Some(format!("*.{ext}"))
}
_ => Some("*".to_owned()),
}
}

fn normalize_domain(domain: &str, port: Option<u16>) -> Option<String> {
if let Some(allow_listed) = normalized_domain_from_allowlist(domain, port) {
return Some(allow_listed);
Expand Down Expand Up @@ -476,6 +498,27 @@ mod tests {
"ListAppViewController"
);

span_description_test!(
span_description_file_write_keep_extension_only,
"data.data (42 KB)",
"file.write",
"*.data"
);

span_description_test!(
span_description_file_read_keep_extension_only,
"Info.plist",
"file.read",
"*.plist"
);

span_description_test!(
span_description_fil_no_extension,
"somefilenamewithnoextension",
"file.read",
"*"
);

#[test]
fn informed_sql_parser() {
let json = r#"
Expand Down
4 changes: 3 additions & 1 deletion relay-server/src/actors/processor.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2265,12 +2265,14 @@ impl EnvelopeProcessorService {
.and_then(|system| system.as_str())
.unwrap_or_default();
op == "http.client"
|| op.starts_with("app.")
|| op.starts_with("ui.load")
|| op.starts_with("db")
&& !(op.contains("clickhouse")
|| op.contains("mongodb")
|| op.contains("redis")
|| op.contains("compiler"))
&& !(op == "db.sql.query" && (description.contains(r#""$"#) || system == "mongodb"))
&& !(op == "db.sql.query" && (description.contains("\"$") || system == "mongodb"))
}

#[cfg(feature = "processing")]
Expand Down
45 changes: 45 additions & 0 deletions relay-server/src/metrics_extraction/event.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -896,6 +896,51 @@ mod tests {
"data": {
"db.system": "redis"
}
},
{
"data": {
"device.class": "2",
"environment": "production",
"http.response.status_code": "200",
"mobile": true,
"release": "sentrydemos.ios.EmpowerPlant@0.0.8+1",
"span.op": "ui.load",
"span.status": "ok",
"transaction": "EmpowerPlantViewController",
"transaction.op": "ui.load",
"user": "id:FADC011D-28AA-40B7-8CA8-839A2AD05168"
},
"description": "viewDidLoad",
"exclusive_time": 15101.696732,
"op": "ui.load",
"parent_span_id": "6ebd1cdbb9424b88",
"span_id": "8cfaf7f29ac345b8",
"start_timestamp": 1695255136.239635,
"status": "ok",
"timestamp": 1695255152.073167,
"trace_id": "2dc90ee797b94299ba5ad82b816fc9f8"
},
{
"data": {
"device.class": "2",
"environment": "production",
"http.response.status_code": "200",
"mobile": true,
"release": "sentrydemos.ios.EmpowerPlant@0.0.8+1",
"span.op": "app.start.cold",
"span.status": "ok",
"transaction": "EmpowerPlantViewController",
"transaction.op": "ui.load",
"user": "id:FADC011D-28AA-40B7-8CA8-839A2AD05168"
},
"description": "Cold Start",
"exclusive_time": 0.0,
"op": "app.start.cold",
"parent_span_id": "6ebd1cdbb9424b88",
"span_id": "0e989cd370034c76",
"start_timestamp": 1695255134.469436,
"timestamp": 1695255136.137952,
"trace_id": "2dc90ee797b94299ba5ad82b816fc9f8"
}
]
}
Expand Down
86 changes: 86 additions & 0 deletions ...ction/snapshots/relay_server__metrics_extraction__event__tests__extract_span_metrics.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1075,4 +1075,90 @@ expression: metrics
"transaction.op": "myop",
},
},
Bucket {
timestamp: UnixTimestamp(1695255152),
width: 0,
name: "d:spans/exclusive_time@millisecond",
value: Distribution(
[
15833.532095,
],
),
tags: {
"environment": "fake_environment",
"http.status_code": "500",
"span.description": "viewDidLoad",
"span.group": "247104252ec2b753",
"span.op": "ui.load",
"span.status": "ok",
"span.status_code": "200",
"transaction": "gEt /api/:version/users/",
"transaction.method": "POST",
"transaction.op": "myop",
},
},
Bucket {
timestamp: UnixTimestamp(1695255152),
width: 0,
name: "d:spans/exclusive_time_light@millisecond",
value: Distribution(
[
15833.532095,
],
),
tags: {
"environment": "fake_environment",
"http.status_code": "500",
"span.description": "viewDidLoad",
"span.group": "247104252ec2b753",
"span.op": "ui.load",
"span.status": "ok",
"span.status_code": "200",
"transaction.method": "POST",
"transaction.op": "myop",
},
},
Bucket {
timestamp: UnixTimestamp(1695255136),
width: 0,
name: "d:spans/exclusive_time@millisecond",
value: Distribution(
[
1668.516159,
],
),
tags: {
"environment": "fake_environment",
"http.status_code": "500",
"span.category": "app",
"span.description": "Cold Start",
"span.group": "2d675185edfeb30c",
"span.op": "app.start.cold",
"span.status_code": "200",
"transaction": "gEt /api/:version/users/",
"transaction.method": "POST",
"transaction.op": "myop",
},
},
Bucket {
timestamp: UnixTimestamp(1695255136),
width: 0,
name: "d:spans/exclusive_time_light@millisecond",
value: Distribution(
[
1668.516159,
],
),
tags: {
"environment": "fake_environment",
"http.status_code": "500",
"span.category": "app",
"span.description": "Cold Start",
"span.group": "2d675185edfeb30c",
"span.op": "app.start.cold",
"span.status_code": "200",
"transaction.method": "POST",
"transaction.op": "myop",
},
},
]

0 comments on commit 5517346

Please sign in to comment.