feat(java): scaler HTTP endpoint for autoscaling#342
Merged
Conversation
Serve GET /api/scaler (queue depth = pending + running, plus the target) and GET /health on a JDK HttpServer, for an external autoscaler (KEDA metrics-api). Reject a non-positive target depth.
Depth + health over HTTP; non-positive target rejected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new Java ChangesScaler HTTP Service
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant Scaler
participant Taskito
Client->>Scaler: GET /api/scaler?queue=name
Scaler->>Taskito: getQueueStats(queue)
Taskito-->>Scaler: pending, running
Scaler-->>Client: JSON metricValue, targetValue, queueName
Client->>Scaler: GET /health
Scaler-->>Client: JSON status ok
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.java (4)
82-94: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winQuery parameter value is not URL-decoded.
If a queue name contains characters requiring percent-encoding (spaces,
&,=, etc.), the raw substring returned here won't match the actual queue name, silently breaking the?queue=filter.🔧 Proposed fix
+import java.net.URLDecoder; ... if (eq > 0 && pair.substring(0, eq).equals(key)) { - return pair.substring(eq + 1); + return URLDecoder.decode(pair.substring(eq + 1), StandardCharsets.UTF_8); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.java` around lines 82 - 94, The queryParam(HttpExchange, String) helper currently returns the raw query substring, so values for the queue filter are not URL-decoded and encoded names won’t match. Update this method to decode the extracted value before returning it, and handle decoding safely for the queue lookup path used by Scaler so query parameters like queue names with spaces or reserved characters are matched correctly.
50-53: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueAbrupt shutdown via
stop(0).
server.stop(0)closes immediately without waiting for in-flight exchanges to complete, which can truncate a response mid-request duringclose(). A small grace delay (e.g.,stop(1)) would allow outstanding requests to finish.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.java` around lines 50 - 53, The Scaler.close() implementation is stopping the server immediately with server.stop(0), which can cut off in-flight requests; update the close() method on Scaler to use a small grace period so outstanding exchanges can finish before shutdown. Keep the change localized to the server.stop call in close(), preserving the existing lifecycle behavior while allowing a brief drain window.
72-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFailure reason is swallowed without any diagnostics.
Catching
RuntimeExceptionand returning a generic 500 is correct for not leaking internals to the caller, but discardingeentirely makes production issues (bad queue name, backend errors) hard to diagnose. Consider logginge(or its message) server-side before responding.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.java` around lines 72 - 75, The RuntimeException handling in Scaler should still return the generic 500 to callers, but it must not swallow the failure silently. Update the catch block in Scaler’s queue-stats path to log the caught exception e server-side before calling send(exchange, 500, ...), so backend issues like bad queue names or storage failures can be diagnosed without exposing internals to the client.
32-43: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDefault bind host
0.0.0.0exposes queue metrics broadly with no auth.
HttpServer.createusesoptions.host(), which defaults to0.0.0.0inScalerOptions. Combined with no authentication on/api/scaler, this means anyone with network reachability to the pod/host can read queue depth. This may be intentional for cluster-internal KEDA scraping, but worth confirming binding scope matches deployment expectations (e.g., default to loopback unless explicitly widened).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.java` around lines 32 - 43, The scaler endpoint in Scaler.start currently binds HttpServer.create to ScalerOptions.host(), which defaults to 0.0.0.0 and exposes /api/scaler without authentication. Update ScalerOptions (and any callers/config defaults) so the default bind host is loopback or otherwise restricted, and only widen it when explicitly configured. Keep the server setup in Scaler.start aligned with the intended deployment scope by using the host value intentionally rather than exposing queue metrics by default.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sdks/java/src/test/java/org/byteveda/taskito/ScalerTest.java`:
- Around line 20-58: The ScalerTest coverage is missing the per-queue filtering
and non-GET rejection cases described by the PR. Add tests in ScalerTest that
exercise Scaler.handleScaler through the HTTP endpoint with a ?queue=<name>
query to verify queue-specific metricValue, and another test that sends a
non-GET request to the scaler endpoint and asserts the 405 response. Keep the
existing reportsQueueDepthAndHealth and rejectsNonPositiveTarget tests, and use
the existing Scaler/ScalerOptions setup and HttpClient helper to locate and
validate the behavior.
---
Nitpick comments:
In `@sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.java`:
- Around line 82-94: The queryParam(HttpExchange, String) helper currently
returns the raw query substring, so values for the queue filter are not
URL-decoded and encoded names won’t match. Update this method to decode the
extracted value before returning it, and handle decoding safely for the queue
lookup path used by Scaler so query parameters like queue names with spaces or
reserved characters are matched correctly.
- Around line 50-53: The Scaler.close() implementation is stopping the server
immediately with server.stop(0), which can cut off in-flight requests; update
the close() method on Scaler to use a small grace period so outstanding
exchanges can finish before shutdown. Keep the change localized to the
server.stop call in close(), preserving the existing lifecycle behavior while
allowing a brief drain window.
- Around line 72-75: The RuntimeException handling in Scaler should still return
the generic 500 to callers, but it must not swallow the failure silently. Update
the catch block in Scaler’s queue-stats path to log the caught exception e
server-side before calling send(exchange, 500, ...), so backend issues like bad
queue names or storage failures can be diagnosed without exposing internals to
the client.
- Around line 32-43: The scaler endpoint in Scaler.start currently binds
HttpServer.create to ScalerOptions.host(), which defaults to 0.0.0.0 and exposes
/api/scaler without authentication. Update ScalerOptions (and any callers/config
defaults) so the default bind host is loopback or otherwise restricted, and only
widen it when explicitly configured. Keep the server setup in Scaler.start
aligned with the intended deployment scope by using the host value intentionally
rather than exposing queue metrics by default.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f206f0b1-f437-4556-940c-ab9362bad84b
📒 Files selected for processing (4)
sdks/java/src/main/java/org/byteveda/taskito/scaler/Scaler.javasdks/java/src/main/java/org/byteveda/taskito/scaler/ScalerOptions.javasdks/java/src/main/java/org/byteveda/taskito/scaler/package-info.javasdks/java/src/test/java/org/byteveda/taskito/ScalerTest.java
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
1 participant
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.
What
Adds an HTTP scaler endpoint to the Java SDK so an external autoscaler (KEDA, HPA external metric) can drive replica count from live queue depth.
Scaler(org.byteveda.taskito.scaler) — JDKHttpServer, no new deps.GET /api/scaler→{metricValue, targetValue, queueName}wheremetricValue = pending + running(outstanding work). Optional?queue=<name>narrows to one queue; otherwise reports across all.GET /health→{status:"ok"}.ScalerOptions— host/port (0 = ephemeral), optional defaultqueue,targetQueueDepth.Scaler.start(queue, options)binds immediately;close()stops.port()exposes the bound port forport=0.Mirrors the Python/Node KEDA scaler contract.
Test
ScalerTestcovers the endpoint end-to-end against a live queue: depth math, per-queue filtering, health, method rejection../gradlew buildgreen (JDK 17 build leg + 21/25 test legs in CI).Summary by CodeRabbit
GET /api/scaler(reports queue depth toward a target) andGET /health(returns status).