Skip to content

fix(profiler): encode debug unwind event sizes as varints - #916

Merged
kaahos merged 2 commits into
mainfrom
paul.fournillon/fix-unwind-failure-encoding
Oct 30, 2026
Merged

kaahos merged 2 commits into
mainfrom
paul.fournillon/fix-unwind-failure-encoding

Conversation

@kaahos

@kaahos kaahos commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?:
Encode debug unwind-failure event sizes with a multi-byte JFR varint.

Motivation:
Long runtime-stub names can exceed the one-byte event-size limit. Reserve a complete size prefix so debug recordings retain valid event boundaries instead of aborting or becoming unreadable.

Additional Notes:

How to test the change?:

./.claude/commands/build-and-summarize :ddprof-lib:gtestDebug \
  :ddprof-lib:gtestRelease_unwindFailureEncoding_ut qualityChecks

Validation for 966e098, including the reviewed documentation-only prerequisite update:

Check State Result
macOS ARM64 full native debug passed Full suite on refreshed prerequisite
Encoding regressions passed Six name lengths, diagnostic fields, maximum counters and following-event boundary
Release encoding guard passed Explicit skip because debug diagnostics are compiled out
macOS ARM64 affected Java smoke passed Six active cases; seven existing mode skips
Formatting and quality checks passed spotlessApply, qualityChecks, diff check
Full PR scope passed Encoding delta byte-identical after prerequisite update
Remote CI passed 113 reported checks succeeded; no failures or pending checks

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a security review.
  • This PR doesn't touch any of that.
  • JIRA: PROF-16298

@dd-octo-sts

dd-octo-sts Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #41051492859 | Commit: 9efc022 | Duration: 19m 18s (longest job)

✅ All 108 test jobs passed

Status Overview

JDK glibc-aarch64/asan glibc-aarch64/debug glibc-aarch64/release glibc-aarch64/tsan glibc-amd64/asan glibc-amd64/debug glibc-amd64/release glibc-amd64/tsan musl-aarch64/debug musl-aarch64/release musl-amd64/debug musl-amd64/release
8 - - - - ✅ ✅ ✅ ✅ - - - -
8-ibm - - - - ✅ ✅ ✅ ✅ - - - -
8-j9 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
8-librca - - - - - - - - ✅ ✅ ✅ ✅
8-orcl - - - - ✅ ✅ ✅ ✅ - - - -
11 - - - - ✅ ✅ ✅ ✅ - - - -
11-j9 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
11-librca - - - - - - - - ✅ ✅ ✅ ✅
17 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
17-graal ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
17-j9 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
17-librca - - - - - - - - ✅ ✅ ✅ ✅
21 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
21-graal ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
21-librca - - - - - - - - ✅ ✅ ✅ ✅
25 ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
25-graal ✅ ✅ ✅ ✅ ✅ ✅ ✅ ✅ - - - -
25-librca - - - - - - - - ✅ ✅ ✅ ✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 108 | Passed: 108 | Failed: 0


Updated: 2026-10-30 13:36:55 UTC

@dd-octo-sts

dd-octo-sts Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 d1b347f6

@kaahos

kaahos commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-28T13:24:43.908939Z 966e098 Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. More of your lovely PRs please.

Reviewed commit: 9ee9826193

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@kaahos
kaahos marked this pull request as ready for review October 28, 2026 13:21
@kaahos
kaahos requested a review from a team as a code owner October 28, 2026 13:21
@kaahos

kaahos commented Oct 28, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@datadog-datadog-us1-prod datadog-datadog-us1-prod Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bits Code Review: PASS

More details

The reserved five-byte prefix matches the existing JFR varint writer, preserving event boundaries for long debug unwind-failure names without changing release behavior.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 966e098

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 966e0983e9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@datadog-datadog-us1-prod

datadog-datadog-us1-prod Bot commented Oct 29, 2026 •

Copy link
Copy Markdown

Pipelines

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: d1b347f | Docs | View more details | Give us feedback!

@jbachorik
jbachorik added this pull request to stack #922 October 29, 2026 15:03
@kaahos
kaahos force-pushed the paul.fournillon/fix-unwind-failure-encoding branch 2 times, most recently from 42d245a to 8a64173 Compare October 29, 2026 17:14
Base automatically changed from paul.fournillon/fix-frozen-virtual-thread-truncation to main October 30, 2026 11:45
@kaahos
kaahos force-pushed the paul.fournillon/fix-unwind-failure-encoding branch from 8a64173 to d1b347f Compare October 30, 2026 11:45

@datadog-datadog-us1-prod datadog-datadog-us1-prod Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bits Code Review: PASS

More details

The reserved varint prefix accommodates bounded unwind-failure names while preserving event boundaries and diagnostic fields. Source review also found the continuation-truncation changes consistent with their callers and regression tests.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Bits Code Review · Commit 8a64173

@kaahos kaahos added test:release Run CI tests with release configuration test:asan Run CI tests with AddressSanitizer configuration test:tsan Run CI tests with ThreadSanitizer configuration test:fuzz test:reliability labels Oct 30, 2026
@kaahos
kaahos merged commit 140a841 into main Oct 30, 2026
402 of 436 checks passed
@kaahos
kaahos deleted the paul.fournillon/fix-unwind-failure-encoding branch October 30, 2026 14:16
@github-actions github-actions Bot added this to the 1.53.0 milestone Oct 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:asan Run CI tests with AddressSanitizer configuration test:fuzz test:release Run CI tests with release configuration test:reliability test:tsan Run CI tests with ThreadSanitizer configuration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant