[jira] [Commented] (YUNIKORN-75) REST API for changing log level

["Adam Antal (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/YUNIKORN-75?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17078218#comment-17078218 ] 

Adam Antal commented on YUNIKORN-75:
------------------------------------

Let's move the design discussion here. I'll give as much detail as I could.

So as much as I understood the k8shim and the core should work together, but they should also work independently.
 - If k8shim is running independently
 -- Creates logger configuration, builds the logger, stores its pointer in the package so that it can be used elsewhere in yunikorn-k8shim and calls the {{zap.ReplaceGlobals}} so it will be the global zap Logger (source: [log/logger.go|https://github.com/apache/incubator-yunikorn-k8shim/blob/dbd74df39403f53b00415210f3410395fb2ad04b/pkg/log/logger.go#L33])
 - If core is running independently
 -- It first checks whether there's a global not no-op zap logger created - if there is, the program saves its pointer for yunikorn-core. Otherwise creates a logger configuration, builds the {{Logger}} object and finally stores its pointer in the package so that it can be used elsewhere in yunikorn-core (source: [log/logger.go|https://github.com/apache/incubator-yunikorn-core/blob/master/pkg/log/logger.go#L34]). Note that there's no {{ReplaceGlobals}} call here.
- If they work together:
 -- As k8shim creates the logger, sets as global logger, and yunikorn-core picks it up.

As far as I could understand, the goal would be to have control over the log level of the *global* logger. That being said, when both yunikorn-core and shim uses the same logger, we want to control the log level of that object. Correct me, if that is not the case.

To control the log level dynamically the {{zap.AtomicLevel}} object should be used for this purpose. The problem is that there's no way to obtain this part of an existing logger object, so you either:
- Store the {{AtomicLevel}} object of the configuration, if the Logger is created from configuration
- Wrap the zap Logger core object and bound it to an AtomicLevel class (current implementation)
The first option is preferred here to work with both scenarios. The problem is how to share this {{AtomicLevel}} object between the components. Though {{ReplaceGlobals}} works fine when it comes to sharing/setting global Logger, but there's no such utility in zap.

My proposal:
- In the k8shim & core case:
 -- Create the {{AtomicLevel}} in the k8shim. After the logger has been created and set as global logger, we should set the {{AtomicLevel}} in the yunikorn-core. This should be done by calling a function from that package (log/logger.go from yunikorn-core), so we introduce a package dependency there - I don't know if that's a concern.
 -- The core REST API will use the AtomicLevel which affects the global logger used by both components
- If only the core is used:
 -- In the current implementation we create a logger there, so we can create an {{AtomicLevel}} additionally and set it to global.
- If only the k8shim is used:
 -- I don't cover this part in this patch. If k8shim does not expose a REST API, it doesn't make sense to store the {{AtomicLevel}} object on k8shim side.

I'll also handle the suggestions from the review given by Sunil, when we agreed on the implementation.

Note that this proposal would comprise of two parts (in yunikorn-k8shim and yunikorn-core) that should be merged at the same time to the repositories.

Could you explain your thoughts on this [~wwei], [~wilfreds], [~sunilg]?

> REST API for changing log level
> -------------------------------
>
>                 Key: YUNIKORN-75
>                 URL: https://issues.apache.org/jira/browse/YUNIKORN-75
>             Project: Apache YuniKorn
>          Issue Type: Improvement
>          Components: core - scheduler
>            Reporter: Adam Antal
>            Assignee: Adam Antal
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 10m
>  Remaining Estimate: 0h
>
> We need a rest API to change the logging level, so we can change the logging level on-the-fly. It can be used to debug in production issues where for most of the time, only INFO level log is needed.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: issues-help@yunikorn.apache.org