Skip to content

Support instrumentations that make structural changes and tolerate already-loaded target classes - #12610

Open
mcculls wants to merge 12 commits into
masterfrom
mcculls/preserve-structure-of-loaded-types
Open

mcculls wants to merge 12 commits into
masterfrom
mcculls/preserve-structure-of-loaded-types

Conversation

@mcculls

@mcculls mcculls commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Introduces Instrumenter.WithStructuralChange, a marker interface for instrumentations whose type advice adds fields, methods, or interfaces to a target type. Each such instrumentation declares a structuralChangeMarker() interface that gets woven into the type when the structural change is applied.
  • Adds MatchRecorder.PreserveLoadedStructure, which narrows a match so an already-loaded class is only retransformed if it already declares the structural-change marker — avoiding VerifyError/redefinition failures from applying structural bytecode changes to classes the JVM has already loaded, while still allowing retransformation of classes that were already structurally changed pre-load.
  • Adds MatchRecorder.CopyMatch to reuse an already-computed match under another id instead of re-running the type matcher.
  • Migrates existing structural-change instrumentations (IAST Taintable visitors, TaskUnwrappingInstrumentation, Vert.x RequestImplInstrumentation) onto the new WithStructuralChange pattern, removing the now-redundant TaintableRedefinitionStrategyListener, MrReturnAdvice, and LoadedTaintableClass.
  • Standardizes how new interfaces are added via advice across TaintableVisitor, UnwrappingVisitor, and RequestImplInstrumentation, extracting a shared CollectionUtils.arrayContains helper.
  • Fixes a TaintedMap bug where the last element wasn't deduplicated
  • Removes the ENABLE_ADVICE_TRANSFORMER test workaround, no longer needed now that structural-change retransformation is handled properly.
  • Adds PreserveLoadedStructureRetransformationForkedTest covering: structural changes surviving retransformation, and an already-loaded (not structurally changed) class correctly not being retransformed.

Test plan

  • New forked test: PreserveLoadedStructureRetransformationForkedTest
  • Existing IAST, Vert.x, and Kafka instrumentation tests updated/passing
  • CollectionUtilsTest covers the new arrayContains utility

🤖 Generated with Claude Code

@mcculls mcculls added tag: ai generated Largely based on code generated by an AI or LLM comp: tooling Build & Tooling type: feature Enhancements and improvements labels Sep 23, 2026
@datadog-prod-us1-3

datadog-prod-us1-3 Bot commented Sep 23, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 4.48%
Overall Coverage: 59.27% (-0.03%)

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

@dd-octo-sts

dd-octo-sts Bot commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.01 s 13.99 s [-0.4%; +0.7%] (no difference)
startup:insecure-bank:tracing:Agent 12.98 s 12.99 s [-0.8%; +0.5%] (no difference)
startup:petclinic:appsec:Agent 17.03 s 16.93 s [-0.3%; +1.5%] (no difference)
startup:petclinic:iast:Agent 16.52 s 17.02 s [-7.2%; +1.3%] (no difference)
startup:petclinic:profiling:Agent 16.76 s 16.73 s [-1.2%; +1.5%] (no difference)
startup:petclinic:sca:Agent 17.05 s 16.89 s [-0.3%; +2.2%] (no difference)
startup:petclinic:tracing:Agent 16.21 s 15.80 s [-1.4%; +6.7%] (no difference)

Commit: cf801e9e · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

@pr-commenter

pr-commenter Bot commented Sep 23, 2026

Copy link
Copy Markdown

