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.

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

KP-8688 Add a syslog handler #11

Merged
merged 6 commits into from
Jan 22, 2025
Merged

Conversation

Traubert
Copy link

(Keep old file-logging as well for now for testing purposes)

(Keep old file-logging as well for now for testing purposes)
Copy link
Collaborator

@mmatthiesencsc mmatthiesencsc left a comment

Choose a reason for hiding this comment

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

The change looks sensible,

Copy link
Collaborator

@mmatthiesencsc mmatthiesencsc left a comment

Choose a reason for hiding this comment

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

So is this preparing the general logger for syslog and not using it as default?

@Traubert Traubert requested a review from janiemi January 21, 2025 07:00
@Traubert
Copy link
Author

@janiemi, I sent a review request, mostly just to let you know that we're trying to make logging from multiple threads more failsafe by using rsyslog on our servers to do the logging. This will also make it easier (hopefully) to direct logs to an external logging service one day. This, and the authserver PR, are for that.

@Traubert Traubert force-pushed the KP-8688-improve-logging branch from 76d8362 to bc4f7d7 Compare January 21, 2025 07:14
@janiemi
Copy link

janiemi commented Jan 21, 2025

I think it’s great if it helps logging from parallel processes avoid messing up the log.

However, does writing to syslog mean writing to the same file as other types of processes using the syslog facility, or would Korp still have a log file of its own? If the former, I’m wondering if the current logging format is too verbose for that.

By the way, to simplify things and to avoid duplication, I have removed support for the plugin-specific config.py configurations from the development version of the plugins and the plugins library that intends to be compatible with the current version of Språkbanken’s Korp backend. pluginconf in __init.py__ will thus be the place for configuration variables and their default values.

(I’m sorry I still haven’t cleaned up the Git branches.)

@Traubert
Copy link
Author

However, does writing to syslog mean writing to the same file as other types of processes using the syslog facility, or would Korp still have a log file of its own? If the former, I’m wondering if the current logging format is too verbose for that.

There's also configuration for syslog to handle the messages in an appropriate way, so they should end up in eg. /var/log/korp/, or even where they always were, if we want. They do currently also go to /var/log/messages, but I think I can get them prevent that with more configuration.

By the way, to simplify things and to avoid duplication, I have removed support for the plugin-specific config.py configurations from the development version of the plugins and the plugins library that intends to be compatible with the current version of Språkbanken’s Korp backend. pluginconf in __init.py__ will thus be the place for configuration variables and their default values.

Ah, ok, good to know. So the way to have our own site-specific configuration will be to have a branch just for that with edits to __init.py__, or make some alterations to it on deployment, or something?

@janiemi
Copy link

janiemi commented Jan 21, 2025

Good that you can configure where the log info is written to. I’m then ready to approve these changes. (I admit that I’m not familiar with the syslog facility.)

Ah, ok, good to know. So the way to have our own site-specific configuration will be to have a branch just for that with edits to __init.py__, or make some alterations to it on deployment, or something?

Definitely not: the plugin configurations are also defined in Korp’s config.py (or typically, instance/config.py). Please see the documentation for some more details.

Even though it might be more modular to have separate config.py files for each plugin, I began to think that it would be also be more complicated and that it might not always be obvious where a configuration value comes from, so I decided to remove support for plugin-specific configuration files.

@Traubert Traubert changed the title WIP KP-8688 Add a syslog handler KP-8688 Add a syslog handler Jan 22, 2025
@Traubert Traubert merged commit 96fa486 into plugins/master Jan 22, 2025
1 check passed
@Traubert Traubert deleted the KP-8688-improve-logging branch January 22, 2025 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants