feat(QTDI-2030): Add health and readiness endpoints to component-server#1245
Open
undx wants to merge 7 commits into
Open
feat(QTDI-2030): Add health and readiness endpoints to component-server#1245undx wants to merge 7 commits into
undx wants to merge 7 commits into
Conversation
Introduces GET /api/v1/health (liveness probe) and GET /api/v1/readiness
(readiness probe) as dedicated Kubernetes-style endpoints, replacing the
insufficient /environment endpoint for cluster health monitoring.
Health checks: heap memory availability (configurable threshold),
component index validity, and Vault connectivity via VaultClient.ping().
Readiness check: component index load state (isStarted()).
Both endpoints return {"status":"UP"|"DOWN","cause":"..."} with HTTP 200/503.
Closes QTDI-2030
Co-Authored-By: GitHub Copilot <copilot@noreply.github.com>
Member
Author
Scope & Design Review — Round 1 — APPROVEDFindingsBLOCKER B1 — Wrong path for readiness endpoint (FIXED ✅)
MAJOR M1 — Memory check divide-by-zero risk with unbounded heap (FIXED ✅)
MINOR Mi1 —
|
| Severity | Count |
|---|---|
| Critical (fixed) | 1 |
| Warning (accepted) | 2 |
CRITICAL C1 — catch (final Exception e) in VaultClient.ping() (FIXED ✅)
- Fix: Changed to
catch (final javax.ws.rs.ProcessingException e)— the correct transport-level exception for JAX-RS client failures.
WARNING W1 — catch (final Throwable t) in HealthService.checkComponentIndex() (ACCEPTED)
- Justification: Intentional per approved plan — OOME can only be caught with
Throwable.
WARNING W2 — MockitoAnnotations.openMocks() instead of @ExtendWith(MockitoExtension.class) (ACCEPTABLE)
- Justification:
mockito-junit-jupiter:4.8.1is incompatible with JUnit 5.10.0 on the test classpath.MockitoAnnotations.openMocks()achieves the same result;@InjectMocksand@Mockannotations are still used as prescribed.
AC Coverage
| AC | Covered? |
|---|---|
| GET /api/v1/health → 200 when healthy | ✅ UT + IT |
| GET /api/v1/readiness → 200 when ready | ✅ UT + IT |
| /health → 503 + cause when memory low | ✅ UT |
| /health → 503 + cause when Vault fails | ✅ UT |
| /readiness → 503 + cause when index not ready | ✅ UT |
| /environment independent from health/readiness | ✅ IT |
| Memory threshold is configurable | ✅ config property |
| Vault check uses VaultClient.ping() | ✅ new method added |
| Error message with cause in JSON | ✅ HealthStatus.cause field |
…ess/readiness - Rename /health → /liveness (LivenessResource + LivenessResourceImpl) The /health endpoint was misleading; /liveness matches Kubernetes naming - Add FatalState singleton to record VirtualMachineError occurrences LivenessResourceImpl now returns DOWN if a fatal JVM error was recorded, instead of performing active synchronous checks that could themselves trigger OOMEs - Add DefaultExceptionHandler VirtualMachineError interception → FatalState Any OOME or other VirtualMachineError during a request marks the JVM as fatal - Add RequestContextFilter + RequestContextHolder to expose request path on the current thread, used by DefaultExceptionHandler for richer error messages - Move Vault check from liveness to ReadinessResourceImpl with 5s cache Synchronous Vault ping on the liveness path risked cascading pod restarts; moved to readiness with a TTL cache and catch(Throwable) + Response.close() - Add VaultClient.ping() timeout (configurable, default 2s) via talend.vault.cache.vault.ping.timeout - Remove HealthService (logic split into LivenessResourceImpl/ReadinessResourceImpl) - New unit tests: FatalStateTest, RequestContextHolderTest, ReadinessResourceImplTest - Rewrite HealthResourceImplIT → LivenessResourceImplIT
Member
Author
Review round 1 — summary
Fixes pushed: What changed
Pending clarifications: 0 Round summary generated by AI. Please resolve threads after verifying the fixes. |
undx
commented
Jul 7, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces Kubernetes probe endpoints for the component-server by adding new liveness/readiness JAX-RS resources, tracking fatal JVM errors to force restarts, and adding a Vault connectivity “ping” used by readiness logic.
Changes:
- Add
/api/v1/livenessand/api/v1/readinessendpoints with a sharedHealthStatusresponse model. - Record
VirtualMachineErroroccurrences viaDefaultExceptionHandlerinto an application-scopedFatalStateso liveness can return503after fatal JVM errors. - Add a
VaultClient.ping()method (with configurable ping timeout) and use it from readiness with simple caching.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
vault-client/src/main/java/org/talend/sdk/components/vault/client/VaultClient.java |
Adds Vault connectivity ping() and a ping-timeout config property. |
component-server-parent/component-server/src/test/java/org/talend/sdk/component/server/service/RequestContextHolderTest.java |
Unit tests for the new request-path ThreadLocal holder. |
component-server-parent/component-server/src/test/java/org/talend/sdk/component/server/service/FatalStateTest.java |
Unit tests for fatal JVM error tracking behavior. |
component-server-parent/component-server/src/test/java/org/talend/sdk/component/server/front/ReadinessResourceImplTest.java |
Unit tests for readiness behavior (index readiness + Vault ping). |
component-server-parent/component-server/src/test/java/org/talend/sdk/component/server/front/LivenessResourceImplIT.java |
Integration tests validating probe endpoints respond successfully in the container. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/service/RequestContextHolder.java |
Introduces ThreadLocal request-path storage for error context. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/service/FatalState.java |
Adds application-scoped fatal error latch used by liveness endpoint. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/service/ComponentManagerService.java |
Exposes isStarted() for readiness gating. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/front/ReadinessResourceImpl.java |
Implements readiness endpoint with component-index check and cached Vault ping. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/front/LivenessResourceImpl.java |
Implements liveness endpoint based on FatalState. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/front/filter/RequestContextFilter.java |
Adds servlet filter that populates/clears RequestContextHolder. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/front/error/DefaultExceptionHandler.java |
Records VirtualMachineError into FatalState including request-path context. |
component-server-parent/component-server/src/main/java/org/talend/sdk/component/server/configuration/ComponentServerConfiguration.java |
Adds (currently unused) heap-threshold config property for health checks. |
component-server-parent/component-server/pom.xml |
Adds mockito-core test dependency for new unit tests. |
component-server-parent/component-server-model/src/main/java/org/talend/sdk/component/server/front/model/HealthStatus.java |
Adds HealthStatus DTO used by probe responses. |
component-server-parent/component-server-api/src/main/java/org/talend/sdk/component/server/api/ReadinessResource.java |
Defines readiness API contract and OpenAPI metadata. |
component-server-parent/component-server-api/src/main/java/org/talend/sdk/component/server/api/LivenessResource.java |
Defines liveness API contract and OpenAPI metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ural refinements - LivenessResource: rename JVM → Application in API response descriptions (U1, U2) - ComponentServerConfiguration: remove unused healthMemoryThreshold field (U3) - ComponentServerConfiguration: add healthVaultEnabled flag (default false) (U4) - ReadinessResourceImpl: gate Vault check behind healthVaultEnabled flag (U4) - ReadinessResourceImpl: fix race — update lastVaultCheck after result (C1) - ReadinessResourceImpl: re-throw VirtualMachineError before catch(Throwable) (C1) - ComponentManagerService: make started field volatile (C5) - DefaultExceptionHandler: replace RequestContextHolder with @context HttpServletRequest (C6) - VaultClient: use dedicated ClientBuilder for ping with connect/read timeout (C7) - FatalState: add reset() method for test isolation (U5) - LivenessResourceImplIT: add test for FatalState.markFatal() → /liveness 503 (U5)
Member
Author
Review round 2 — summary
Fixes pushed: Round summary generated by AI. Please resolve threads after verifying the fixes. |
|
Comment on lines
+210
to
+229
| public boolean ping() { | ||
| if ("no-vault".equals(setup.getVaultUrl())) { | ||
| return true; | ||
| } | ||
| Response response = null; | ||
| try { | ||
| response = vault | ||
| .path("v1/sys/health") | ||
| .request() | ||
| .get(); | ||
| return true; | ||
| } catch (final javax.ws.rs.ProcessingException e) { | ||
| log.warn("Vault ping failed: {}", e.getMessage()); | ||
| return false; | ||
| } finally { | ||
| if (response != null) { | ||
| response.close(); | ||
| } | ||
| } | ||
| } |
Comment on lines
+62
to
+67
| if (exception instanceof VirtualMachineError) { | ||
| final String requestPath = request != null ? request.getRequestURI() : "(unknown)"; | ||
| final String cause = exception.getClass().getSimpleName() + " during request " | ||
| + requestPath + ": " + exception.getMessage(); | ||
| fatalState.markFatal(cause); | ||
| } |
Comment on lines
+41
to
+49
| @Test | ||
| void livenessReturns200WhenNoFatalError() { | ||
| final Response response = base.path("liveness").request(APPLICATION_JSON_TYPE).get(); | ||
| assertEquals(200, response.getStatus()); | ||
| final HealthStatus status = response.readEntity(HealthStatus.class); | ||
| assertNotNull(status); | ||
| assertEquals("UP", status.getStatus()); | ||
| assertNull(status.getCause()); | ||
| } |
Comment on lines
+51
to
+58
| @Test | ||
| void readinessReturns200WhenReady() { | ||
| final Response response = base.path("readiness").request(APPLICATION_JSON_TYPE).get(); | ||
| assertEquals(200, response.getStatus()); | ||
| final HealthStatus status = response.readEntity(HealthStatus.class); | ||
| assertNotNull(status); | ||
| assertEquals("UP", status.getStatus()); | ||
| } |
Comment on lines
+60
to
+70
| @Test | ||
| void livenessAndReadinessAreIndependentFromEnvironment() { | ||
| final Response envResponse = base.path("environment").request(APPLICATION_JSON_TYPE).get(); | ||
| assertEquals(200, envResponse.getStatus()); | ||
|
|
||
| final Response livenessResponse = base.path("liveness").request(APPLICATION_JSON_TYPE).get(); | ||
| assertEquals(200, livenessResponse.getStatus()); | ||
|
|
||
| final Response readinessResponse = base.path("readiness").request(APPLICATION_JSON_TYPE).get(); | ||
| assertEquals(200, readinessResponse.getStatus()); | ||
| } |
Comment on lines
+75
to
+85
| try { | ||
| final Response response = base.path("liveness").request(APPLICATION_JSON_TYPE).get(); | ||
| assertEquals(503, response.getStatus()); | ||
| final HealthStatus status = response.readEntity(HealthStatus.class); | ||
| assertNotNull(status); | ||
| assertEquals("DOWN", status.getStatus()); | ||
| assertNotNull(status.getCause()); | ||
| } finally { | ||
| // reset the singleton state so other tests are not affected | ||
| fatalState.reset(); | ||
| } |
Comment on lines
+74
to
+98
| final long now = System.currentTimeMillis(); | ||
| if (now - lastVaultCheck.get() < VAULT_CACHE_TTL_MS) { | ||
| return cachedVaultResult.get() | ||
| ? new HealthStatus(STATUS_UP, null) | ||
| : new HealthStatus(STATUS_DOWN, "Vault is not reachable"); | ||
| } | ||
| try { | ||
| final boolean reachable = vaultClient.ping(); | ||
| cachedVaultResult.set(reachable); | ||
| lastVaultCheck.set(now); | ||
| if (!reachable) { | ||
| log.warn("Readiness check: Vault is not reachable"); | ||
| return new HealthStatus(STATUS_DOWN, "Vault is not reachable"); | ||
| } | ||
| } catch (final VirtualMachineError vme) { | ||
| throw vme; | ||
| } catch (final Throwable t) { | ||
| cachedVaultResult.set(false); | ||
| lastVaultCheck.set(now); | ||
| final String cause = "Vault connectivity check failed: " + t.getMessage(); | ||
| log.warn("Readiness check failed: {}", cause); | ||
| return new HealthStatus(STATUS_DOWN, cause); | ||
| } | ||
| return new HealthStatus(STATUS_UP, null); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements two new Kubernetes-style probe endpoints for the component-server, resolving recurring cluster restart failures in Talend Cloud (AU/AP incidents).
The existing
/environmentendpoint was insufficient because it returns 200 even when the server cannot process requests. This PR introduces:503 {"status":"DOWN","cause":"..."\}if any check fails, triggering a pod restart.503while the index is being built, preventing premature traffic routing.Key design decisions:
talend.server.health.memory.threshold(default: 10%)Runtime.maxMemory() == Long.MAX_VALUEguard prevents false DOWN on unbounded heapvaultUrl == "no-vault"(standalone mode)catch (Throwable t)in component index check is intentional — required to catch OOME (documented in PR comment below)Jira: QTDI-2030
Checklist
mvn clean verifypasses locallymvn spotless:apply)AI generated code
https://internal.qlik.dev/general/ways-of-working/code-reviews/#guidelines-for-ai-generated-code