diff --git a/src/main/java/com/databricks/jdbc/exception/DatabricksBatchUpdateException.java b/src/main/java/com/databricks/jdbc/exception/DatabricksBatchUpdateException.java index a9554f1e68..484cabc3e3 100644 --- a/src/main/java/com/databricks/jdbc/exception/DatabricksBatchUpdateException.java +++ b/src/main/java/com/databricks/jdbc/exception/DatabricksBatchUpdateException.java @@ -24,7 +24,7 @@ public DatabricksBatchUpdateException( exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), internalErrorCode.toString(), - reason, + this, TelemetryLogLevel.ERROR); } @@ -39,7 +39,7 @@ public DatabricksBatchUpdateException( exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), SQLState, - reason, + this, TelemetryLogLevel.ERROR); } diff --git a/src/main/java/com/databricks/jdbc/exception/DatabricksDriverException.java b/src/main/java/com/databricks/jdbc/exception/DatabricksDriverException.java index f7a2b57fe9..a8a8890c92 100644 --- a/src/main/java/com/databricks/jdbc/exception/DatabricksDriverException.java +++ b/src/main/java/com/databricks/jdbc/exception/DatabricksDriverException.java @@ -22,7 +22,7 @@ public DatabricksDriverException(String reason, Throwable cause, String sqlState exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), sqlState, - reason, + this, TelemetryLogLevel.ERROR); } @@ -31,7 +31,7 @@ public DatabricksDriverException(String reason, String sqlState) { exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), sqlState, - reason, + this, TelemetryLogLevel.ERROR); } } diff --git a/src/main/java/com/databricks/jdbc/exception/DatabricksSQLException.java b/src/main/java/com/databricks/jdbc/exception/DatabricksSQLException.java index ed84e87878..74e788716c 100644 --- a/src/main/java/com/databricks/jdbc/exception/DatabricksSQLException.java +++ b/src/main/java/com/databricks/jdbc/exception/DatabricksSQLException.java @@ -35,7 +35,7 @@ public DatabricksSQLException( exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), sqlState, - reason, + this, statementId, chunkIndex, TelemetryLogLevel.ERROR); @@ -60,7 +60,7 @@ public DatabricksSQLException( DatabricksDriverErrorCode internalError, boolean silentExceptions) { super(reason, sqlState, internalError.getCode()); - logTelemetryEvent(sqlState, reason, silentExceptions); + logTelemetryEvent(sqlState, silentExceptions); } public DatabricksSQLException(String reason, String sqlState, int vendorCode) { @@ -70,20 +70,20 @@ public DatabricksSQLException(String reason, String sqlState, int vendorCode) { public DatabricksSQLException( String reason, String sqlState, int vendorCode, boolean silentExceptions) { super(reason, sqlState, vendorCode); - logTelemetryEvent(sqlState, reason, silentExceptions); + logTelemetryEvent(sqlState, silentExceptions); } public DatabricksSQLException(String reason, String sqlState, int vendorCode, Throwable cause) { super(reason, sqlState, vendorCode, cause); - logTelemetryEvent(sqlState, reason, false); + logTelemetryEvent(sqlState, false); } - private void logTelemetryEvent(String sqlState, String reason, boolean silentExceptions) { + private void logTelemetryEvent(String sqlState, boolean silentExceptions) { if (!silentExceptions) { exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), sqlState, - reason, + this, TelemetryLogLevel.ERROR); } else { // These are errors that are thrown to call a fallback method (e.g. metadata column not @@ -92,7 +92,7 @@ private void logTelemetryEvent(String sqlState, String reason, boolean silentExc exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), sqlState, - reason, + this, TelemetryLogLevel.TRACE); } } diff --git a/src/main/java/com/databricks/jdbc/exception/DatabricksTimeoutException.java b/src/main/java/com/databricks/jdbc/exception/DatabricksTimeoutException.java index 66f58cc02d..da407982b5 100644 --- a/src/main/java/com/databricks/jdbc/exception/DatabricksTimeoutException.java +++ b/src/main/java/com/databricks/jdbc/exception/DatabricksTimeoutException.java @@ -15,7 +15,7 @@ public DatabricksTimeoutException( TelemetryHelper.exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), internalError.name(), - reason, + this, TelemetryLogLevel.ERROR); } @@ -25,7 +25,7 @@ public DatabricksTimeoutException( TelemetryHelper.exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), internalError.name(), - reason, + this, TelemetryLogLevel.ERROR); } } diff --git a/src/main/java/com/databricks/jdbc/exception/DatabricksTransactionException.java b/src/main/java/com/databricks/jdbc/exception/DatabricksTransactionException.java index 893bffe154..12228c46bb 100644 --- a/src/main/java/com/databricks/jdbc/exception/DatabricksTransactionException.java +++ b/src/main/java/com/databricks/jdbc/exception/DatabricksTransactionException.java @@ -36,7 +36,7 @@ public class DatabricksTransactionException extends SQLException { public DatabricksTransactionException( String reason, String sqlState, int vendorCode, Throwable cause) { super(reason, sqlState, vendorCode, cause); - logTelemetryEvent(sqlState, reason); + logTelemetryEvent(sqlState); } /** @@ -98,11 +98,11 @@ private static int getVendorCodeFromCause(Throwable cause) { return DatabricksVendorCode.getVendorCode(cause); } - private void logTelemetryEvent(String sqlState, String reason) { + private void logTelemetryEvent(String sqlState) { exportFailureLog( DatabricksThreadContextHolder.getConnectionContext(), sqlState, - reason, + this, TelemetryLogLevel.ERROR); } } diff --git a/src/main/java/com/databricks/jdbc/telemetry/TelemetryHelper.java b/src/main/java/com/databricks/jdbc/telemetry/TelemetryHelper.java index 4e45cfe131..47d7ef12c5 100644 --- a/src/main/java/com/databricks/jdbc/telemetry/TelemetryHelper.java +++ b/src/main/java/com/databricks/jdbc/telemetry/TelemetryHelper.java @@ -30,8 +30,10 @@ import com.google.common.annotations.VisibleForTesting; import java.nio.charset.Charset; import java.time.Instant; +import java.util.Arrays; import java.util.UUID; import java.util.concurrent.ConcurrentHashMap; +import java.util.stream.Collectors; public class TelemetryHelper { private static final JdbcLogger LOGGER = JdbcLoggerFactory.getLogger(TelemetryHelper.class); @@ -40,6 +42,9 @@ public class TelemetryHelper { private static final ConcurrentHashMap connectionParameterCache = new ConcurrentHashMap<>(); private static final String APP_NAME_SYSTEM_PROPERTY = "app.name"; + // Upper bound on the number of stack frames serialized into the telemetry stack_trace field, + // to keep payloads bounded for deep stacks. + private static final int MAX_STACK_TRACE_FRAMES = 100; @VisibleForTesting static final String TELEMETRY_FEATURE_FLAG_NAME = @@ -134,22 +139,22 @@ private static void exportTelemetryEvent( public static void exportFailureLog( IDatabricksConnectionContext connectionContext, String errorName, - String errorMessage, + Throwable throwable, TelemetryLogLevel logLevel) { String statementId = DatabricksThreadContextHolder.getStatementId(); exportFailureLog( - connectionContext, errorName, errorMessage, statementId, /* chunkIndex */ null, logLevel); + connectionContext, errorName, throwable, statementId, /* chunkIndex */ null, logLevel); } public static void exportFailureLog( IDatabricksConnectionContext connectionContext, String errorName, - String errorMessage, + Throwable throwable, String statementId, Long chunkIndex, TelemetryLogLevel logLevel) { DriverErrorInfo errorInfo = - new DriverErrorInfo().setErrorName(errorName).setStackTrace(errorMessage); + new DriverErrorInfo().setErrorName(errorName).setStackTrace(formatStackTrace(throwable)); StatementTelemetryDetails telemetryDetails; if (statementId == null) { telemetryDetails = new StatementTelemetryDetails(null); @@ -161,6 +166,29 @@ public static void exportFailureLog( exportTelemetryEvent(connectionContext, telemetryDetails, errorInfo, chunkIndex, logLevel); } + /** + * Formats a throwable's stack frames (class, method, file, line) into a newline-separated string + * for the telemetry {@code stack_trace} field. + * + *

Only code locations are serialized — never the throwable's message. Exception messages in + * the driver routinely embed user data (SQL text, hostnames, literal filter values), so excluding + * the message keeps that PII out of telemetry by construction. + * + * @param throwable the throwable whose frames to format; may be {@code null} + * @return the newline-separated frames (capped at {@link #MAX_STACK_TRACE_FRAMES}), or {@code + * null} if {@code throwable} is {@code null} + */ + @VisibleForTesting + static String formatStackTrace(Throwable throwable) { + if (throwable == null) { + return null; + } + return Arrays.stream(throwable.getStackTrace()) + .limit(MAX_STACK_TRACE_FRAMES) + .map(StackTraceElement::toString) + .collect(Collectors.joining("\n")); + } + public static String getStatementIdString(StatementId statementId) { return statementId != null ? statementId.toSQLExecStatementId() diff --git a/src/test/java/com/databricks/jdbc/telemetry/TelemetryHelperTest.java b/src/test/java/com/databricks/jdbc/telemetry/TelemetryHelperTest.java index 09672c673d..1117c14ac0 100644 --- a/src/test/java/com/databricks/jdbc/telemetry/TelemetryHelperTest.java +++ b/src/test/java/com/databricks/jdbc/telemetry/TelemetryHelperTest.java @@ -82,7 +82,10 @@ void testErrorTelemetryToNoAuthTelemetryClientDoesNotThrowError() { assertDoesNotThrow( () -> TelemetryHelper.exportFailureLog( - connectionContext, TEST_STRING, TEST_STRING, TelemetryLogLevel.ERROR)); + connectionContext, + TEST_STRING, + new RuntimeException(TEST_STRING), + TelemetryLogLevel.ERROR)); } } @@ -91,6 +94,36 @@ void testGetDriverSystemConfigurationDoesNotThrowError() { assertDoesNotThrow(TelemetryHelper::getDriverSystemConfiguration); } + @Test + void testFormatStackTraceIncludesFramesButNotMessage() { + // Exception messages in the driver can embed user data (SQL text, literal values); the + // telemetry stack_trace field must carry code locations only, never the message. + String sensitiveMessage = "Failed query: SELECT ssn FROM patients WHERE ssn = '123-45-6789'"; + Throwable throwable = new RuntimeException(sensitiveMessage); + + String formatted = TelemetryHelper.formatStackTrace(throwable); + + assertNotNull(formatted); + // Frames are present: this test method's frame is at the top of the captured trace. + assertTrue( + formatted.contains("TelemetryHelperTest"), + "stack_trace should contain code-location frames"); + assertTrue( + formatted.contains("testFormatStackTraceIncludesFramesButNotMessage"), + "stack_trace should include the frame where the throwable was created"); + // The message (and any PII in it) must never appear. + assertFalse( + formatted.contains(sensitiveMessage), + "stack_trace must not contain the exception message"); + assertFalse( + formatted.contains("123-45-6789"), "stack_trace must not contain PII from the message"); + } + + @Test + void testFormatStackTraceWithNullReturnsNull() { + assertNull(TelemetryHelper.formatStackTrace(null)); + } + @ParameterizedTest @MethodSource("failureLogParameters") void testExportFailureLogWithVariousParameters(String statementId, Long chunkIndex) { @@ -199,7 +232,9 @@ void testExportFailureLogWithNullContextAndNullStatementId() { DatabricksThreadContextHolder.clearConnectionContext(); DatabricksThreadContextHolder.clearStatementInfo(); assertDoesNotThrow( - () -> TelemetryHelper.exportFailureLog(null, "err", "msg", TelemetryLogLevel.ERROR)); + () -> + TelemetryHelper.exportFailureLog( + null, "err", new RuntimeException("msg"), TelemetryLogLevel.ERROR)); } @Test