Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
6 changes: 3 additions & 3 deletions csharp/doc/telemetry-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -1793,7 +1793,7 @@ The JDBC driver uses Jackson with explicit `@JsonProperty("session_id")` annotat
"session_id": "abc123",
"sql_statement_id": "def456",
"system_configuration": { "driver_version": "1.0.0", "runtime_name": ".NET 8.0" },
"driver_connection_params": { "http_path": "/sql/1.0/warehouses/xyz", "host_info": { "host_url": "https://..." } },
"driver_connection_params": { "http_path": "/sql/1.0/warehouses/xyz", "host_info": { "host_url": "host.cloud.databricks.com", "port": 443 } },
"auth_type": "pat",
"sql_operation": { "statement_type": "STATEMENT_QUERY", "execution_result": "EXECUTION_RESULT_EXTERNAL_LINKS" },
"operation_latency_ms": 254
Expand Down Expand Up @@ -2373,8 +2373,8 @@ Every field in the `OssSqlDriverTelemetryLog` proto must be populated and verifi
|-------------|---------------|------------|
| `http_path` | e.g. `"/sql/1.0/warehouses/abc123"` | Non-empty, starts with `/` |
| `mode` | `DRIVER_MODE_THRIFT` or `DRIVER_MODE_SEA` | Not `UNSPECIFIED` |
| `host_info.host_url` | e.g. `"https://host.cloud.databricks.com:443"` | Non-empty, starts with `https://` |
| `host_info.port` | Port number (may be 0 if embedded in URL) | — |
| `host_info.host_url` | e.g. `"host.cloud.databricks.com"` (bare hostname, matches JDBC) | Non-empty, no scheme, no port suffix |
| `host_info.port` | Port number, e.g. `443` | > 0 |
| `auth_mech` | e.g. `DRIVER_AUTH_MECH_PAT` | Not `UNSPECIFIED` |
| `auth_flow` | e.g. `DRIVER_AUTH_FLOW_TOKEN_PASSTHROUGH` | Not `UNSPECIFIED` |
| `enable_arrow` | `true` | Boolean |
Expand Down
5 changes: 4 additions & 1 deletion csharp/src/Telemetry/ConnectionTelemetry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,10 @@ internal static Proto.DriverConnectionParameters BuildDriverConnectionParams(
Mode = Proto.DriverMode.Types.Type.Thrift,
HostInfo = new Proto.HostDetails
{
HostUrl = $"https://{host}:{port}",
// Bare hostname, matching JDBC. Scheme is implicit (always https) and
// port is reported in the sibling Port field, so including them here
// was redundant and forced analysts to normalize across drivers.
HostUrl = host,
Port = port
},
AuthMech = authMech,
Expand Down
4 changes: 3 additions & 1 deletion csharp/test/E2E/Telemetry/ClientTelemetryE2ETests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1114,7 +1114,9 @@ public void Telemetry_QueryExecution_CapturesAllFields()
Assert.NotEqual(AdbcDrivers.Databricks.Telemetry.Proto.DriverMode.Types.Type.Unspecified, connParams.Mode);
Assert.NotNull(connParams.HostInfo);
Assert.False(string.IsNullOrEmpty(connParams.HostInfo.HostUrl), "HostUrl should be populated");
Assert.Contains("https://", connParams.HostInfo.HostUrl);
// host_url is a bare hostname (matches JDBC). Scheme/port must not be embedded
// (PECO-2987). Port is reported separately in host_info.port.
Assert.DoesNotContain("://", connParams.HostInfo.HostUrl);
Assert.NotEqual(AdbcDrivers.Databricks.Telemetry.Proto.DriverAuthMech.Types.Type.Unspecified, connParams.AuthMech);
Assert.NotEqual(AdbcDrivers.Databricks.Telemetry.Proto.DriverAuthFlow.Types.Type.Unspecified, connParams.AuthFlow);

Expand Down
66 changes: 66 additions & 0 deletions csharp/test/E2E/Telemetry/ConnectionParametersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,72 @@ public async Task ConnectionParams_HostInfoPort_IsPopulated()
}
}

/// <summary>
/// Regression test for PECO-2987: driver_connection_params.host_info.host_url was
/// emitted as <c>"https://&lt;host&gt;:443"</c>, differing from JDBC which emits a bare
/// hostname. Analysts joining on host across drivers had to normalize. host_url must
/// now match JDBC: bare hostname, no scheme, no port suffix. Port is reported
/// separately in host_info.port.
/// </summary>
[SkippableFact]
public async Task ConnectionParams_HostInfoHostUrl_IsBareHostname()
{
CapturingTelemetryExporter exporter = null!;
AdbcConnection? connection = null;

try
{
var properties = TestEnvironment.GetDriverParameters(TestConfiguration);

// Resolve the expected host using the same precedence as DatabricksConnection.GetHost().
string? expectedHost = null;
if (properties.TryGetValue(SparkParameters.HostName, out string? hostName)
&& !string.IsNullOrEmpty(hostName))
{
expectedHost = hostName;
}
else if (properties.TryGetValue(AdbcOptions.Uri, out string? uri)
&& !string.IsNullOrEmpty(uri)
&& Uri.TryCreate(uri, UriKind.Absolute, out Uri? parsedUri))
{
expectedHost = parsedUri.Host;
}

Assert.False(string.IsNullOrEmpty(expectedHost),
$"Test configuration must set {SparkParameters.HostName} or {AdbcOptions.Uri}");

(connection, exporter) = TelemetryTestHelpers.CreateConnectionWithCapturingTelemetry(properties);

using var statement = connection.CreateStatement();
statement.SqlQuery = "SELECT 1 AS test_value";
var result = statement.ExecuteQuery();
using var reader = result.Stream;

statement.Dispose();

var logs = await TelemetryTestHelpers.WaitForTelemetryEvents(exporter, expectedCount: 1);
TelemetryTestHelpers.AssertLogCount(logs, 1);

var protoLog = TelemetryTestHelpers.GetProtoLog(logs[0]);

Assert.NotNull(protoLog.DriverConnectionParams);
Assert.NotNull(protoLog.DriverConnectionParams.HostInfo);
string actualHostUrl = protoLog.DriverConnectionParams.HostInfo.HostUrl;

Assert.False(string.IsNullOrEmpty(actualHostUrl), "host_url should be populated");
Assert.Equal(expectedHost, actualHostUrl);
Assert.DoesNotContain("://", actualHostUrl);
Assert.DoesNotContain(":", actualHostUrl);

OutputHelper?.WriteLine($"✓ host_info.host_url: {actualHostUrl}");
}
finally
{
connection?.Dispose();
TelemetryTestHelpers.ClearExporterOverride();
}
}

/// <summary>
/// Tests that enable_arrow is set to true for ADBC driver.
/// </summary>
Expand Down
2 changes: 1 addition & 1 deletion docs/designs/fix-telemetry-gaps-design.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ flowchart TD
|---|---|---|
| `http_path` | Populated | |
| `mode` | Populated | Hardcoded to THRIFT |
| `host_info` | Populated | |
| `host_info` | Populated | `host_url` is bare hostname (matches JDBC, PECO-2987); `port` reported separately |
| `auth_mech` | Populated | PAT or OAUTH |
| `auth_flow` | Populated | TOKEN_PASSTHROUGH or CLIENT_CREDENTIALS |
| `use_proxy` | **NOT SET** | |
Expand Down
Loading