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

Make log level of slf4j logger configurable #3451

Closed
iwasakims opened this issue Sep 13, 2023 · 2 comments · Fixed by #3463
Closed

Make log level of slf4j logger configurable #3451

iwasakims opened this issue Sep 13, 2023 · 2 comments · Fixed by #3463
Labels
feature_request New feature request, awaiting triage triage all new issues awaiting classification

Comments

@iwasakims
Copy link
Contributor

iwasakims commented Sep 13, 2023

Feature Request

Which Areas Would Be Affected?

boot, monitor

Why Is the Feature Desired?

We can not get log of underlying libraries using slf4j if the log level is equal to or smaller than INFO since the log level of slf4j logger is hard coded.

It is inconvenient for developing modules depending on libraries using slf4j. Technology-Aws depending on AWS SDK is the example.

We can not temporalily use another binding like slf4j-simple if MonitorProvider is in the classpath. Since the MonitorProvider is provided by org.eclipse.edc:boot, most of the code depending on EDC could be affected.

S3DataPlaneIntegrationTest STANDARD_ERROR
    SLF4J: Class path contains multiple SLF4J providers.
    SLF4J: Found provider [org.eclipse.edc.boot.monitor.MonitorProvider@47c64cfe]
    SLF4J: Found provider [org.slf4j.simple.SimpleServiceProvider@6ce90bc5]
    SLF4J: See https://www.slf4j.org/codes.html#multiple_bindings for an explanation.
    SLF4J: Actual provider is of type [org.eclipse.edc.boot.monitor.MonitorProvider@47c64cfe]
OpenJDK 64-Bit Server VM warning: Sharing is only supported for boot loader classes because bootstrap classpath has been appended

Solution Proposal

Making the log level of slf4j logger configurable as done for slf4j-simple.

Making the feature opt-in could be an option. If the feature is enabled by loading the extension, EDC Config is available on initialize. Modules not loading BaseRuntime (e.g. Technology-AWS) can use alternative slf4j binding during development.

@iwasakims iwasakims added feature_request New feature request, awaiting triage triage all new issues awaiting classification labels Sep 13, 2023
@paullatzelsperger
Copy link
Member

AFAIR the MonitorProvider was added a long time ago to satisfy some transitive dependency. Do we even need it anymore or can we remove it?
That would mean that people can just register their downstream implementation of a Monitor, e.g. an Slf4jMonitor.

@iwasakims
Copy link
Contributor Author

I submitted #3463 which removes the MonitorProvider from the code path of BaseRuntime. The SLF4J-Monitor bridge feature still could be useful for users of existing monitor like LoggerMonitorExtension to make log destination settings simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature_request New feature request, awaiting triage triage all new issues awaiting classification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants