Skip to content

Commit

Permalink
Reduce Log4J API Exposures (hyperledger#5189)
Browse files Browse the repository at this point in the history
Reduce the number of places that expose Log4J classes as a part of the
interfaces for methods and classes. While Log4j remains the default we
still need to be able to function when the Log4J jars are removed from
the classpath.

Signed-off-by: Danno Ferrin <danno.ferrin@swirldslabs.com>
  • Loading branch information
shemnon authored and eum602 committed Nov 3, 2023
1 parent 0456978 commit 6edbf35
Show file tree
Hide file tree
Showing 22 changed files with 151 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import org.hyperledger.besu.tests.acceptance.dsl.privacy.PrivacyNode;
import org.hyperledger.besu.tests.acceptance.dsl.privacy.account.PrivacyAccountResolver;
import org.hyperledger.besu.tests.web3j.generated.EventEmitter;
import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;
import org.hyperledger.enclave.testutil.EnclaveEncryptorType;
import org.hyperledger.enclave.testutil.EnclaveType;

Expand All @@ -34,7 +34,6 @@
import java.util.Collection;
import java.util.Optional;

import org.apache.logging.log4j.Level;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
Expand Down Expand Up @@ -99,7 +98,7 @@ public PrivacyGroupAcceptanceTest(

@Test
public void nodeCanCreatePrivacyGroup() {
Log4j2ConfiguratorUtil.setLevel("", Level.DEBUG);
LogConfigurator.setLevel("", "DEBUG");
final String privacyGroupId =
alice.execute(
privacyTransactions.createPrivacyGroup(
Expand Down
18 changes: 13 additions & 5 deletions besu/src/main/java/org/hyperledger/besu/Besu.java
Original file line number Diff line number Diff line change
Expand Up @@ -62,18 +62,26 @@ public static void main(final String... args) {
}

private static Logger setupLogging() {
InternalLoggerFactory.setDefaultFactory(Log4J2LoggerFactory.INSTANCE);
try {
// This call is to test if log4j classes are available
((Log4J2LoggerFactory) Log4J2LoggerFactory.INSTANCE).newInstance("");
InternalLoggerFactory.setDefaultFactory(Log4J2LoggerFactory.INSTANCE);
} catch (Throwable t) {
System.out.printf(
"Could not set netty log4j logger factory: %s - %s%n",
t.getClass().getSimpleName(), t.getMessage());
}
try {
System.setProperty(
"vertx.logger-delegate-factory-class-name",
"io.vertx.core.logging.Log4j2LogDelegateFactory");
System.setProperty(
"log4j.configurationFactory", BesuLoggingConfigurationFactory.class.getName());
System.setProperty("log4j.skipJansi", String.valueOf(false));
} catch (SecurityException e) {
System.out.println(
"Could not set logging system property as the security manager prevented it:"
+ e.getMessage());
} catch (Throwable t) {
System.out.printf(
"Could not set logging system property: %s - %s%n",
t.getClass().getSimpleName(), t.getMessage());
}
final Logger logger = LoggerFactory.getLogger(Besu.class);
Thread.setDefaultUncaughtExceptionHandler(slf4jExceptionHandler(logger));
Expand Down
18 changes: 7 additions & 11 deletions besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@
import org.hyperledger.besu.services.StorageServiceImpl;
import org.hyperledger.besu.services.kvstore.InMemoryStoragePlugin;
import org.hyperledger.besu.util.InvalidConfigurationException;
import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;
import org.hyperledger.besu.util.NetworkUtility;
import org.hyperledger.besu.util.PermissioningConfigurationValidator;
import org.hyperledger.besu.util.number.Fraction;
Expand Down Expand Up @@ -240,7 +240,6 @@
import net.consensys.quorum.mainnet.launcher.config.ImmutableLauncherConfig;
import net.consensys.quorum.mainnet.launcher.exception.LauncherException;
import net.consensys.quorum.mainnet.launcher.util.ParseArgsHelper;
import org.apache.logging.log4j.Level;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.units.bigints.UInt256;
import org.slf4j.Logger;
Expand Down Expand Up @@ -1440,7 +1439,6 @@ protected BesuCommand(
* @param args arguments to Besu command
* @return success or failure exit code.
*/
@VisibleForTesting
public int parse(
final IExecutionStrategy resultHandler,
final BesuParameterExceptionHandler parameterExceptionHandler,
Expand Down Expand Up @@ -1553,7 +1551,6 @@ private void addSubCommands(final InputStream in) {
private void registerConverters() {
commandLine.registerConverter(Address.class, Address::fromHexStringStrict);
commandLine.registerConverter(Bytes.class, Bytes::fromHexString);
commandLine.registerConverter(Level.class, Level::valueOf);
commandLine.registerConverter(MetricsProtocol.class, MetricsProtocol::fromString);
commandLine.registerConverter(UInt256.class, (arg) -> UInt256.valueOf(new BigInteger(arg)));
commandLine.registerConverter(Wei.class, (arg) -> Wei.of(Long.parseUnsignedLong(arg)));
Expand Down Expand Up @@ -1796,14 +1793,14 @@ private void setReleaseMetrics() {
*/
public void configureLogging(final boolean announce) {
// To change the configuration if color was enabled/disabled
Log4j2ConfiguratorUtil.reconfigure();
LogConfigurator.reconfigure();
// set log level per CLI flags
final Level logLevel = loggingLevelOption.getLogLevel();
final String logLevel = loggingLevelOption.getLogLevel();
if (logLevel != null) {
if (announce) {
System.out.println("Setting logging level to " + logLevel.name());
System.out.println("Setting logging level to " + logLevel);
}
Log4j2ConfiguratorUtil.setAllLevels("", logLevel);
LogConfigurator.setLevel("", logLevel);
}
}

Expand Down Expand Up @@ -3109,7 +3106,7 @@ private void addShutdownHook(final Runner runner) {
try {
besuPluginContext.stopPlugins();
runner.close();
Log4j2ConfiguratorUtil.shutdown();
LogConfigurator.shutdown();
} catch (final Exception e) {
logger.error("Failed to stop Besu");
}
Expand Down Expand Up @@ -3306,7 +3303,6 @@ private List<EnodeURL> buildEnodes(
*
* @return instance of BesuParameterExceptionHandler
*/
@VisibleForTesting
public BesuParameterExceptionHandler parameterExceptionHandler() {
return new BesuParameterExceptionHandler(this::getLogLevel);
}
Expand Down Expand Up @@ -3454,7 +3450,7 @@ private static boolean ensureGoQuorumCompatibilityModeNotUsedOnMainnet(
}

@VisibleForTesting
Level getLogLevel() {
String getLogLevel() {
return loggingLevelOption.getLogLevel();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,30 @@
import java.io.PrintWriter;
import java.util.function.Supplier;

import org.apache.logging.log4j.Level;
import picocli.CommandLine;
import picocli.CommandLine.Model.CommandSpec;

/** The custom parameter exception handler for Besu PicoCLI. */
public class BesuParameterExceptionHandler implements CommandLine.IParameterExceptionHandler {

private final Supplier<Level> levelSupplier;
private final Supplier<String> levelSupplier;

/**
* Instantiates a new Besu parameter exception handler.
*
* @param levelSupplier the logging level supplier
*/
public BesuParameterExceptionHandler(final Supplier<Level> levelSupplier) {
public BesuParameterExceptionHandler(final Supplier<String> levelSupplier) {
this.levelSupplier = levelSupplier;
}

@Override
public int handleParseException(final CommandLine.ParameterException ex, final String[] args) {
final CommandLine cmd = ex.getCommandLine();
final PrintWriter err = cmd.getErr();
final Level logLevel = levelSupplier.get();
if (logLevel != null && Level.DEBUG.isMoreSpecificThan(logLevel)) {
final String logLevel = levelSupplier.get();
if (logLevel != null
&& (logLevel.equals("DEBUG") || logLevel.equals("TRACE") || logLevel.equals("ALL"))) {
ex.printStackTrace(err);
} else {
err.println(ex.getMessage());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import java.util.Set;

import org.apache.logging.log4j.Level;
import picocli.CommandLine;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Spec;
Expand All @@ -38,7 +37,7 @@ public static LoggingLevelOption create() {
/** The Picocli CommandSpec. Visible for testing. Injected by Picocli framework at runtime. */
@Spec CommandSpec spec;

private Level logLevel;
private String logLevel;

/**
* Sets log level.
Expand All @@ -52,9 +51,9 @@ public static LoggingLevelOption create() {
public void setLogLevel(final String logLevel) {
if ("FATAL".equalsIgnoreCase(logLevel)) {
System.out.println("FATAL level is deprecated");
this.logLevel = Level.ERROR;
this.logLevel = "ERROR";
} else if (ACCEPTED_VALUES.contains(logLevel.toUpperCase())) {
this.logLevel = Level.getLevel(logLevel.toUpperCase());
this.logLevel = logLevel.toUpperCase();
} else {
throw new CommandLine.ParameterException(
spec.commandLine(), "Unknown logging value: " + logLevel);
Expand All @@ -66,7 +65,7 @@ public void setLogLevel(final String logLevel) {
*
* @return the log level
*/
public Level getLogLevel() {
public String getLogLevel() {
return logLevel;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,11 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.JsonRpcConfiguration;
import org.hyperledger.besu.ethereum.retesteth.RetestethConfiguration;
import org.hyperledger.besu.ethereum.retesteth.RetestethService;
import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;

import java.net.InetAddress;
import java.nio.file.Path;

import org.apache.logging.log4j.Level;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import picocli.CommandLine.Command;
Expand Down Expand Up @@ -106,10 +105,10 @@ private InetAddress autoDiscoverDefaultIP() {

private void prepareLogging() {
// set log level per CLI flags
final Level logLevel = loggingLevelOption.getLogLevel();
final String logLevel = loggingLevelOption.getLogLevel();
if (logLevel != null) {
System.out.println("Setting logging level to " + logLevel.name());
Log4j2ConfiguratorUtil.setAllLevels("", logLevel);
System.out.println("Setting logging level to " + logLevel);
LogConfigurator.setLevel("", logLevel);
}
}

Expand All @@ -132,7 +131,7 @@ public void run() {
() -> {
try {
retestethService.close();
Log4j2ConfiguratorUtil.shutdown();
LogConfigurator.shutdown();
} catch (final Exception e) {
LOG.error("Failed to stop Besu Retesteth");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
import io.vertx.core.json.JsonObject;
import org.apache.commons.io.FileUtils;
import org.apache.commons.text.StringEscapeUtils;
import org.apache.logging.log4j.Level;
import org.apache.tuweni.bytes.Bytes;
import org.apache.tuweni.toml.Toml;
import org.apache.tuweni.toml.TomlParseResult;
Expand Down Expand Up @@ -4979,7 +4978,7 @@ public void logLevelHasNullAsDefaultValue() {
public void logLevelIsSetByLoggingOption() {
final TestBesuCommand command = parseCommand("--logging", "WARN");

assertThat(command.getLogLevel()).isEqualTo(Level.WARN);
assertThat(command.getLogLevel()).isEqualTo("WARN");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ public void setUp() {
@Test
public void fatalLevelEqualsToError() {
levelOption.setLogLevel("fatal");
assertThat(levelOption.getLogLevel()).isEqualTo(Level.ERROR);
assertThat(levelOption.getLogLevel()).isEqualTo("ERROR");
}

@Test
Expand All @@ -49,7 +49,7 @@ public void setsExpectedLevels() {
.forEach(
level -> {
levelOption.setLogLevel(level.name());
assertThat(levelOption.getLogLevel()).isEqualTo(level);
assertThat(levelOption.getLogLevel()).isEqualTo(level.name());
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
import org.hyperledger.besu.ethereum.mainnet.feemarket.BaseFeeMarket;
import org.hyperledger.besu.ethereum.mainnet.feemarket.LondonFeeMarket;
import org.hyperledger.besu.ethereum.worldstate.WorldStateArchive;
import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;

import java.util.ArrayList;
import java.util.List;
Expand Down Expand Up @@ -109,7 +109,7 @@ public void setUp() {
@Test
public void reorgsAcrossTDDToDifferentTargetsWhenNotFinal() {
// Add N blocks to chain from genesis, where total diff is < TTD
Log4j2ConfiguratorUtil.setLevelDebug(BlockHeaderValidator.class.getName());
LogConfigurator.setLevel(BlockHeaderValidator.class.getName(), "DEBUG");
List<Block> endOfWork = subChain(genesisState.getBlock().getHeader(), 10, Difficulty.of(100L));
endOfWork.stream().forEach(this::appendBlock);
assertThat(blockchain.getChainHead().getHeight()).isEqualTo(10L);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,12 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;

import java.util.Arrays;
import java.util.Optional;
import java.util.Set;

import org.apache.logging.log4j.Level;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -45,12 +44,11 @@ public String getName() {
@Override
public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
try {
final String rawLogLevel = requestContext.getRequiredParameter(0, String.class);
if (!VALID_PARAMS.contains(rawLogLevel)) {
final String logLevel = requestContext.getRequiredParameter(0, String.class);
if (!VALID_PARAMS.contains(logLevel)) {
return new JsonRpcErrorResponse(
requestContext.getRequest().getId(), JsonRpcError.INVALID_PARAMS);
}
final Level logLevel = Level.toLevel(rawLogLevel);
final Optional<String[]> optionalLogFilters =
requestContext.getOptionalParameter(1, String[].class);
optionalLogFilters.ifPresentOrElse(
Expand All @@ -64,8 +62,8 @@ public JsonRpcResponse response(final JsonRpcRequestContext requestContext) {
}
}

private void setLogLevel(final String logFilter, final Level logLevel) {
LOG.debug("Setting {} logging level to {} ", logFilter, logLevel.name());
Log4j2ConfiguratorUtil.setAllLevels(logFilter, logLevel);
private void setLogLevel(final String logFilter, final String logLevel) {
LOG.debug("Setting {} logging level to {} ", logFilter, logLevel);
LogConfigurator.setLevel(logFilter, logLevel);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcErrorResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcResponse;
import org.hyperledger.besu.ethereum.api.jsonrpc.internal.response.JsonRpcSuccessResponse;
import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;

import org.apache.logging.log4j.Level;
import org.junit.Before;
Expand All @@ -40,7 +40,7 @@ public class AdminChangeLogLevelTest {
@Before
public void before() {
adminChangeLogLevel = new AdminChangeLogLevel();
Log4j2ConfiguratorUtil.setAllLevels("", Level.INFO);
LogConfigurator.setLevel("", "INFO");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
*/
package org.hyperledger.besu.ethereum.vm;

import org.hyperledger.besu.util.Log4j2ConfiguratorUtil;
import org.hyperledger.besu.util.LogConfigurator;

import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -61,7 +61,7 @@ public void execution() {
} catch (final RuntimeException | AssertionError e) {
if (!"trace".equalsIgnoreCase(originalRootLogLevel)
|| !"trace".equalsIgnoreCase(originalEvmLogLevel)) {
// try again, this time with more logging so we can capture more information.
// try again, this time with more logging, so we can capture more information.
System.setProperty("root.log.level", "trace");
System.setProperty("evm.log.level", "trace");
resetLogging();
Expand All @@ -73,7 +73,7 @@ public void execution() {
}

private void resetLogging() {
Log4j2ConfiguratorUtil.reconfigure();
LogConfigurator.reconfigure();
}

/** Subclasses should implement this method to run the actual JUnit test. */
Expand Down
Loading

0 comments on commit 6edbf35

Please sign in to comment.