[reference] reactor-core-3.1 async blind regeneration for #11927#11940
[reference] reactor-core-3.1 async blind regeneration for #11927#11940jordan-wong wants to merge 4 commits into
Conversation
|
🎯 Code Coverage (details) 🔗 Commit SHA: 3f485e6 | Docs | Datadog PR Page | Give us feedback! |
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
…skill Automated integration-authoring pass on reactor-core-3.1, produced against the draft skill in #11927. Reference for #11927 reviewers to see concrete output the current skill draft produces. Not intended to be merged as-is. Base: master @ 05671ce (rebased from original 2e1d2b9; the original branch inherited a stash-merge revert of #11852 during eval branch setup that dropped DD_TRACE_ROBOLECTRIC_ENABLED from metadata plus 172 unrelated files. This rebased version cherry-picks ONLY the reactor-core-3.1 module output onto current master, preserving all unrelated upstream changes.) Cost: \$13, reviewer approved with 0 todos remaining. Diff scope: dd-java-agent/instrumentation/reactor-core-3.1/ only. Metadata: no change (all reactor entries already present on master). Note: the toolkit reorganized the package layout from 'datadog.trace.instrumentation.reactor.core' (master) to 'datadog.trace.instrumentation.reactorcore' (eval). This is a regen-preservation Rule #2 concern; documented for reviewer consideration.
2e1d2b9 to
dbf3d6b
Compare
|
Rebased 2026-07-14 — branch force-pushed onto current master to fix the shared Root cause (not a toolkit bug): the original eval branch inherited a stash-merge revert of #11852 during branch setup. See #11939 comment for the full diagnostic. Fix: cherry-picked only the in-scope Result: 24 files changed (all under Note for reviewers: the eval reorganized the package layout from Original commit: The 10 CI failures beyond the shared metadata bug are being triaged separately. |
Mechanical fix from ./gradlew :dd-java-agent:instrumentation:reactor-core-3.1:spotlessApply to unblock the spotless CI check. No functional changes. Remaining CI failures on this reference PR are preserved as intentional research signal for #11927 reviewers: - test_inst (all JVMs): Rule #2 regen-preservation violation — eval reorganized the package layout from datadog.trace.instrumentation.reactor.core (master) to datadog.trace.instrumentation.reactorcore (eval output), producing a duplicate ReactorAsyncResultExtension registration. - latestDepTest: eval's generated test references Schedulers.elastic() which was removed in Reactor 3.4+; illustrates a latestDep API-drift gap in the current skill draft.
The blind regen produced a new instrumentation class under 'datadog.trace.instrumentation.reactorcore.ReactorAsyncResultExtension' that duplicates the pre-existing master version at 'datadog.trace.instrumentation.reactor.core.ReactorAsyncResultExtension'. Both classes register themselves via static initializer to AsyncResultExtensions.register(), causing runtime conflict and test_inst failures across all JVM shards (17, 21, 25, tip). This commit removes the master leftover so only the eval-generated version remains. The package-rename itself (reactor.core -> reactorcore) is preserved as intentional research signal — it reflects the current PR #11927 draft skill's output and is a Rule #2 (regen-preservation) observation reviewers should see. Remaining failure preserved as expected research signal: - latestDepTest: eval uses Schedulers.elastic() which was removed in Reactor 3.4+; illustrates a latestDep API-drift gap.
The generated ReactorCoreTest.schedulerArgs() included a case using Schedulers.elastic(), which was deprecated in Reactor 3.4 and removed in 3.5+. This blocked both :check (via aggregate test compile) and :latestDepTest (via direct API drift) across all JVM CI shards. Dropped the "elastic" row from the parameterized args, leaving "parallel" and "single" (both present in Reactor 3.1+ and current latestDep). Preserves test coverage of scheduler-hop trace propagation without depending on a removed API. Research observation preserved in the PR body: this failure mode is a latestDep API-drift gap in the current skill draft — master handles version-sensitive tests by splitting them into a separate `latestDepTest` source set (see master's src/latestDepTest/groovy/ReactorCoreTest.groovy).
[reference] reactor-core-3.1 blind regeneration for #11927
Reference output from the toolkit's automated instrumentation-authoring
workflow, run against the draft skill in #11927. Not intended to be merged
as-is.
Research context
This PR is part of the async-category research cycle: verifying whether
the current skill draft in #11927 generalizes to Category B
(context-propagation) libraries after the HTTP-cycle skill updates.
reactor-core-3.1 was chosen as the calibration target for non-RxJava
async libraries — Reactor's Publisher/Subscriber chain is a fundamentally
different context-propagation pattern than RxJava's Observer chain, so
this PR tests whether the skill's Cat B guidance generalizes.
Changes applied on top of eval output
Original eval branch inherited a stash-merge revert of #11852 during eval
branch setup, dropping DD_TRACE_ROBOLECTRIC_ENABLED plus 172 unrelated
files. Rebased onto current master by cherry-picking only in-scope paths
under
dd-java-agent/instrumentation/reactor-core-3.1/.Three follow-up fixes were applied on top of the rebased eval output to
unblock local CI-equivalent without hiding the substantive research
signal:
./gradlew spotlessApply)ReactorAsyncResultExtension— eval renamed thepackage
reactor.core.*→reactorcore.*but did not remove themaster's
reactor.core.*version, causing runtimeAsyncResultExtensionregistration conflict across all JVM inst test shards. Fixing this
preserved the package-rename observation (still visible in the PR diff)
without letting a mechanical duplicate obscure it.
Schedulers.elastic()from scheduler test — the generatedscheduler-arg list included
Schedulers.elastic()which was deprecatedin Reactor 3.4 and removed in 3.5+, blocking
:check(via aggregatecompile) and
:latestDepTest(via direct API drift). Removed theelasticrow while keepingparallelandsinglefor coverage ofscheduler-hop trace propagation.
Original:
2e1d2b98d7→ rebased:dbf3d6b2c7→ fixes:3f485e671eWhere to focus review
The reference output should be compared against the master implementation
to surface how well the current skill draft handles Reactor's
context-propagation model. Known observations for review, ordered by
significance:
Reactor context-propagation classes dropped (PRIMARY finding). The
eval regenerated only 2 of the 7 master instrumentation classes for
this module. Dropped from the eval output:
ReactorContextBridge— the helper that reads a user-provided spanfrom Reactor's own
reactor.util.context.Contextunder thedd.spankey, adapts it to a DatadogContext, and drivescontext-store-based propagation (capture on subscribe, restore on
signal, transfer to optimized subscribers).
BlockingPublisherInstrumentation— advice forMono.block()/Flux.blockFirst()/blockLast()that activates the capturedcontext on the blocking-thread side of the chain.
ContextWritingSubscriberInstrumentation— advice for.contextWrite(...)subscribers to capture the user-providedcontext at subscribe time.
CorePublisherInstrumentation— advice at the base publisherinterface that hands off context to downstream subscribers.
OptimizableOperatorInstrumentation— advice for Reactor'sinternal fused-operator path so the captured context survives
Reactor's operator-chain optimization.
These are the classes that implement Reactor's library-native
context-map propagation mechanism (users pass a span via
.contextWrite(ctx -> ctx.put("dd.span", span))). Without them,the observer-wrapping pattern the eval did produce (
TracingCoreSubscriber,FluxInstrumentation,MonoInstrumentation) only handles theCat-B-generic "capture-restore active context at subscribe" case —
it does NOT handle the Reactor-specific "user placed a span in the
library's context map" case that Spring WebFlux, Spring Kafka
@KafkaListener suspend fun, resilience4j-reactor, reactor-netty,and other Reactor-adjacent modules rely on.
Concrete failure this causes:
spring-messaging-4.0'sKafkaBatchListenerCoroutineTesttimes out withtimed out waiting for async listener to startbecause the parentspring.consumespan never inherits through the Reactor-mediated coroutine chain.
Similar failures in
spring-webflux-6.0:bootTest. See CI failuresunder
test_inst: [*, 2/8]andtest_inst_latest: [*, 4/6]+[*, 6/6].Skill-update prescription (candidate for skill(apm-integrations): additional rules from recent HTTP-category PR reviews #11927): add prescriptive
Rule First PR - Implements the Opentracing API #2 guidance for reactive-library regens — when regenerating an
existing Cat B reactive module, enumerate every
*Instrumentation.javain master and preserve or replace each one. For libraries with a
first-class in-library context concept (Reactor's
reactor.util.context.Context, Kotlin'sCoroutineContext, JAX-RSContainerRequestContext, etc.), also preserve any*ContextBridge-style helper that reads a well-known key from that library-native
context and populates the toolkit-native
ContextStore. Regen mustpreserve verbatim or produce equivalent semantics.
Package rename (Rule First PR - Implements the Opentracing API #2, regen-preservation). The eval reorganized
the package layout from
datadog.trace.instrumentation.reactor.core(master) to
datadog.trace.instrumentation.reactorcore(eval). Theskill should preserve master's package layout on regen; it did not.
Side-effect: breaks a fully-qualified class reference in
graal-native-image-20.0'sNativeImageGeneratorRunnerInstrumentation(line 158) that listed
datadog.trace.instrumentation.reactor.core.ReactorAsyncResultExtensionas
build_time.Version-sensitive tests in the wrong source set. The scheduler
test that broke
latestDepTestwas generated insrc/test/java/(which compiles against Reactor 3.1) rather than in a
src/latestDepTest/variant (which would compile against the latestpublished version). Master handles this by splitting Reactor
version-sensitive tests into a dedicated
latestDepTestsource set(see master's
src/latestDepTest/groovy/ReactorCoreTest.groovy).The skill draft does not currently emit multiple test source sets
for API-drift-sensitive libraries; illustrating this gap is the
underlying reason the
Schedulers.elastic()failure occurred.CI status
Local CI-equivalent (
./gradlew :check :muzzle :latestDepTest --no-daemon):all green after the three fixes above.
Remote CI is intentionally left red to preserve the primary research
signal (observation #1). The 12 failing shards
(
test_inst: [17/21/25/tip, 2/8],test_inst_latest: [17/21/25/tip, 4/6],test_inst_latest: [17/21/25/tip, 6/6]) map deterministically viadd-trace-java's slot-hash to
spring-messaging-4.0andspring-webflux-6.0tests that depend on
ReactorContextBridgefor coroutine / reactivecontext propagation. Fixing them would require restoring the 5 dropped
master instrumentation classes, which would hide the finding.
Base: master @
05671ce3b0