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

feat(console monitor): Log level can be set via program arg #4476

Merged
merged 17 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 14 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
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,10 @@
import org.eclipse.edc.boot.system.injection.ProviderMethod;
import org.eclipse.edc.boot.system.injection.ProviderMethodScanner;
import org.eclipse.edc.boot.system.injection.lifecycle.ExtensionLifecycleManager;
import org.eclipse.edc.spi.EdcException;
import org.eclipse.edc.spi.monitor.ConsoleMonitor;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.spi.result.Result;
import org.eclipse.edc.spi.system.MonitorExtension;
import org.eclipse.edc.spi.system.ServiceExtension;
import org.eclipse.edc.spi.system.ServiceExtensionContext;
Expand All @@ -37,6 +39,7 @@
import java.util.Set;
import java.util.function.Supplier;
import java.util.stream.Collectors;
import java.util.stream.Stream;

public class ExtensionLoader {

Expand Down Expand Up @@ -89,7 +92,11 @@ private static Supplier<Object> getDefaultProviderInvoker(ServiceExtensionContex

static @NotNull Monitor loadMonitor(List<MonitorExtension> availableMonitors, String... programArgs) {
if (availableMonitors.isEmpty()) {
return new ConsoleMonitor(!Set.of(programArgs).contains("--no-color"));
var parseResult = parseLogLevel(programArgs);
if (parseResult.failed()) {
throw new EdcException(parseResult.getFailureDetail());
}
return new ConsoleMonitor((ConsoleMonitor.Level) parseResult.getContent(), !Set.of(programArgs).contains(ConsoleMonitor.COLOR_PROG_ARG));
}

if (availableMonitors.size() > 1) {
Expand Down Expand Up @@ -127,4 +134,27 @@ public <T> List<T> loadExtensions(Class<T> type, boolean required) {
return serviceLocator.loadImplementors(type, required);
}

/**
* Parses the ConsoleMonitor log level from the program args. If no log level is provided, defaults to Level default.
*/
private static Result<?> parseLogLevel(String[] programArgs) {
return Stream.of(programArgs)
.filter(arg -> arg.startsWith(ConsoleMonitor.LEVEL_PROG_ARG + "="))
.map(arg -> arg.split("="))
.map(arg -> {
var validValueMessage = String.format("Valid values for the console level are %s", Stream.of(ConsoleMonitor.Level.values()).toList());

if (arg.length != 2) {
return Result.failure(String.format("Value missing for the --log-level argument. %s", validValueMessage));
}
try {
return Result.success(ConsoleMonitor.Level.valueOf(arg[1]));
} catch (IllegalArgumentException e) {
return Result.failure(String.format("Invalid value %s for the --log-level argument. %s", arg[1], validValueMessage));
}
})
.findFirst()
.orElse(Result.success(ConsoleMonitor.Level.getDefaultLevel()));
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.EnumSource;
import org.mockito.Mockito;

import java.util.ArrayList;
Expand Down Expand Up @@ -98,6 +100,33 @@ void loadMonitor_whenNoMonitorExtension() {
assertTrue(monitor instanceof ConsoleMonitor);
}

@ParameterizedTest
@EnumSource
void loadMonitor_programArgsSetConsoleMonitorLogLevel(ConsoleMonitor.Level level) {

var monitor = ExtensionLoader.loadMonitor(new ArrayList<>(), ConsoleMonitor.LEVEL_PROG_ARG + "=" + level.toString());
Copy link
Member

@paullatzelsperger paullatzelsperger Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also test various combinations of spaces, upper, lower case etc.:

"--log-level = INFO", "--log-level= INFO", "--log-level =INFO", "--log-level=info",...

This could also be done with a ParamterizedTest and a @ValueSource.

Not saying all of these variants should get accepted, IMO it's fine to only accept "--log-level=XYZ", but to have a definitive test and to guard against regression.


assertThat(monitor).extracting("level").isEqualTo(level);
}

@Test
void loadMonitor_consoleMonitorDefaultLogLevelWhenNoArgs() {

var monitor = ExtensionLoader.loadMonitor(new ArrayList<>());

assertThat(monitor).extracting("level").isEqualTo(ConsoleMonitor.Level.getDefaultLevel());
}

@Test
void loadMonitor_consoleMonitorDefaultLogLevelWhenWrongArgs() {
rafaelmag110 marked this conversation as resolved.
Show resolved Hide resolved
assertThatThrownBy(() -> ExtensionLoader.loadMonitor(new ArrayList<>(), ConsoleMonitor.LEVEL_PROG_ARG + "=INF"))
.isInstanceOf(EdcException.class)
.hasMessageContaining("Invalid value INF for the --log-level argument.");
assertThatThrownBy(() -> ExtensionLoader.loadMonitor(new ArrayList<>(), ConsoleMonitor.LEVEL_PROG_ARG + "="))
.isInstanceOf(EdcException.class)
.hasMessageContaining("Value missing for the --log-level argument.");
}

@Test
void selectOpenTelemetryImpl_whenNoOpenTelemetry() {
var openTelemetry = ExtensionLoader.selectOpenTelemetryImpl(emptyList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,21 @@ public class ConsoleMonitor implements Monitor {
private static final String INFO = "INFO";
private static final String DEBUG = "DEBUG";

public static final String LEVEL_PROG_ARG = "--log-level";
public static final String COLOR_PROG_ARG = "--no-color";


private final boolean useColor;

private final Level level;
private Level level;
private final String prefix;

public ConsoleMonitor() {
this(true);
}

public ConsoleMonitor(boolean useColor) {
this(null, Level.DEBUG, useColor);
this(Level.getDefaultLevel(), true);
}

public ConsoleMonitor(@Nullable String runtimeName, Level level) {
this(runtimeName, level, true);
public ConsoleMonitor(Level level, boolean useColor) {
this(null, level, useColor);
}

public ConsoleMonitor(@Nullable String runtimeName, Level level, boolean useColor) {
Expand Down Expand Up @@ -117,5 +117,10 @@ public enum Level {
Level(int value) {
this.value = value;
}

public static Level getDefaultLevel() {
return Level.DEBUG;
}

}
}
Loading