-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
Reworking logging in mreg. #515
Conversation
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: - Logs are context aware, and context is tracked via a request_id. - Each new request (and other non-API-based context initiators) creates a new request_id. - Associated log entries are given the same request_id. - This request_id is also returned to the client via the header X-Request-ID. - Console output is colorized via Rich - The environment variable MREG_LOG_LEVEL controls the logging output. - To see an example in action, try `CI=True MREG_LOG_LEVEL=debug pytest mreg/api/v1/tests/test_hostgroups.py -vv -s`, or see https://gist.github.com/terjekv/8e97915c8b66c30241ff421a087e510d An example of the output can be seen here:
- Request_id is now added implicitly to all log messages, using a structlog processor. This alleviates the need for each log entry to manually insert it. - Add some visual touches to the logs to make them more readable on the console (only). This does not affect production logging.
- Remove local implementation of contextvars. - Add support for correlation_id to passed through the calls. - This change also removes the request_id for non-request-sourced events. - Events without request_id are colored with a black dot with colored output.
a8ba843
to
54c1253
Compare
Note that there is no review or filtering of the content returned from http requests, nor on the contents offered in POST data. |
- The log file gets raw json, no colors or sorting. - The console should be unchanged. - If the environment variable CI isn't set, the console gets raw JSON like the file. - The log file is rotated based on size (MREG_LOG_FILE_SIZE, defaults to 10MB) and a number of version are kept (MREG_LOG_FILE_COUNT, defaults to 5).
- File name can be set by MREG_LOG_FILE_NAME (may contain slashes) - Log directory is extrated from the filename, and created if needed.
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 |
- LOG_FILE_NAME now contains the full path, not just the issued file name.
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. |
When testing the latest changes locally, I get no log file at all. Tried both with and without setting + - 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 |
Weird. Does it work if you run it with pytest or similar directly, ie, outside of a container? |
Huh. It creates the logs folder, but not the file? That's really odd. |
Right, this seems to be a bind issue. If I do |
Good news, it turns out I had used the wrong path for the volume. It worked after I corrected it. |
At last it ensured I added some more checking and validation. :) |
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:
CI=True MREG_LOG_LEVEL=debug pytest mreg/api/v1/tests/test_hostgroups.py -vv -s
, or see https://gist.github.com/terjekv/8e97915c8b66c30241ff421a087e510dLogging to file is also supported:
An example of the output can be seen here:
#515 (comment)