Skip to content

[reference] reactor-core-3.1 async blind regeneration for #11927#11940

Draft
jordan-wong wants to merge 4 commits into
masterfrom
eval/reactor-core-3.1-async-blind-regen-20260714
Draft

[reference] reactor-core-3.1 async blind regeneration for #11927#11940
jordan-wong wants to merge 4 commits into
masterfrom
eval/reactor-core-3.1-async-blind-regen-20260714

Conversation

@jordan-wong

@jordan-wong jordan-wong commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

[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:

  • spotless formatting (mechanical, ./gradlew spotlessApply)
  • removed duplicate ReactorAsyncResultExtension — eval renamed the
    package reactor.core.*reactorcore.* but did not remove the
    master's reactor.core.* version, causing runtime AsyncResultExtension
    registration 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.
  • dropped Schedulers.elastic() from scheduler test — the generated
    scheduler-arg list included Schedulers.elastic() which was deprecated
    in Reactor 3.4 and removed in 3.5+, blocking :check (via aggregate
    compile) and :latestDepTest (via direct API drift). Removed the
    elastic row while keeping parallel and single for coverage of
    scheduler-hop trace propagation.

Original: 2e1d2b98d7 → rebased: dbf3d6b2c7 → fixes: 3f485e671e

Where 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:

  1. 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 span
      from Reactor's own reactor.util.context.Context under the
      dd.span key, adapts it to a Datadog Context, and drives
      context-store-based propagation (capture on subscribe, restore on
      signal, transfer to optimized subscribers).
    • BlockingPublisherInstrumentation — advice for Mono.block() /
      Flux.blockFirst() / blockLast() that activates the captured
      context on the blocking-thread side of the chain.
    • ContextWritingSubscriberInstrumentation — advice for
      .contextWrite(...) subscribers to capture the user-provided
      context at subscribe time.
    • CorePublisherInstrumentation — advice at the base publisher
      interface that hands off context to downstream subscribers.
    • OptimizableOperatorInstrumentation — advice for Reactor's
      internal 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 the
    Cat-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's
    KafkaBatchListenerCoroutineTest times out with timed out waiting for async listener to start because the parent spring.consume
    span never inherits through the Reactor-mediated coroutine chain.
    Similar failures in spring-webflux-6.0:bootTest. See CI failures
    under test_inst: [*, 2/8] and test_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.java
    in master and preserve or replace each one. For libraries with a
    first-class in-library context concept (Reactor's
    reactor.util.context.Context, Kotlin's CoroutineContext, JAX-RS
    ContainerRequestContext, 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 must
    preserve verbatim or produce equivalent semantics.

  2. 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). The
    skill 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's NativeImageGeneratorRunnerInstrumentation
    (line 158) that listed datadog.trace.instrumentation.reactor.core.ReactorAsyncResultExtension
    as build_time.

  3. Version-sensitive tests in the wrong source set. The scheduler
    test that broke latestDepTest was generated in src/test/java/
    (which compiles against Reactor 3.1) rather than in a
    src/latestDepTest/ variant (which would compile against the latest
    published version). Master handles this by splitting Reactor
    version-sensitive tests into a dedicated latestDepTest source 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 via
dd-trace-java's slot-hash to spring-messaging-4.0 and spring-webflux-6.0
tests that depend on ReactorContextBridge for coroutine / reactive
context propagation. Fixing them would require restoring the 5 dropped
master instrumentation classes, which would hide the finding.

Base: master @ 05671ce3b0

@datadog-datadog-us1-prod

datadog-datadog-us1-prod Bot commented Jul 14, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 100.00%
Overall Coverage: 57.17% (+0.00%)

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

@dd-octo-sts

dd-octo-sts Bot commented Jul 14, 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.06 s 13.97 s [-0.2%; +1.5%] (no difference)
startup:insecure-bank:tracing:Agent 13.01 s 13.05 s [-1.0%; +0.4%] (no difference)
startup:petclinic:appsec:Agent 16.95 s 16.76 s [+0.2%; +2.0%] (maybe worse)
startup:petclinic:iast:Agent 16.92 s 16.96 s [-1.1%; +0.5%] (no difference)
startup:petclinic:profiling:Agent 16.66 s 16.93 s [-2.4%; -0.7%] (maybe better)
startup:petclinic:sca:Agent 16.96 s 16.82 s [-0.1%; +1.7%] (no difference)
startup:petclinic:tracing:Agent 16.18 s 16.10 s [-0.6%; +1.6%] (no difference)

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

…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.
@jordan-wong jordan-wong force-pushed the eval/reactor-core-3.1-async-blind-regen-20260714 branch from 2e1d2b9 to dbf3d6b Compare July 14, 2026 12:04
@jordan-wong

Copy link
Copy Markdown
Contributor Author

Rebased 2026-07-14 — branch force-pushed onto current master to fix the shared config-inversion-linter failure (same root cause as PR #11939).

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 dd-java-agent/instrumentation/reactor-core-3.1/ paths from the original terminal SHA (2e1d2b98d7) onto fresh master (05671ce3b0). All reactor metadata entries were already present on master, so no metadata change is needed.

Result: 24 files changed (all under reactor-core-3.1/), no sibling contamination.

Note for reviewers: the eval reorganized the package layout from datadog.trace.instrumentation.reactor.core (master) to datadog.trace.instrumentation.reactorcore (eval output). This is a Rule #2 (regen-preservation) concern worth flagging — the master module's package layout should typically be preserved verbatim on regen.

Original commit: 2e1d2b98d7 → rebased: dbf3d6b2c7

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.
@jordan-wong jordan-wong added tag: do not merge Do not merge changes and removed type: feature Enhancements and improvements labels Jul 14, 2026
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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

inst: reactor Reactor instrumentation tag: do not merge Do not merge changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant