A draft for making a decision on managing logging configurations #28603
Conversation
|
Thanks for the pull request, @ebberg! I've created OSPR-6006 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
openedx-webhooks
added
community manager review
open-source-contribution
openedx-webhooks
added
NEED-CLA
open-source-contribution
|
@ebberg Thank you for the contribution! Once you have signed our Contributor Agreement and it has been processed, you will receive a confirmation via email from our Legal team. We then will be able to review this and all your future code contributions. |
|
@ebberg is part of the MIT Open Learning team and should be covered by the MIT organizational agreement.
… Once you have signed our Contributor Agreement and it has been processed, you will receive a confirmation via email from our Legal team. We then will be able to review this and all your future code contributions.
|
|
@natabene sure thing, it's: berge@mit.edu |
|
@ebberg Could you please also tell me your first and last name, so I can add you to our contributors list? |
openedx-webhooks
added
awaiting prioritization
and removed
community manager review
labels
nedbat
changed the title
| Decision | ||
| ======== | ||
|
|
||
| Let us add an optional override dictionary to `get_logging_config`. This would |
There was a problem hiding this comment.
Are you talking about setting a param to https://github.com/edx/edx-platform/blob/ac8b4f5a6dfdc4ccc6433aeae60d42f6aa341207/openedx/core/lib/logsettings.py#L13 that would allow us to replace the entire logger config dictionary? Thus allowing any operator to fully manage how logging is done on their deployment of edx-platform?
There was a problem hiding this comment.
@ebberg checking back in on this. Trying to understand what piece you want to make overridable.
There was a problem hiding this comment.
The general idea is to be able to support customizing the logging configuration by passing in configuration values. The specifics of the mechanism are still subject to debate and design work. For the time being we have written a plugin to override the LOGGING setting but it might be worth exploring how to support some measure of modification without having to resort to a plugin.
There was a problem hiding this comment.
That makes sense, I agree that making the logging settings more configurable is valuable. I think we can go a bit further though and explore also making it easier to understand and explain.
It sounds like the current suggestion is to have more config settings that can be passed to the get_logger_config function that will allow for more flexibility?
| implementations is less. But the initial comment to the implementation is worth | ||
| reproducing in full here: | ||
|
|
||
| Return the appropriate logging config dictionary. You should assign the |
There was a problem hiding this comment.
I don't quite understand what this means. If say, common.py has set LOGGING and then we override that by pulling from a yaml file in production.py, what's the concern? What's the negative impact of resetting the LOGGING variable that I'm missing?
| Decision | ||
| ======== | ||
|
|
||
| Let us add an optional override dictionary to `get_logging_config`. This would |
There was a problem hiding this comment.
That makes sense, I agree that making the logging settings more configurable is valuable. I think we can go a bit further though and explore also making it easier to understand and explain.
It sounds like the current suggestion is to have more config settings that can be passed to the get_logger_config function that will allow for more flexibility?
|
@ebberg Happy 2022, I hope all is well. Just checking to see if you have time to implement feedback. |
|
ebberg is not longer with MIT and we've decided to close this ADR PR for now. We might revisit it later. |
|
@ebberg Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
|
@pdpinch Please make sure to offboard ebberg from TCRIL's list. |
Description
This is a draft for an ADR, that has implications on logging configuration for the edx platform, and would affect the kinds of additional logging management in applications. This is of specific interest for devops/sre edx platform work -- in other words, "Developer" and "Operator" roles.
Supporting information
I'm new to making changes on the EdX Platform, and my references are the following. Apologies in advance if I've left out anything important in this initial draft.
I'm referencing:
Testing instructions
Verifying this change is implemented backwards-compatible with existing usages of log configuration is essential. Also essential for ergonomics: reasonable errors for malformed logging overrides.
Deadline
"Soon-ish," as log ingestion and analysis for priority projects might get blocked on this, hindering maintenance efforts.
Other information
It might be useful for me or others to add specific use cases for this change, with the understanding that the list of potential use cases might change over time.
Thank you
Thanks for your time in reading, happy to try out contributions to open edx software, and look forward to any discussion and feedback on this ADR.