Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add /healthz endpoint for health monitoring in S3Proxy #763

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Aazme
Copy link

@Aazme Aazme commented Jan 21, 2025

This PR introduces a new /healthz endpoint in S3Proxy to provide a basic health check mechanism.

Changes:

  • Added a /healthz route to handle GET requests.

  • Implemented handleVersionRequest method to return a JSON response:
    { "status": "OK" }

  • Updated S3ProxyHandler to route and process health check requests.

This endpoint serves as a lightweight solution for monitoring the service's availability.

- Implemented a new `/healthz` endpoint to handle GET requests.
- Added the `handleVersionRequest` method to return a JSON response with health status:
  - Response: `{ "status": "OK" }`
- Updated `S3ProxyHandler` to route requests to `/healthz` and handle responses effectively.
- The endpoint provides a simple mechanism for health monitoring and status validation.
@Aazme Aazme changed the title Remove explicit dependency on Apache commons-io Add /healthz endpoint for health monitoring in S3Proxy Jan 21, 2025
Copy link
Owner

@gaul gaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address Checkstyle warnings:

Warning:  src/main/java/org/gaul/s3proxy/S3ProxyHandler.java:[2042] (regexp) RegexpSingleline: Line has trailing spaces.
Warning:  src/main/java/org/gaul/s3proxy/S3ProxyHandler.java:[2044] (regexp) RegexpSingleline: Line has trailing spaces.

@@ -2029,6 +2035,18 @@ private void handlePutBlob(HttpServletRequest request,

response.addHeader(HttpHeaders.ETAG, maybeQuoteETag(eTag));
}
private void handleVersionRequest(HttpServletResponse response) throws IOException {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be called handleStatuszRequest?

response.setContentType("application/json");
response.setCharacterEncoding("UTF-8");

String versionInfo = "{ \"status\": \"OK\"}";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra space between { and \".

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should return the current time, the time s3proxy launched, and the git hash? Are there any well-known statusz formats this could follow?

@@ -2029,6 +2035,18 @@ private void handlePutBlob(HttpServletRequest request,

response.addHeader(HttpHeaders.ETAG, maybeQuoteETag(eTag));
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add newline before new method.

@@ -293,6 +293,12 @@ public final void doHandle(HttpServletRequest baseRequest,
String uri = request.getRequestURI();
String originalUri = request.getRequestURI();

// Check for the /version endpoint
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove useless comment.

Copy link
Owner

@gaul gaul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you exercise this in some kind of test, e.g., AwsSdkAnonymousTest?

response.setContentType("application/json");
response.setCharacterEncoding("UTF-8");

String versionInfo = "{ \"status\": \"OK\"}";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this should return the current time, the time s3proxy launched, and the git hash? Are there any well-known statusz formats this could follow?

@gaul
Copy link
Owner

gaul commented Jan 30, 2025

I'm preparing a new release so please try to address my comments before the weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants