Fix Finatra test server lifecycle

[AlexeyKuznetsov-DD]

What Does This Do

• Runs the embedded Finatra test servers with nonExitingMain instead of the production main wrapper, so a lifecycle error can no longer call System.exit(1) inside the Gradle test worker.
• Tracks the server thread with a FutureTask, so a startup or graceful-shutdown failure is rethrown as an ordinary test failure with its original stack trace.
• Registers the latest-dependency lifecycle observer before starting its server thread, closing a race where a startup event could fire before the observer was attached.
• Makes FinatraServer.awaitStart return whether the latch actually fired instead of silently discarding the result.
• Renames the latestPre207Test suite to latestPre207ForkedTest and pins it to [20.6.0,20.7.0) so it actually runs — see below.

Motivation

The Finatra Java 8 forked test intermittently completed its V1 JUnit suite successfully and then lost the Gradle test worker with exit code 1:

Execution failed for task ':dd-java-agent:instrumentation:finatra-2.9:forkedTest'.
> Process 'Gradle Test Executor ...' finished with non-zero exit value 1

The corresponding FinatraServerV1ForkedTest report was already complete with 72 tests, 0 failures, 0 errors — so the process died after the suite finished, not because of an instrumentation assertion.

The tests were invoking Twitter Util's production App.main, which turns any lifecycle error into a process exit (util-app, com/twitter/app/App.scala):

final def main(args: Array[String]): Unit = {
  try nonExitingMain(args)
  catch { case t: Throwable => exitOnError(t) }   // -> System.exit(1)
}

final def nonExitingMain(args: Array[String]): Unit = {
  ...
  close(defaultCloseGracePeriod)
  if (!suppressGracefulShutdownErrors) Await.result(this, closeDeadline - Time.now)  // throws
}

Because the server ran inside the Gradle worker, an intermittent graceful-shutdown error terminated the whole worker and hid the underlying exception from the test runner. nonExitingMain preserves the identical application lifecycle but lets the harness capture the failure.

Note this improves diagnosability rather than guaranteeing the flake disappears: if the intermittent shutdown error is inherent (e.g. a close timeout under CI load), the suite will now fail in cleanupSpec with a real stack trace instead of vanishing. If that turns out to be the case, the follow-up knob is suppressGracefulShutdownErrors on the test server.

Implementation Notes

The server thread's outcome is carried by a FutureTask, which provides exception capture, a bounded join, and rethrow-with-cause in one construct — no AtomicReference, no manual try/catch, no join + liveness assertion:

serverMain = new FutureTask<Void>({
  testServer.nonExitingMain("-admin.port=:0", "-http.port=:" + port)
}, null)
Thread serverThread = new Thread(serverMain, "finatra-server")
serverThread.setDaemon(true)
serverThread.start()

void stopServer(HttpServer httpServer) {
  Await.ready(httpServer.close(), TIMEOUT)
  // Rethrows a shutdown failure reported by the server thread, or times out if it did not stop
  serverMain.get(TIMEOUT.inMilliseconds(), TimeUnit.MILLISECONDS)
}

In the latest-dependency test the existing Observer already delivers startup failures through startupPromise.setException(...), so no extra handling is needed there.

The latestPre207 Suite

While validating the fix on older Finatra versions, the latestPre207Test suite turned out to be inert in two independent ways on master:

1. It executed zero tests. dd-trace-java.configure-tests.gradle.kts excludes **/*ForkedTest* from any task not named forkedTest*/*ForkedTest, and both concrete classes in src/test are FinatraServerV0ForkedTest / FinatraServerV1ForkedTest.
2. Its version override never applied. The suite extends test, which declares a static finatra-http_2.11:19.12.0; that version satisfies [,20.7.0), so conflict resolution kept it:

Selection reasons:
  - By conflict resolution: between versions 19.12.0 and [,20.7.0)   -> 19.12.0

This is visible in master's lockfile, where 19.12.0 is listed for both testCompileClasspath and latestPre207TestCompileClasspath.

Renaming the suite alone would only have added 72 duplicate tests against the same jar forkedTest already covers, so the dependency is now declared as [20.6.0,20.7.0). The lower bound excludes 19.12.0 from the range, which flips conflict resolution:

Selection reasons:
  - By conflict resolution: between versions 20.6.0 and 19.12.0      -> 20.6.0

The range (rather than a hard pin) keeps the suite tracking any future 20.6.x release. The lockfile was regenerated with resolveAndLockAll --write-locks; no dependency coordinate was dropped, and allLatestDepTests still picks the renamed suite up.

Validation

./gradlew :dd-java-agent:instrumentation:finatra-2.9:forkedTest :dd-java-agent:instrumentation:finatra-2.9:latestDepTest -PtestJvm=8 (latestDepTest is finalizedBy latestPre207ForkedTest):

Suite	Finatra	Tests	Failures	Errors
forkedTest — V0 / V1	19.12.0	72 / 72	0	0
latestDepTest — FinatraServer270Test	21.2.0	72	0	0
latestPre207ForkedTest — V0 / V1	20.6.0	72 / 72	0	0

43 of the 72 are pre-existing assumeTrue skips for HttpServerTest features this integration does not opt into (AppSec blocking, IG body callbacks, websockets, RUM); that count is unchanged from master.

./gradlew :dd-java-agent:instrumentation:finatra-2.9:spotlessCheck passes.

This changes test behaviour only; production Finatra applications and tracer instrumentation are unaffected.

Contributor Checklist

• Format the title according to the contribution guidelines
• Assign the type: and (comp: or inst:) labels in addition to any other useful labels
• Avoid using close, fix, or any linking keywords when referencing an issue
  Use solves instead, and assign the PR milestone to the issue
• Update the CODEOWNERS file on source file addition, migration, or deletion
• Update public documentation with any new configuration flags or behaviors
• Once approved, use merge queue to merge the PR

Jira ticket: N/A

[dd-octo-sts]

🟢 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.82 s	14.69 s	[+0.0%; +1.8%] (maybe worse)
startup:insecure-bank:tracing:Agent	13.61 s	13.66 s	[-1.2%; +0.5%] (no difference)
startup:petclinic:appsec:Agent	16.96 s	16.85 s	[-0.2%; +1.6%] (no difference)
startup:petclinic:iast:Agent	16.94 s	17.14 s	[-2.0%; -0.4%] (maybe better)
startup:petclinic:profiling:Agent	16.73 s	16.87 s	[-1.8%; +0.3%] (no difference)
startup:petclinic:sca:Agent	16.83 s	16.46 s	[+1.3%; +3.1%] (significantly worse)
startup:petclinic:tracing:Agent	16.06 s	16.11 s	[-1.2%; +0.6%] (no difference)

Commit: 5c8f18f5 · 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.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 200604d8c4

ℹ️ 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]

More details

The static review finds no reportable defect. The lifecycle changes keep server failures inside the test process and make the older-version suite use Finatra 20.6.x.

Was this helpful? React 👍 or 👎

_{Open Bits AI session}

_{🤖 Datadog Autotest · Commit 200604d · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest}