Skip to content

Commit

Permalink
feat(spans): Keep domain + extension for image resources (#2826)
Browse files Browse the repository at this point in the history
Group `resource.img` spans by domain + extension. Known path segments
like `data` are also allowed.

This PR does not yet enable span (metrics) ingestion for all orgs (still
restricted to `all-modules`).

ref: getsentry/sentry#61161
  • Loading branch information
jjbayer authored Dec 11, 2023
1 parent 13a7c59 commit 771e87a
Show file tree
Hide file tree
Showing 3 changed files with 70 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
- Add size limits on metric related envelope items. ([#2800](https://github.com/getsentry/relay/pull/2800))
- Include the size offending item in the size limit error message. ([#2801](https://github.com/getsentry/relay/pull/2801))
- Add metric_bucket data category. ([#2824](https://github.com/getsentry/relay/pull/2824))
- Keep only domain and extension for image resource span grouping. ([#2826](https://github.com/getsentry/relay/pull/2826))

## 23.11.2

Expand Down
2 changes: 1 addition & 1 deletion relay-dynamic-config/src/defaults.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const MOBILE_OPS: &[&str] = &["app.*", "ui.load*"];
const MONGODB_QUERIES: &[&str] = &["*\"$*", "{*", "*({*", "*[{*"];

/// A list of patterns for resource span ops we'd like to ingest.
const RESOURCE_SPAN_OPS: &[&str] = &["resource.script", "resource.css"];
const RESOURCE_SPAN_OPS: &[&str] = &["resource.script", "resource.css", "resource.img"];

/// Adds configuration for extracting metrics from spans.
///
Expand Down
85 changes: 68 additions & 17 deletions relay-event-normalization/src/normalize/span/description/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ pub(crate) fn scrub_span_description(span: &Span) -> Option<String> {
sql::scrub_queries(db_system, description)
}
}
("resource", _) => scrub_resource(description),
("resource", ty) => scrub_resource(ty, description),
("ui", "load") => {
// `ui.load` spans contain component names like `ListAppViewController`, so
// they _should_ be low-cardinality.
Expand Down Expand Up @@ -207,7 +207,7 @@ enum UrlType {
}

/// Scrubber for spans with `span.op` "resource.*".
fn scrub_resource(string: &str) -> Option<String> {
fn scrub_resource(resource_type: &str, string: &str) -> Option<String> {
let (url, ty) = match Url::parse(string) {
Ok(url) => (url, UrlType::Full),
Err(url::ParseError::RelativeUrlWithoutBase) => {
Expand Down Expand Up @@ -262,7 +262,7 @@ fn scrub_resource(string: &str) -> Option<String> {
.path_segments()
.and_then(|s| s.last())
.unwrap_or_default();
let last_segment = scrub_resource_filename(last_segment);
let last_segment = scrub_resource_filename(resource_type, last_segment);

if segments.is_empty() {
format!("{scheme}://{domain}/{last_segment}")
Expand All @@ -282,11 +282,14 @@ fn scrub_resource(string: &str) -> Option<String> {
Some(formatted)
}

fn scrub_resource_filename(path: &str) -> String {
let (mut base_path, mut extension) = path.rsplit_once('.').unwrap_or((path, ""));
fn scrub_resource_filename<'a>(ty: &str, path: &'a str) -> Cow<'a, str> {
if path.is_empty() {
return Cow::Borrowed("");
}
let (mut basename, mut extension) = path.rsplit_once('.').unwrap_or((path, ""));
if extension.contains('/') {
// Not really an extension
base_path = path;
basename = path;
extension = "";
}

Expand All @@ -298,14 +301,20 @@ fn scrub_resource_filename(path: &str) -> String {
extension = "";
}

let mut segments = base_path.split('/').map(scrub_resource_segment);
let mut joined = segments.join("/");
if !extension.is_empty() {
joined.push('.');
joined.push_str(extension);
}
let basename = if ty == "img" {
Cow::Borrowed("*")
} else {
scrub_resource_segment(basename)
};

joined
if extension.is_empty() {
basename
} else {
let mut filename = basename.to_string();
filename.push('.');
filename.push_str(extension);
Cow::Owned(filename)
}
}

fn scrub_resource_segment(segment: &str) -> Cow<str> {
Expand Down Expand Up @@ -555,14 +564,35 @@ mod tests {
resource_query_params2,
"https://data.domain.com/data/guide123.gif?jzb=3f535634H467g5-2f256f&ct=1234567890&v=1.203.0_prod",
"resource.img",
"https://*.domain.com/data/guide*.gif"
"https://*.domain.com/data/*.gif"
);

span_description_test!(
resource_query_params2_script,
"https://data.domain.com/data/guide123.js?jzb=3f535634H467g5-2f256f&ct=1234567890&v=1.203.0_prod",
"resource.script",
"https://*.domain.com/data/guide*.js"
);

span_description_test!(
resource_no_ids,
"https://data.domain.com/js/guide.js",
"resource.script",
"https://*.domain.com/js/guide.js"
);

span_description_test!(
resource_no_ids_img_known_segment,
"https://data.domain.com/data/guide.gif",
"resource.img",
"https://*.domain.com/data/guide.gif"
"https://*.domain.com/data/*.gif"
);

span_description_test!(
resource_no_ids_img,
"https://data.domain.com/something/guide.gif",
"resource.img",
"https://*.domain.com/*/*.gif"
);

span_description_test!(
Expand Down Expand Up @@ -604,6 +634,13 @@ mod tests {
random_string1,
"https://static.domain.com/6gezWf_qs4Wc12Nz9rpLOx2aw2k/foo-99",
"resource.img",
"https://*.domain.com/*/*"
);

span_description_test!(
random_string1_script,
"https://static.domain.com/6gezWf_qs4Wc12Nz9rpLOx2aw2k/foo-99",
"resource.script",
"https://*.domain.com/*/foo-*"
);

Expand Down Expand Up @@ -653,7 +690,7 @@ mod tests {
resource_url_with_fragment,
"https://data.domain.com/data/guide123.gif#url=someotherurl",
"resource.img",
"https://*.domain.com/data/guide*.gif"
"https://*.domain.com/data/*.gif"
);

span_description_test!(
Expand Down Expand Up @@ -725,7 +762,14 @@ mod tests {
resource_img_comma_with_extension,
"https://example.org/p/fit=cover,width=150,height=150,format=auto,quality=90/media/photosV2/weird-stuff-123-234-456.jpg",
"resource.img",
"https://example.org/*/media/*/weird-stuff-*-*-*.jpg"
"https://example.org/*/media/*/*.jpg"
);

span_description_test!(
resource_script_comma_with_extension,
"https://example.org/p/fit=cover,width=150,height=150,format=auto,quality=90/media/photosV2/weird-stuff-123-234-456.js",
"resource.script",
"https://example.org/*/media/*/weird-stuff-*-*-*.js"
);

span_description_test!(
Expand All @@ -735,6 +779,13 @@ mod tests {
"/*/*"
);

span_description_test!(
resource_script_path_with_comma,
"/help/purchase-details/1,*,0&fmt=webp&qlt=*,1&fit=constrain,0&op_sharpen=0&resMode=sharp2&iccEmbed=0&printRes=*",
"resource.script",
"/*/*"
);

span_description_test!(
resource_script_random_path_only,
"/ERs-sUsu3/wd4/LyMTWg/Ot1Om4m8cu3p7a/QkJWAQ/FSYL/GBlxb3kB",
Expand Down

0 comments on commit 771e87a

Please # to comment.