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

Revert "FIR-32836: log is simplified now - no lombok in log (#405)" #444

Merged
merged 3 commits into from
Jul 18, 2024
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
1 change: 0 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,6 @@ dependencies {
implementation 'net.jodah:expiringmap:0.5.11'
implementation 'org.apache.commons:commons-text:1.12.0'
implementation 'org.lz4:lz4-java:1.8.0'
implementation 'org.slf4j:jul-to-slf4j:2.0.13'

implementation fileTree(dir: 'libs', includes: ['*.jar'])

Expand Down
1 change: 1 addition & 0 deletions lombok.config
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
lombok.anyConstructor.addConstructorProperties = true
config.stopBubbling = true
lombok.addLombokGeneratedAnnotation = true
lombok.log.custom.declaration = com.firebolt.jdbc.log.FireboltLogger com.firebolt.jdbc.util.LoggerUtil.getLogger(NAME)
2 changes: 2 additions & 0 deletions src/integrationTest/java/integration/IntegrationTest.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package integration;

import com.firebolt.jdbc.client.HttpClientConfig;
import lombok.CustomLog;
import lombok.SneakyThrows;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.TestInstance;
Expand All @@ -16,6 +17,7 @@
import static com.firebolt.jdbc.connection.FireboltConnectionUserPassword.SYSTEM_ENGINE_NAME;
import static org.junit.jupiter.api.Assertions.assertNotNull;

@CustomLog
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@Tag("common")
public abstract class IntegrationTest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.firebolt.jdbc.type.FireboltDataType;
import com.firebolt.jdbc.type.array.FireboltArray;
import integration.IntegrationTest;
import lombok.CustomLog;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand All @@ -26,6 +27,7 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNull;

@CustomLog
class PreparedStatementArrayTest extends IntegrationTest {
enum PreparedStatementValueSetter {
ARRAY {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import integration.ConnectionInfo;
import integration.IntegrationTest;
import lombok.Builder;
import lombok.CustomLog;
import lombok.EqualsAndHashCode;
import lombok.Value;
import org.junit.jupiter.api.AfterEach;
Expand All @@ -32,6 +33,7 @@
import java.sql.ResultSet;
import java.sql.SQLException;
import java.sql.Statement;
import java.sql.Time;
import java.sql.Timestamp;
import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -49,6 +51,7 @@
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

@CustomLog
class PreparedStatementTest extends IntegrationTest {

@BeforeEach
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import com.firebolt.jdbc.statement.FireboltStatement;
import integration.EnvironmentCondition;
import integration.IntegrationTest;
import lombok.CustomLog;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
Expand All @@ -16,15 +17,13 @@
import java.sql.SQLException;
import java.sql.Statement;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;

import static integration.EnvironmentCondition.Attribute.databaseVersion;
import static integration.EnvironmentCondition.Comparison.GE;
import static org.junit.jupiter.api.Assertions.assertEquals;

@CustomLog
class StatementCancelTest extends IntegrationTest {
private static final Logger log = Logger.getLogger(StatementCancelTest.class.getName());

@BeforeEach
void beforeEach() {
Expand Down Expand Up @@ -92,11 +91,11 @@ private void verifyThatNoMoreRecordsAreAdded(Connection connection, String table
// data is available.
long waitForResultTime = insertTime / 2;
long waitForResultDelay = waitForResultTime / 10;
log.log(Level.INFO, "verifyThatNoMoreRecordsAreAdded insertTime={0}, waitForResultTime={0}", new Object[] {insertTime, waitForResultTime});
log.info("verifyThatNoMoreRecordsAreAdded insertTime={}, waitForResultTime={}", insertTime, waitForResultTime);
int count0;
int i = 0;
for (count0 = count(connection, tableName); i < 10; count0 = count(connection, tableName), i++) {
log.log(Level.INFO, "verifyThatNoMoreRecordsAreAdded count0={0}", count0);
log.info("verifyThatNoMoreRecordsAreAdded count0={}", count0);
if (count0 > 0) {
break;
}
Expand All @@ -109,7 +108,7 @@ private void verifyThatNoMoreRecordsAreAdded(Connection connection, String table
int count1 = count(connection, tableName);
Thread.sleep(insertTime); // waiting to see if more records are being added
int count2 = count(connection, tableName);
log.log(Level.INFO, "verifyThatNoMoreRecordsAreAdded count1={0}, count2={1}", new Object[] {count1, count2});
log.info("verifyThatNoMoreRecordsAreAdded count1={}, count2={}", count1, count2);
assertEquals(count1, count2);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import integration.EnvironmentCondition;
import integration.IntegrationTest;
import kotlin.collections.ArrayDeque;
import lombok.CustomLog;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
Expand Down Expand Up @@ -42,6 +43,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

@CustomLog
class StatementTest extends IntegrationTest {

@BeforeEach
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import integration.ConnectionInfo;
import integration.EnvironmentCondition;
import integration.IntegrationTest;
import lombok.CustomLog;
import org.junit.Assert;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
Expand All @@ -33,8 +34,6 @@
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Stream;

import static com.firebolt.jdbc.connection.FireboltConnectionUserPassword.SYSTEM_ENGINE_NAME;
Expand All @@ -49,6 +48,7 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

@CustomLog
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
public class SystemEngineTest extends IntegrationTest {

Expand All @@ -62,14 +62,12 @@ public class SystemEngineTest extends IntegrationTest {
private static final String TABLE1 = TABLE + "_1";
private static final String TABLE2 = TABLE + "_2";

private static final Logger log = Logger.getLogger(SystemEngineTest.class.getName());

@BeforeAll
void beforeAll() {
try {
executeStatementFromFile("/statements/system/ddl.sql", getSystemEngineName());
} catch (Exception e) {
log.log(Level.WARNING, "Could not execute statement", e);
log.warn("Could not execute statement", e);
}
}

Expand All @@ -78,7 +76,7 @@ void afterAll() {
try {
executeStatementFromFile("/statements/system/cleanup.sql", getSystemEngineName());
} catch (Exception e) {
log.log(Level.WARNING, "Could not execute statement", e);
log.warn("Could not execute statement", e);
}
}

Expand Down Expand Up @@ -112,6 +110,7 @@ void shouldThrowExceptionWhenExecutingWrongQueryWithJsonError() throws SQLExcept
}

@Test
@Tag("v2")
void shouldFailToSelectFromCustomDbUsingSystemEngine() throws SQLException {
ConnectionInfo current = integration.ConnectionInfo.getInstance();
String systemEngineJdbcUrl = new ConnectionInfo(current.getPrincipal(), current.getSecret(),
Expand Down Expand Up @@ -178,7 +177,7 @@ void useDatabase(String entityType) throws SQLException {
try (Statement statement = connection.createStatement()) {
statement.executeUpdate(query);
} catch (SQLException e) { // catch just in case to do our best to clean everything even if test has failed
log.log(Level.WARNING, "Cannot perform query " + query, e);
log.warn("Cannot perform query {}", query, e);
}
}
}
Expand Down Expand Up @@ -342,8 +341,9 @@ void shouldExecuteEngineManagementQueries() throws SQLException {
format("DROP DATABASE \"%s\"", SECOND_DATABASE_NAME)}) {
try (Statement statement = connection.createStatement()) {
statement.executeUpdate(query);
} catch (SQLException e) { // catch just in case to do our best to clean everything even if test has failed
log.log(Level.WARNING, "Cannot perform query " + query, e);
} catch (
SQLException e) { // catch just in case to do our best to clean everything even if test has failed
log.warn("Cannot perform query {}", query, e);
}
}
}
Expand Down
7 changes: 3 additions & 4 deletions src/integrationTest/java/integration/tests/TimeoutTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.firebolt.jdbc.connection.FireboltConnection;
import integration.EnvironmentCondition;
import integration.IntegrationTest;
import lombok.CustomLog;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Tag;
Expand All @@ -14,18 +15,16 @@
import java.sql.Statement;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;

import static integration.EnvironmentCondition.Attribute.databaseVersion;
import static integration.EnvironmentCondition.Comparison.GE;
import static java.lang.String.format;
import static org.junit.jupiter.api.Assertions.assertTrue;

@CustomLog
class TimeoutTest extends IntegrationTest {
private static final int MIN_TIME_SECONDS = 350;
private static final Map<Integer, Long> SERIES_SIZE = Map.of(1, 80000000000L, 2, 180000000000L);
private static final Logger log = Logger.getLogger(TimeoutTest.class.getName());
private long startTime;

@BeforeEach
Expand All @@ -37,7 +36,7 @@ void before() {
void after() {
long endTime = System.nanoTime();
long elapsedTimeSeconds = (endTime - startTime) / 1_000_000_000;
log.log(Level.INFO, "Time elapsed: {0} seconds", elapsedTimeSeconds);
log.info("Time elapsed: {} seconds", elapsedTimeSeconds);
assertTrue(elapsedTimeSeconds > MIN_TIME_SECONDS, format("Test is too short. It took %d but should take at least %d seconds", elapsedTimeSeconds, MIN_TIME_SECONDS));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.firebolt.jdbc.testutils.AssertionUtil;
import integration.IntegrationTest;
import io.zonky.test.db.postgres.embedded.EmbeddedPostgres;
import lombok.CustomLog;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -36,6 +37,7 @@
import static java.sql.Types.TIMESTAMP_WITH_TIMEZONE;
import static org.junit.jupiter.api.Assertions.assertEquals;

@CustomLog
@DefaultTimeZone("UTC")
class TimestampTest extends IntegrationTest {
private static final TimeZone UTC_TZ = TimeZone.getTimeZone("UTC");
Expand Down
13 changes: 6 additions & 7 deletions src/main/java/com/firebolt/FireboltDriver.java
Original file line number Diff line number Diff line change
@@ -1,28 +1,27 @@
package com.firebolt;

import com.firebolt.jdbc.connection.FireboltConnection;
import com.firebolt.jdbc.util.LoggerUtil;
import com.firebolt.jdbc.exception.FireboltSQLFeatureNotSupportedException;
import com.firebolt.jdbc.util.PropertyUtil;
import com.firebolt.jdbc.util.VersionUtil;
import lombok.CustomLog;

import java.sql.Connection;
import java.sql.Driver;
import java.sql.DriverPropertyInfo;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.util.Properties;
import java.util.logging.Logger;

@CustomLog
public class FireboltDriver implements Driver {

public static final String JDBC_FIREBOLT = "jdbc:firebolt:";
private static final Logger rootLog;
private static final Logger log;

static {
try {
java.sql.DriverManager.registerDriver(new FireboltDriver());
rootLog = LoggerUtil.getRootLogger();
log = Logger.getLogger(FireboltDriver.class.getName());
log.info("Firebolt Driver registered");
} catch (SQLException ex) {
throw new RuntimeException("Cannot register the driver");
Expand Down Expand Up @@ -60,7 +59,7 @@ public boolean jdbcCompliant() {
}

@Override
public Logger getParentLogger() {
return rootLog;
public Logger getParentLogger() throws SQLFeatureNotSupportedException {
throw new FireboltSQLFeatureNotSupportedException();
}
}
9 changes: 4 additions & 5 deletions src/main/java/com/firebolt/jdbc/client/FireboltClient.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.firebolt.jdbc.exception.ServerError.Error.Location;
import com.firebolt.jdbc.resultset.compress.LZ4InputStream;
import com.firebolt.jdbc.util.CloseableUtil;
import lombok.CustomLog;
import lombok.Getter;
import lombok.NonNull;
import okhttp3.Call;
Expand Down Expand Up @@ -48,13 +49,12 @@
import static java.util.Optional.ofNullable;

@Getter
@CustomLog
public abstract class FireboltClient implements CacheListener {

private static final String HEADER_AUTHORIZATION = "Authorization";
private static final String HEADER_AUTHORIZATION_BEARER_PREFIX_VALUE = "Bearer ";
private static final String HEADER_USER_AGENT = "User-Agent";
private static final String HEADER_PROTOCOL_VERSION = "Firebolt-Protocol-Version";
private static final Logger log = Logger.getLogger(FireboltClient.class.getName());
private static final Pattern plainErrorPattern = Pattern.compile("Line (\\d+), Column (\\d+): (.*)$", Pattern.MULTILINE);
private final OkHttpClient httpClient;
private final String headerUserAgentValue;
Expand Down Expand Up @@ -181,13 +181,12 @@ protected String getResponseAsString(Response response) throws SQLException, IOE
return response.body().string();
}

@SuppressWarnings("java:S2139") // TODO: Exceptions should be either logged or rethrown but not both
private String extractErrorMessage(Response response, boolean isCompress) throws SQLException {
byte[] entityBytes;
try {
entityBytes = response.body() != null ? response.body().bytes() : null;
} catch (IOException e) {
log.log(Level.WARNING, "Could not parse response containing the error message from Firebolt", e);
log.warn("Could not parse response containing the error message from Firebolt", e);
String errorResponseMessage = format("Server failed to execute query%ninternal error:%n%s",
getInternalErrorWithHeadersText(response));
throw new FireboltException(errorResponseMessage, response.code(), e);
Expand All @@ -202,7 +201,7 @@ private String extractErrorMessage(Response response, boolean isCompress) throws
return new BufferedReader(new InputStreamReader(is, StandardCharsets.UTF_8)).lines()
.collect(Collectors.joining("\n")) + "\n";
} catch (Exception e) {
log.log(Level.WARNING, "Could not decompress error from server");
log.warn("Could not decompress error from server");
}
}
return new String(entityBytes, StandardCharsets.UTF_8);
Expand Down
14 changes: 8 additions & 6 deletions src/main/java/com/firebolt/jdbc/client/HttpClientConfig.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,20 @@
package com.firebolt.jdbc.client;

import com.firebolt.jdbc.client.config.OkHttpClientCreator;
import com.firebolt.jdbc.connection.settings.FireboltProperties;
import okhttp3.OkHttpClient;

import java.io.IOException;
import java.security.KeyManagementException;
import java.security.KeyStoreException;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;
import java.util.logging.Logger;

import com.firebolt.jdbc.client.config.OkHttpClientCreator;
import com.firebolt.jdbc.connection.settings.FireboltProperties;

import lombok.CustomLog;
import okhttp3.OkHttpClient;

@CustomLog
public class HttpClientConfig {
private static final Logger log = Logger.getLogger(HttpClientConfig.class.getName());

private static OkHttpClient instance;

private HttpClientConfig() {
Expand Down
Loading
Loading