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.

Sign up for GitHub

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 design note for health and status endpoints #49

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

Add design note for health and status endpoints #49

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ with enough detail to review the intent and direction of the feature.
- [Exportable Subflow](designs/exportable-subflow/README.md)
- [Node Timeout API](designs/timeout-api.md)
- [Overwrite Values in settings.js](designs/overwrite-settings.md)
- [Add Health and Status Endpoints to Admin API](designs/admin-api-health-status.md)

#### In-progress

Expand Down
36 changes: 36 additions & 0 deletions designs/admin-api-health-status.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
state: draft | in-progress | complete
---

# Add Health and Status Endpoints to Admin API

## Summary

In addition to the available methods in the Admin API laid out [here](https://nodered.org/docs/api/admin/methods/), this document proposes the creation of `health` and `status` endpoints. These endpoints would be useful for application monitoring but could find broader use as well.

While users can add this functionality manully to their own instances of Node-RED, this feature set feels sufficiently primitive and general that it would be useful to expose it as part of the Admin API.

## Authors

- [@szep](https://discourse.nodered.org/u/szep)

## Details

Two endpoints would be added to the Admin API:

1. `GET /health` -- on success returns status code 200 and payload `{"status": "ok"}`
Copy link
Member

Choose a reason for hiding this comment

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

this is for the overall Node-RED service ? - so yes agree 200 OK fine. (maybe if we want to be detailed could add one for starting and stopping - obviously no response when actually stopped)

Copy link
Author

Choose a reason for hiding this comment

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

I think that's a great idea!

Copy link
Member

Choose a reason for hiding this comment

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

I think the /health end point will need to be configurable - both in terms of whether the endpoint is enabled at all, and what url it is exposed on.

We also have two separate route handlers in Node-RED - the admin side and the node side. We need to decide which handler these end-points are on. They are probably admin routes - although we know some users will disable the admin route handler entirely for their production system.

As there will be existing flows that serve up their own /health end point, we may have to disable it by default - and allow it to be enabled via the settings file.

- Likely don't need to discuss what a fail condition would be since this endpoint is designed to indicate simply that Node-RED is up and running
1. `GET /status` -- on success returns status code 200 and payload:
Copy link
Member

Choose a reason for hiding this comment

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

Which status object ? They are currently for nodes. Is this scoped to nodes ? in which case then does every node need to be able to provide status ? (imho no) - or to the overall runtime in which case what should it actually contain ?

Copy link
Author

Choose a reason for hiding this comment

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

@dceejay, I'm imagining it returning an array of the same status objects that the status node provides. Only nodes that provide a status would be included in the return payload.

Copy link
Member

Choose a reason for hiding this comment

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

none of the status is held as state anywhere apart from in the client browser - hence doing what you suggest now means we have to hold all that state centrally for all nodes just in case - whether there is a browser client or not. I think that is not something we want to do on the offchance someone wants it. I can see the use of an overall status of the runtime - eg maybe showing things like uptime, name of running flow, maybe process cpu load ? that sort of thing - but not state of every single node.

Copy link
Member

Choose a reason for hiding this comment

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

none of the status is held as state anywhere apart from in the client browser

That's not entirely true. The runtime does have to hold the current status value of all nodes so that when a new editor connects, it is able to deliver the status value to get it in sync.

That said, the node status is meant to be ephemeral internal state of the flow - used by the flow itself in its operation - it wasn't something intended to be exposed via admin apis. A flow could be created to expose node status if the flow author wants to do so - but that isn't a pattern I have ever seen mentioned in the community.

So it is technically possible to do, I would like to understand the specific use case for it a bit more.


```JSON
{
"statuses": [
<STATUS OBJECTS IDENTICAL TO
THOSE FROM STATUS NODE>
]
}
```

## History

- 2021-03-04 - Initial proposal submitted