DanielLeens opened a new pull request, #11641:
URL: https://github.com/apache/seatunnel/pull/11641
### Purpose of this pull request
Fix a flaky `unit-test` job failure that is unrelated to the PRs it blocks.
`org.apache.seatunnel.engine.server.metrics.MetricsApiTest#metricsApiTest`
intermittently fails with:
```
java.lang.AssertionError:
1 expectation failed.
Expected status code <200> but was <500>.
at
org.apache.seatunnel.engine.server.metrics.MetricsApiTest.metricsApiTest(MetricsApiTest.java:56)
```
It was observed on five different, unrelated PRs (`#10874`, `#10958`,
`#10973`, `#11060`, `#11382`) on both `unit-test (8, ubuntu-latest)` and
`unit-test (11, ubuntu-latest)`. None of those PRs touch telemetry, metrics or
the REST layer, so the failure is a property of the test itself.
**Root cause**
`@BeforeAll` starts the member and the test issues `GET /metrics` right away
— the failing runs show the whole test class living for ~1.1s with the request
answered ~40ms after `createHazelcastInstance()` returned.
The HTTP listener starts accepting as soon as the member reaches `STARTED`,
but `MetricsServlet` iterates `CollectorRegistry` collectors that read
coordinator-owned state which is still being wired up at that moment (the same
runs log `The Node is not ready yet, Node state STARTING` and a
`CoordinatorService` pool with `poolSize: 0`). A collector that runs before its
backing service is available raises, `ExceptionHandlingFilter` turns it into a
500, and the test fails.
The existing assertion also throws the server's answer away.
`ExceptionHandlingFilter` puts the originating stack trace in the response
body, but Rest Assured's `statusCode(200)` only reports the code, which is why
these CI failures could not be diagnosed from the logs.
### Does this PR introduce _any_ user-facing change?
No. Test-only change; no production code is touched.
- Poll the endpoint until it is serviceable (bounded at 60s, 1s interval)
instead of querying it exactly once. A healthy endpoint answers on the first
poll and costs nothing; an endpoint that never recovers still fails the test.
- Assert the status with the response body attached, so any future failure
carries the server-side stack trace.
All three metric families that were asserted before
(`process_start_time_seconds`, `engine_state_store_local_owned_entries`,
`engine_state_store_checkpoint_monitor_jobs`) are still required, so the test
is not weakened — it only stops depending on an unspecified "endpoint is fully
warm the instant the member is STARTED" timing guarantee.
### How was this patch tested?
Existing `MetricsApiTest` is the test; it keeps every assertion it had.
Correctness is verified by CI on this PR head.
### Check list
* [x] If any new Jar binary package adding in your PR, please add License
Notice according [New License
Guide](https://github.com/apache/seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [x] If necessary, please update the documentation to describe the new
feature. https://github.com/apache/seatunnel/tree/dev/docs
* [x] If necessary, please update `incompatible-changes.md` to describe the
incompatibility caused by this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
