supervisor icon indicating copy to clipboard operation
supervisor copied to clipboard

Add enhanced logging REST endpoints using systemd-journal-gatewayd

Open agners opened this issue 3 years ago • 6 comments

Proposed change

Add /host/logs/entries and /host/logs/{identifier}/entries to expose log entries from systemd-journald running on the host. Use systemd-journal-gatewayd which exposes the logs to the Supervisor via Unix socket.

Current two query string parameters are allowed: boot and follow. The first will only return logs since last boot. The second will keep the HTTP request open and send new log entries as they get added to the systemd-journal.

Forward the Range header to systemd-journal-gatewayd. This allows to select only a certain amount of log data. The Range header is a standard header to select only partial amount of data. However, the "entries=" prefix is custom for systemd-journal-gatewayd, denoting that the numbers following represent log entries (as opposed to bytes or other metrics).

Type of change

  • [ ] Dependency upgrade
  • [ ] Bugfix (non-breaking change which fixes an issue)
  • [x] New feature (which adds functionality to the supervisor)
  • [ ] Breaking change (fix/feature causing existing functionality to break)
  • [ ] Code quality improvements to existing code or addition of tests

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request: https://github.com/home-assistant/developers.home-assistant/pull/1127
  • Link to cli pull request:

Checklist

  • [x] The code change is tested and works locally.
  • [ ] Local tests pass. Your PR cannot be merged unless tests pass
  • [ ] There is no commented out code in this PR.
  • [ ] I have followed the development checklist
  • [ ] The code has been formatted using Black (black --fast supervisor tests)
  • [ ] Tests have been added to verify that the new code works.

If API endpoints of add-on configuration are added/changed:

agners avatar Nov 05 '21 16:11 agners

  1. Please write a generic RFC on how we can handle logs API call for signaling to use streams or just return a range.
  2. Params are not allowed, use path for this (append /boot or /stream at the end i.e.) however, stream is part of the RFC.
  3. /host/logs/entries and /host/logs/{identifier}/entries is not how we handle Paths. use /host/logs/all/entries for all

pvizeli avatar Nov 08 '21 09:11 pvizeli

Maybe it would make sense to offload it to /journal/ I guess

pvizeli avatar Nov 08 '21 09:11 pvizeli

2. Params are not allowed, use path for this (append /boot or /stream at the end i.e.) however, stream is part of the RFC.

With Params you mean query string right?

Any specific reasons they are not allowed? In RESTful API's the URL is used as resource identifier, in this case the Log. The parameters I defined (follow and boot) are just modifiers or selectors what part of this resource should be returned. It is quite common to use GET query strings in RESTful APIs, e.g. often used for filtering.

agners avatar Nov 08 '21 10:11 agners

Reading up a bit about RESTful API design it seems that multiple URI's for the same resource as well as GET query parameters for filtering/paging are "allowed" and commonly used... Now the question is: is boot or follow a "filter" or merely a different URI for the same resource.

I'd say follow is more a filter type of thing, and boot warrants a separate URI. So I think this might be a good RESTful API design:

/host/logs/all/entries
/host/logs/{identifier}/entries
/host/logs/boot/list
/host/logs/boot/{bootid}/entries

All of them would support the follow query string parameter.

agners avatar Nov 08 '21 10:11 agners

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Jan 07 '22 11:01 github-actions[bot]

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

github-actions[bot] avatar Mar 08 '22 22:03 github-actions[bot]

The query to get boot IDs used in this PR is equivalent to running journalctl -o json MESSAGE_ID=b07a249cd024414a82dd00cd181378ff | jq '._BOOT_ID' on the host. A 100% accurate list of boot IDs for a given host can be found by doing journalcl --list-boots. We are not using this for two reasons:

  1. It was found to be very expensive memory and CPU wise, particularly on devices like rpi3s
  2. systemd-journal-gatewayd does not support it so we would need some other mechanism for running it on the host and getting the results

The query being used does not have these issues but isn't 100% accurate. In my testing it missed some older boots on some systems. It also would obviously miss any boot that ended before systemd finished starting up since that is when messages with that ID are logged.

I believe it is good enough for our use here. That being said, I would appreciate more evidence to back that up if anyone wants to run those two queries on a host and share the compare their results.

mdegat01 avatar Sep 28 '22 00:09 mdegat01

We need in a future PR:

  • Supported flag for journald socket evaluation

pvizeli avatar Oct 04 '22 09:10 pvizeli