-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Adding /admin GET method which will return GOOD to denote the health of Azkaban web server #3053
base: master
Are you sure you want to change the base?
Conversation
…of Azkaban web server
Codecov Report
@@ Coverage Diff @@
## master #3053 +/- ##
============================================
+ Coverage 41.49% 41.53% +0.03%
- Complexity 4654 4655 +1
============================================
Files 596 597 +1
Lines 40037 40051 +14
Branches 4654 4654
============================================
+ Hits 16614 16634 +20
+ Misses 22049 22039 -10
- Partials 1374 1378 +4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the changes look good. I am wondering
- why don't we use
/status
itself to solve the same problem? - can we consider returning a JSON response? The clients who are using can parse this easily and if we choose to add more fields in the response it will be convenient.
@svnarumugam , we have been using /status but it is not just for health check. It queries lot of things from database and sometimes slow when it comes to pinging /status for every hit coming to Azkaban web server. We need light API. |
@jakhani Can we change the name of this endpoint? /health? /monitor? /ping? /heartbeat? |
try { | ||
resp.setContentType(JSON_MIME_TYPE); | ||
resp.getOutputStream().println("GOOD"); | ||
resp.setStatus(HttpServletResponse.SC_OK); | ||
} catch (final Exception e) { | ||
logger.error("Error!! while reporting status: ", e); | ||
resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getMessage()); | ||
} finally { | ||
resp.getOutputStream().close(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the sendResponse
method defined in the AbstractAzkabanServlet
class?
Could you please add an example response in the PR description? |
/admin call will return GOOD. It can be used to integrate with health and monitoring system for web server.