Repository navigation
feat(teapot): Name the faulting shader of a GPU crash from uploaded DXIL binaries - #129684
tustanivsky wants to merge 28 commits into
Conversation
github-actions
Bot
added
the
Scope: Backend
06e5fcc to
8e206d6
Compare
Register two debug file kinds for GPU shader symbolication: dxil (DXC shader binary, DXBC magic, code_id = 16-hex Aftermath hash) and dxilpdb (DXC shader PDB, MSF magic with an ILDN part). Both share the DXC hash as debug_id and arrive through the existing chunk-upload and DIF assemble API, which now accepts an optional code_id. When teapot answers "partial" with dxil/dxil_pdb entries in missing_difs, look the files up by code_id / debug_id, presign the objectstore-backed ones and call teapot once more with shader_binaries / shader_pdbs and a "-2" idempotency key. Any failure in that round falls back to the first response. Add SENTRY_TEAPOT_INLINE_ATTACHMENT_FALLBACK (default off) so a dev server whose Relay keeps attachments out of objectstore can still hand teapot presigned URLs.
Resolve shader DIFs with one presign helper and emit the teapot request entries directly, instead of a dataclass and four lookup helpers. Inline the dxil code_id and shader_group validation like the other DIF kinds and reduce the shader group backfill to its condition.
The native frame renderer shows package, function and filename only, so the group teapot puts into frame.module never reached the page. Write it to the gpu.shader_group, gpu.material and gpu.shader_class tags, the gpu_crash context, the culprit and the frame's filename when UE's sentinel or nothing fills that slot.
Relay streams every attachment to objectstore before the GPU crash task runs, so an attachment without a stored_id is an error, not a case to paper over.
The assemble endpoint answers from the existing row for a known checksum and never runs the task, so a group that a later cook supplies for an already uploaded dxil binary was dropped.
The Linux decoder never reads a shader PDB, so only the DXIL binary is stored, resolved and sent to teapot. The dxilpdb kind, the sibling PDB lookup and the shader_pdbs request field return with line numbers.
8e206d6 to
57d7064
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
## Summary Sentry can now name the shader that caused a GPU crash, but only if it has the game's compiled shaders. Unreal Engine builds export them as `.dxil` files (one per shader, over ten thousand per game). This PR lets `sentry debug-files upload` upload them as `dxil` debug files and makes the upload path fast enough for sets of that size. This is the toolkit version of getsentry/sentry-cli#3489, which was implemented earlier to test with the Unreal SDK which has not migrated to the Sentry toolkit yet (getsentry/sentry-unreal#1680). ## Changes - The scanner recognises a DXIL container by its header, before the files reach the object parser, which does not know shaders. The debug id is the 32-hex compiler hash the engine uses as the file name. `dxil` joins the `--type` list; `--id` accepts the bare file name or the hyphenated id. Shaders have none of the symbol/unwind/source features the other formats are filtered on, so those filters do not apply to them. - The GPU crash decoder identifies a shader by NVIDIA's own hash of it, which only NVIDIA's Windows library can compute. The Sentry Unreal plugin computes it at build time and writes `sentry-shader-manifest.tsv` next to the shaders. The CLI reads that manifest once per directory and sends the hash and the shader group as the new `code_id` and `shader_group` assemble fields. Without a manifest the shader is uploaded without them. - Assemble requests are sent in batches of a thousand files. - Missing chunks are uploaded in batched requests, packed up to the server's `chunksPerRequest` and `maxRequestSize` limits, with a chunk shared by several files sent once. They were sent one request per chunk, so ten thousand small files took a lot of time. - The assemble request is repeated right after the chunks are delivered, and the processing wait starts from that point. Before, the wait started before the upload; when the upload alone outlasted it, the command exited without the request that starts processing, and reported the files as not found or timed out although every chunk was on the server. The last three points apply to every debug file type. Files that are not shaders otherwise take exactly the old path. `debug-files find` and `check` do not learn about shaders here. ## Possible Follow-ups Shader PDBs would add source file and line context to GPU crashes. They are left out for now because the server cannot use them yet, but the manifest already lists the PDB that belongs to each binary. ## Related - getsentry/sentry#129684 adds the `dxil` debug file format and the assemble fields - getsentry/teapot#22 names the shader from the binary - getsentry/sentry-unreal#1678 exports and hashes the shaders after the cook - getsentry/sentry-cli#3489 is the legacy CLI version of this change
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ea98991. Configure here.
| } | ||
|
|
||
|
|
||
| def test_client_sends_sources_only_when_given(mock_objectstore: mock.Mock) -> None: |
There was a problem hiding this comment.
Out of scope for this PR but I passed by as I was pinged and noticed you're mocking Objectstore in this test file. You don't really need to do that as it's available as a devservice. As far as I know we do proper tests against it everywhere (such as in symbolicator) rather than mocking.
There was a problem hiding this comment.
Thanks, good point. I believe mock_objectstore came with the original Teapot integration and this test just follows the rest of the file. Although migrating to devservice in a follow-up so this PR stays focused makes perfect sense.
Teapot looks up shader binaries in Sentry after the decode. Send max_decode_ms two seconds below the request timeout, so Teapot answers with what it has before the client gives up.
mujacica
left a comment
mujacica
left a comment
There was a problem hiding this comment.
Went through this end to end together with getsentry/teapot#22 (getsentry/ops#25348 is merged). The approach looks good. Three things I'd fix before merging, plus one smaller one, all inline:
- The 3 s decode budget turns slow but working decodes into 504s.
- Sending
sourcesneeds its own off switch. code_idis never filled in on a re-upload.- Smaller: the culprit comes from
frames[0], which on page faults isn't necessarily the shader that faulted.
For DXIL we're going with Sentry's own debug-file store and asking customers to upload, so the internal sentry source is the only one this needs for now.
|
|
||
| # Teapot's budget ends before the timeout, so it answers with what it has | ||
| # instead of being cut off. `MIN_DECODE_BUDGET_MS` is teapot's lower limit. | ||
| DECODE_BUDGET_MARGIN_MS = 2000 |
There was a problem hiding this comment.
Blocking: with the default 5 s timeout this sends max_decode_ms=3000. Teapot's budget covers the decode plus the lookups, and when the decode itself runs over, Teapot returns a 504 ("decode budget exceeded" in SymbolicateController.cpp), not a partial result. 504 is in RETRYABLE_STATUS, so we retry, fail the same way, and count it toward the circuit breaker.
A real page-fault dump in our samples takes ~3.2 s to decode. It works today and would fail after this PR. The teapot.timeout-seconds comment ("decode is sub-second") is out of date as well.
Suggestion: raise the timeout to ~15 s (DEFAULT_TIMEOUT here and the option default) and keep the 2 s margin, so a 13 s budget. Two attempts still fit the task's 120 s deadline. Worth checking Teapot's p99 request duration before settling on the number. The comment above could also say that the budget stops the lookups early, since the decode itself isn't cut short.
There was a problem hiding this comment.
Done, timeout was increased to 15s.
| event_id, | ||
| cast(TeapotAttachment, dump), | ||
| cast(Sequence[TeapotAttachment], shaders), | ||
| sources=[get_internal_source(project)], |
There was a problem hiding this comment.
Every GPU crash now sends the project's debug-file source with the system token, and Teapot makes up to 16 lookups per crash, including for projects that never uploaded a shader. Right now the only way to turn that off is TEAPOT_SOURCE_ENABLED in Teapot, which needs a deploy. Could we gate it on an option?
register("teapot.shader-sources.enabled", default=False, flags=FLAG_AUTOMATOR_MODIFIABLE)
sources = [get_internal_source(project)] if options.get("teapot.shader-sources.enabled") else NoneOptional: also skip it when get_last_upload(project.id) is empty, i.e. the project never uploaded any debug file.
There was a problem hiding this comment.
Fair point, added a new option (off by default) + skipped get_last_upload check.
| and debug_file.file_format == "dxil" | ||
| and not (debug_file.data or {}).get("shader_group") | ||
| ): | ||
| debug_file.update(data={**(debug_file.data or {}), "shader_group": shader_group}) |
There was a problem hiding this comment.
This fills in a missing shader_group, but not a missing code_id. If a binary first went up without a manifest entry, re-uploading it with one has the same checksum and lands here, so code_id stays empty and Teapot can never find that shader.
if debug_file.file_format == "dxil":
updates = {}
if shader_group and not (debug_file.data or {}).get("shader_group"):
updates["data"] = {**(debug_file.data or {}), "shader_group": shader_group}
code_id = (file.get("code_id") or "").lower()
if code_id and not debug_file.code_id and _SHADER_CODE_ID_RE.match(code_id):
updates["code_id"] = code_id
if updates:
debug_file.update(**updates)_cached_dif_lacks_shader_group needs the same for code_id (the cached dif has codeId). A test would cover it: upload without code_id, re-upload with it, then list by code_id.
There was a problem hiding this comment.
Good catch, re-uploading a known binary now fills in a missing code_id (including when the assemble status is cached). Added a test for both paths.
| def _shader_group(response: Mapping[str, Any]) -> str | None: | ||
| """The UE shader group recorded with the faulting frame's dxil debug file, if any.""" | ||
| frames = response.get("frames") or [] | ||
| first = frames[0] if frames and isinstance(frames[0], dict) else {} |
There was a problem hiding this comment.
Teapot builds frames in active-shader order (build_frames), not by which warp faulted. On a page fault with more than one active shader, frames[0] can be a different shader from the one that faulted, and then the culprit, gpu.material and gpu.shader_class point at the wrong material.
Suggest taking the group from the frame Teapot marks as the faulting one (proposed data.faulted in getsentry/teapot#22), and not overriding the culprit on page faults when no frame is marked.
There was a problem hiding this comment.
The group, culprit and material tags now come from the frame Teapot marks with data.faulted. The shader hash, type and faulting PC follow the same frame so they don't point at a different shader. On a page fault without a mark, no shader is named.
A page fault dump can take a few seconds to decode, more than the 3 s budget that the 5 s timeout left, and a decode over the budget gets a 504 that is retried and counts toward the circuit breaker. Two attempts of 15 s still fit the task's 120 s deadline. The budget comment now says that it stops the shader binary lookups early.
With the source, Teapot looks up the shader binaries of every GPU crash with the system token. The option teapot.shader-sources.enabled, off by default, turns that on and off without a Teapot deploy.
A dxil binary that went up without a manifest entry has no code_id, so Teapot can never find it. Uploading the same file again with a code_id now stores it, as it already did for the shader group, also when the assemble status of the file is cached.
Teapot's frames follow the order of the active shaders, so the first frame is not always the shader that faulted. The shader group, culprit, material, shader class, shader hash and faulting PC now come from the frame that Teapot marks with data.faulted. Without a mark the first frame is used, except on a page fault, where it can be any running shader and no shader is named. The decoder's own name of a renamed frame is kept in raw_function.
When a game's GPU hangs or faults, the NVIDIA Aftermath SDK writes a crash dump and Sentry sends it to Teapot (Sentry's GPU crash decoder) to turn it into an event. The dump identifies the shader that was running only by a 64-bit hash. Teapot has no name to go with that hash, so today the event shows a placeholder such as
compute_01and the developer has to guess which shader it was.This PR closes that gap with debug files, the same way native crashes get symbolicated. A game can upload its compiled shaders (DXIL binaries) as debug files, keyed by that Aftermath hash. Sentry passes Teapot the project's debug file source, as it does for Symbolicator, and Teapot fetches the binaries it needs itself. The event then shows the shader's real entry point and, for material shaders, the material and pass it belongs to.
Key Changes
dxildebug file format. The debug ID is the compiler's hash from the file name, the code ID is the Aftermath hash computed by the uploader, and an optional shader group from the Unreal Engine shader symbol export is stored with the file. Both arrive as new optional fields of the assemble request; re-uploading a known binary fills in a missing group or code ID.get_internal_source(project)to its Teapot request when the optionteapot.shader-sources.enabledis on (off by default), the same entry with the system token that Symbolicator receives, and Teapot fetches the binaries it needs from there.max_decode_mstwo seconds below the request timeout (13 s with the default 15 s), so Teapot answers with what it has before the client gives up.gpu.shader_group,gpu.materialandgpu.shader_classtags, part of the GPU crash context, and the frame's filename when Teapot provides none, so it is visible in the issue list and on the stack trace.No migration. Events without uploaded shaders are processed exactly as before. Teapot reaches the debug files endpoint at
system.internal-url-prefix, as Symbolicator does.Known Limitations
Testing
Unit tests cover the debug file format and the group backfill, the source and the decode budget passed to Teapot, renewal on code ID lookups, and the event fields.
Verified end to end against a local stack with a UE 5.8 game, for a global compute shader, a material pixel shader and the sample project's GPU hang test: all events completed with the shader name and group and no missing debug files.
With a shader's binary deleted, the crash still becomes a normal GPU event with the placeholder name, status
partialand one missing debug file.Before:
After:
Related Items
--type dxil