Skip to content

Add opt-in instrumentation support for generated lambda classes - #12346

Open
amarziali wants to merge 38 commits into
masterfrom
andrea.marziali/lambda
Open

amarziali wants to merge 38 commits into
masterfrom
andrea.marziali/lambda

Conversation

@amarziali

@amarziali amarziali commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

What Does This Do?

Adds opt-in infrastructure for applying Datadog instrumentations to classes generated by LambdaMetafactory.

Generated lambda classes are normally defined without passing through the standard Java agent transformation path. This change instruments InnerClassLambdaMetafactory at the byte-generation point and routes eligible lambda bytecode through the existing transformer before class definition.

The infrastructure is disabled by default and can be enabled with:

dd.trace.lambda.enabled=true

Enabling it alone does not select any production lambda classes. An enabled instrumentation must explicitly implement Instrumenter.ForLambda and register the exact functional interface it supports.

Notable Changes

  • Introduces Instrumenter.ForLambda for instrumentations targeting generated implementations of an exact functional interface.
  • Instruments java.lang.invoke.InnerClassLambdaMetafactory immediately after lambda bytecode generation.
  • Routes eligible generated classes through the existing matching, context-store, helper-injection, and transformation pipeline.
  • Supports the metafactory layouts used by JDK 8, JDK 11+, and the ClassFile API path used by newer JDKs.
  • Uses the module-aware ClassFileTransformer entry point on JDK 9+.
  • Applies only transformations registered for the generated lambda’s functional interface.
  • Avoids hidden-lambda cache pollution when symbolic class names are reused.
  • Skips the mechanism during GraalVM Native Image builds.

Compatibility and Limitations

Lambda transformation is intentionally best-effort:

  • The feature is disabled by default.
  • Reentrant lambda generation during transformation is skipped to prevent recursion.
  • CDS/AppCDS lambda classes that bypass runtime metafactory generation are not covered.
  • GraalVM Native Image builder JVMs are excluded. Instrumented lambda shapes conflict with Graal’s lambda-renaming analysis.
  • If a future JDK changes the metafactory byte-generation layout, the agent leaves the metafactory unchanged and emits a warning.
  • Security restrictions that prevent access to the defining loader, module, or protection domain cause transformation to fall back to the original bytes.

Motivation

Additional Notes

Contributor Checklist

Jira ticket: [PROJ-IDENT]

@amarziali

Copy link
Copy Markdown
Contributor Author

@DataDog review

@datadog-prod-us1-6

datadog-prod-us1-6 Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

🎯 Code Coverage (details)
• Patch Coverage: 0.00%
• Overall Coverage: 58.96% (-0.14%)

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

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

Datadog Autotest: FAIL

On JDK 8 and JDK 11, the new structure matcher requires a field that does not exist. The lambda transformer does not run, and Runnable lambdas still use wrappers.

Open Bits AI session

🤖 Datadog Autotest · Commit 054841c · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

@amarziali
amarziali force-pushed the andrea.marziali/lambda branch from 054841c to 5742c20 Compare August 31, 2026 12:41
@amarziali

Copy link
Copy Markdown
Contributor Author

@DataDog review

@amarziali
amarziali requested a review from mcculls August 31, 2026 13:04

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

Datadog Autotest: PASS

More details

The PR instruments generated Runnable lambdas so executor propagation keeps task identity, with supported JDK paths and a Graal build guard. This update fixes the earlier JDK 8 and 11 samBase mismatch and adds no new finding; the documented concurrent-reuse limitation remains, and no review finding is open.

Was this helpful? React 👍 or 👎

Open Bits AI session

🤖 Datadog Autotest · Commit 5655a6f · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

@amarziali

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 31, 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-09-09T13:28:01.820141Z 490090a Draft marked ready
ℹ️ 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.

@amarziali
amarziali requested a review from bric3 August 31, 2026 13:09
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 5655a6ff32

ℹ️ 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".

@dd-octo-sts

dd-octo-sts Bot commented Aug 31, 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.86 s 14.70 s [+0.3%; +2.0%] (maybe worse)
startup:insecure-bank:tracing:Agent 13.56 s 13.63 s [-1.2%; +0.2%] (no difference)
startup:petclinic:appsec:Agent 17.06 s 16.96 s [-0.2%; +1.4%] (no difference)
startup:petclinic:iast:Agent 17.01 s 17.03 s [-0.9%; +0.8%] (no difference)
startup:petclinic:profiling:Agent 16.87 s 17.00 s [-2.2%; +0.6%] (no difference)
startup:petclinic:sca:Agent 16.93 s 16.78 s [-0.0%; +1.9%] (no difference)
startup:petclinic:tracing:Agent 15.65 s 16.22 s [-7.6%; +0.7%] (no difference)

