[PECOBLR-435] added application name to track BiTool by shivam2680 · Pull Request #837 · databricks/databricks-jdbc

shivam2680 · 2025-05-22T16:37:20Z

Description

Added application name url param. This will be used to set client app name in telemetry. Earlier, it was set to null.
Now we follow fallback mechanism to set user agent and client app in telemetry

UserAgentEntry url param
ApplicationName url param
Connection.setClientInfo("ApplicationName", "xyz")
System property app.name

Testing

Manual
Script:

String jdbcUrl =
        "jdbc:databricks://e2-dogfood.staging.cloud.databricks.com:443/default;transportMode=http;ssl=1;httpPath=;AuthMech=3;applicationname=test-abcd";
    Connection con =
        DriverManager.getConnection(jdbcUrl, "token", "xx");
    System.out.println("Connection established......");
    con.setClientInfo("applicationname", "test-shivam");
    System.out.println("updated config");
    System.out.println(getDriverSystemConfiguration().toString());

Additional Notes to the Reviewer

NO_CHANGELOG=true

samikshya-db

Minor comments, LGTM otherwise. Thanks for this!

samikshya-db · 2025-05-22T20:47:02Z

        DatabricksConnectionContextFactory.create(url, info);
    DriverUtil.setUpLogging(connectionContext);
    UserAgentManager.setUserAgent(connectionContext);
+    updateClientAppName(connectionContext.getApplicationName());


can you call this function at the layer that builds the telemetry connection config itself? This can be abstracted out of here.

you mean under exportFailureLog(ctx) functions? or when we are constructing DRIVER_SYSTEM_CONFIGURATION

gopalldb · 2025-05-23T09:35:45Z

        DatabricksConnectionContextFactory.create(url, info);
    DriverUtil.setUpLogging(connectionContext);
    UserAgentManager.setUserAgent(connectionContext);
+    updateClientAppName(connectionContext.getApplicationName());


We can set this in following order:

set from UserAgentEntry connection property

set from ApplicationName connection property

set from ApplicationName client info property

samikshya-db · 2025-06-02T09:11:07Z

    IDatabricksConnectionContext connectionContext =
        DatabricksConnectionContextFactory.create(url, info);
    DriverUtil.setUpLogging(connectionContext);
+    // This initializes both user agent headers and telemetry


Can we remove this comment? This seems misleading for someone having telemetry = OFF.

samikshya-db · 2025-06-02T09:15:00Z

