-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(bqjdbc): optimize formatter in BigQueryJdbcRootLogger #12877
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
|
|
||
| package com.google.cloud.bigquery.jdbc; | ||
|
|
||
| import com.google.common.base.Strings; | ||
| import java.io.IOException; | ||
| import java.lang.management.ManagementFactory; | ||
| import java.nio.file.Files; | ||
|
|
@@ -24,7 +25,6 @@ | |
| import java.nio.file.StandardCopyOption; | ||
| import java.text.SimpleDateFormat; | ||
| import java.util.Date; | ||
| import java.util.Optional; | ||
| import java.util.logging.ConsoleHandler; | ||
| import java.util.logging.FileHandler; | ||
| import java.util.logging.Formatter; | ||
|
|
@@ -49,6 +49,25 @@ class BigQueryJdbcRootLogger { | |
| private static Path currentLogPath = null; | ||
| private static int fileCounter = 0; | ||
|
|
||
| static final long PROCESS_ID = | ||
| Long.parseLong(ManagementFactory.getRuntimeMXBean().getName().split("@")[0]); | ||
|
|
||
| static String getThreadName(long threadId) { | ||
| Thread current = Thread.currentThread(); | ||
| if (current.getId() == threadId) { | ||
| return current.getName(); | ||
| } | ||
| int count = Thread.activeCount(); | ||
| Thread[] threads = new Thread[count * 2]; | ||
| int actualCount = Thread.enumerate(threads); | ||
|
Neenu1995 marked this conversation as resolved.
Outdated
|
||
| for (int i = 0; i < actualCount; i++) { | ||
| if (threads[i].getId() == threadId) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe worth adding local cache for id->name?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This fallback is an expensive, low-frequency edge case. The memory overhead of maintaining the cache exceeds the performance gains from faster lookups. |
||
| return threads[i].getName(); | ||
| } | ||
| } | ||
| return ""; | ||
| } | ||
|
|
||
| static { | ||
| logger.setUseParentHandlers(false); | ||
| storageLogger.setUseParentHandlers(true); | ||
|
|
@@ -62,49 +81,41 @@ class BigQueryJdbcRootLogger { | |
|
|
||
| public static Formatter getFormatter() { | ||
| return new Formatter() { | ||
| private static final String PATTERN = "yyyy-MM-dd HH:mm:ss.SSS"; | ||
| private static final String FORMAT = | ||
| "%1$s %2$5s %3$d --- [%4$-7.15s] %5$-50s %6$-20s: %7$s%8$s"; | ||
| private final ThreadLocal<SimpleDateFormat> dateFormatter = | ||
| ThreadLocal.withInitial(() -> new SimpleDateFormat("yyyy-MM-dd HH:mm:ss.SSS")); | ||
| private static final int MAX_THREAD_NAME_LENGTH = 15; | ||
|
|
||
| /** | ||
| * Returns the thread for the given thread id. | ||
| * | ||
| * @param threadId ID for the thread being logged. | ||
| * @return returns the thread | ||
| */ | ||
| Optional<Thread> getThread(long threadId) { | ||
| return Thread.getAllStackTraces().keySet().stream() | ||
| .filter(thread -> thread.getId() == threadId) | ||
| .findFirst(); | ||
| } | ||
|
|
||
| @Override | ||
| public String format(LogRecord record) { | ||
| String date = new SimpleDateFormat(PATTERN).format(new Date(record.getMillis())); | ||
| String threadName = | ||
| getThread(record.getThreadID()) | ||
| .map(Thread::getName) | ||
| .map( | ||
| name -> | ||
| name.length() > MAX_THREAD_NAME_LENGTH | ||
| ? name.substring(name.length() - MAX_THREAD_NAME_LENGTH) | ||
| : name) | ||
| .orElse(""); | ||
| long processId = | ||
| Long.parseLong(ManagementFactory.getRuntimeMXBean().getName().split("@")[0]); | ||
| String date = dateFormatter.get().format(new Date(record.getMillis())); | ||
|
Neenu1995 marked this conversation as resolved.
Outdated
|
||
|
|
||
| long threadId = record.getThreadID(); | ||
| String threadName = getThreadName(threadId); | ||
|
|
||
| if (threadName.length() > MAX_THREAD_NAME_LENGTH) { | ||
| threadName = threadName.substring(threadName.length() - MAX_THREAD_NAME_LENGTH); | ||
|
logachev marked this conversation as resolved.
|
||
| } | ||
|
|
||
| String sourceClassName = record.getLoggerName(); | ||
| String sourceMethodName = record.getSourceMethodName(); | ||
| return String.format( | ||
| FORMAT, | ||
| date, | ||
| record.getLevel().getName(), | ||
| processId, | ||
| threadName, | ||
| sourceClassName, | ||
| sourceMethodName, | ||
| record.getMessage(), | ||
| System.lineSeparator()); | ||
|
|
||
| StringBuilder sb = new StringBuilder(256); | ||
| sb.append(date) | ||
|
Neenu1995 marked this conversation as resolved.
|
||
| .append(" ") | ||
| .append(Strings.padStart(record.getLevel().getName(), 5, ' ')) | ||
| .append(" ") | ||
| .append(PROCESS_ID) | ||
| .append(" --- [") | ||
| .append(Strings.padEnd(threadName, 7, ' ')) | ||
| .append("] ") | ||
| .append(Strings.padEnd(sourceClassName != null ? sourceClassName : "", 50, ' ')) | ||
| .append(" ") | ||
| .append(Strings.padEnd(sourceMethodName != null ? sourceMethodName : "", 20, ' ')) | ||
| .append(": ") | ||
| .append(record.getMessage()) | ||
| .append(System.lineSeparator()); | ||
|
|
||
| return sb.toString(); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| /* | ||
| * Copyright 2026 Google LLC | ||
| * | ||
| * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| * you may not use this file except in compliance with the License. | ||
| * You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, software | ||
| * distributed under the License is distributed on an "AS IS" BASIS, | ||
| * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
|
|
||
| package com.google.cloud.bigquery.jdbc; | ||
|
|
||
| import static org.junit.jupiter.api.Assertions.assertEquals; | ||
| import static org.junit.jupiter.api.Assertions.assertNotNull; | ||
| import static org.junit.jupiter.api.Assertions.assertTrue; | ||
|
|
||
| import java.util.logging.Formatter; | ||
| import java.util.logging.Level; | ||
| import java.util.logging.LogRecord; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| public class BigQueryJdbcRootLoggerTest { | ||
|
|
||
| @Test | ||
| public void testGetFormatterFormat() { | ||
| Formatter formatter = BigQueryJdbcRootLogger.getFormatter(); | ||
| assertNotNull(formatter); | ||
|
|
||
| LogRecord record = new LogRecord(Level.INFO, "Test message"); | ||
| record.setMillis(1713715200000L); | ||
| record.setLoggerName("TestLogger"); | ||
| record.setSourceMethodName("testMethod"); | ||
|
|
||
| String formatted = formatter.format(record); | ||
|
|
||
| assertTrue(formatted.contains("INFO")); | ||
| assertTrue(formatted.contains("Test message")); | ||
| assertTrue(formatted.contains("TestLogger")); | ||
| assertTrue(formatted.contains("testMethod")); | ||
| assertTrue(formatted.contains("---")); | ||
| } | ||
|
|
||
| @Test | ||
| public void testThreadNameTruncation() { | ||
| Formatter formatter = BigQueryJdbcRootLogger.getFormatter(); | ||
| LogRecord record = new LogRecord(Level.INFO, "Test message"); | ||
|
|
||
| String formatted = formatter.format(record); | ||
| int startIndex = formatted.indexOf("--- [") + 5; | ||
| int endIndex = formatted.indexOf("]", startIndex); | ||
| String threadPart = formatted.substring(startIndex, endIndex).trim(); | ||
|
|
||
| assertTrue(threadPart.length() <= 15); | ||
| } | ||
|
|
||
| @Test | ||
| public void testGetThreadName() { | ||
| Thread current = Thread.currentThread(); | ||
| String name = BigQueryJdbcRootLogger.getThreadName(current.getId()); | ||
| assertEquals(current.getName(), name); | ||
| } | ||
|
|
||
| @Test | ||
| public void testGetThreadNameNotFound() { | ||
| String name = BigQueryJdbcRootLogger.getThreadName(-1); | ||
| assertEquals("", name); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.