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

Fix how the CLI logs #910

Merged
merged 1 commit into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 22 additions & 95 deletions smithy-cli/src/main/java/software/amazon/smithy/cli/Cli.java
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
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() {
}
}
}
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