telefonicaid / fiware-orion

Context Broker and CEF building block for context data management, providing NGSI interfaces.
https://fiware-orion.rtfd.io/
GNU Affero General Public License v3.0
212 stars 265 forks source link

add health route api #4119

Open tzzed opened 2 years ago

tzzed commented 2 years ago

Is your feature request related to a problem / use case? Please describe. It will be great to add a /health route api for orion broker.

Describe the solution you'd like Get the status of the broker can be very usefull.

Describe alternatives you've considered Instead of health route I use the /version to know it orion broker is started.

Describe why you need this feature A simple case is to know if the orion broker is started.

Additional information Add any other information, diagrams or screenshots about the feature request here.

Do you have the intention to implement the solution

fgalan commented 2 years ago

It is an interesting idea!

Just to know what you have in mind... which information do you would include in the health response? Could you even draft an example JSON on how it would look like?

Thanks for the feedback!

fgalan commented 2 years ago

As minor side note:

Note that the use case you describe:

A simple case is to know if the orion broker is started.

Can be solved with /version operations, that not only check that Orion is started but also how long it has been (in the uptime) field ;)

tzzed commented 2 years ago

It depends. But we can include some informations that are in docker inspect. something like this can be a motivation.

 "State": {
            "Name": "/fiware-orion",
            "RestartCount": 0,
            "Driver": "overlay2",
            "Platform": "linux",
            "Status": "running",
            "Running": true,
            "Paused": false,
            "Restarting": false,
            "OOMKilled": false,
            "Dead": false,
            "Pid": 28700,
            "ExitCode": 0,
            "Error": "",
            "StartedAt": "2022-05-05T13:11:55.7592099Z",
            "FinishedAt": "0001-01-01T00:00:00Z",

}
fgalan commented 2 years ago

It depends. But we can include some informations that are in docker inspect.

Note that Orion is an independent process. I doesn't always runs inside a docker container, so the information returned by health should be "neutral" in that sense.

tzzed commented 2 years ago

I know but using the informations from docker inspect as source of inspiration can be helpful.

Anjali-NEC commented 1 year ago

Hi @fgalan sir,

I would like to work on this issue, please let me know if this is still required or not? and please suggest what would be the response of the /health API. Thanks

fgalan commented 1 year ago

The issue is still valid but it would require to analize what to return as response for the /health request. Unless you have a proposal for that (please add a comment with it in this issue in that case) I'd suggest to pick another issue.

tzzed commented 1 year ago

Hi @fgalan what about mixing some informations from /version and enities + subscription.

