unioslo / mreg

GNU General Public License v3.0
7 stars 13 forks source link

Reworking logging in mreg. #515

Closed terjekv closed 11 months ago

terjekv commented 1 year ago

This change migrates the entire logging backend to structlog (https://www.structlog.org/en/stable/) and implements an internal framework to offer the following funtionality:

Logging to file is also supported:

An example of the output can be seen here: https://github.com/unioslo/mreg/pull/515#issuecomment-1647920124

coveralls commented 1 year ago

Coverage Status

coverage: 99.185% (-0.06%) from 99.243% when pulling 238d5fef877d29e789efee4d3798c9a64061e9d9 on terjekv:logging_reworked into 1920fdcbc8a67e2c7886dced4d43a4e249e5dbea on unioslo:master.

terjekv commented 1 year ago
Screenshot 2023-07-24 at 23 43 11
terjekv commented 11 months ago

Note that there is no review or filtering of the content returned from http requests, nor on the contents offered in POST data.

oyvindhagberg commented 11 months ago

I tested the container image from the artifact in https://github.com/unioslo/mreg/actions/runs/5961486496. I got it to log something to the console after setting MREG_LOG_LEVEL=INFO. It also created logs/app.log, but the log file remained empty... Any ideas?

terjekv commented 11 months ago

I'm willing to bet that the path to the file is broken. Fixed a bug and added some hard validation of the writability of the log file.

oyvindhagberg commented 11 months ago

When testing the latest changes locally, I get no log file at all. Tried both with and without setting CI. I'm running docker compose up with the following changes to docker-compose.yml:

+      - MREG_LOG_LEVEL=INFO
+      #- CI=yes
+    volumes:
+      - /mnt/oyvind/mreg/mregsite:/app/mreg/mregsite
+      - /mnt/oyvind/mreg/logs:/app/mreg/logs

I test by first running manage.py create_mreg_superuser inside the container, then using mreg-cli to authenticate and run host add something -f.

terjekv commented 11 months ago

Weird. Does it work if you run it with pytest or similar directly, ie, outside of a container?

terjekv commented 11 months ago

Huh. It creates the logs folder, but not the file? That's really odd.

terjekv commented 11 months ago

Right, this seems to be a bind issue. If I do docker exec -it /bin/sh mreg-mreg and cat /app/logs/app.log I see data in the log file.

oyvindhagberg commented 11 months ago

Good news, it turns out I had used the wrong path for the volume. It worked after I corrected it.

terjekv commented 11 months ago

At last it ensured I added some more checking and validation. :)