Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
688cf75
Add support for configuring HTTP connection pool size
gopalldb Feb 12, 2025
2e6bfaa
Changes for default value
gopalldb Feb 12, 2025
8ee0fa1
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Feb 21, 2025
5f7d0c1
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Feb 24, 2025
2e28c9e
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Feb 25, 2025
a720802
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Feb 25, 2025
e88657b
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Feb 26, 2025
2670e43
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 1, 2025
c4925ee
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 2, 2025
5a5f1de
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 4, 2025
bf41453
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 5, 2025
b81f4ad
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 6, 2025
1e790a4
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 6, 2025
5ed0c08
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 17, 2025
73029b4
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 19, 2025
ffc2aae
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 27, 2025
7cf9cc8
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Mar 30, 2025
f55ca04
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Apr 2, 2025
ceb2fcb
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Apr 7, 2025
77552fc
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Apr 7, 2025
0690dea
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Apr 7, 2025
638a11a
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Apr 10, 2025
a72870e
Adding extra null check for thread local variables in Telemetry
gopalldb Apr 25, 2025
99c83f8
.
gopalldb Apr 25, 2025
5da3b1e
Merge branch 'main' of github.com:databricks/databricks-jdbc
gopalldb Apr 25, 2025
39ee46b
Merge branch 'main' into threads
gopalldb Apr 25, 2025
3d03893
Add extra null check
gopalldb Apr 25, 2025
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
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ public static TelemetryClientFactory getInstance() {

public ITelemetryClient getTelemetryClient(
IDatabricksConnectionContext connectionContext, DatabricksConfig databricksConfig) {
if (connectionContext != null && connectionContext.isTelemetryEnabled()) {
if (connectionContext != null
&& connectionContext.isTelemetryEnabled()
&& databricksConfig != null) {
return telemetryClients.computeIfAbsent(
connectionContext.getConnectionUuid(),
k ->
Expand Down
70 changes: 40 additions & 30 deletions src/main/java/com/databricks/jdbc/telemetry/TelemetryHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -113,44 +113,54 @@ static void exportLatencyLog(
long latencyMilliseconds,
SqlExecutionEvent executionEvent,
StatementId statementId) {
TelemetryEvent telemetryEvent =
new TelemetryEvent()
.setLatency(latencyMilliseconds)
.setSqlOperation(executionEvent)
.setDriverConnectionParameters(getDriverConnectionParameter(connectionContext));
if (statementId != null) {
telemetryEvent.setSqlStatementId(statementId.toString());
// Though we already handle null connectionContext in the downstream implementation,
// we are adding this check for extra sanity
if (connectionContext != null) {
TelemetryEvent telemetryEvent =
new TelemetryEvent()
.setLatency(latencyMilliseconds)
.setSqlOperation(executionEvent)
.setDriverConnectionParameters(getDriverConnectionParameter(connectionContext));
if (statementId != null) {
telemetryEvent.setSqlStatementId(statementId.toString());
}
TelemetryFrontendLog telemetryFrontendLog =
new TelemetryFrontendLog()
.setFrontendLogEventId(getEventUUID())
.setContext(getLogContext())
.setEntry(new FrontendLogEntry().setSqlDriverLog(telemetryEvent));
TelemetryClientFactory.getInstance()
.getTelemetryClient(
connectionContext, DatabricksThreadContextHolder.getDatabricksConfig())
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we check DatabricksConfig too? DatabricksThreadContextHolder.getDatabricksConfig

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needed for authenticated client. Added the check there.

.exportEvent(telemetryFrontendLog);
}
TelemetryFrontendLog telemetryFrontendLog =
new TelemetryFrontendLog()
.setFrontendLogEventId(getEventUUID())
.setContext(getLogContext())
.setEntry(new FrontendLogEntry().setSqlDriverLog(telemetryEvent));
TelemetryClientFactory.getInstance()
.getTelemetryClient(connectionContext, DatabricksThreadContextHolder.getDatabricksConfig())
.exportEvent(telemetryFrontendLog);
}

public static void exportLatencyLog(
IDatabricksConnectionContext connectionContext,
long latencyMilliseconds,
DriverVolumeOperation volumeOperationEvent) {
TelemetryFrontendLog telemetryFrontendLog =
new TelemetryFrontendLog()
.setFrontendLogEventId(getEventUUID())
.setContext(getLogContext())
.setEntry(
new FrontendLogEntry()
.setSqlDriverLog(
new TelemetryEvent()
.setLatency(latencyMilliseconds)
.setVolumeOperation(volumeOperationEvent)
.setDriverConnectionParameters(
getDriverConnectionParameter(connectionContext))));
// Though we already handle null connectionContext in the downstream implementation,
// we are adding this check for extra sanity
if (connectionContext != null) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should introduce this weird ad-hoc check somewhere deep in the code when we are actually exporting the logs. This is happening already. So the PR is a no-op

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we are already doing that. It is just that to make this future proof, where we accidently add dependency on connectionContext in any downstream logic and breaking the functionality. Telemetry is common to all executions. In current impl you're right that this is a no-op.

TelemetryFrontendLog telemetryFrontendLog =
new TelemetryFrontendLog()
.setFrontendLogEventId(getEventUUID())
.setContext(getLogContext())
.setEntry(
new FrontendLogEntry()
.setSqlDriverLog(
new TelemetryEvent()
.setLatency(latencyMilliseconds)
.setVolumeOperation(volumeOperationEvent)
.setDriverConnectionParameters(
getDriverConnectionParameter(connectionContext))));

TelemetryClientFactory.getInstance()
.getTelemetryClient(connectionContext, DatabricksThreadContextHolder.getDatabricksConfig())
.exportEvent(telemetryFrontendLog);
TelemetryClientFactory.getInstance()
.getTelemetryClient(
connectionContext, DatabricksThreadContextHolder.getDatabricksConfig())
.exportEvent(telemetryFrontendLog);
}
}

private static DriverConnectionParameters getDriverConnectionParameter(
Expand Down
Loading