Skip to content

upload-debug-info-to-sentry.py: Upload binaries in addition to debug files#462

Merged
edolstra merged 2 commits into
mainfrom
more-sentry-upload
May 18, 2026
Merged

upload-debug-info-to-sentry.py: Upload binaries in addition to debug files#462
edolstra merged 2 commits into
mainfrom
more-sentry-upload

Conversation

@edolstra
Copy link
Copy Markdown
Collaborator

@edolstra edolstra commented May 18, 2026

Motivation

This should provide better stack unwinding in Sentry.

Context

Summary by CodeRabbit

  • Chores
    • Discovered binaries and libraries are now added to the upload list as they are found during processing.
    • Messaging clarified when no separate debug information is available, and processing continues without duplicating entries.

Review Change Stack

…files

This should provide better stack unwinding in Sentry.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 513515f3-d298-44d1-85e1-1ee725851e9c

📥 Commits

Reviewing files that changed from the base of the PR and between 3d701da and e7cd684.

📒 Files selected for processing (1)
  • maintainers/upload-debug-info-to-sentry.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • maintainers/upload-debug-info-to-sentry.py

📝 Walkthrough

Walkthrough

The ELF processing loop now appends each discovered non‑Darwin binary to the upload list immediately, then attempts build-id extraction and debuginfo lookup. If no separate debuginfo is found, the script logs that and proceeds (the binary is already queued).

Changes

Binary upload timing

Layer / File(s) Summary
Upload binary unconditionally before build-id resolution
maintainers/upload-debug-info-to-sentry.py
Each ELF library/executable is appended to debug_files immediately as the loop iterates. In the missing separate debuginfo path, the script logs that no separate debug info exists and continues without re-appending, since the binary is already in the upload list.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • DeterminateSystems/nix-src#434: Yes—both PRs modify maintainers/upload-debug-info-to-sentry.py’s non-Darwin debug-info upload flow.
  • DeterminateSystems/nix-src#435: Both PRs modify maintainers/upload-debug-info-to-sentry.py’s ELF/debug-info resolution flow to ensure .so/binaries are added to the upload list when build-id or separate debuginfo lookup fails.

Suggested reviewers

  • cole-h
  • grahamc

Poem

🐰 I found a lib out on the trail,

Queued it first, so uploads won't fail,
When debug info hides or takes a nap,
The binary's safe in the Sentry map! 📦✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: uploading binaries in addition to debug files, which is the primary modification to the script.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch more-sentry-upload

Comment @coderabbitai help to get the list of available commands and usage tips.

cole-h
cole-h previously approved these changes May 18, 2026
grahamc
grahamc previously approved these changes May 18, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
maintainers/upload-debug-info-to-sentry.py (1)

134-137: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Remove duplicate append to prevent uploading the same binary twice.

Line 131 now unconditionally appends every library to the upload list. Line 136 appends the library again when there's no build ID, creating a duplicate entry in files_to_upload. The duplicate will cause the same file to be uploaded twice to Sentry.

🐛 Proposed fix to remove duplicate append
     build_id = get_build_id(lib)
     if build_id is None:
         print(f"  {lib} (no build ID, uploading binary)", file=sys.stderr)
-        debug_files.append(lib)
         continue
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@maintainers/upload-debug-info-to-sentry.py` around lines 134 - 137, The code
is appending the same library twice to the upload list: one unconditional append
earlier (files_to_upload or debug_files) and a second append in the no-build-id
branch (debug_files.append(lib)); remove the duplicate by deleting the
conditional append inside the if build_id is None block (or move the
unconditional append into the else branch) so each library is added exactly
once; update references to build_id, debug_files and files_to_upload accordingly
to ensure only a single append happens per library.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@maintainers/upload-debug-info-to-sentry.py`:
- Around line 134-137: The code is appending the same library twice to the
upload list: one unconditional append earlier (files_to_upload or debug_files)
and a second append in the no-build-id branch (debug_files.append(lib)); remove
the duplicate by deleting the conditional append inside the if build_id is None
block (or move the unconditional append into the else branch) so each library is
added exactly once; update references to build_id, debug_files and
files_to_upload accordingly to ensure only a single append happens per library.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3801a0e4-70f4-4cab-8c96-fd25b69d6170

📥 Commits

Reviewing files that changed from the base of the PR and between b181e64 and 3d701da.

📒 Files selected for processing (1)
  • maintainers/upload-debug-info-to-sentry.py

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 18, 2026

@github-actions github-actions Bot temporarily deployed to pull request May 18, 2026 14:28 Inactive
@edolstra edolstra enabled auto-merge May 18, 2026 14:28
@edolstra edolstra added this pull request to the merge queue May 18, 2026
@edolstra edolstra removed this pull request from the merge queue due to a manual request May 18, 2026
@edolstra edolstra dismissed stale reviews from grahamc and cole-h via e7cd684 May 18, 2026 14:36
@edolstra edolstra enabled auto-merge May 18, 2026 14:36
@github-actions github-actions Bot temporarily deployed to pull request May 18, 2026 14:40 Inactive
@edolstra edolstra added this pull request to the merge queue May 18, 2026
Merged via the queue into main with commit dcda71e May 18, 2026
29 checks passed
@edolstra edolstra deleted the more-sentry-upload branch May 18, 2026 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants