Skip to content

Commit

Permalink
Fix how the CLI logs
Browse files Browse the repository at this point in the history 
This commit updates the CLI to use a nicer log message format, and to
also properly configure JUL when a logging level is given. We need to
later make changes to the Gradle plugin to enable configuring the CLI
loggers.
  • Loading branch information
@mtdowling
mtdowling committed Sep 15, 2021
1 parent 3b6052b commit a681a0a
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 97 deletions.
117 changes: 22 additions & 95 deletions smithy-cli/src/main/java/software/amazon/smithy/cli/Cli.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
Expand All @@ -17,19 +17,11 @@

import java.io.PrintWriter;
import java.io.StringWriter;
import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.TreeMap;
import java.util.function.Consumer;
import java.util.logging.ConsoleHandler;
import java.util.logging.Formatter;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import java.util.logging.SimpleFormatter;
import software.amazon.smithy.utils.SmithyUnstableApi;
import software.amazon.smithy.utils.StringUtils;

Expand Down Expand Up @@ -68,15 +60,14 @@ public final class Cli {
/** Configures whether or not to use ANSI colors. */
static boolean useAnsiColors = isAnsiColorSupported();

private static final SimpleDateFormat FORMAT = new SimpleDateFormat("HH:mm:ss.SSS");

// Note that we don't use a method reference here in case System.out or System.err are changed.
private static Consumer<String> stdout = s -> System.out.println(s);
private static Consumer<String> stderr = s -> System.err.println(s);

private final String applicationName;
private final ClassLoader classLoader;
private Map<String, Command> commands = new TreeMap<>();
private final Map<String, Command> commands = new TreeMap<>();
private boolean configureLogging;

/**
* Creates a new CLI with the given name.
Expand Down Expand Up @@ -107,6 +98,15 @@ public void addCommand(Command command) {
commands.put(command.getName(), command);
}

/**
* Set to true to configure logging for the CLI.
*
* @param configureLogging Set to true to configure logging.
*/
public void setConfigureLogging(boolean configureLogging) {
this.configureLogging = configureLogging;
}

/**
* Execute the command line using the given arguments.
*
Expand Down Expand Up @@ -152,6 +152,16 @@ public void run(String[] args) {
}
}

private void configureLogging(Arguments parsedArguments) {
// Logging is configured if --logging is provided, if --debug is provided, or if configureLogging is used.
boolean needsConfiguration = configureLogging || parsedArguments.has(DEBUG) || parsedArguments.has(LOGGING);

if (needsConfiguration) {
Level level = Level.parse(parsedArguments.parameter(LOGGING, Level.ALL.getName()));
LoggingUtil.configureLogging(parsedArguments.has(DEBUG), level);
}
}

/**
* Configures a custom STDOUT printer.
*
Expand Down Expand Up @@ -224,44 +234,6 @@ private static boolean isAnsiColorSupported() {
return System.console() != null && System.getenv().get("TERM") != null;
}

private void configureLogging(Arguments parsedArgs) {
boolean configureLogging = parsedArgs.has(DEBUG) || parsedArgs.has(LOGGING);

if (!configureLogging) {
return;
}

Level level = Level.parse(parsedArgs.parameter(LOGGING, Level.ALL.getName()));

// Remove any currently present console loggers.
Logger rootLogger = Logger.getLogger("");
removeConsoleHandler(rootLogger);

if (parsedArgs.has(DEBUG)) {
// Debug ignores the given logging level and just logs everything.
CliLogHandler handler = new CliLogHandler(new DebugFormatter());
handler.setLevel(Level.ALL);
rootLogger.addHandler(handler);
rootLogger.setLevel(Level.ALL);
for (Handler h : rootLogger.getHandlers()) {
h.setLevel(Level.ALL);
}
} else if (level != Level.OFF) {
CliLogHandler handler = new CliLogHandler(new BasicFormatter());
handler.setLevel(level);
rootLogger.addHandler(handler);
}
}

private static void removeConsoleHandler(Logger rootLogger) {
for (Handler handler : rootLogger.getHandlers()) {
if (handler instanceof ConsoleHandler) {
// Remove any console log handlers.
rootLogger.removeHandler(handler);
}
}
}

private boolean hasArgument(String[] args, String name) {
for (String arg : args) {
if (arg.equals(name)) {
Expand Down Expand Up @@ -373,49 +345,4 @@ private void writeArgHelp(Parser.Argument arg, StringBuilder sink) {
sink.append(" ...");
}
}

private static final class BasicFormatter extends SimpleFormatter {
@Override
public synchronized String format(LogRecord r) {
return FORMAT.format(new Date(r.getMillis()))
+ " [" + r.getLevel().getLocalizedName() + "] "
+ r.getMessage() + System.lineSeparator();
}
}

private static final class DebugFormatter extends SimpleFormatter {
@Override
public synchronized String format(LogRecord r) {
return FORMAT.format(new Date(r.getMillis()))
+ " [" + r.getLevel().getLocalizedName() + "] ["
+ r.getLoggerName() + "] "
+ r.getMessage() + System.lineSeparator();
}
}

/**
* Logs messages to the CLI's redirect stderr.
*/
private static final class CliLogHandler extends Handler {
private final Formatter formatter;

CliLogHandler(Formatter formatter) {
this.formatter = formatter;
}

@Override
public void publish(LogRecord record) {
if (isLoggable(record)) {
Cli.stderr(formatter.format(record));
}
}

@Override
public void flush() {
}

@Override
public void close() {
}
}
}
113 changes: 113 additions & 0 deletions smithy-cli/src/main/java/software/amazon/smithy/cli/LoggingUtil.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

package software.amazon.smithy.cli;

import java.text.SimpleDateFormat;
import java.util.Date;
import java.util.logging.ConsoleHandler;
import java.util.logging.Formatter;
import java.util.logging.Handler;
import java.util.logging.Level;
import java.util.logging.LogRecord;
import java.util.logging.Logger;
import java.util.logging.SimpleFormatter;

/**
* Package-private utilities for configuring CLI logging.
*/
final class LoggingUtil {

private static final SimpleDateFormat FORMAT = new SimpleDateFormat("HH:mm:ss.SSS");

private LoggingUtil() {}

static void configureLogging(boolean debug, Level level) {
Logger rootLogger = Logger.getLogger("");
removeConsoleHandler(rootLogger);
addCliHandler(debug, level, rootLogger);
}

private static void addCliHandler(boolean debug, Level level, Logger rootLogger) {
if (level != Level.OFF) {
Handler handler = debug
// Debug ignores the given logging level and just logs everything.
? new CliLogHandler(new DebugFormatter())
: new CliLogHandler(new BasicFormatter());
handler.setLevel(level);
rootLogger.addHandler(handler);
}

rootLogger.setLevel(level);
for (Handler h : rootLogger.getHandlers()) {
h.setLevel(level);
}
}

private static void removeConsoleHandler(Logger rootLogger) {
for (Handler handler : rootLogger.getHandlers()) {
if (handler instanceof ConsoleHandler) {
// Remove any console log handlers.
rootLogger.removeHandler(handler);
}
}
}

private static final class BasicFormatter extends SimpleFormatter {
@Override
public synchronized String format(LogRecord r) {
return FORMAT.format(new Date(r.getMillis()))
+ " " + r.getLevel().getLocalizedName() + " - "
+ r.getMessage();
}
}

private static final class DebugFormatter extends SimpleFormatter {
@Override
public synchronized String format(LogRecord r) {
return FORMAT.format(new Date(r.getMillis()))
+ " [" + Thread.currentThread().getName() + "] "
+ r.getLevel().getLocalizedName() + " "
+ r.getLoggerName() + " - "
+ r.getMessage();
}
}

/**
* Logs messages to the CLI's redirect stderr.
*/
private static final class CliLogHandler extends Handler {
private final Formatter formatter;

CliLogHandler(Formatter formatter) {
this.formatter = formatter;
}

@Override
public void publish(LogRecord record) {
if (isLoggable(record)) {
Cli.stderr(formatter.format(record));
}
}

@Override
public void flush() {
}

@Override
public void close() {
}
}
}
12 changes: 10 additions & 2 deletions smithy-cli/src/main/java/software/amazon/smithy/cli/SmithyCli.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ public final class SmithyCli {
public static final String SEVERITY = "--severity";

private ClassLoader classLoader = getClass().getClassLoader();
private boolean configureLogging;

private SmithyCli() {}

Expand All @@ -51,7 +52,9 @@ public static SmithyCli create() {
*/
public static void main(String... args) {
try {
SmithyCli.create().run(args);
SmithyCli cli = SmithyCli.create();
cli.configureLogging = true;
cli.run(args);
} catch (CliError e) {
System.exit(e.code);
} catch (Exception e) {
Expand Down Expand Up @@ -85,12 +88,17 @@ public void run(List<String> args) {
* @param args Arguments to parse and execute.
*/
public void run(String... args) {
createCliRunner().run(args);
}

private Cli createCliRunner() {
Cli cli = new Cli("smithy", classLoader);
cli.addCommand(new ValidateCommand());
cli.addCommand(new BuildCommand());
cli.addCommand(new DiffCommand());
cli.addCommand(new SelectCommand());
cli.addCommand(new AstCommand());
cli.run(args);
cli.setConfigureLogging(configureLogging);
return cli;
}
}
Loading

0 comments on commit a681a0a

Please sign in to comment.