Add diagnostics #654

mernst · 2025-09-17T03:24:07Z

Summary by CodeRabbit

Bug Fixes
- Prevents a null-related crash when formatting object strings.
- Ensures tracking decision logic returns the same result while adding per-point decision logging.
Chores
- Adds extensive debug/logging for class names, implemented interfaces, methods, and parameter details to aid troubleshooting.
- Improves error messages when loading the instrumenter by including the instrumenter’s class name.

coderabbitai · 2025-09-17T03:28:28Z

📝 Walkthrough

Walkthrough

Adds verbose debug logging to Chicory instrumentation, adds entry and per-PPT decision logs to DCInstrument.should_track (keeps prior inclusion semantics), updates Premain's exception text to include the instrumenter name, and adds a null guard in DCRuntime.obj_str. No public/API signature changes.

Changes

Cohort / File(s)	Summary
Chicory instrumentation debug logs `java/daikon/chicory/Instrument24.java`	Added import and extensive debug logging: convert internal class name to binary form, print implemented interfaces, per-method name/type logs in instrumentation, detailed parameter logs in `create_method_info`, and a commented placeholder; no functional changes to instrumentation logic.
DComp tracking decision logic and logs `java/daikon/dcomp/DCInstrument.java`	Added entry log and per-PPT decision logs in `should_track`, updated early-exit messages for JDK/toString checks, now computes `shouldIgnore = Instrument.shouldIgnore(...)`, logs decision, and returns `!shouldIgnore` (preserving prior return semantics).
Premain error messaging `java/daikon/dcomp/Premain.java`	Extended `RuntimeException` message in `premain` to include the instrumenter class name when transformer loading fails; behavior and exception type unchanged.
Null guard in obj_str `java/daikon/dcomp/DCRuntime.java`	Added `tostring != null` guard before comparing to `default_tostring` in `obj_str` to avoid NPE; fallback formatting behavior preserved.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Caller
  participant DC as DCInstrument.should_track
  Caller->>DC: should_track(className, methodName, pptName)
  activate DC
  DC->>DC: log "considering PPT for potential data collection"
  alt JDK class or toString early-exit
    DC-->>Caller: return (excluded)
  else
    DC->>DC: shouldIgnore = Instrument.shouldIgnore(className, methodName, pptName)
    DC->>DC: log "ignored" if shouldIgnore true, else "included"
    DC-->>Caller: return !shouldIgnore
  end
  deactivate DC

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Make call to indexOf() more efficient #649 — modifies java/daikon/chicory/Instrument24.java; likely related due to overlapping edits in the same instrumentation file.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title "Add diagnostics" is concise and directly reflects the primary change in the diff, which introduces diagnostic/logging statements across instrumentation and runtime classes; it therefore communicates the main intent without extraneous detail.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.38.6)

java/daikon/dcomp/DCInstrument.java

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

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

coderabbitai

Actionable comments posted: 3

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a2d9176 and 2562b98.

📒 Files selected for processing (3)

java/daikon/chicory/Instrument24.java (7 hunks)
java/daikon/dcomp/DCInstrument.java (1 hunks)
java/daikon/dcomp/Premain.java (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

java/daikon/dcomp/DCInstrument.java (2)

java/daikon/chicory/Instrument.java (1)

Instrument (60-1329)

java/daikon/dcomp/Instrument.java (1)

Instrument (27-317)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
GitHub Check: codespecs.daikon (typecheck_latest_ubuntu_jdk25)
GitHub Check: codespecs.daikon (typecheck_bundled_ubuntu_jdk25)
GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)

🔇 Additional comments (7)

java/daikon/dcomp/Premain.java (1)

216-216: LGTM: clearer exception message

Including the instrumenter class name and preserving the cause improves diagnostics. No issues.

java/daikon/chicory/Instrument24.java (6)

34-35: LGTM: needed import for interface logging

Import of ClassEntry is appropriate for interface introspection; no issues.

372-376: LGTM: clearer class name logging

Converts internal name to binary; good for readable diagnostics.

576-577: LGTM: method signature diagnostics

Additional method name/type logs aid triage; gated by debug.

1384-1389: LGTM: pre-adjustment param-name logging

Helpful when correlating with later LVT-based renames; debug-gated.

1428-1429: LGTM: post-adjustment param-name logging

Confirms final param names after LVT reconciliation.

1453-1458: LGTM: parameter type logging

Adds useful visibility; matches surrounding logging style.

java/daikon/chicory/Instrument24.java

java/daikon/dcomp/DCInstrument.java