Kafka / producer-benchmark

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master mcculls/preserve-structure-of-loaded-types
git_commit_date 1790160832 1790178724
git_commit_sha 3723256 cf801e9
See matching parameters
Baseline Candidate
ci_job_date 1790179945 1790179945
ci_job_id 2072862214 2072862214
ci_pipeline_id 139542705 139542705
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
jdkVersion 11.0.31 11.0.31
jmhVersion 1.36 1.36
jvm /usr/lib/jvm/java-11-openjdk-amd64/bin/java /usr/lib/jvm/java-11-openjdk-amd64/bin/java
jvmArgs -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=15002 -Dhttps.proxyHost=127.0.0.1 -Dhttps.proxyPort=15002 -Dhttp.nonProxyHosts=localhost *.localhost
kernel_version Linux runner-zfyrx7zua-project-304-concurrent-0-eceg986u 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-zfyrx7zua-project-304-concurrent-0-eceg986u 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
vmName OpenJDK 64-Bit Server VM OpenJDK 64-Bit Server VM
vmVersion 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean throughput
scenario:not-instrumented/KafkaProduceBenchmark.benchProduce same
scenario:only-tracing-dsm-disabled-benchmarks/KafkaProduceBenchmark.benchProduce same
scenario:only-tracing-dsm-enabled-benchmarks/KafkaProduceBenchmark.benchProduce same

@pr-commenter

pr-commenter Bot commented Sep 23, 2026

Copy link
Copy Markdown

Kafka / consumer-benchmark

Parameters

Baseline Candidate
baseline_or_candidate baseline candidate
git_branch master mcculls/preserve-structure-of-loaded-types
git_commit_date 1790160832 1790178724
git_commit_sha 3723256 cf801e9
See matching parameters
Baseline Candidate
ci_job_date 1790179978 1790179978
ci_job_id 2072862216 2072862216
ci_pipeline_id 139542705 139542705
cpu_model Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz Intel(R) Xeon(R) Platinum 8259CL CPU @ 2.50GHz
jdkVersion 11.0.31 11.0.31
jmhVersion 1.36 1.36
jvm /usr/lib/jvm/java-11-openjdk-amd64/bin/java /usr/lib/jvm/java-11-openjdk-amd64/bin/java
jvmArgs -Dhttp.proxyHost=127.0.0.1 -Dhttp.proxyPort=15002 -Dhttps.proxyHost=127.0.0.1 -Dhttps.proxyPort=15002 -Dhttp.nonProxyHosts=localhost *.localhost
kernel_version Linux runner-zfyrx7zua-project-304-concurrent-0-rj24j2sb 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux Linux runner-zfyrx7zua-project-304-concurrent-0-rj24j2sb 6.8.0-1031-aws #33~22.04.1-Ubuntu SMP Thu Jun 26 14:22:30 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
vmName OpenJDK 64-Bit Server VM OpenJDK 64-Bit Server VM
vmVersion 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu 11.0.31+11-post-1ubuntu1-22.04.2-Ubuntu

Summary

Found 0 performance improvements and 0 performance regressions! Performance is the same for 3 metrics, 0 unstable metrics.

See unchanged results
scenario Δ mean throughput
scenario:not-instrumented/KafkaConsumerBenchmark.benchConsume same
scenario:only-tracing-dsm-disabled-benchmarks/KafkaConsumerBenchmark.benchConsume unsure
[-23565.345op/s; -2648.434op/s] or [-7.493%; -0.842%]
scenario:only-tracing-dsm-enabled-benchmarks/KafkaConsumerBenchmark.benchConsume same

@mcculls
mcculls marked this pull request as ready for review September 23, 2026 14:26
@mcculls
mcculls requested review from a team as code owners September 23, 2026 14:26
@mcculls
mcculls requested review from amarziali, claponcet, dromanol and ygree and removed request for a team September 23, 2026 14:26

@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: 058842ae68

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal-api/src/main/java/datadog/trace/util/CollectionUtils.java

@datadog-prod-us1-3 datadog-prod-us1-3 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.

⚠️ Autotest was unable to complete this review. View session

Please try again by commenting @autotest review.

This branch has not been deployed

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

Labels

comp: tooling Build & Tooling tag: ai generated Largely based on code generated by an AI or LLM type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants