Skip to content

Address code review feedback for build-and-summarize - #775

Draft
jbachorik wants to merge 2 commits into
mainfrom
jb/fix-build-and-summarize
Draft

jbachorik wants to merge 2 commits into
mainfrom
jb/fix-build-and-summarize

Conversation

@jbachorik

Copy link
Copy Markdown
Collaborator

What does this PR do?:

Fixes three bugs in the deterministic Gradle log parser inside .claude/commands/build-and-summarize (added in an uncommitted local change prior to this PR, replacing a headless-claude-spawn approach with a self-contained Python parser):

  • An IndexError when a stray non-blank, non-* line appears between the FAILURE: banner and the * What went wrong: header, leaving headlines empty at index time.
  • Test/module counts being misattributed under parallel Gradle builds, since they were attached to "whatever > Task line was last seen," including non-test tasks that interleave with a test task's completion line.
  • Failure headlines being paired with failed tasks by list position instead of identity, so with --continue and multiple failing tasks, headlines could land on the wrong task or go missing entirely.

Also removes an unverifiable comment claiming managed settings pin a specific model and bill real money for log parsing — that claim isn't backed by anything in this file.

Motivation:

A code review of the uncommitted parser changes surfaced these correctness bugs before they shipped.

Additional Notes:

None.

How to test the change?:

Verified by hand-running the parser logic against a synthetic Gradle log that reproduces all three bugs (stray line before a headline, parallel task interleaving, multi-failure --continue output) and confirming correct output with no crash.

For Datadog employees:

  • This PR doesn't touch any of that.
  • JIRA: N/A

@jbachorik jbachorik added the AI label Sep 1, 2026
@jbachorik jbachorik changed the title Fix build-and-summarize log-parsing bugs Address code review feedback for build-and-summarize Sep 1, 2026
Fixes an IndexError on stray log lines, test-count misattribution
under parallel builds, and positional (rather than identity-based)
pairing of failure headlines with failed tasks. Also removes an
unverifiable comment about managed-settings model billing.
@jbachorik
jbachorik force-pushed the jb/fix-build-and-summarize branch from dc33515 to 30357ff Compare September 1, 2026 15:46
@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Scan-Build Report

User:runner@runnervmlun5p
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Thu Sep 17 10:01:13 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs1
Logic error
Dereference of null pointer1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Logic errorDereference of null pointerfaultInjection.cppcrashNow242

@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #35208081255 | Commit: b6b97da | Duration: 15m 54s (longest job)

All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
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: 32 | Passed: 32 | Failed: 0


Updated: 2026-09-17 10:19:00 UTC

@dd-octo-sts

dd-octo-sts Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

39 passed, 1 failed out of 40 configurations

Test Matrix

Platform JDK 8 JDK 11 JDK 17 JDK 21 JDK 25
glibc-x64-hotspot
glibc-x64-openj9
glibc-arm64-hotspot
glibc-arm64-openj9
musl-x64-hotspot
musl-x64-openj9
musl-arm64-hotspot
musl-arm64-openj9

Failure Details

glibc-x64-hotspot-jdk25

Profiler-only:

"  Skipped (tracer not expected in this configuration)"
""
"[7/8] Checking for unexpected events..."
"  ? No unexpected events found"
""
"[8/8] Scenario-specific validation (ddprof_only)..."
"  Validating profiler-only scenario..."
"  ? Profiler-only scenario checks passed"
""
"=== Validation Summary ==="
"ExecutionSample:            0.0 events (datadog.ExecutionSample)"
"Stack traces:               0.0 samples"
"Thread diversity:           0.0 threads"
"Allocation samples:         0.0 events (jdk.ObjectAllocationSample)"
"ThreadAllocationStatistics: 43 events"
""
"VALIDATION_FAILED: One or more checks did not pass"

Script executed successfully: 425 commands
VALIDATION_FAILED: Validation checks did not pass

Links

@datadog-official

datadog-official Bot commented Sep 17, 2026

Copy link
Copy Markdown

Pipelines

Unblock PR with BitsAI

❌ Errors

Your PR has failed checks. Please review the issues below and take necessary action before merging.

🚦 2 Pipeline jobs failed

DataDog/java-profiler | integration-test-x64-glibc: [hotspot, 25] — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

DataDog/java-profiler | post-pr-comment — 🔧 Needs a code fix, caused by this PR

View more details · View in GitLab

Useful? React with 👍 / 👎

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant