Skip to content

Commit

Permalink
Replace stack walking getLogger with explicit calls (#84480)
Browse files Browse the repository at this point in the history
Replace the no-args LogManager::getLogger calls with the single-arg
variant that accepts a j.l.Class reference, which avoids the stack walk
of the no-args variant. The no-args variant determines the caller's
class by looking at the stack frame two positions from itself. The use
of the 1-args variant is more explicit and avoids the need for the stack
walk, while retaining the very same behaviour. Standardizing on the
1-args variant will reduce the need to have different ways to retrieve
logger references.
  • Loading branch information
ChrisHegarty authored Mar 2, 2022
1 parent e209a10 commit ae8eeae
Show file tree
Hide file tree
Showing 29 changed files with 34 additions and 29 deletions.
6 changes: 4 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -482,9 +482,11 @@ that the logs fill up with noise and the useful signal is lost.

Elasticsearch uses Log4J for logging. In most cases you should log via a
`Logger` named after the class that is writing the log messages, which you can
do by declaring a static field of the class as follows:
do by declaring a static field of the class. For example:

private static final Logger logger = LogManager.getLogger();
class Foo {
private static final Logger logger = LogManager.getLogger(Foo.class);
}

In rare situations you may want to configure your `Logger` slightly
differently, perhaps specifying a different class or maybe using one of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@ org.apache.logging.log4j.Logger#error(java.lang.Object, java.lang.Throwable)
org.apache.logging.log4j.Logger#fatal(java.lang.Object)
org.apache.logging.log4j.Logger#fatal(java.lang.Object, java.lang.Throwable)

@defaultMessage Use getLogger(Class)
org.apache.logging.log4j.LogManager#getLogger()

# This is permitted in test code, where we have a Checkstyle rule to guard
# against unsafe uses. This leniency does not extend to server code.
java.lang.String#formatted(java.lang.Object[]) @ Uses default locale - use String#format(Locale, String, Object...) instead
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public final class ChannelActionListener<Response extends TransportResponse, Req
implements
ActionListener<Response> {

private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(ChannelActionListener.class);

private final TransportChannel channel;
private final Request request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
*/
public class HttpClientStatsTracker {

private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(HttpClientStatsTracker.class);

private final ThreadPool threadPool;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@
* all of the {@link Rounding.Prepared#fixedRoundingPoints() fixed rounding points}.
*/
class DateHistogramAggregator extends BucketsAggregator implements SizedBucketAggregator {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(DateHistogramAggregator.class);

/**
* Build an {@link Aggregator} for a {@code date_histogram} aggregation.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@
* Reads data in a time series style way.
*/
public class TimeSeriesMetrics {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(TimeSeriesMetrics.class);

private final int bucketBatchSize;
private final int docBatchSize;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ protected interface BlobStoreHttpHandler extends HttpHandler {
private static ExecutorService executorService;
protected Map<String, HttpHandler> handlers;

private static final Logger log = LogManager.getLogger();
private static final Logger log = LogManager.getLogger(ESMockAPIBasedRepositoryIntegTestCase.class);

@BeforeClass
public static void startHttpServer() throws Exception {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public class ExpressionModel {

public static final Predicate<FieldExpression.FieldValue> NULL_PREDICATE = field -> field.getValue() == null;

private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(ExpressionModel.class);

private final Map<String, Object> fieldValues;
private final Map<String, Predicate<FieldExpression.FieldValue>> fieldPredicates;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@

public class EqlParser {

private static final Logger log = LogManager.getLogger();
private static final Logger log = LogManager.getLogger(EqlParser.class);

private final boolean DEBUG = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public class IdentityProviderPlugin extends Plugin implements ActionPlugin {

private static final Setting<Boolean> ENABLED_SETTING = Setting.boolSetting("xpack.idp.enabled", false, Setting.Property.NodeScope);

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(IdentityProviderPlugin.class);
private boolean enabled;
private Settings settings;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public class TransportDeleteSamlServiceProviderAction extends HandledTransportAc
DeleteSamlServiceProviderRequest,
DeleteSamlServiceProviderResponse> {

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(TransportDeleteSamlServiceProviderAction.class);
private final SamlServiceProviderIndex index;

@Inject
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class TransportPutSamlServiceProviderAction extends HandledTransportActio
PutSamlServiceProviderRequest,
PutSamlServiceProviderResponse> {

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(TransportPutSamlServiceProviderAction.class);
private final SamlServiceProviderIndex index;
private final SamlIdentityProvider identityProvider;
private final Clock clock;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public class ApplicationActionsResolver extends AbstractLifecycleComponent {
Setting.Property.NodeScope
);

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(ApplicationActionsResolver.class);

private final ServiceProviderDefaults defaults;
private final Client client;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public static UserPrivileges noAccess(String principal) {
}
}

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(UserPrivilegeResolver.class);
private final Client client;
private final SecurityContext securityContext;
private final ApplicationActionsResolver actionsResolver;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
*/
public class SuccessfulAuthenticationResponseMessageBuilder {

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(SuccessfulAuthenticationResponseMessageBuilder.class);

private final Clock clock;
private final SamlIdentityProvider idp;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
*/
public class SamlIdentityProvider {

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(SamlIdentityProvider.class);

private final String entityId;
private final Map<String, URL> ssoEndpoints;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
*/
public class SamlServiceProviderIndex implements Closeable {

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(SamlServiceProviderIndex.class);

private final Client client;
private final ClusterService clusterService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ private State(Map<String, WildcardServiceProvider> services) {
}
}

private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(WildcardServiceProviderResolver.class);

private final Settings settings;
private final ScriptService scriptService;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
public final class SamlInit {

private static final AtomicBoolean INITIALISED = new AtomicBoolean(false);
private static final Logger LOGGER = LogManager.getLogger();
private static final Logger LOGGER = LogManager.getLogger(SamlInit.class);

private SamlInit() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class SecondaryAuthenticator {
*/
public static final String SECONDARY_AUTH_HEADER_NAME = "es-secondary-authorization";

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(SecondaryAuthenticator.class);
private final SecurityContext securityContext;
private final AuthenticationService authenticationService;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public final class SetSecurityUserProcessor extends AbstractProcessor {

public static final String TYPE = "set_security_user";

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(SetSecurityUserProcessor.class);

private final SecurityContext securityContext;
private final Settings settings;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import org.elasticsearch.xpack.security.rest.action.SecurityBaseRestHandler;

public abstract class OpenIdConnectBaseRestHandler extends SecurityBaseRestHandler {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(OpenIdConnectBaseRestHandler.class);

private static final String OIDC_REALM_TYPE = OpenIdConnectRealmSettings.TYPE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
* Rest handler that authenticates the user based on the information provided as parameters of the redirect_uri
*/
public class RestOpenIdConnectAuthenticateAction extends OpenIdConnectBaseRestHandler implements RestRequestFilter {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(RestOpenIdConnectAuthenticateAction.class);

static final ObjectParser<OpenIdConnectAuthenticateRequest, Void> PARSER = new ObjectParser<>(
"oidc_authn",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
* Generates an oAuth 2.0 authentication request as a URL string and returns it to the REST client.
*/
public class RestOpenIdConnectPrepareAuthenticationAction extends OpenIdConnectBaseRestHandler {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(RestOpenIdConnectPrepareAuthenticationAction.class);

static final ObjectParser<OpenIdConnectPrepareAuthenticationRequest, Void> PARSER = new ObjectParser<>(
"oidc_prepare_authentication",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
* A REST handler that attempts to authenticate a user based on the provided SAML response/assertion.
*/
public class RestSamlAuthenticateAction extends SamlBaseRestHandler implements RestRequestFilter {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(RestSamlAuthenticateAction.class);

static class Input {
String content;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
* An abstract implementation of {@link SecurityBaseRestHandler} that performs a license check for the SAML realm type
*/
public abstract class SamlBaseRestHandler extends SecurityBaseRestHandler {
private static final Logger logger = LogManager.getLogger();
private static final Logger logger = LogManager.getLogger(SamlBaseRestHandler.class);

private static final String SAML_REALM_TYPE = SamlRealmSettings.TYPE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public class SecuritySystemIndices {
public static final String INTERNAL_SECURITY_PROFILE_INDEX_8 = ".security-profile-8";
public static final String SECURITY_PROFILE_ALIAS = ".security-profile";

private final Logger logger = LogManager.getLogger();
private final Logger logger = LogManager.getLogger(SecuritySystemIndices.class);

private final SystemIndexDescriptor mainDescriptor;
private final SystemIndexDescriptor tokenDescriptor;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@

public class SqlParser {

private static final Logger log = LogManager.getLogger();
private static final Logger log = LogManager.getLogger(SqlParser.class);

private final boolean DEBUG = false;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public NotificationService(
// Used for testing only
NotificationService(String type, Settings settings, List<Setting<?>> pluginSecureSettings) {
this.type = type;
this.logger = LogManager.getLogger();
this.logger = LogManager.getLogger(NotificationService.class);
this.bootSettings = settings;
this.pluginSecureSettings = pluginSecureSettings;
}
Expand Down

0 comments on commit ae8eeae

Please sign in to comment.