Skip to content

Commit

Permalink
Improve servicetalk-test-resources/log4j2.xml (#1181)
Browse files Browse the repository at this point in the history
Motivation:

Wire logger and h2 frame logger are controlled by the logger configuration
after #1123. It allows us to use these logger names in tests and enabled them
via system property when required for local debugging.
Having 2 different logger names for client and server controlled by a single
system property does not help much. We can use thread name to understand if
a logger statement belongs to a client or to a server. A single logger name
simplifies usage in tests.

Modifications:

- Keep only a single `servicetalk-tests-wire-logger` logger name;
- Add `servicetalk-tests-h2-frame-logger` logger name and
`servicetalk.logger.h2FrameLogLevel` system property to control visibility
for h2 frames;
- Update all tests to use new names;

Result:

Separate system property to control wire logger and h2 frame logger.
  • Loading branch information
idelpivnitskiy authored Oct 15, 2020
1 parent 2c92ed0 commit a0bf19d
Show file tree
Hide file tree
Showing 8 changed files with 34 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@
import io.servicetalk.transport.api.HostAndPort;
import io.servicetalk.transport.api.ServerContext;
import io.servicetalk.transport.api.ServiceTalkSocketOptions;
import io.servicetalk.transport.netty.internal.ExecutionContextRule;

import org.junit.After;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.Timeout;
Expand All @@ -50,8 +52,10 @@
import static io.servicetalk.concurrent.api.AsyncCloseables.newCompositeCloseable;
import static io.servicetalk.concurrent.api.Publisher.never;
import static io.servicetalk.concurrent.api.Single.succeeded;
import static io.servicetalk.grpc.api.GrpcExecutionStrategies.defaultStrategy;
import static io.servicetalk.transport.netty.internal.AddressUtils.localAddress;
import static io.servicetalk.transport.netty.internal.AddressUtils.serverHostAndPort;
import static io.servicetalk.transport.netty.internal.ExecutionContextRule.cached;
import static java.time.Duration.ofSeconds;
import static java.util.Arrays.asList;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
Expand All @@ -60,34 +64,44 @@

@RunWith(Parameterized.class)
public class KeepAliveTest {
private final TesterClient client;
private final ServerContext ctx;

@ClassRule
public static final ExecutionContextRule SERVER_CTX = cached("server-io", "server-executor");
@ClassRule
public static final ExecutionContextRule CLIENT_CTX = cached("client-io", "client-executor");

@Rule
public final Timeout timeout = new ServiceTalkTestTimeout(1, MINUTES);

private final TesterClient client;
private final ServerContext ctx;
private final long idleTimeoutMillis;

public KeepAliveTest(final boolean keepAlivesFromClient,
final Function<String, H2ProtocolConfig> protocolConfigSupplier,
final long idleTimeoutMillis) throws Exception {
this.idleTimeoutMillis = idleTimeoutMillis;
GrpcServerBuilder serverBuilder = GrpcServers.forAddress(localAddress(0));
GrpcServerBuilder serverBuilder = GrpcServers.forAddress(localAddress(0))
.ioExecutor(SERVER_CTX.ioExecutor())
.executionStrategy(defaultStrategy(SERVER_CTX.executor()));
if (!keepAlivesFromClient) {
serverBuilder.protocols(protocolConfigSupplier.apply("servicetalk-tests-server-wire-logger"));
serverBuilder.protocols(protocolConfigSupplier.apply("servicetalk-tests-wire-logger"));
} else {
serverBuilder.socketOption(ServiceTalkSocketOptions.IDLE_TIMEOUT, idleTimeoutMillis)
.protocols(HttpProtocolConfigs.h2()
.enableFrameLogging("servicetalk-tests-server-wire-logger").build());
.enableFrameLogging("servicetalk-tests-h2-frame-logger").build());
}
ctx = serverBuilder.listenAndAwait(new ServiceFactory(new InfiniteStreamsService()));
GrpcClientBuilder<HostAndPort, InetSocketAddress> clientBuilder =
GrpcClients.forAddress(serverHostAndPort(ctx));
GrpcClients.forAddress(serverHostAndPort(ctx))
.ioExecutor(CLIENT_CTX.ioExecutor())
.executionStrategy(defaultStrategy(CLIENT_CTX.executor()));
if (keepAlivesFromClient) {
clientBuilder.protocols(protocolConfigSupplier.apply("servicetalk-tests-client-wire-logger"));
clientBuilder.protocols(protocolConfigSupplier.apply("servicetalk-tests-wire-logger"));
} else {
clientBuilder.socketOption(ServiceTalkSocketOptions.IDLE_TIMEOUT, idleTimeoutMillis)
.protocols(HttpProtocolConfigs.h2()
.enableFrameLogging("servicetalk-tests-client-wire-logger").build());
.enableFrameLogging("servicetalk-tests-h2-frame-logger").build());
}
client = clientBuilder.build(new TesterProto.Tester.ClientFactory());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ protected ConnectionSetup(boolean useUds, boolean viaProxy, boolean awaitRequest
HttpServers.forAddress(localAddress(0)))
.ioExecutor(SERVER_CTX.ioExecutor())
.executionStrategy(defaultStrategy(SERVER_CTX.executor()))
.enableWireLogging("servicetalk-tests-server-wire-logger")
.enableWireLogging("servicetalk-tests-wire-logger")
.appendConnectionAcceptorFilter(original -> new DelegatingConnectionAcceptor(original) {
@Override
public Completable accept(final ConnectionContext context) {
Expand Down Expand Up @@ -190,7 +190,7 @@ public Completable accept(final ConnectionContext context) {
HttpClients.forResolvedAddress(serverContext.listenAddress()))
.ioExecutor(CLIENT_CTX.ioExecutor())
.executionStrategy(defaultStrategy(CLIENT_CTX.executor()))
.enableWireLogging("servicetalk-tests-client-wire-logger")
.enableWireLogging("servicetalk-tests-wire-logger")
.buildStreaming();
connection = client.reserveConnection(client.get("/")).toFuture().get();
connection.onClose().whenFinally(clientConnectionClosed::countDown).subscribe();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ public GracefulConnectionClosureHandlingTest(boolean initiateClosureFromClient,
HttpServers.forAddress(localAddress(0)))
.ioExecutor(SERVER_CTX.ioExecutor())
.executionStrategy(defaultStrategy(SERVER_CTX.executor()))
.enableWireLogging("servicetalk-tests-server-wire-logger")
.enableWireLogging("servicetalk-tests-wire-logger")
.appendConnectionAcceptorFilter(original -> new DelegatingConnectionAcceptor(original) {
@Override
public Completable accept(final ConnectionContext context) {
Expand Down Expand Up @@ -196,7 +196,7 @@ public Completable accept(final ConnectionContext context) {
HttpClients.forResolvedAddress(serverContext.listenAddress()))
.ioExecutor(CLIENT_CTX.ioExecutor())
.executionStrategy(defaultStrategy(CLIENT_CTX.executor()))
.enableWireLogging("servicetalk-tests-client-wire-logger")
.enableWireLogging("servicetalk-tests-wire-logger")
.appendConnectionFactoryFilter(cf -> initiateClosureFromClient ?
new OnClosingConnectionFactoryFilter<>(cf, onClosing) : cf)
.buildStreaming();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@
import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_1_1;
import static io.servicetalk.http.api.HttpProtocolVersion.HTTP_2_0;
import static io.servicetalk.http.netty.HttpProtocolConfigs.h1Default;
import static io.servicetalk.http.netty.HttpProtocolConfigs.h2Default;
import static io.servicetalk.http.netty.HttpProtocolConfigs.h2;

enum HttpProtocol {
HTTP_1(h1Default(), HTTP_1_1),
HTTP_2(h2Default(), HTTP_2_0);
HTTP_2(h2().enableFrameLogging("servicetalk-tests-h2-frame-logger").build(), HTTP_2_0);

final HttpProtocolConfig config;
final HttpProtocolVersion version;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public ServerRespondsOnClosingTest() throws Exception {
DefaultHttpExecutionContext httpExecutionContext = new DefaultHttpExecutionContext(DEFAULT_ALLOCATOR,
fromNettyEventLoop(channel.eventLoop()), immediate(), noOffloadsStrategy());
final HttpServerConfig httpServerConfig = new HttpServerConfig();
httpServerConfig.tcpConfig().enableWireLogging("servicetalk-tests-server-wire-logger");
httpServerConfig.tcpConfig().enableWireLogging("servicetalk-tests-wire-logger");
ReadOnlyHttpServerConfig config = httpServerConfig.asReadOnly();
ConnectionObserver connectionObserver = NoopConnectionObserver.INSTANCE;
HttpService service = (ctx, request, responseFactory) -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ TcpClientConfig getTcpClientConfig() {
securityConfig.disableHostnameVerification();
tcpClientConfig.secure(securityConfig.asReadOnly());
}
tcpClientConfig.enableWireLogging("servicetalk-tests-wire-logger");
return tcpClientConfig;
}

Expand All @@ -123,6 +124,7 @@ TcpServerConfig getTcpServerConfig() {
securityConfig.keyManager(DefaultTestCerts::loadServerPem, DefaultTestCerts::loadServerKey);
tcpServerConfig.secure(securityConfig.asReadOnly());
}
tcpServerConfig.enableWireLogging("servicetalk-tests-wire-logger");
return tcpServerConfig;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ public SecureTcpTransportObserverErrorsTest(ErrorReason errorReason,
this.errorReason = errorReason;
this.clientProvider = clientProvider;
this.serverProvider = serverProvider;
// clientConfig.enableWireLogging("servicetalk-tests-client-wire-logger");
// serverConfig.enableWireLogging("servicetalk-tests-server-wire-logger");
ClientSecurityConfig clientSecurityConfig = defaultClientSecurityConfig(clientProvider);
ServerSecurityConfig serverSecurityConfig = defaultServerSecurityConfig(serverProvider);
switch (errorReason) {
Expand Down
5 changes: 3 additions & 2 deletions servicetalk-test-resources/src/main/resources/log4j2.xml
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,16 @@
<Properties>
<Property name="rootLevel">${sys:servicetalk.logger.rootLevel:-INFO}</Property>
<Property name="wireLogLevel">${sys:servicetalk.logger.wireLogLevel:-OFF}</Property>
<Property name="h2FrameLogLevel">${sys:servicetalk.logger.h2FrameLogLevel:-OFF}</Property>
</Properties>
<Appenders>
<Console name="Console" target="SYSTEM_OUT">
<PatternLayout pattern="%d %30t [%-5level] %-30logger{1} - %msg%n"/>
</Console>
</Appenders>
<Loggers>
<Logger name="servicetalk-tests-client-wire-logger" level="${wireLogLevel}"/>
<Logger name="servicetalk-tests-server-wire-logger" level="${wireLogLevel}"/>
<Logger name="servicetalk-tests-wire-logger" level="${wireLogLevel}"/>
<Logger name="servicetalk-tests-h2-frame-logger" level="${h2FrameLogLevel}"/>
<Root level="${rootLevel}">
<AppenderRef ref="Console"/>
</Root>
Expand Down

0 comments on commit a0bf19d

Please sign in to comment.