+import com.databricks.sdk.core.UserAgent;
+
+/** Helper class for determining and setting user agent and client app name. */
+public class UserAgentHelper {


Any reason we split the existing UserAgentManager helper into a different UserAgent helper? (looks like an unnecessary split in my opinion.)

I just wanted the fallback mechanism utility to be separate. But, I agree, merging makes sense.

samikshya-db · 2025-06-02T09:28:19Z

+    // Preserve existing custom headers (like User-Agent) and add/update auth headers
+    Map<String, String> newHeaders = new HashMap<>(customHeaders);
+    if (refreshedHeaders != null) {
+      newHeaders.putAll(refreshedHeaders);
+    }
+    customHeaders = newHeaders;


Minor improved version :

Suggested change

// Preserve existing custom headers (like User-Agent) and add/update auth headers

Map<String, String> newHeaders = new HashMap<>(customHeaders);

if (refreshedHeaders != null) {

newHeaders.putAll(refreshedHeaders);

}

customHeaders = newHeaders;

if (refreshed != null) {

customHeaders = new HashMap<>(customHeaders);

customHeaders.putAll(refreshed);

}

samikshya-db · 2025-06-02T09:29:36Z

+    Map<String, String> headers = new HashMap<>(customHeaders);
+    headers.put("User-Agent", UserAgentManager.getUserAgentString());
+    customHeaders = headers;


Suggested change

Map<String, String> headers = new HashMap<>(customHeaders);

headers.put("User-Agent", UserAgentManager.getUserAgentString());

customHeaders = headers;

customHeaders = new HashMap<>(customHeaders);

customHeaders.put("User-Agent", UserAgentManager.getUserAgentString());

samikshya-db · 2025-06-02T09:34:22Z

-    customHeaders =
-        refreshedHeaders != null ? new HashMap<>(refreshedHeaders) : Collections.emptyMap();
+    // Preserve existing custom headers (like User-Agent) and add/update auth headers
+    Map<String, String> newHeaders = new HashMap<>(customHeaders);


Does that mean we don't add user-Agent info in our current thrift flow? Do we pass it from DatabricksHttpClient? I see that DatabricksHttpClient needs change too.

Not really.
we do add user agent info in current thrift flow. thrift client is constructed from databrickshttpclient. While constructing, we do set userAgent in makeClosableHttpClient.
I've added logic to update user agent in thrift client because we want to update it when setClientInfo property as well.

(Based on offline discussion) : SDK and thrift are not exhaustive set of places where this would be required. Any other call using HTTPClient will have the incorrect userAgent

samikshya-db · 2025-06-02T12:43:51Z

  void resetAccessToken(String newAccessToken);

+  /** Update the user agent headers when application name changes */
+  void updateUserAgent();


we are going to remove this, right?

samikshya-db · 2025-06-02T17:17:30Z

    }
    try {
+      String userAgentString = UserAgentManager.getUserAgentString();
+      if (!isNullOrEmpty(userAgentString) && !request.containsHeader("User-Agent")) {


do we need to do

if (!isNullOrEmpty(userAgentString) || !ObjectEquals(request.containsHeader("User-Agent"), userAgentString) {

inorder to replace the potentially stale userAgent (i.e., before customer sets it via session? )

I discussed this also with @gopalldb , the plan is to introduce client info property and applicationname url param as fallback for userAgentEntry url param. UserAgentEntry always takes precedence.

samikshya-db

Minor comment, looks good otherwise! Thanks for the clean code!

jayantsing-db · 2025-06-02T17:22:48Z

UserAgentEntry url param
ApplicationName url param

what is the rationale for requiring two separate params to trace user activity?

samikshya-db · 2025-06-02T17:28:16Z

@jayantsing-db - this is in hope to reduce the number of unknown BI tool entries on the backend.

Apparently, applicationName is a standard way to set the BI tool. More context here : https://databricks.slack.com/archives/C08AS8EFRNC/p1747302484928519?thread_ts=1747287041.406849&cid=C08AS8EFRNC

added application name to track BiTool

4af6d6b

shivam2680 temporarily deployed to azure-prod May 22, 2025 16:38 — with GitHub Actions Inactive

add tests

bc52608

shivam2680 temporarily deployed to azure-prod May 22, 2025 17:07 — with GitHub Actions Inactive

shivam2680 temporarily deployed to azure-prod May 22, 2025 17:08 — with GitHub Actions Inactive

shivam2680 marked this pull request as ready for review May 22, 2025 17:09

shivam2680 requested review from deeksha-db, gopalldb and samikshya-db May 22, 2025 17:09

samikshya-db reviewed May 22, 2025

View reviewed changes

some changes

c28d24c

gopalldb reviewed May 23, 2025

View reviewed changes

refactored code; update user agent and telemetry client app

764ede9

shivam2680 requested a review from gopalldb May 23, 2025 13:04

gopalldb reviewed May 26, 2025

View reviewed changes

Comment thread src/main/java/com/databricks/jdbc/common/DatabricksJdbcUrlParams.java

gopalldb reviewed May 26, 2025

View reviewed changes

Comment thread src/main/java/com/databricks/jdbc/common/util/UserAgentHelper.java Outdated

update user agent thrift mode as well

e737325

shivam2680 temporarily deployed to azure-prod May 26, 2025 09:29 — with GitHub Actions Inactive

shivam2680 requested review from gopalldb, jayantsing-db, samikshya-db and vikrantpuppala May 26, 2025 09:31

samikshya-db reviewed May 28, 2025

View reviewed changes

Comment thread src/main/java/com/databricks/jdbc/common/util/UserAgentHelper.java Outdated

gopalldb reviewed May 29, 2025

View reviewed changes

Comment thread src/main/java/com/databricks/jdbc/common/util/UserAgentHelper.java Outdated

gopalldb approved these changes May 29, 2025

View reviewed changes

changes fn to package-private.

8b10ecf

samikshya-db reviewed Jun 2, 2025

View reviewed changes

Comment thread src/test/java/com/databricks/jdbc/common/util/UserAgentHelperTest.java Outdated

resolve conflicts

a268d5f

shivam2680 temporarily deployed to azure-prod June 2, 2025 05:39 — with GitHub Actions Inactive

make parameterized test

1f57dbc

shivam2680 temporarily deployed to azure-prod June 2, 2025 07:15 — with GitHub Actions Inactive

shivam2680 had a problem deploying to azure-prod June 2, 2025 07:15 — with GitHub Actions Failure

spotless

0408c4a

shivam2680 requested review from gopalldb and samikshya-db June 2, 2025 08:19

shivam2680 temporarily deployed to azure-prod June 2, 2025 08:20 — with GitHub Actions Inactive

samikshya-db reviewed Jun 2, 2025

View reviewed changes

merge UserAgentHelper and UserAgentManager

8043483

shivam2680 temporarily deployed to azure-prod June 2, 2025 10:37 — with GitHub Actions Inactive

addressed reviews

0e1b2f3

shivam2680 temporarily deployed to azure-prod June 2, 2025 10:54 — with GitHub Actions Inactive

samikshya-db reviewed Jun 2, 2025

View reviewed changes

update DatabricksHttpClient

83fd62c

shivam2680 temporarily deployed to azure-prod June 2, 2025 14:11 — with GitHub Actions Inactive

shivam2680 requested a review from samikshya-db June 2, 2025 15:10

samikshya-db reviewed Jun 2, 2025

View reviewed changes

samikshya-db approved these changes Jun 2, 2025

View reviewed changes

shivam2680 merged commit 3957211 into databricks:main Jun 3, 2025
15 of 16 checks passed

Conversation

shivam2680 commented May 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Additional Notes to the Reviewer

Uh oh!

samikshya-db left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samikshya-db left a comment

Choose a reason for hiding this comment

Uh oh!

jayantsing-db commented Jun 2, 2025

Uh oh!

samikshya-db commented Jun 2, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shivam2680 commented May 22, 2025 •

edited

Loading