Commit: 3adeef5a · 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.

@amarziali
amarziali marked this pull request as ready for review August 31, 2026 15:06
@amarziali
amarziali requested review from a team as code owners August 31, 2026 15:06
@amarziali
amarziali requested review from jordan-wong and removed request for a team August 31, 2026 15:06

@datadog-prod-us1-6 datadog-prod-us1-6 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 start this review.

Please try again by commenting @autotest review.

@dd-octo-sts

dd-octo-sts Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Hi! 👋 Thanks for your pull request! 🎉

To help us review it, please make sure to:

  • Add at least one type, and one component or instrumentation label to the pull request

If you need help, please check our contributing guidelines.

@amarziali amarziali added inst: java Core Java language instrumentation tag: concurrency Virtual Threads, Coroutines, Async, RX, Executors type: feature Enhancements and improvements labels Aug 31, 2026

@datadog-prod-us1-6 datadog-prod-us1-6 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 start this review.

Please try again by commenting @autotest review.

@amarziali

Copy link
Copy Markdown
Contributor Author

@DataDog review

@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: a721063dff

ℹ️ 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-prod-us1-6 datadog-prod-us1-6 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.

Datadog Autotest: FAIL

A hidden Runnable lambda can put its type description in the shared name cache. A later real class with the same legal binary name can receive wrong instrumentation on JDK 21 and later.

Open Bits AI session

🤖 Datadog Autotest · Commit a721063 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

@amarziali

Copy link
Copy Markdown
Contributor Author

@DataDog review

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

Datadog Autotest: FAIL

The new module has no Gradle lock file. Its dependency graph can change between builds.

Open Bits AI session

🤖 Datadog Autotest · Commit b11c8c6 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

Comment thread dd-java-agent/instrumentation/java/java-lambda/java-lambda-1.8/build.gradle Outdated

@bric3 bric3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It looks good overall, but I think some parts need to be adjusted a bit, e.g. in the AgentInstaller.

I made a flow diagram to actually try to understand what this will provide ... when we have ForLambda instrumentation? Is my understanding correct ?

flowchart TB
    subgraph install["1 · Install lambda instrumentation"]
        direction TB
        A["Collect enabled <code>Instrumenter.ForLambda</code> implementations"]:::tracer
        B["<code>AgentBuilder</code> builds the decorated <code>ClassFileTransformer</code>"]:::third
        C["<code>onBeforeInstall</code> publishes the transformer to <code>LambdaTransformerHolder</code>"]:::tracer
        D["Register the transformer and retransform loaded <code>InnerClassLambdaMetafactory</code>"]:::jdk
        E["Inject a pre-definition call to <code>LambdaTransformerHelper.transform(...)</code> into <code>InnerClassLambdaMetafactory</code>"]:::tracer
        A --> B --> C --> D --> E
    end
    subgraph generate["2 · Prepare the lambda implementation (linkage) and invoke the lambda"]
        direction TB
        F["Application or library first executes an unlinked lambda <code>invokedynamic</code> call site"]:::third
        G["<code>InnerClassLambdaMetafactory</code> generates the lambda implementation bytes"]:::jdk
        H{"Can the agent transform this lambda?"}:::tracer
        O["Return the original generated bytes"]:::tracer
        F --> G --> H
        H -->|no| O

        subgraph transform["Transform before definition"]
            direction TB
            I["Call the Byte Buddy transformer with the declaring class's module, loader and protection domain"]:::tracer
            J["Byte Buddy runs Datadog's matching and transformation pipeline"]:::third
            K["Select <code>ForLambda</code> transformations by exact interface, lambda matcher, class-loader matcher and muzzle compatibility"]:::tracer
            L["Apply advice, inject helpers and add eligible context fields"]:::tracer
            M["Run cleanup: <code>SharedTypePools.endTransform(...)</code> releases owned type-pool state, then clear per-thread lambda markers"]:::tracer
            I --> J --> K --> L --> M
            J -. no change or failure .-> M
        end

        N["<code>InnerClassLambdaMetafactory</code> defines the lambda class from the returned bytes"]:::jdk
        P["Application invokes the lambda; injected behavior runs if a <code>ForLambda</code> transformation matched"]:::third
        H -->|yes| I
        M -->|transformed or original bytes| N
        O --> N
        N --> P
    end
    %% Connect groups to preserve their top-to-bottom direction within the left-to-right layout.
    install -. transformer published and generation hook installed .-> generate
    style install fill:#f3e8ff,stroke:#9333ea,stroke-width:2px,color:#581c87
    style generate fill:#ecfeff,stroke:#0891b2,stroke-width:2px,color:#164e63
    style transform fill:#fff1f2,stroke:#e11d48,stroke-width:2px,color:#881337
    classDef jdk fill:#dbeafe,stroke:#2563eb,color:#172554
    classDef tracer fill:#dcfce7,stroke:#16a34a,color:#14532d
    classDef third fill:#ffedd5,stroke:#ea580c,color:#7c2d12

