feat(spans): Keep domain + extension for image resources #2826
Conversation
| let basename = if ty == "img" { | ||
| Cow::Borrowed("*") | ||
| } else { | ||
| scrub_resource_segment(basename) | ||
| }; |
There was a problem hiding this comment.
This is the main functional change in this PR: for img spans, only keep the extension.
| let mut segments = base_path.split('/').map(scrub_resource_segment); | ||
| let mut joined = segments.join("/"); |
There was a problem hiding this comment.
Drive-by fix: Segments are already handled in scrub_resource, so no need to do it again here.
| 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" |
There was a problem hiding this comment.
@DominikB2014 these tests illustrate what the new span descriptions will look like.
| "resource.img", | ||
| "https://example.org/*/media/*/weird-stuff-*-*-*.jpg" | ||
| "https://example.org/p/fit=cover,width=150,height=150,format=auto,quality=90/media/photosV2/weird-stuff-123-234-456.js", | ||
| "resource.script", |
There was a problem hiding this comment.
Why are we changing existing tests, instead of adding new ones? Should we update the test names if we update what they're testing?
There was a problem hiding this comment.
The intention of the test was to make sure commas don't make it into the scrubbed description, and that is now trivially true for resource.img because the entire base name gets removed. But you are right, I will duplicate the tests instead.
|
One thing we might have to consider in the future is that we can still have images that don't have the span.op:resource.img, once merged, I can monitor how it looks in this case. |
#2826 introduced grouping `resource.img` spans by domain + extension, allowing us to enable span metrics for this op. Now also enable `resource.img` for the spans dataset.
Group
resource.imgspans by domain + extension. Known path segments likedataare also allowed.This PR does not yet enable span ingestion for all orgs (still restricted to
all-modules).ref: getsentry/sentry#61161