mernst · 2025-09-17T05:08:00Z

~~The pull request is failing due to diagnostics accessing not-fully-initialized objects. Maybe other changes need to be merged before this pull request, or this one needs more guards.~~
There was a logic problem.

coderabbitai

Actionable comments posted: 3

♻️ Duplicate comments (2)

java/daikon/chicory/Instrument24.java (2)

382-386: Log interfaces using binary names (repeat)

Use binary names for consistency with class-name logging.

-    for (ClassEntry ce : classModel.interfaces()) {
-      debugInstrument.log("  %s%n", ce.asInternalName());
-    }
+    for (ClassEntry ce : classModel.interfaces()) {
+      debugInstrument.log("  %s%n", Signatures.internalFormToBinaryName(ce.asInternalName()));
+    }

832-836: Remove dead commented debug block (repeat)

The commented BranchInstruction snippet adds noise; delete it.

-        // debug code
-        // case BranchInstruction bi -> {
-        // System.out.printf("  %s : %s%n", bi, bi.target());
-        // codeList.add(ce);
-        // }

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 86d33d7 and 607b95f.

📒 Files selected for processing (2)

java/daikon/chicory/Instrument24.java (7 hunks)
java/daikon/dcomp/DCInstrument.java (1 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

java/daikon/dcomp/DCInstrument.java (1)

java/daikon/chicory/Instrument.java (1)

Instrument (60-1329)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: codespecs.daikon (typecheck_bundled_ubuntu_jdk25)
GitHub Check: codespecs.daikon (typecheck_latest_ubuntu_jdk25)
GitHub Check: codespecs.daikon (kvasir_ubuntu_jdk25)
GitHub Check: codespecs.daikon (quick_ubuntu_jdk25)
GitHub Check: codespecs.daikon (nonquick_ubuntu_jdk25)
GitHub Check: codespecs.daikon (nontxt_ubuntu_jdk25)
GitHub Check: codespecs.daikon (misc_ubuntu_jdk25)

🔇 Additional comments (4)

java/daikon/dcomp/DCInstrument.java (1)

3074-3081: Inclusion logic fix — correct and aligns with docs

Negation of shouldIgnore and clearer logging look good.

java/daikon/chicory/Instrument24.java (3)

34-34: Needed import for interface logging

Import of ClassEntry is appropriate.

372-376: Class-name logging — OK

Converts internal to binary name; suppression is reasonable.

575-577: Additional method debug — OK

Method name/type logging under debug is helpful.

java/daikon/chicory/Instrument24.java

java/daikon/dcomp/DCInstrument.java

Add diagnostics

2562b98

mernst assigned markro49 Sep 17, 2025

coderabbitai bot reviewed Sep 17, 2025

View reviewed changes

java/daikon/chicory/Instrument24.java Show resolved Hide resolved

java/daikon/chicory/Instrument24.java Show resolved Hide resolved

java/daikon/dcomp/DCInstrument.java Show resolved Hide resolved

mernst added 3 commits September 17, 2025 00:39

Guard against null

86d33d7

Fix polarity

ea31ac7

Merge ../daikon-branch-master into logging

607b95f

coderabbitai bot reviewed Sep 17, 2025

View reviewed changes

java/daikon/chicory/Instrument24.java Show resolved Hide resolved

java/daikon/dcomp/DCInstrument.java Show resolved Hide resolved

java/daikon/dcomp/DCInstrument.java Show resolved Hide resolved

markro49 approved these changes Sep 17, 2025

View reviewed changes

markro49 assigned mernst and unassigned markro49 Sep 17, 2025

mernst merged commit b39ddaf into codespecs:master Sep 18, 2025
74 of 105 checks passed

mernst deleted the logging branch September 18, 2025 05:55

coderabbitai bot mentioned this pull request Sep 19, 2025

fix some javadoc and clean up some logging messages #679

Merged

coderabbitai bot mentioned this pull request Oct 6, 2025

Add additional version of DynComp for Java 24 #685

Open

coderabbitai bot mentioned this pull request Jan 9, 2026

Diagnostics for DynComp #739

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add diagnostics #654

Add diagnostics #654

Uh oh!

mernst commented Sep 17, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Sep 17, 2025 •

edited

Loading

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mernst commented Sep 17, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Add diagnostics #654

Add diagnostics #654

Uh oh!

Conversation

mernst commented Sep 17, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Pre-merge checks and finishing touches

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mernst commented Sep 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mernst commented Sep 17, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 17, 2025 •

edited

Loading

mernst commented Sep 17, 2025 •

edited

Loading