feat(explorer): add rpc for profile flamegraph tool #103293
Conversation
github-actions
bot
added
the
Scope: Backend
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #103293 +/- ##
===========================================
- Coverage 80.67% 80.67% -0.01%
===========================================
Files 9243 9243
Lines 394826 394912 +86
Branches 25152 25152
===========================================
+ Hits 318538 318596 +58
- Misses 75841 75869 +28
Partials 447 447 |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| project_id = row.get("project.id") | ||
| min_start_ts = row.get("min(precise.start_ts)") | ||
| max_end_ts = row.get("max(precise.finish_ts)") | ||
| break |
There was a problem hiding this comment.
Bug: Aggregate Query Mixes Profile Data
The aggregate query at lines 349-367 doesn't use GROUP BY when selecting non-aggregated columns (profile.id, profiler.id, project.id) alongside aggregation functions (min(precise.start_ts), max(precise.finish_ts)). When the wildcard query matches spans with different full profile IDs sharing the same prefix, the aggregates compute min/max timestamps across all profiles while returning an arbitrary profile ID. This causes fetch_profile_data to receive timestamps from different profiles than the one being fetched, potentially leading to incorrect or missing profile data.
There was a problem hiding this comment.
pretty sure it automatically applies the group by
|
|
||
|
|
||
| @pytest.mark.django_db(databases=["default", "control"]) | ||
| class TestRpcGetProfileFlamegraph(APITransactionTestCase, SpanTestCase, SnubaTestCase): |
There was a problem hiding this comment.
i'm wondering if a transaction test case is actually necessary? (it is much slower than the regular APITestCase)
| assert result["external_id"] == "12345678" | ||
|
|
||
|
|
||
| @pytest.mark.django_db(databases=["default", "control"]) |
There was a problem hiding this comment.
i don't think this decorator is necessary?
|
Semgrep found 1 Risk: Affected versions of django are vulnerable to Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'). The ORM methods QuerySet.filter(), QuerySet.exclude(), QuerySet.get() and the Q() class can be tricked into SQL injection when you pass a specially crafted dictionary via **kwargs that includes a malicious Fix: Upgrade this library to at least version 5.2.8 at sentry/uv.lock:305. Reference(s): GHSA-frmv-pr5f-9mcr, CVE-2025-64459 |
![semgrep-code-getsentry[bot]](https://avatars.githubusercontent.com/in/1153895?s=60&v=4){kind=small}
| return {"error": "Organization not found"} | ||
|
|
||
| # Get all projects for the organization | ||
| projects = list(Project.objects.filter(organization=organization, status=ObjectStatus.ACTIVE)) |
There was a problem hiding this comment.
Risk: Affected versions of django are vulnerable to Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'). The ORM methods QuerySet.filter(), QuerySet.exclude(), QuerySet.get() and the Q() class can be tricked into SQL injection when you pass a specially crafted dictionary via **kwargs that includes a malicious _connector entry. This bypasses the normal query parameterization and lets an attacker inject arbitrary SQL into the WHERE clause.
Fix: Upgrade this library to at least version 5.2.8 at sentry/uv.lock:305.
Reference(s): GHSA-frmv-pr5f-9mcr, CVE-2025-64459
🎈 Fixed in commit a12ee26 🎈
Adds an RPC that gets a profile flamegraph given just a profile id (8 char or 32 char). Automatically finds the full ID and distinguishes between transaction and continuous profiles. The caveat is we require tracing for profiling to work, but this is true in 99% of cases already.