Loading

Comment thread docs/how_instrumentations_work.md
Comment thread dd-java-agent/agent-installer/src/test/java/datadog/application/LambdaTarget.java Outdated
Class<?> classBeingRedefined,
ProtectionDomain pd) {

String lambdaInterface = lambdaMatchers.isEmpty() ? null : TypePoolFacade.lambdaInterface();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

could we find a quicker way to shortcut this when the feature is not installed?

I'd like to avoid repeatedly calling HashMap.isEmpty() during premain when we know this will never be false - we have a static config flag gating this feature which we could store as a static local constant


static BitSet memoizeHierarchy(TypeDescription type, Map<String, BitSet> localMemos) {
if (noMatchFilter.contains(type.getName())) {
if (isCacheable(type) && noMatchFilter.contains(type.getName())) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

noMatchFilter is designed to be a very quick filter - I'm concerned that we're adding an instanceOf and method call to isCacheable here before it.


long fromTick = InstrumenterMetrics.tick();
SharedTypeInfo<BitSet> sharedMemo = memos.find(name);
SharedTypeInfo<BitSet> sharedMemo = cacheable ? memos.find(name) : null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm struggling a bit to work out how isCacheable layers on top of memoization.

Also note that this code will very soon move out to dd-instrument-java, where it will be memoization of a more general matching process (i.e. outside of byte-buddy.)

Is this isCacheable property something that I'll need to also move over, or is it specific to how we use byte-buddy?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good question. The goal was to avoid reusing cached matcher results for generated lambda classes. They are parsed from temporary in-memory bytes, and the same class name may later refer to different bytecode, so a cached result could be stale. In 94d2bd4, TypeFactory.isLambdaTarget(...) identifies that specific target directly, and the shared type and matcher caches skip it. So this is specific to this lambda transformation path so IMHO I don't think that there’s any general property that needs to move to dd-instrument-java.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ok, are these names like lambda names with slashes? or can they re-use general class names?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, the definition mechanism varies by JDK: older JDKs use anonymous classes and newer JDKs use hidden classes. In both cases our hook runs before definition, so it sees the nominal bytecode name without the runtime /... suffix.

* Cleans up local caches if this callback owns the active transformation. Reference identity is
* the ownership token, so transformer wrappers must pass the original callback buffer unchanged.
*/
void endTransform(byte[] classFileBuffer) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Quick question - is this needed because we're now nesting transform calls?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes as far as I could see, while transforming one class, class loading can trigger another transformation callback on the same thread. I remember having had failing tests. Byte Buddy blocks the inner callback from doing a full transformation, but its wrapper still runs the usual cleanup afterward (endTransform). That cleanup clears the temporary TypeFactory state used to describe the class currently being transformed. Without this check, the inner callback could accidentally clear the outer transformation’s state before it has finished.
This is documented in 5feb161 and tested in 790a7a8

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks - I suspect it could be a side-effect of how we're plugging into the metafactory

}

@Override
public boolean isCacheable() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This property is quite invasive, causing changes across a lot of the type caching layer - I'm thinking there might be a better way to do this with less changes (especially as I want to move parts of this to the dd-instrument-java library)

@mcculls mcculls left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the idea, but would like to see this more of a "no-op" when the feature is not enabled.

There are also some changes in code which will soon move out of this codebase, and we'll need to decide whether those changes will still be required.

@jordan-wong jordan-wong left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, other than 1 test I would add for coverage

!Platform.isNativeImageBuilder()
&& InstrumenterConfig.get()
.isIntegrationEnabled(Collections.singleton("lambda"), false);
if (lambdaTransformationEnabled) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would consider adding a regression/ unit test for lambdaTransformationEnabled
PR description mentions

Skips the mechanism during GraalVM Native Image builds.

as a noteable feature

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

inst: java Core Java language instrumentation tag: concurrency Virtual Threads, Coroutines, Async, RX, Executors type: feature Enhancements and improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants