Skip to content

Commit 80ca302

Browse files
samikshya-dbsamikshya-db
authored
Fix passing of encoded userAgent string in BITool+ Add system configuration in every telemetry request (#940)
## Description - The intention of appName in telemetry is to understand BI tools using our driver. And we do not want that to break our UserAgent. This causes DBeaver to hang as it has a app name as `DBeaver%2F25.1.4.202508031529` (and encoded strings are not allowed in our userAgent check). This PR fixes that. - To create dashboards, we need system configuration in every telemetry row. This PR also adds that. ## Testing - Added unit tests - Also tested manually using DBeaver Client ## Additional Notes to the Reviewer - The userAgent is sent for all customers, and it may not be appropriate for us to send info like processName and appName as part of this. Moreover, UserAgent is supposed to be fixed for a JVM. The `unknown` string issue on thrift table will be addressed by telemetry now. (i.e., we will do a join between two tables to identify unknowns) - BI tool Error that is fixed in this PR : <img width="812" height="21" alt="Screenshot 2025-08-14 at 4 42 33 PM" src="https://github.com/user-attachments/assets/77a3c01d-0306-4535-95da-67ae73ecc82e" />
1 parent 1236e7f commit 80ca302

File tree

7 files changed

+164
-144
lines changed

7 files changed

+164
-144
lines changed

‎NEXT_CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,6 @@ AbstractArrowResultChunk
1414
- Fixed Statement.getUpdateCount to return -1 for non-DML queries.
1515
- Fixed Statement.setMaxRows(0) to be interepeted as no limit.
1616
- Fixed retry behaviour to not throw an exception when there is no retry-after header for 503 and 429 status codes.
17+
- Fixed encoded UserAgent parsing in BI tools.
1718
---
1819
*Note: When making changes, please add your change under the appropriate section with a brief description.*

‎src/main/java/com/databricks/jdbc/api/impl/DatabricksConnection.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.databricks.jdbc.log.JdbcLoggerFactory;
2424
import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode;
2525
import com.databricks.jdbc.telemetry.TelemetryClientFactory;
26+
import com.databricks.jdbc.telemetry.TelemetryHelper;
2627
import com.google.common.annotations.VisibleForTesting;
2728
import java.sql.*;
2829
import java.util.*;
@@ -58,6 +59,7 @@ public DatabricksConnection(
5859
DatabricksThreadContextHolder.setConnectionContext(connectionContext);
5960
this.session = new DatabricksSession(connectionContext, testDatabricksClient);
6061
UserAgentManager.setUserAgent(connectionContext);
62+
TelemetryHelper.updateTelemetryAppName(connectionContext, null);
6163
}
6264

6365
@Override

‎src/main/java/com/databricks/jdbc/api/impl/DatabricksSession.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import com.databricks.jdbc.common.DatabricksClientType;
1010
import com.databricks.jdbc.common.DatabricksJdbcUrlParams;
1111
import com.databricks.jdbc.common.IDatabricksComputeResource;
12-
import com.databricks.jdbc.common.util.UserAgentManager;
1312
import com.databricks.jdbc.dbclient.IDatabricksClient;
1413
import com.databricks.jdbc.dbclient.IDatabricksMetadataClient;
1514
import com.databricks.jdbc.dbclient.impl.sqlexec.DatabricksEmptyMetadataClient;
@@ -21,6 +20,7 @@
2120
import com.databricks.jdbc.exception.DatabricksTemporaryRedirectException;
2221
import com.databricks.jdbc.log.JdbcLogger;
2322
import com.databricks.jdbc.log.JdbcLoggerFactory;
23+
import com.databricks.jdbc.telemetry.TelemetryHelper;
2424
import com.databricks.jdbc.telemetry.latency.DatabricksMetricsTimedProcessor;
2525
import com.databricks.sdk.support.ToStringer;
2626
import com.google.common.annotations.VisibleForTesting;
@@ -266,7 +266,7 @@ public void setClientInfoProperty(String name, String value) {
266266

267267
// If application name is being set, update both telemetry and user agent
268268
if (name.equalsIgnoreCase(DatabricksJdbcUrlParams.APPLICATION_NAME.getParamName())) {
269-
UserAgentManager.updateUserAgentAndTelemetry(connectionContext, value);
269+
TelemetryHelper.updateTelemetryAppName(connectionContext, value);
270270
}
271271

272272
clientInfoProperties.put(name, value);

‎src/main/java/com/databricks/jdbc/common/util/UserAgentManager.java‎

Lines changed: 20 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
11
package com.databricks.jdbc.common.util;
22

3-
import static com.databricks.jdbc.common.util.WildcardUtil.isNullOrEmpty;
4-
53
import com.databricks.jdbc.api.internal.IDatabricksConnectionContext;
6-
import com.databricks.jdbc.telemetry.TelemetryHelper;
4+
import com.databricks.jdbc.log.JdbcLogger;
5+
import com.databricks.jdbc.log.JdbcLoggerFactory;
76
import com.databricks.sdk.core.UserAgent;
7+
import java.net.URLDecoder;
8+
import java.nio.charset.StandardCharsets;
89

910
public class UserAgentManager {
11+
private static final JdbcLogger LOGGER = JdbcLoggerFactory.getLogger(UserAgentManager.class);
1012
private static final String SDK_USER_AGENT = "databricks-sdk-java";
1113
private static final String JDBC_HTTP_USER_AGENT = "databricks-jdbc-http";
1214
private static final String DEFAULT_USER_AGENT = "DatabricksJDBCDriverOSS";
1315
private static final String CLIENT_USER_AGENT_PREFIX = "Java";
1416
public static final String USER_AGENT_SEA_CLIENT = "SQLExecHttpClient";
1517
public static final String USER_AGENT_THRIFT_CLIENT = "THttpClient";
16-
private static final String APP_NAME_SYSTEM_PROPERTY = "app.name";
1718
private static final String VERSION_FILLER = "version";
1819

1920
/**
@@ -25,61 +26,23 @@ public static void setUserAgent(IDatabricksConnectionContext connectionContext)
2526
// Set the base product and client info
2627
UserAgent.withProduct(DEFAULT_USER_AGENT, DriverUtil.getDriverVersion());
2728
UserAgent.withOtherInfo(CLIENT_USER_AGENT_PREFIX, connectionContext.getClientUserAgent());
28-
29-
// Update application name for both telemetry and user agent
30-
updateUserAgentAndTelemetry(connectionContext, null);
31-
}
32-
33-
/**
34-
* Determines the application name using a fallback mechanism: 1. useragententry url param 2.
35-
* applicationname url param 3. client info property "applicationname" 4. System property app.name
36-
*
37-
* @param connectionContext The connection context
38-
* @param clientInfoAppName The application name from client info properties, can be null
39-
* @return The determined application name or null if none is found
40-
*/
41-
static String determineApplicationName(
42-
IDatabricksConnectionContext connectionContext, String clientInfoAppName) {
43-
// First check URL params
44-
String appName = connectionContext.getCustomerUserAgent();
45-
if (!isNullOrEmpty(appName)) {
46-
return appName;
47-
}
48-
49-
// Then check application name URL param
50-
appName = connectionContext.getApplicationName();
51-
if (!isNullOrEmpty(appName)) {
52-
return appName;
29+
if (connectionContext.getCustomerUserAgent() == null) {
30+
return;
5331
}
54-
55-
// Then check client info property
56-
if (!isNullOrEmpty(clientInfoAppName)) {
57-
return clientInfoAppName;
58-
}
59-
60-
// Finally check system property
61-
return System.getProperty(APP_NAME_SYSTEM_PROPERTY);
62-
}
63-
64-
/**
65-
* Updates both the telemetry client app name and HTTP user agent headers. To be called during
66-
* connection initialization and when app name changes.
67-
*
68-
* @param connectionContext The connection context
69-
* @param clientInfoAppName Optional client info app name, can be null
70-
*/
71-
public static void updateUserAgentAndTelemetry(
72-
IDatabricksConnectionContext connectionContext, String clientInfoAppName) {
73-
String appName = determineApplicationName(connectionContext, clientInfoAppName);
74-
if (!isNullOrEmpty(appName)) {
75-
// Update telemetry
76-
TelemetryHelper.updateClientAppName(appName);
77-
78-
// Update HTTP user agent
79-
int i = appName.indexOf('/');
80-
String customerName = (i < 0) ? appName : appName.substring(0, i);
81-
String customerVersion = (i < 0) ? VERSION_FILLER : appName.substring(i + 1);
32+
try {
33+
String decodedUA =
34+
URLDecoder.decode(
35+
connectionContext.getCustomerUserAgent(),
36+
StandardCharsets.UTF_8); // This is for encoded userAgentString
37+
int i = decodedUA.indexOf('/');
38+
String customerName = (i < 0) ? decodedUA : decodedUA.substring(0, i);
39+
String customerVersion = (i < 0) ? VERSION_FILLER : decodedUA.substring(i + 1);
8240
UserAgent.withOtherInfo(customerName, UserAgent.sanitize(customerVersion));
41+
} catch (Exception e) {
42+
LOGGER.debug(
43+
"Failed to set user agent for customer userAgent entry {}, Error {}",
44+
connectionContext.getCustomerUserAgent(),
45+
e);
8346
}
8447
}
8548

‎src/main/java/com/databricks/jdbc/telemetry/TelemetryHelper.java‎

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public class TelemetryHelper {
3030
// Cache to store unique DriverConnectionParameters for each connectionUuid
3131
private static final ConcurrentHashMap<String, DriverConnectionParameters>
3232
connectionParameterCache = new ConcurrentHashMap<>();
33+
private static final String APP_NAME_SYSTEM_PROPERTY = "app.name";
3334

3435
@VisibleForTesting
3536
static final String TELEMETRY_FEATURE_FLAG_NAME =
@@ -55,12 +56,6 @@ public static DriverSystemConfiguration getDriverSystemConfiguration() {
5556
return DRIVER_SYSTEM_CONFIGURATION;
5657
}
5758

58-
public static void updateClientAppName(String clientAppName) {
59-
if (!isNullOrEmpty(clientAppName)) {
60-
DRIVER_SYSTEM_CONFIGURATION.setClientAppName(clientAppName);
61-
}
62-
}
63-
6459
public static boolean isTelemetryAllowedForConnection(IDatabricksConnectionContext context) {
6560
if (context != null && context.forceEnableTelemetry()) {
6661
return true;
@@ -88,6 +83,7 @@ private static void exportTelemetryEvent(
8883
}
8984
TelemetryEvent telemetryEvent =
9085
new TelemetryEvent()
86+
.setDriverSystemConfiguration(DRIVER_SYSTEM_CONFIGURATION)
9187
.setDriverConnectionParameters(getDriverConnectionParameter(connectionContext))
9288
.setSessionId(DatabricksThreadContextHolder.getSessionId())
9389
.setDriverErrorInfo(errorInfo) // This is only set for failure logs
@@ -312,4 +308,50 @@ public static OperationType mapMethodToOperationType(String methodName) {
312308
return OperationType.TYPE_UNSPECIFIED;
313309
}
314310
}
311+
312+
/**
313+
* Sets/updates client app name in telemetry
314+
*
315+
* @param connectionContext The connection context
316+
* @param clientInfoAppName The application name from client info properties, can be null
317+
*/
318+
public static void updateTelemetryAppName(
319+
IDatabricksConnectionContext connectionContext, String clientInfoAppName) {
320+
String appName = determineApplicationName(connectionContext, clientInfoAppName);
321+
if (!isNullOrEmpty(appName)) {
322+
DRIVER_SYSTEM_CONFIGURATION.setClientAppName(appName);
323+
}
324+
}
325+
326+
/**
327+
* Determines the application name using a fallback mechanism: 1. useragententry url param 2.
328+
* applicationname url param 3. client info property "applicationname" 4. System property app.name
329+
*
330+
* @param connectionContext The connection context
331+
* @param clientInfoAppName The application name from client info properties, can be null
332+
* @return The determined application name or null if none is found
333+
*/
334+
@VisibleForTesting
335+
static String determineApplicationName(
336+
IDatabricksConnectionContext connectionContext, String clientInfoAppName) {
337+
// First check URL params
338+
String appName = connectionContext.getCustomerUserAgent();
339+
if (!isNullOrEmpty(appName)) {
340+
return appName;
341+
}
342+
343+
// Then check application name URL param
344+
appName = connectionContext.getApplicationName();
345+
if (!isNullOrEmpty(appName)) {
346+
return appName;
347+
}
348+
349+
// Then check client info property
350+
if (!isNullOrEmpty(clientInfoAppName)) {
351+
return clientInfoAppName;
352+
}
353+
354+
// Finally check system property
355+
return System.getProperty(APP_NAME_SYSTEM_PROPERTY);
356+
}
315357
}

‎src/test/java/com/databricks/jdbc/common/util/UserAgentManagerTest.java‎

Lines changed: 28 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,8 @@
11
package com.databricks.jdbc.common.util;
22

33
import static com.databricks.jdbc.TestConstants.*;
4-
import static com.databricks.jdbc.common.util.UserAgentManager.determineApplicationName;
4+
import static com.databricks.jdbc.common.util.UserAgentManager.USER_AGENT_SEA_CLIENT;
55
import static com.databricks.jdbc.common.util.UserAgentManager.getUserAgentString;
6-
import static com.databricks.jdbc.common.util.UserAgentManager.updateUserAgentAndTelemetry;
76
import static org.junit.jupiter.api.Assertions.*;
87
import static org.mockito.Mockito.when;
98

@@ -12,14 +11,10 @@
1211
import com.databricks.jdbc.exception.DatabricksSQLException;
1312
import com.databricks.jdbc.telemetry.TelemetryHelper;
1413
import java.util.Properties;
15-
import java.util.stream.Stream;
1614
import org.junit.jupiter.api.AfterEach;
1715
import org.junit.jupiter.api.BeforeEach;
1816
import org.junit.jupiter.api.Test;
1917
import org.junit.jupiter.api.extension.ExtendWith;
20-
import org.junit.jupiter.params.ParameterizedTest;
21-
import org.junit.jupiter.params.provider.Arguments;
22-
import org.junit.jupiter.params.provider.MethodSource;
2318
import org.mockito.Mock;
2419
import org.mockito.MockedStatic;
2520
import org.mockito.Mockito;
@@ -42,88 +37,44 @@ public void tearDown() {
4237
telemetryHelperMock.close();
4338
}
4439

45-
private static Stream<Arguments> provideApplicationNameTestCases() {
46-
return Stream.of(
47-
// Test case 1: UserAgentEntry takes precedence
48-
Arguments.of(
49-
"MyUserAgent",
50-
"AppNameValue",
51-
"ClientInfoApp",
52-
"MyUserAgent",
53-
"When useragententry is set"),
54-
// Test case 2: ApplicationName is used when UserAgentEntry is null
55-
Arguments.of(
56-
null,
57-
"AppNameValue",
58-
"ClientInfoApp",
59-
"AppNameValue",
60-
"When useragententry is not set but applicationname is"),
61-
// Test case 3: ClientInfo is used when both UserAgentEntry and ApplicationName are null
62-
Arguments.of(
63-
null,
64-
null, // applicationName
65-
"ClientInfoApp",
66-
"ClientInfoApp",
67-
"When URL params are not set but client info is provided"));
68-
}
69-
70-
@ParameterizedTest
71-
@MethodSource("provideApplicationNameTestCases")
72-
public void testDetermineApplicationName(
73-
String customerUserAgent,
74-
String applicationName,
75-
String clientInfoApp,
76-
String expectedResult) {
77-
// Setup only necessary stubs
78-
Mockito.lenient().when(connectionContext.getCustomerUserAgent()).thenReturn(customerUserAgent);
79-
Mockito.lenient().when(connectionContext.getApplicationName()).thenReturn(applicationName);
80-
81-
// Execute
82-
String result = determineApplicationName(connectionContext, clientInfoApp);
83-
84-
// Verify
85-
assertEquals(expectedResult, result);
40+
@Test
41+
public void testUpdateUserAgent() {
42+
// Test that user agent is updated even without a version
43+
when(connectionContext.getCustomerUserAgent()).thenReturn("TestApp");
44+
when(connectionContext.getClientUserAgent()).thenReturn(USER_AGENT_SEA_CLIENT);
45+
UserAgentManager.setUserAgent(connectionContext);
46+
String userAgent = getUserAgentString();
47+
assertTrue(userAgent.contains("TestApp/version"));
8648
}
8749

8850
@Test
89-
public void testDetermineApplicationName_WithSystemProperty() {
90-
// When falling back to system property
91-
when(connectionContext.getCustomerUserAgent()).thenReturn(null);
92-
when(connectionContext.getApplicationName()).thenReturn(null);
93-
94-
System.setProperty("app.name", "SystemPropApp");
95-
try {
96-
String result = determineApplicationName(connectionContext, null);
97-
assertEquals("SystemPropApp", result);
98-
} finally {
99-
System.clearProperty("app.name");
100-
}
51+
public void testEncodedUserAgent() {
52+
// Test that user agent is updated even if it is encoded
53+
when(connectionContext.getCustomerUserAgent())
54+
.thenReturn("DBeaverEncoded%2F25.1.4.202508031529");
55+
when(connectionContext.getClientUserAgent()).thenReturn(USER_AGENT_SEA_CLIENT);
56+
UserAgentManager.setUserAgent(connectionContext);
57+
String userAgent = getUserAgentString();
58+
assertTrue(userAgent.contains("DBeaverEncoded/25.1.4.202508031529"));
10159
}
10260

10361
@Test
104-
public void testUpdateUserAgentAndTelemetry() {
105-
// Test that both telemetry and user agent are updated
106-
when(connectionContext.getCustomerUserAgent()).thenReturn("TestApp");
107-
// when(UserAgent.sanitize("version")).thenReturn("version");
108-
109-
updateUserAgentAndTelemetry(connectionContext, null);
110-
111-
telemetryHelperMock.verify(() -> TelemetryHelper.updateClientAppName("TestApp"));
62+
public void testIncorrectUserAgentDoesNotThrowException() {
63+
when(connectionContext.getCustomerUserAgent()).thenReturn("DBeaverInvalid~25.1.4.202508031529");
64+
when(connectionContext.getClientUserAgent()).thenReturn(USER_AGENT_SEA_CLIENT);
65+
UserAgentManager.setUserAgent(connectionContext);
11266
String userAgent = getUserAgentString();
113-
assertTrue(userAgent.contains("TestApp/version"));
67+
assertFalse(userAgent.contains("DBeaverInvalid"));
11468
}
11569

11670
@Test
117-
public void testUpdateUserAgentAndTelemetry_WithVersion() {
118-
// Test with app name containing version
119-
when(connectionContext.getCustomerUserAgent()).thenReturn("MyApp/1.2.3");
120-
// when(UserAgent.sanitize("1.2.3")).thenReturn("1.2.3");
121-
122-
updateUserAgentAndTelemetry(connectionContext, null);
123-
124-
telemetryHelperMock.verify(() -> TelemetryHelper.updateClientAppName("MyApp/1.2.3"));
71+
public void testUserAgentWithSlash() {
72+
// Test that user agent with added version is updated
73+
when(connectionContext.getCustomerUserAgent()).thenReturn("MyAppSlash/25.1.4.202508031529");
74+
when(connectionContext.getClientUserAgent()).thenReturn(USER_AGENT_SEA_CLIENT);
75+
UserAgentManager.setUserAgent(connectionContext);
12576
String userAgent = getUserAgentString();
126-
assertTrue(userAgent.contains("MyApp/1.2.3"));
77+
assertTrue(userAgent.contains("MyAppSlash/25.1.4.202508031529"));
12778
}
12879

12980
@Test

0 commit comments

Comments
 (0)