feat(spans): Record resource spans for clustering #58689
Conversation
github-actions
bot
added
the
Scope: Backend
| ) | ||
| assert len(mocked_record.mock_calls) == expected | ||
|
|
||
|
|
There was a problem hiding this comment.
We only look at the scrubbed description for resource spans, so this test case seemed redundant (there is already a test_record_span_desc_url).
| if len(tokens) != 2: | ||
| return None | ||
| url = tokens[1] | ||
| return urlparse(url).path |
There was a problem hiding this comment.
Resource spans do not have a GET prefix, so this helper can be removed.
Codecov Report
@@ Coverage Diff @@
## master #58689 +/- ##
==========================================
- Coverage 80.59% 80.58% -0.01%
==========================================
Files 5151 5151
Lines 225411 225405 -6
Branches 37983 37982 -1
==========================================
- Hits 181667 181648 -19
- Misses 38192 38203 +11
- Partials 5552 5554 +2
|
| @@ -201,37 +199,28 @@ def record_span_descriptions( | |||
|
|
|||
|
|
|||
| def _get_span_description_to_store(span: Mapping[str, Any]) -> Optional[str]: | |||
| if not span.get("op", "").startswith("http"): | |||
| if not span.get("op") in ("resource.css", "resource.js"): | |||
There was a problem hiding this comment.
Is resource.js an op? I don't see it documented in dev docs. Related -- should we include more resource.* spans?
There was a problem hiding this comment.
Thanks for catching this:
/// A list of patterns for resource span ops we'd like to ingest.
const RESOURCE_SPAN_OPS: &[&str] = &["resource.script", "resource.css"];
| @@ -201,37 +199,28 @@ def record_span_descriptions( | |||
|
|
|||
|
|
|||
| def _get_span_description_to_store(span: Mapping[str, Any]) -> Optional[str]: | |||
| if not span.get("op", "").startswith("http"): | |||
| if not span.get("op") in ("resource.css", "resource.js"): | |||
There was a problem hiding this comment.
Thanks for catching this:
/// A list of patterns for resource span ops we'd like to ingest.
const RESOURCE_SPAN_OPS: &[&str] = &["resource.script", "resource.css"];
|
After some discussion we decided to go with a deterministic approach for now. See getsentry/relay#2654. |
|
Update: We will not enforce discovered rules in relay, but will use the clusterer to collect patterns that might be converted to manual rules. |
We previously only recorded span description for URL clustering if the span op was
http. For those spans, the clustering results are not used anymore because we scrub HTTP spans more radically these days.Record & cluster resource spans instead, because for those, we still keep URL segments under specific circumstances.
Update 2023-11-14: This PR now also removes the clustering rules from project configs entirely, and as an experiment sets the span clustering threshold to 50.