Skip to content

Commit

Permalink
Just make it an instance
Browse files Browse the repository at this point in the history
  • Loading branch information
bryce-anderson committed Dec 6, 2023
1 parent b20368c commit a32b203
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 58 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ final class HttpMessageDiscardWatchdogClientFilterTest {

private static final Logger LOGGER = LoggerFactory.getLogger(HttpMessageDiscardWatchdogClientFilterTest.class);

private final LoggerStringWriter loggerStringWriter = new LoggerStringWriter();

@RegisterExtension
static final ExecutionContextExtension SERVER_CTX =
ExecutionContextExtension.cached("server-io", "server-executor")
Expand All @@ -68,12 +70,12 @@ final class HttpMessageDiscardWatchdogClientFilterTest {

@BeforeEach
public void setup() {
LoggerStringWriter.reset();
loggerStringWriter.reset();
}

@AfterEach
public void tearDown() {
LoggerStringWriter.remove();
loggerStringWriter.remove();
}

/**
Expand Down Expand Up @@ -132,7 +134,7 @@ protected Single<StreamingHttpResponse> request(final StreamingHttpRequester del
}
}

String output = LoggerStringWriter.stableAccumulated(1000);
String output = loggerStringWriter.stableAccumulated(1000);
LOGGER.info("Logger output: {}", output);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,16 @@ final class HttpMessageDiscardWatchdogServiceFilterTest {
ExecutionContextExtension.cached("client-io", "client-executor")
.setClassLevel(true);

private final LoggerStringWriter loggerStringWriter = new LoggerStringWriter();

@BeforeEach
public void setup() {
LoggerStringWriter.reset();
loggerStringWriter.reset();
}

@AfterEach
public void tearDown() {
LoggerStringWriter.remove();
loggerStringWriter.remove();
}

@ParameterizedTest(name = "{displayName} [{index}] transformer={0}")
Expand All @@ -91,7 +93,7 @@ public Single<StreamingHttpResponse> handle(final HttpServiceContext ctx,
assertEquals(0, response.payloadBody().readableBytes());
}

String output = LoggerStringWriter.stableAccumulated(CI ? 5000 : 1000);
String output = loggerStringWriter.stableAccumulated(CI ? 5000 : 1000);
if (!output.contains("Discovered un-drained HTTP response message body which " +
"has been dropped by user code")) {
throw new AssertionError("Logs didn't contain the expected output:\n" + output);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,23 +44,24 @@

public final class LoggerStringWriter {
private static final Logger LOGGER = LoggerFactory.getLogger(LoggerStringWriter.class);
private static final ThreadLocal<ConcurrentStringWriter> THREAD_LOCAL_APPENDER = new ThreadLocal<>();

private LoggerStringWriter() {
// no instances.
}
// protected by synchronization on `this`.
private ConcurrentStringWriter writer;

/**
* Clear the content of the {@link #accumulated()}.
* <p>
* Note that the underlying logger may be initialized by this method and it must always be
* followed up with a {@link #remove()} call at the end of tests to clean up logger state.
*/
public static void reset() {
public void reset() {
getStringWriter().reset();
}

/**
* Remove the underlying in-memory log appender.
*/
public static void remove() {
public void remove() {
removeStringWriter();
}

Expand All @@ -69,7 +70,7 @@ public static void remove() {
*
* @return the accumulated content that has been logged.
*/
public static String accumulated() {
public String accumulated() {
return getStringWriter().toString();
}

Expand All @@ -83,7 +84,7 @@ public static String accumulated() {
* @throws TimeoutException If the {@code totalWaitTimeMillis} duration has been exceeded and the
* {@link #accumulated()} has not yet stabilize.
*/
public static String stableAccumulated(int totalWaitTimeMillis) throws InterruptedException, TimeoutException {
public String stableAccumulated(int totalWaitTimeMillis) throws InterruptedException, TimeoutException {
return stableAccumulated(totalWaitTimeMillis, 10);
}

Expand All @@ -98,7 +99,7 @@ public static String stableAccumulated(int totalWaitTimeMillis) throws Interrupt
* @throws TimeoutException If the {@code totalWaitTimeMillis} duration has been exceeded and the
* {@link #accumulated()} has not yet stabilize.
*/
public static String stableAccumulated(int totalWaitTimeMillis, final long sleepDurationMs)
public String stableAccumulated(int totalWaitTimeMillis, final long sleepDurationMs)
throws InterruptedException, TimeoutException {
// We force a unique log entry, and wait for it to ensure the content from the local thread has been flushed.
String forcedLogEntry = "forced log entry to help for flush on current thread " +
Expand Down Expand Up @@ -157,23 +158,20 @@ public static void assertContainsMdcPair(String value, String expectedLabel, Str
assertThat(value.substring(beginIndex, beginIndex + expectedValue.length()), is(expectedValue));
}

private static ConcurrentStringWriter getStringWriter() {
ConcurrentStringWriter writer = THREAD_LOCAL_APPENDER.get();
private synchronized ConcurrentStringWriter getStringWriter() {
if (writer == null) {
final LoggerContext context = (LoggerContext) LogManager.getContext(false);
writer = addWriterAppender(context, DEBUG);
THREAD_LOCAL_APPENDER.set(writer);
}
return writer;
}

private static void removeStringWriter() {
ConcurrentStringWriter writer = THREAD_LOCAL_APPENDER.get();
private synchronized void removeStringWriter() {
if (writer == null) {
return;
}
removeWriterAppender(writer, (LoggerContext) LogManager.getContext(false));
THREAD_LOCAL_APPENDER.remove();
writer = null;
}

private static ConcurrentStringWriter addWriterAppender(final LoggerContext context, Level level) {
Expand Down Expand Up @@ -224,17 +222,13 @@ public void write(char[] cbuf, int off, int len) throws IOException {
}

@Override
public void flush() throws IOException {
synchronized (stringWriter) {
stringWriter.flush();
}
public void flush() {
// this is a no-op for `StringWriter`
}

@Override
public void close() throws IOException {
synchronized (stringWriter) {
stringWriter.close();
}
public void close() {
// this is a no-op for `StringWriter`
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
import static io.servicetalk.concurrent.api.Single.succeeded;
import static io.servicetalk.http.netty.HttpClients.forSingleAddress;
import static io.servicetalk.log4j2.mdc.utils.LoggerStringWriter.assertContainsMdcPair;
import static io.servicetalk.log4j2.mdc.utils.LoggerStringWriter.stableAccumulated;
import static io.servicetalk.opentelemetry.http.TestUtils.SPAN_STATE_SERIALIZER;
import static io.servicetalk.opentelemetry.http.TestUtils.TRACING_TEST_LOG_LINE_PREFIX;
import static io.servicetalk.opentelemetry.http.TestUtils.TestTracingClientLoggerFilter;
Expand All @@ -61,18 +60,19 @@
class OpenTelemetryHttpRequestFilterTest {

private static final Logger LOGGER = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
private final LoggerStringWriter loggerStringWriter = new LoggerStringWriter();

@RegisterExtension
static final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create();

@BeforeEach
public void setup() {
LoggerStringWriter.reset();
loggerStringWriter.reset();
}

@AfterEach
public void tearDown() {
LoggerStringWriter.remove();
loggerStringWriter.remove();
}

@Test
Expand All @@ -86,7 +86,7 @@ void testInjectWithNoParent() throws Exception {
HttpResponse response = client.request(client.get(requestUrl)).toFuture().get();
TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(1);
Expand Down Expand Up @@ -116,7 +116,7 @@ void testInjectWithAParent() throws Exception {
HttpResponse response = client.request(client.get(requestUrl)).toFuture().get();
TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(2);
Expand Down Expand Up @@ -173,7 +173,7 @@ void testInjectWithAParentCreated() throws Exception {
} finally {
span.end();
}
verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(3);
Expand Down Expand Up @@ -219,7 +219,7 @@ void testCaptureHeader() throws Exception {
.addHeader("some-request-header", "request-header-value")).toFuture().get();
TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@

import static io.servicetalk.concurrent.api.Single.succeeded;
import static io.servicetalk.http.netty.HttpClients.forSingleAddress;
import static io.servicetalk.log4j2.mdc.utils.LoggerStringWriter.stableAccumulated;
import static io.servicetalk.opentelemetry.http.OpenTelemetryHttpRequestFilterTest.verifyTraceIdPresentInLogs;
import static io.servicetalk.opentelemetry.http.TestUtils.SPAN_STATE_SERIALIZER;
import static io.servicetalk.opentelemetry.http.TestUtils.TRACING_TEST_LOG_LINE_PREFIX;
Expand All @@ -60,14 +59,16 @@ class OpenTelemetryHttpServerFilterTest {
@RegisterExtension
static final OpenTelemetryExtension otelTesting = OpenTelemetryExtension.create();

private final LoggerStringWriter loggerStringWriter = new LoggerStringWriter();

@BeforeEach
public void setup() {
LoggerStringWriter.reset();
loggerStringWriter.reset();
}

@AfterEach
public void tearDown() {
LoggerStringWriter.remove();
loggerStringWriter.remove();
}

@Test
Expand All @@ -78,7 +79,7 @@ void testInjectWithNoParent() throws Exception {
HttpResponse response = client.request(client.get(requestUrl)).toFuture().get();
TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(1);
Expand Down Expand Up @@ -125,7 +126,7 @@ void testInjectWithAParent() throws Exception {
HttpResponse response = client.request(client.get(requestUrl)).toFuture().get();
TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(2);
Expand Down Expand Up @@ -176,7 +177,7 @@ void testInjectWithNewTrace() throws Exception {
} finally {
span.end();
}
verifyTraceIdPresentInLogs(stableAccumulated(1000), "/",
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), "/",
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(2);
Expand Down Expand Up @@ -209,7 +210,7 @@ void testCaptureHeaders() throws Exception {
.toFuture().get();
TestSpanState serverSpanState = response.payloadBody(SPAN_STATE_SERIALIZER);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.getTraceId(), serverSpanState.getSpanId(),
TRACING_TEST_LOG_LINE_PREFIX);
assertThat(otelTesting.getSpans()).hasSize(1);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@
import static io.servicetalk.http.api.HttpRequestMethod.GET;
import static io.servicetalk.http.api.HttpResponseStatus.OK;
import static io.servicetalk.http.netty.HttpClients.forSingleAddress;
import static io.servicetalk.log4j2.mdc.utils.LoggerStringWriter.stableAccumulated;
import static io.servicetalk.opentracing.asynccontext.AsyncContextInMemoryScopeManager.SCOPE_MANAGER;
import static io.servicetalk.opentracing.http.TestUtils.SPAN_STATE_SERIALIZER;
import static io.servicetalk.opentracing.http.TestUtils.TRACING_TEST_LOG_LINE_PREFIX;
Expand Down Expand Up @@ -87,18 +86,20 @@
class TracingHttpRequesterFilterTest {
private static final Logger LOGGER = LoggerFactory.getLogger(TracingHttpRequesterFilterTest.class);

private final LoggerStringWriter loggerStringWriter = new LoggerStringWriter();

@Mock
private Tracer mockTracer;

@BeforeEach
public void setup() {
initMocks(this);
LoggerStringWriter.reset();
loggerStringWriter.reset();
}

@AfterEach
public void tearDown() {
LoggerStringWriter.remove();
loggerStringWriter.remove();
}

@Test
Expand Down Expand Up @@ -130,8 +131,8 @@ void testInjectWithNoParent() throws Exception {
assertEquals(OK.code(), lastFinishedSpan.tags().get(HTTP_STATUS.getKey()));
assertFalse(lastFinishedSpan.tags().containsKey(ERROR.getKey()));

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl, serverSpanState.traceId,
serverSpanState.spanId, null, TRACING_TEST_LOG_LINE_PREFIX);
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.traceId, serverSpanState.spanId, null, TRACING_TEST_LOG_LINE_PREFIX);
}
}
}
Expand Down Expand Up @@ -171,9 +172,10 @@ void testInjectWithParent() throws Exception {
assertEquals(requestUrl, lastFinishedSpan.tags().get(HTTP_URL.getKey()));
assertEquals(OK.code(), lastFinishedSpan.tags().get(HTTP_STATUS.getKey()));
assertFalse(lastFinishedSpan.tags().containsKey(ERROR.getKey()));
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl,
serverSpanState.traceId, serverSpanState.spanId, serverSpanState.parentSpanId,
TRACING_TEST_LOG_LINE_PREFIX);

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl, serverSpanState.traceId,
serverSpanState.spanId, serverSpanState.parentSpanId, TRACING_TEST_LOG_LINE_PREFIX);
} finally {
clientSpan.finish();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
import static io.servicetalk.http.api.HttpSerializers.textSerializerUtf8;
import static io.servicetalk.http.netty.AsyncContextHttpFilterVerifier.verifyServerFilterAsyncContextVisibility;
import static io.servicetalk.http.netty.HttpClients.forSingleAddress;
import static io.servicetalk.log4j2.mdc.utils.LoggerStringWriter.stableAccumulated;
import static io.servicetalk.opentracing.asynccontext.AsyncContextInMemoryScopeManager.SCOPE_MANAGER;
import static io.servicetalk.opentracing.http.TestUtils.SPAN_STATE_SERIALIZER;
import static io.servicetalk.opentracing.http.TestUtils.TRACING_TEST_LOG_LINE_PREFIX;
Expand Down Expand Up @@ -84,19 +83,20 @@
@ExtendWith(MockitoExtension.class)
class TracingHttpServiceFilterTest {
private static final Logger LOGGER = LoggerFactory.getLogger(TracingHttpServiceFilterTest.class);
private final LoggerStringWriter loggerStringWriter = new LoggerStringWriter();

@Mock
private Tracer mockTracer;

@BeforeEach
public void setup() {
initMocks(this);
LoggerStringWriter.reset();
loggerStringWriter.reset();
}

@AfterEach
public void tearDown() {
LoggerStringWriter.remove();
loggerStringWriter.remove();
}

private static ServerContext buildServer(CountingInMemorySpanEventListener spanListener) throws Exception {
Expand Down Expand Up @@ -204,7 +204,7 @@ private void assertSpan(final CountingInMemorySpanEventListener spanListener, fi
assertNull(lastFinishedSpan);
}

verifyTraceIdPresentInLogs(stableAccumulated(1000), requestUrl, serverSpanState.traceId,
verifyTraceIdPresentInLogs(loggerStringWriter.stableAccumulated(1000), requestUrl, serverSpanState.traceId,
serverSpanState.spanId, serverSpanState.parentSpanId, TRACING_TEST_LOG_LINE_PREFIX);
}

Expand Down
Loading

0 comments on commit a32b203

Please sign in to comment.