{
"entities": "nb_entities",
"subscriptions": "nb_subscriptions",
"orion": {
    "version": "3.8.0",
    "uptime": "3 d, 8 h, 10 m, 33 s",
    "release_date": "Thu Jan 12 12:06:24 UTC 2023",
    "machine": "x86_64",
    "doc": "https://fiware-orion.rtfd.io/en/3.8.0/",   
}
fgalan commented 1 year ago

I think that kind of information (number of entities and number of subscriptions) fits better in the statistics or metrics requests. Maybe the later, as it provides information per subservice and service along with its aggregations, and entities and subscriptions belongs to subservice+service.

ArqamFarooqui110719 commented 6 months ago

Hi @fgalan, I also think it's a very good idea to implement /health API for Orion.

The purpose of /health API for any application is to show the running status of the application. Keeping that point in mind, I'd suggest that, /version API should keep the data of "version of Orion and it's dependency" and /health API should return "current running status of Orion and compilation info". For example:

Version API:

curl localhost:1026/version

/version API Response after /health API (it should only return version details of Orion and it's dependent):

  "version" : "3.x",
  "git_hash" : "xvz",
  "release_date" : "Mon Jan 29 08:45:34 UTC 2024",
  "doc" : "https://fiware-orion.rtfd.io/en/3.x.0/",
  "libversions": { xyz }

health API:

curl localhost:1026/health

health API response (it should return health/status of Orion):

  "status" : "healthy/running"
  "uptime" : "x days"
  "compile_time" : "x days"
  "compiled_by" : "x days"
  "compiled_in" : "x days"
  "machine" : "x86_64"
  "port" : "1026"

I think with the above approach, the purpose of both the APIs will be fulfilled and also we will be able to add /health API.

In case, there will be any new and valid information need to be added into any of these APIs, we can add that later also, if required. and for now we can go with the above details.

What is your opinion on this, @fgalan?

ArqamFarooqui110719 commented 6 months ago

Hi @fgalan Please provide your opinion on above comment?

fgalan commented 6 months ago

In my opinion, it's not a good idea to touch the existing GET /version API request to remove fields. Some existing applications may be using these fields for health-checking so removing them (to move to that new /health) could be a potential backward compatibility problem with little gain.

(Side note: adding new fields to GET /version wouldn't be risky)

Taking that into account, it would be possible to implement a GET /health operation if we find it useful. Which fields would include that operation (not already include in /version and in the the statistics or metrics APIs)? Could you provide a proposal (draft JSON), maybe re-formuling your previous proposal taking into account the requirement on not removing fields in the GET /version operation ?

ArqamFarooqui110719 commented 4 months ago

In my opinion, it's not a good idea to touch the existing GET /version API request to remove fields. Some existing applications may be using these fields for health-checking so removing them (to move to that new /health) could be a potential backward compatibility problem with little gain. (Side note: adding new fields to GET /version wouldn't be risky)

I agree with your opinion, we should not touch existing /version API.

Taking that into account, it would be possible to implement a GET /health operation if we find it useful. Which fields would include that operation (not already include in /version and in the the statistics or metrics APIs)? Could you provide a proposal (draft JSON), maybe re-formuling your previous proposal taking into account the requirement on not removing fields in the GET /version operation ?

As I mentioned in previous comment, it's very good to have /health API for Orion and the general purpose for /health API is to give the health status of application or to show the running status of the application. Keeping that point in mind, the response for /health API could be:

{
  "status" : "healthy/running"
  "uptime" : "x days"
  "compile_time" : "x days"
  "compiled_by" : "..."
  "compiled_in" : "..."
  "machine" : "x86_64"
  "port" : "1026"
}

There are some redundant fields in above draft but I think those information are more related with /health API, so we should include them. And we can remove/add some fields given in the above draft after discussion. What is your opinion on this @fgalan ?

fgalan commented 4 months ago

Taking into account the proposed fields:

Regarding the other ones:

Anyway, I think that only these two fields is a bit poor and doesn't justify the cost of developing a new operation in the API. I have the feeling that more information should be include here, providing information of the internal status of the contextBroker process, although I'm afraid I cannot provide more precise information (maybe you could take inspiration on some existing service with a similar health API?).

I think we need more suggestions on this before starting to implement this issue.

ArqamFarooqui110719 commented 4 months ago

Will discuss on this once we will have more information. Meanwhile I'll look into some similar health APIs.

fgalan commented 4 months ago

Some ideas: consumed memory, used file descriptors, number of threads, etc.

ArqamFarooqui110719 commented 4 months ago

Regarding the other ones:

  • status: apart from healthy status, which other possibilities you suggest?
  • uptime: fine with this.

Other possibilities could be:

{
  "status" : "healthy" / "unhealthy" / "healthy(with warning)",
  ...
}

Anyway, I think that only these two fields is a bit poor and doesn't justify the cost of developing a new operation in the API. I have the feeling that more information should be include here, providing information of the internal status of the contextBroker process, although I'm afraid I cannot provide more precise information (maybe you could take inspiration on some existing service with a similar health API?).

I think we need more suggestions on this before starting to implement this issue.

What about having some below details in /health API like memory utilization and os-utilization:

{
  "status" : "healthy",
  "uptime" : "-",
  "memory-utilization" : "-",
  "os-utilization" : "-"
}

Not sure about database connectivity field in /health API?

fgalan commented 4 months ago

Not sure about database connectivity field in /health API?

It could make sense.

ArqamFarooqui110719 commented 4 months ago

Some ideas: consumed memory, used file descriptors, number of threads, etc.

Yes, I was also thinking about these fields.

ArqamFarooqui110719 commented 4 months ago

Not sure about database connectivity field in /health API?

It could make sense.

I am looking for some more information, I'll update the JSON draft with updated fields. (e.g. Status, uptime, Memory utilization, OS utilization, Threads, database connectivity etc.)