Skip to content
Permalink
Browse files
[HUDI-1176] Upgrade hudi to log4j2 (#5366)
* Move to log4j2

cr: https://code.amazon.com/reviews/CR-71010705

* Upgrade unit tests to log4j2

* update exclusion

Co-authored-by: Brandon Scheller <bschelle@amazon.com>
  • Loading branch information
bschell and Brandon Scheller committed Jun 28, 2022
1 parent ed823f1 commit fd7d25ab630cdd7a3adf21d8a30e03ab718d65d4
Showing 22 changed files with 235 additions and 79 deletions.
@@ -48,8 +48,16 @@

<!-- Logging -->
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-1.2-api</artifactId>
</dependency>

<!-- Hadoop -->
@@ -201,8 +201,16 @@

<!-- Logging -->
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-1.2-api</artifactId>
</dependency>

<dependency>
@@ -55,8 +55,16 @@

<!-- Logging -->
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-1.2-api</artifactId>
</dependency>

<!-- Parquet -->
@@ -28,8 +28,8 @@
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.hudi.config.HoodieWriteCommitCallbackConfig;
import org.apache.hudi.config.HoodieWriteConfig;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.io.Closeable;
import java.io.IOException;
@@ -32,8 +32,8 @@
import org.apache.http.entity.StringEntity;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.io.Closeable;
import java.io.IOException;
@@ -34,8 +34,8 @@
import com.fasterxml.jackson.databind.node.ArrayNode;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.fasterxml.jackson.databind.node.TextNode;
import org.apache.log4j.LogManager;
import org.apache.log4j.Logger;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import java.io.IOException;
import java.util.List;
@@ -23,11 +23,13 @@
import org.apache.http.StatusLine;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.log4j.AppenderSkeleton;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Appender;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.Logger;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.ArgumentCaptor;
@@ -52,10 +54,10 @@
public class TestCallbackHttpClient {

@Mock
AppenderSkeleton appender;
Appender appender;

@Captor
ArgumentCaptor<LoggingEvent> logCaptor;
ArgumentCaptor<LogEvent> logCaptor;

@Mock
CloseableHttpClient httpClient;
@@ -66,8 +68,17 @@ public class TestCallbackHttpClient {
@Mock
StatusLine statusLine;

@BeforeEach
void prepareAppender() {
when(appender.getName()).thenReturn("MockAppender");
when(appender.isStarted()).thenReturn(true);
when(appender.isStopped()).thenReturn(false);
((Logger) LogManager.getLogger(HoodieWriteCommitHttpCallbackClient.class)).addAppender(appender);
}

@AfterEach
void resetMocks() {
((Logger) LogManager.getLogger(HoodieWriteCommitHttpCallbackClient.class)).removeAppender(appender);
reset(appender, httpClient, httpResponse, statusLine);
}

@@ -83,44 +94,41 @@ private void mockResponse(int statusCode) {

@Test
public void sendPayloadShouldLogWhenRequestFailed() throws IOException {
Logger.getRootLogger().addAppender(appender);
when(httpClient.execute(any())).thenThrow(IOException.class);

HoodieWriteCommitHttpCallbackClient hoodieWriteCommitCallBackHttpClient =
new HoodieWriteCommitHttpCallbackClient("fake_api_key", "fake_url", httpClient);
hoodieWriteCommitCallBackHttpClient.send("{}");

verify(appender).doAppend(logCaptor.capture());
assertEquals("Failed to send callback.", logCaptor.getValue().getRenderedMessage());
verify(appender).append(logCaptor.capture());
assertEquals("Failed to send callback.", logCaptor.getValue().getMessage().getFormattedMessage());
assertEquals(Level.WARN, logCaptor.getValue().getLevel());
}

@Test
public void sendPayloadShouldLogUnsuccessfulSending() {
Logger.getRootLogger().addAppender(appender);
mockResponse(401);
when(httpResponse.toString()).thenReturn("unauthorized");

HoodieWriteCommitHttpCallbackClient hoodieWriteCommitCallBackHttpClient =
new HoodieWriteCommitHttpCallbackClient("fake_api_key", "fake_url", httpClient);
hoodieWriteCommitCallBackHttpClient.send("{}");

verify(appender).doAppend(logCaptor.capture());
assertEquals("Failed to send callback message. Response was unauthorized", logCaptor.getValue().getRenderedMessage());
verify(appender).append(logCaptor.capture());
assertEquals("Failed to send callback message. Response was unauthorized", logCaptor.getValue().getMessage().getFormattedMessage());
assertEquals(Level.WARN, logCaptor.getValue().getLevel());
}

@Test
public void sendPayloadShouldLogSuccessfulSending() {
Logger.getRootLogger().addAppender(appender);
mockResponse(202);

HoodieWriteCommitHttpCallbackClient hoodieWriteCommitCallBackHttpClient =
new HoodieWriteCommitHttpCallbackClient("fake_api_key", "fake_url", httpClient);
hoodieWriteCommitCallBackHttpClient.send("{}");

verify(appender).doAppend(logCaptor.capture());
assertTrue(logCaptor.getValue().getRenderedMessage().startsWith("Sent Callback data"));
verify(appender).append(logCaptor.capture());
assertTrue(logCaptor.getValue().getMessage().getFormattedMessage().startsWith("Sent Callback data"));
assertEquals(Level.INFO, logCaptor.getValue().getLevel());
}

@@ -23,11 +23,13 @@
import org.apache.http.StatusLine;
import org.apache.http.client.methods.CloseableHttpResponse;
import org.apache.http.impl.client.CloseableHttpClient;
import org.apache.log4j.AppenderSkeleton;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Appender;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.Logger;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
@@ -55,10 +57,10 @@
public class TestDatadogHttpClient {

@Mock
AppenderSkeleton appender;
Appender appender;

@Captor
ArgumentCaptor<LoggingEvent> logCaptor;
ArgumentCaptor<LogEvent> logCaptor;

@Mock
CloseableHttpClient httpClient;
@@ -69,8 +71,17 @@ public class TestDatadogHttpClient {
@Mock
StatusLine statusLine;

@BeforeEach
void prepareAppender() {
when(appender.getName()).thenReturn("MockAppender");
when(appender.isStarted()).thenReturn(true);
when(appender.isStopped()).thenReturn(false);
((Logger) LogManager.getLogger(DatadogHttpClient.class)).addAppender(appender);
}

@AfterEach
void resetMocks() {
((org.apache.logging.log4j.core.Logger) LogManager.getLogger(DatadogHttpClient.class)).removeAppender(appender);
reset(appender, httpClient, httpResponse, statusLine);
}

@@ -106,41 +117,41 @@ public void validateApiKeyShouldThrowExceptionWhenResponseNotSuccessful() {

@Test
public void sendPayloadShouldLogWhenRequestFailed() throws IOException {
Logger.getRootLogger().addAppender(appender);
((Logger) LogManager.getLogger(DatadogHttpClient.class)).addAppender(appender);
when(httpClient.execute(any())).thenThrow(IOException.class);

DatadogHttpClient ddClient = new DatadogHttpClient(ApiSite.US, "foo", true, httpClient);
ddClient.send("{}");

verify(appender).doAppend(logCaptor.capture());
assertEquals("Failed to send to Datadog.", logCaptor.getValue().getRenderedMessage());
verify(appender).append(logCaptor.capture());
assertEquals("Failed to send to Datadog.", logCaptor.getValue().getMessage().getFormattedMessage());
assertEquals(Level.WARN, logCaptor.getValue().getLevel());
}

@Test
public void sendPayloadShouldLogUnsuccessfulSending() {
Logger.getRootLogger().addAppender(appender);
((Logger) LogManager.getLogger(DatadogHttpClient.class)).addAppender(appender);
mockResponse(401);
when(httpResponse.toString()).thenReturn("unauthorized");

DatadogHttpClient ddClient = new DatadogHttpClient(ApiSite.US, "foo", true, httpClient);
ddClient.send("{}");

verify(appender).doAppend(logCaptor.capture());
assertEquals("Failed to send to Datadog. Response was unauthorized", logCaptor.getValue().getRenderedMessage());
verify(appender).append(logCaptor.capture());
assertEquals("Failed to send to Datadog. Response was unauthorized", logCaptor.getValue().getMessage().getFormattedMessage());
assertEquals(Level.WARN, logCaptor.getValue().getLevel());
}

@Test
public void sendPayloadShouldLogSuccessfulSending() {
Logger.getRootLogger().addAppender(appender);
((Logger) LogManager.getLogger(DatadogHttpClient.class)).addAppender(appender);
mockResponse(202);

DatadogHttpClient ddClient = new DatadogHttpClient(ApiSite.US, "foo", true, httpClient);
ddClient.send("{}");

verify(appender).doAppend(logCaptor.capture());
assertTrue(logCaptor.getValue().getRenderedMessage().startsWith("Sent metrics data"));
verify(appender).append(logCaptor.capture());
assertTrue(logCaptor.getValue().getMessage().getFormattedMessage().startsWith("Sent metrics data"));
assertEquals(Level.DEBUG, logCaptor.getValue().getLevel());
}

@@ -24,10 +24,11 @@

import com.codahale.metrics.MetricFilter;
import com.codahale.metrics.MetricRegistry;
import org.apache.log4j.AppenderSkeleton;
import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.log4j.spi.LoggingEvent;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.core.Appender;
import org.apache.logging.log4j.core.LogEvent;
import org.apache.logging.log4j.core.Logger;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
@@ -44,15 +45,16 @@
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;

@ExtendWith(MockitoExtension.class)
public class TestDatadogReporter {

@Mock
AppenderSkeleton appender;
Appender appender;

@Captor
ArgumentCaptor<LoggingEvent> logCaptor;
ArgumentCaptor<LogEvent> logCaptor;

@Mock
MetricRegistry registry;
@@ -62,6 +64,7 @@ public class TestDatadogReporter {

@AfterEach
void resetMocks() {
((Logger) LogManager.getLogger(DatadogReporter.class)).removeAppender(appender);
reset(appender, registry, client);
}

@@ -75,14 +78,18 @@ public void stopShouldCloseEnclosedClient() throws IOException {

@Test
public void stopShouldLogWhenEnclosedClientFailToClose() throws IOException {
Logger.getRootLogger().addAppender(appender);
when(appender.getName()).thenReturn("MockAppender");
when(appender.isStarted()).thenReturn(true);
when(appender.isStopped()).thenReturn(false);
((Logger) LogManager.getLogger(DatadogReporter.class)).addAppender(appender);

doThrow(IOException.class).when(client).close();

new DatadogReporter(registry, client, "foo", Option.empty(), Option.empty(),
MetricFilter.ALL, TimeUnit.SECONDS, TimeUnit.SECONDS).stop();

verify(appender).doAppend(logCaptor.capture());
assertEquals("Error disconnecting from Datadog.", logCaptor.getValue().getRenderedMessage());
verify(appender).append(logCaptor.capture());
assertEquals("Error disconnecting from Datadog.", logCaptor.getValue().getMessage().getFormattedMessage());
assertEquals(Level.WARN, logCaptor.getValue().getLevel());
}

@@ -63,8 +63,16 @@ See https://github.com/GoogleCloudPlatform/cloud-opensource-java/wiki/The-Google

<!-- Logging -->
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-1.2-api</artifactId>
</dependency>

<dependency>
@@ -165,8 +165,16 @@

<!-- Logging -->
<dependency>
<groupId>log4j</groupId>
<artifactId>log4j</artifactId>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-api</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-core</artifactId>
</dependency>
<dependency>
<groupId>org.apache.logging.log4j</groupId>
<artifactId>log4j-1.2-api</artifactId>
</dependency>

<dependency>

0 comments on commit fd7d25a

Please sign in to comment.