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

Makes SubstituteLogger location aware #438

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ppkarwasz
Copy link
Contributor

Currently, SubstituteLogger neither supports location information from upstream callers nor passes location information to the delegated logger.

This PR:

  • implements LocationAwareLogger, which allows upstream callers to pass location information both through the LocationAwareLogger API and the fluent API.
  • sets the correct caller boundary, when the classic API methods are invoked.

This is related to #425.

Currently, `SubstituteLogger` neither supports location information from upstream callers nor passes location information to the delegated logger.

This PR:

- implements `LocationAwareLogger`, which allows upstream callers to pass location information both through the `LocationAwareLogger` API and the fluent API.
- sets the correct caller boundary, when the classic API methods are invoked.

Signed-off-by: Piotr P. Karwasz <[email protected]>
@ppkarwasz ppkarwasz force-pushed the fix/location-aware-substitute branch from 58ad526 to 679bc22 Compare October 23, 2024 07:17
@ceki
Copy link
Member

ceki commented Oct 23, 2024

@ppkarwasz Thank you for this PR.

For the moment, I fail to see the circumstances under which the LocationAwareLogger.log() method may be called. Still investigating though.

@ppkarwasz
Copy link
Contributor Author

For the moment, I fail to see the circumstances under which the LocationAwareLogger.log() method may be called. Still investigating though.

I am not entirely sure when SubstituteLogger is used (or if it is used at all these days), but I assume that it could appear in OSGi environments if:

  1. The SLF4J bundle is activated first (or used by another bundle without being activated).
  2. The Logback bundle is activated only later.

Since LocationAwareLogger is the oldest available mechanism to transfer caller boundary information, it is commonly used in API-to-API bridges, e.g. the Log4j API-to-SLF4J bridge or Spring JCL-to-SLF4J bridge.

Newer mechanisms such as LoggingEventAware (for logger implementations) or CallerBoundaryAware (for logger builder implementations) are less common. The default LoggingEventBuilder also requires the underlying logger to either implement LocationAwareLogger or LoggingEventAware:

protected void log(LoggingEvent aLoggingEvent) {
if(aLoggingEvent.getCallerBoundary() == null) {
setCallerBoundary(DLEB_FQCN);
}
if(logger instanceof LoggingEventAware) {
((LoggingEventAware) logger).log(aLoggingEvent);
} else if(logger instanceof LocationAwareLogger) {
logViaLocationAwareLoggerAPI((LocationAwareLogger) logger, aLoggingEvent);
} else {
logViaPublicSLF4JLoggerAPI(aLoggingEvent);
}
}

ppkarwasz added a commit to ppkarwasz/slf4j that referenced this pull request Oct 28, 2024
The `JDK14LoggerAdapter` class uses the wrong caller boundary for location unaware methods.

This PR:

- Sets the correct caller boundary for classic API method calls (`AbstractLogger`),
- Adds the missing `LoggingEventAware` interface and fixes its implementation. The interface was previously implemented, but not declared.
- Adds a test for qos-ch#425.

The `SubstituteLogger` tests depend on qos-ch#438.
ppkarwasz added a commit to ppkarwasz/slf4j that referenced this pull request Oct 28, 2024
The `JDK14LoggerAdapter` class uses the wrong caller boundary for location unaware methods.

This PR:

- Sets the correct caller boundary for classic API method calls (`AbstractLogger`),
- Adds the missing `LoggingEventAware` interface and fixes its implementation. The interface was previously implemented, but not declared.
- Adds a test for qos-ch#425.

The `SubstituteLogger` tests depend on qos-ch#438.

Signed-off-by: Piotr P. Karwasz <[email protected]>
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.

2 participants