Skip to content

feat(jms): JMS 1.1 instrumentation (AI-generated, blind test)#11170

Draft
jordan-wong wants to merge 1 commit intomasterfrom
eval/jms-new-integration-pr
Draft

feat(jms): JMS 1.1 instrumentation (AI-generated, blind test)#11170
jordan-wong wants to merge 1 commit intomasterfrom
eval/jms-new-integration-pr

Conversation

@jordan-wong
Copy link
Copy Markdown
Contributor

@jordan-wong jordan-wong commented Apr 21, 2026

Summary

  • AI-generated JMS 1.1 instrumentation via apm-instrumentation-toolkit `new_integration` workflow
  • Blind test: original instrumentation deleted before generation; agent had no access to original code
  • LLM reviewer approved after 3 iterations (fixed messaging.system/destination/kind tags, messaging.operation, TopicPublisher/QueueSender, error handling)
  • Run 9 in the Java eval doc

Note on module naming: The generated code lives under `javax-jms-1.1-gen/` so all files appear as new in this diff. The existing `javax-jms-1.1/` module is untouched. This is a draft for review only — not intended for merge as-is (the `-gen` suffix and `settings.gradle.kts` entry would need cleanup before merging).

Generated artifacts (in `dd-java-agent/instrumentation/jms/javax-jms-1.1-gen/`):

  • `JMSConsumerInstrumentation` — `MessageConsumer.receive` / `receiveNoWait` / `receive(timeout)`
  • `MessageConsumerInstrumentation` — interface-level consumer instrumentation
  • `MessageListenerInstrumentation` — `MessageListener.onMessage` for async consumers
  • `MessageProducerInstrumentation` — `MessageProducer.send` overloads
  • `JmsDecorator` — messaging span decoration with `messaging.system=jms`, `messaging.destination`, `messaging.operation`
  • `MessageExtractAdapter` / `MessageInjectAdapter` — context propagation via JMS message properties
  • `JmsTest.groovy` — 914-line Spock test suite with real ActiveMQ broker, JMS 1.1 + legacy 1.0 API coverage

Metrics: $40.13 / ~6.5 hours / new_integration workflow

Test plan

  • `./gradlew :dd-java-agent:instrumentation:jms:javax-jms-1.1-gen:compileJava`
  • `./gradlew :dd-java-agent:instrumentation:jms:javax-jms-1.1-gen:spotlessCheck`
  • `./gradlew :dd-java-agent:instrumentation:jms:javax-jms-1.1-gen:muzzle`
  • `./gradlew :dd-java-agent:instrumentation:jms:javax-jms-1.1-gen:test`
  • `./gradlew :dd-java-agent:instrumentation:jms:javax-jms-1.1-gen:latestDepTest`
  • Expert review: correct instrumentation points for JMS semantics?
  • Expert review: context propagation correct for async consumers?

🤖 Generated with apm-instrumentation-toolkit + Claude Code

Generated by apm-instrumentation-toolkit new_integration workflow.
Blind test — original instrumentation deleted before generation.
LLM reviewer approved after 3 iterations.

Module placed under javax-jms-1.1-gen/ to appear as fully new files
in the PR diff (avoids confusion with existing javax-jms-1.1 module).
This is a draft for review only — not intended for merge as-is.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@jordan-wong jordan-wong force-pushed the eval/jms-new-integration-pr branch from 33eaa6e to 0cdf553 Compare April 21, 2026 11:44
@jordan-wong
Copy link
Copy Markdown
Contributor Author

Comparison: Generated vs Existing JMS 1.1 Instrumentation

This comment summarizes a diff analysis between the AI-generated code in this PR (javax-jms-1.1-gen/) and the existing production instrumentation (javax-jms-1.1/ on master).

What's identical

  • Decorator (JmsDecorator) — byte-for-byte identical. All span tags, operation names, messaging.system, messaging.destination, messaging.destination.kind, messaging.operation, Data Streams integration, BROKER_DECORATE for time-in-queue spans, and estimatePayloadSize() are correct.
  • Build config — identical muzzle range ([1.1-rev-1,)), same ActiveMQ test dependency, same latestDepTest config.
  • MessageExtractAdapter / MessageInjectAdapter — identical.
  • MessageListenerInstrumentation — identical structure and logic.

Architectural differences

Aspect Original Generated
Module registration 4 independent @AutoService classes JavaxJmsModule orchestrating 5 instrumentations via typeInstrumentations()
Namespace Hardcoded javax Constructor-parameterized (jakarta-ready scaffolding)
Context store Declared per-instrumentation Centralized in JavaxJmsModule
Advice composition Single advice per method pattern Dual advice pattern (@AppliesOn for conditional advice)
Call-depth tracking class Message.class MessageProducer.class / MessageConsumer.class (varies)

What the generated code adds (not in original)

  • Time-in-queue spans — full JMS_DELIVER broker spans with batch IDs and producer timestamp injection. Original has none.
  • Session mode tracking — AUTO_ACKNOWLEDGE / CLIENT_ACKNOWLEDGE / TRANSACTED modes managed via SessionInstrumentation. Original ignores session mode entirely.
  • Message.acknowledge instrumentationMessageInstrumentation properly closes client-ack spans. Original misses this.
  • Listener wrappingDatadogMessageListener proxy manages async span lifecycle (try-with-resources, session state transitions, error handling). Original only extracts context.
  • Message-Driven Bean support — dedicated MDBMessageConsumerInstrumentation for EJB MDB lifecycle. Original treats MDBs as plain MessageListeners.
  • Central module classJavaxJmsModule centralizes context store definitions and instrumentation registration.
  • Per-destination propagation control — config flags check isJmsPropagationDisabledForDestination() per producer. Original applies propagation globally.
  • Consumer close() instrumentation — cleans up auto-ack spans when consumer closes. Original has no close() handling.
  • Consumer setMessageListener() instrumentation — wraps listeners with DatadogMessageListener at registration time.
  • Multi-level span hierarchyJMS_DELIVER (broker latency) + JMS_CONSUME (client consumption) + iteration spans for polling loops. Original produces flat producer/consumer/onMessage spans only.

What the generated code misses or gets wrong

  • JMS 2.0 JMSConsumer — original has JMSConsumerInstrumentation targeting javax.jms.JMSConsumer (JMS 2.0 simplified API), including receiveBody() variants. Generated drops this entirely — unclear if JMSConsumer is handled through namespace parameterization or is a genuine gap.
  • Call-depth tracking class — generated uses MessageProducer.class / MessageConsumer.class instead of original's Message.class. Could cause threading issues with concurrent producers/consumers on the same thread.
  • Dual-advice propagation gateProducerContextPropagationAdvice is @AppliesOn(CONTEXT_TRACKING). If that feature flag is off, context propagation silently doesn't happen.
  • Jakarta support incomplete — namespace parameterization scaffolding is present but JavaxJmsModule hardcodes "javax" in its constructor. No jakarta variant wired up.
  • IBM MQ DSM narrowing — DSM-only optimization is gated on an IBM MQ check. May incorrectly skip DSM for other MQ providers.
  • @AutoService registration riskJavaxJmsModule is @AutoService and registers child instrumentations. If any child is also @AutoService, there's a double-registration risk.

Test coverage delta

Original has a basic Groovy test suite. Generated adds:

  • Decorator unit tests
  • Separate JMS 1.1 vs JMS 2.0 test classes
  • Message-Driven Bean tests (MDB1, MDB2, MDBTest, MDBBad)
  • Spring JMS integration tests (SpringListenerJMS1/2Test, SpringTemplateJMS1/2Test)
  • Time-in-queue forked tests
  • Test listener utilities

Questions for expert reviewer

  1. Is JMSConsumer (JMS 2.0 receiveBody()) actually covered by the generated code, or is this a regression?
  2. CallDepthThreadLocalMap keyed on MessageProducer.class vs original's Message.class — correct for concurrent scenarios?
  3. @AppliesOn(CONTEXT_TRACKING) on propagation advice — is silent no-op when flag is off intentional?
  4. JavaxJmsModule + @AutoService child instrumentations — any double-registration risk?
  5. IBM MQ DSM gate — should this be broader?

@PerfectSlayer PerfectSlayer added the tag: do not merge Do not merge changes label Apr 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tag: do not merge Do not merge changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants