feat(sourcemaps): Add logic to insert data into new debug ids table #44885
Conversation
|
🚨 Warning: This pull request contains Frontend and Backend changes! It's discouraged to make changes to Sentry's Frontend and Backend in a single pull request. The Frontend and Backend are not atomically deployed. If the changes are interdependent of each other, they must be separated into two pull requests and be made forward or backwards compatible, such that the Backend or Frontend can be safely deployed independently. Have questions? Please ask in the |
0d6cc46 to
80477e9
Compare
size-limit report 📦
|
| }, | ||
| "files/_/_/index.js": { | ||
| "url": "~/index.js", | ||
| "type": "source", |
There was a problem hiding this comment.
source cannot have debug ids, this has to be minified_source.
|
|
||
| schema = { | ||
| "type": "object", | ||
| "properties": { |
There was a problem hiding this comment.
I would put an projects key into the schema and then allow the request to specify one or more projects for association. That would also mean that assemble_artifacts takes a list of project IDs rather than one.
There was a problem hiding this comment.
Yep, I still didn't implement the new schema for the endpoint
src/sentry/tasks/assemble.py
Outdated
| file_type = info.get("type", None) | ||
|
|
||
| if debug_id is not None and file_type is not None: | ||
| debug_ids_with_types.append((SourceFileType.from_lowercase_key(file_type), debug_id)) |
There was a problem hiding this comment.
This could return None, needs to be guarded against.
src/sentry/tasks/assemble.py
Outdated
| files = manifest.get("files", {}) | ||
| for filename, info in files.items(): | ||
| headers = _normalize_headers(info.get("headers", {})) | ||
| debug_id = _normalize_debug_id(headers.get("debug-id", None)) |
There was a problem hiding this comment.
I would explicitly check if the debug-id is not None before calling into normalize_debug_id and swallowing the error there.
tests/sentry/tasks/test_assemble.py
Outdated
| assert details is None | ||
|
|
||
| for debug_id in debug_ids: | ||
| debug_id_artifact_bundles = DebugIdArtifactBundle.objects.filter(debug_id=debug_id) |
There was a problem hiding this comment.
It would be dangerous to write code like this outside of tests becauase it can leak cross organization data (eg: same debug id existing in more than one org). Before someone copy pastes this elsewhere, please make sure that this has the organization_id as constraint too.
There was a problem hiding this comment.
Oh yeah, thanks for the heads-up! I forgot about that
src/sentry/tasks/assemble.py
Outdated
| debug_ids_with_types = [] | ||
|
|
||
| files = manifest.get("files", {}) | ||
| for filename, info in files.items(): |
There was a problem hiding this comment.
Rename filename to file_path.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #44885 +/- ##
==========================================
- Coverage 80.24% 80.19% -0.06%
==========================================
Files 4725 4725
Lines 198780 199078 +298
Branches 12006 12030 +24
==========================================
+ Hits 159519 159646 +127
- Misses 38999 39170 +171
Partials 262 262
|
This PR aims at implementing the logic for extracting debug_ids information from the
manifest.jsonof the uploaded.zipartifact bundle.The new logic reads the updated
manifest.jsonfile and looks for theheader.debug-idandprojectfields.A sample
manifest.jsoncan be seen here:{ "org": "123", "files": { "files/_/_/index.js.map": { "url": "~/index.js.map", "type": "source_map", "headers": { "debug-id": "eb6e60f1-65ff-4f6f-adff-f1bbeded627b" } }, "files/_/_/index.js": { "url": "~/index.js", "type": "source", "headers": { "Sourcemap": "index.js.map", "debug-id": "eb6e60f1-65ff-4f6f-adff-f1bbeded627b" } } } }