Skip to content

Conversation

@sindilevich
Copy link
Contributor

Closes #189.

As noted in the PR, the hardcoded values for the top-level status property aren't playing well with other monitoring systems, such as Netfilx's Eureka Discovery server. Other monitoring systems may even expect a whole different JSON as status of an application.

Since Kubernetes assures liveness of a container by an HTTP response with an HTTP code >= 200 and < 400, the hardcoded ok makes no difference to the health check process. Therefore, a response body of: { status: 'up' }, with HTTP code 200, would easily serve as a liveness indicator for both Kubernetes and the Netfilx's Eureka Discovery server.

The proposed resolution in this PR, is to add two new configuration properties: statusOkResponse and statusErrorResponse. Those properties will allow to globally configure the HTTP response body, used to respond with on a successful or an unsuccessful healthcheck respectively.

An unsuccessful healthcheck may override the global statusErrorResponse property by setting the statusResponse property on the Error object.

As an example, the following configuration excerpt (for Express) can help monitoring application liveness for both Kubernetes and the Netfilx's Eureka Discovery server:

const http = require('http');
const express = require('express');
const app = express();

app.get('/', (req, res) => {
  res.send('ok');
});

const server = http.createServer(app);

const options = {
  // opts,
  statusOkResponse: { status: 'up' },
  statusErrorResponse: {status: 'down' }
};

createTerminus(server, options);

server.listen(PORT || 3000);

The new configuration property: statusOkResponse will allow to globally configure the HTTP response body, used to respond with on a successful healthcheck.
When no healthtchecks were defined, the value of statusOkResponse would be returned as is. Therefore, memoizing its string representation would boost performance.
The new configuration property: statusErrorResponse will allow to globally configure the HTTP response body, used to respond with on an unsuccessful healthcheck.
An unsuccessful healthcheck may override the global statusErrorResponse property by setting the statusResponse property on the Error object.
@sindilevich
Copy link
Contributor Author

@gergelyke, @rxmarbles, any chance you can review the PR and, hopefully, merge it in any time soon? Being able to set custom status responses is a real need for me, and am eagerly waiting for the new version of terminus with this functionality!

@sindilevich
Copy link
Contributor Author

@gergelyke, @rxmarbles, pinging to make sure there is a possibility for you to review the PR, and, hopefully, merge it in. The proposed functionality is of high demand in my project.

@rxmarbles rxmarbles requested a review from gergelyke October 26, 2021 15:43
@rxmarbles
Copy link
Member

@sindilevich sorry for the delay in this. Looks like this PR has conflicts. Mind taking care of that so this can land?

@sindilevich sindilevich requested a review from a team as a code owner February 25, 2023 20:50
@sindilevich
Copy link
Contributor Author

@rxmarbles, thank you for finding time to look into my PR!
I've resolved all the conflicts with the main branch.
Would greatly appreciate your review and an approval!
Hope the PR'd finally land!

@rxmarbles
Copy link
Member

@gergelyke Mind giving this a review when you have a chance.

Copy link
Collaborator

@gergelyke gergelyke left a comment

Choose a reason for hiding this comment

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

as this is backward compatible, I am 👌👍👏

@sindilevich
Copy link
Contributor Author

@gergelyke, @rxmarbles. thank you for investing your time, reviewing and approving the PR!
Would greatly appreciate this PR be merged and land in the next version of this great tool!
Thank you!

@rxmarbles rxmarbles merged commit 49e187a into godaddy:main Apr 14, 2023
@sindilevich sindilevich deleted the custom-status-response branch April 14, 2023 09:21
@sindilevich
Copy link
Contributor Author

@rxmarbles, thank you so much for advancing merging the fix!

rxmarbles pushed a commit that referenced this pull request Apr 16, 2023
* Added statusOkResponse to configuration

The new configuration property: statusOkResponse will allow to globally configure the HTTP response body, used to respond with on a successful healthcheck.

* Memoize the value of statusOkResponse

When no healthtchecks were defined, the value of statusOkResponse would be returned as is. Therefore, memoizing its string representation would boost performance.

* Added statusErrorResponse to configuration

The new configuration property: statusErrorResponse will allow to globally configure the HTTP response body, used to respond with on an unsuccessful healthcheck.
An unsuccessful healthcheck may override the global statusErrorResponse property by setting the statusResponse property on the Error object.

* Incorporated use of headers
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.

Option to configure top-level successful response's value

3 participants