diff --git a/dotnet/NEXT_CHANGELOG.md b/dotnet/NEXT_CHANGELOG.md index d033ef68..10a15589 100644 --- a/dotnet/NEXT_CHANGELOG.md +++ b/dotnet/NEXT_CHANGELOG.md @@ -8,6 +8,11 @@ ### Bug Fixes +- The SDK now identifies itself as `zerobus-sdk-dotnet/` in the `User-Agent` + header. The identifier was only sent when the caller supplied one explicitly, so a + default-configured client sent the Rust core's own identifier and .NET traffic was + reported as Rust. An explicit `SdkIdentifier(...)` still takes precedence. No public + API change. - Fixed a use-after-free in which a custom `IHeadersProvider` could be freed while the Rust core was still inside a `GetHeaders()` call into it during connection recovery. Provider ownership is now handed to the FFI via the new diff --git a/dotnet/src/Zerobus/ZerobusSdkBuilder.cs b/dotnet/src/Zerobus/ZerobusSdkBuilder.cs index 2862734d..6d399150 100644 --- a/dotnet/src/Zerobus/ZerobusSdkBuilder.cs +++ b/dotnet/src/Zerobus/ZerobusSdkBuilder.cs @@ -1,3 +1,4 @@ +using System.Reflection; using Databricks.Zerobus.Native; namespace Databricks.Zerobus; @@ -43,6 +44,22 @@ public sealed class ZerobusSdkBuilder : IDisposable private bool _disableTls; private int _consumed; // 0 = live, 1 = consumed/disposed + // Sent when the caller doesn't set SdkIdentifier(); without it the Rust core's own + // identifier goes on the wire and .NET traffic is attributed to Rust. + private static readonly string DefaultSdkIdentifier = $"zerobus-sdk-dotnet/{SdkVersion()}"; + + // The informational version carries from the csproj, unlike AssemblyVersion, + // which callers may pin separately. SourceLink appends "+" to it. + private static string SdkVersion() + { + var informational = typeof(ZerobusSdkBuilder).Assembly + .GetCustomAttribute()?.InformationalVersion; + + return string.IsNullOrEmpty(informational) + ? "0.0.0" + : informational.Split('+')[0]; + } + internal ZerobusSdkBuilder() { } /// @@ -81,6 +98,10 @@ public ZerobusSdkBuilder UnityCatalogUrl(string unityCatalogUrl) /// Wrapper SDKs use this to identify themselves; end-user code should prefer /// instead. /// + /// + /// Without this call the SDK sends zerobus-sdk-dotnet/<version>. A blank + /// or whitespace-only value falls back to that same default. + /// /// The SDK identifier string. /// This builder, for chaining. /// Thrown if is null. @@ -144,8 +165,11 @@ public ZerobusSdk Build() NativeMethods.SdkBuilderEndpoint(builderPtr, _endpoint); if (_unityCatalogUrl is not null) NativeMethods.SdkBuilderUnityCatalogUrl(builderPtr, _unityCatalogUrl); - if (_sdkIdentifier is not null) - NativeMethods.SdkBuilderSdkIdentifier(builderPtr, _sdkIdentifier); + // The core substitutes its own identifier only for a literal empty override, not + // for whitespace, so a blank value would otherwise reach the wire verbatim. + NativeMethods.SdkBuilderSdkIdentifier( + builderPtr, + string.IsNullOrWhiteSpace(_sdkIdentifier) ? DefaultSdkIdentifier : _sdkIdentifier); if (_applicationName is not null) NativeMethods.SdkBuilderApplicationName(builderPtr, _applicationName); if (_disableTls) diff --git a/dotnet/tests/Zerobus.IntegrationTests/SdkIdentifierIntegrationTests.cs b/dotnet/tests/Zerobus.IntegrationTests/SdkIdentifierIntegrationTests.cs new file mode 100644 index 00000000..194adf8c --- /dev/null +++ b/dotnet/tests/Zerobus.IntegrationTests/SdkIdentifierIntegrationTests.cs @@ -0,0 +1,122 @@ +using NUnit.Framework; + +namespace Databricks.Zerobus.IntegrationTests; + +[TestFixture] +[Parallelizable(ParallelScope.Children)] +public class SdkIdentifierIntegrationTests : IntegrationTestBase +{ + [Test] + public async Task DefaultSdkIdentifier_SentInUserAgent() + { + await using var fixture = await MockServerFixture.StartAsync(); + + fixture.MockServer.InjectResponses(TestTableName, + [ + MockResponses.CreateStreamResponse("test_stream_sdk_identifier"), + MockResponses.RecordAckResponse(0), + ]); + + using var sdk = CreateDefaultSdk(fixture); + + using var stream = sdk.CreateStreamWithHeadersProvider( + CreateTableProperties(), new TestHeadersProvider(), CreateDefaultOptions()); + + stream.IngestRecord("test record data"u8.ToArray()); + stream.Flush(); + + var observedHeaders = fixture.MockServer.GetLastRequestHeaders(TestTableName); + Assert.That(observedHeaders.TryGetValue("user-agent", out var userAgent), Is.True); + Assert.That(userAgent, Does.StartWith("zerobus-sdk-dotnet/")); + } + + [Test] + public async Task ExplicitSdkIdentifier_TakesPrecedenceOverDefault() + { + await using var fixture = await MockServerFixture.StartAsync(); + + fixture.MockServer.InjectResponses(TestTableName, + [ + MockResponses.CreateStreamResponse("test_stream_sdk_identifier_override"), + MockResponses.RecordAckResponse(0), + ]); + + using var sdk = ZerobusSdk.CreateBuilder() + .Endpoint(fixture.ServerUrl) + .UnityCatalogUrl("https://mock-uc.com") + .SdkIdentifier("custom-wrapper/9.9.9") + .DisableTls() + .Build(); + + using var stream = sdk.CreateStreamWithHeadersProvider( + CreateTableProperties(), new TestHeadersProvider(), CreateDefaultOptions()); + + stream.IngestRecord("test record data"u8.ToArray()); + stream.Flush(); + + var observedHeaders = fixture.MockServer.GetLastRequestHeaders(TestTableName); + Assert.That(observedHeaders.TryGetValue("user-agent", out var userAgent), Is.True); + Assert.That(userAgent, Does.StartWith("custom-wrapper/9.9.9")); + } + + [Test] + public async Task BlankSdkIdentifier_FallsBackToDefault() + { + await using var fixture = await MockServerFixture.StartAsync(); + + fixture.MockServer.InjectResponses(TestTableName, + [ + MockResponses.CreateStreamResponse("test_stream_sdk_identifier_blank"), + MockResponses.RecordAckResponse(0), + ]); + + using var sdk = ZerobusSdk.CreateBuilder() + .Endpoint(fixture.ServerUrl) + .UnityCatalogUrl("https://mock-uc.com") + .SdkIdentifier(" ") + .DisableTls() + .Build(); + + using var stream = sdk.CreateStreamWithHeadersProvider( + CreateTableProperties(), new TestHeadersProvider(), CreateDefaultOptions()); + + stream.IngestRecord("test record data"u8.ToArray()); + stream.Flush(); + + // The core only substitutes its default for a literal empty override, not whitespace. + var observedHeaders = fixture.MockServer.GetLastRequestHeaders(TestTableName); + Assert.That(observedHeaders.TryGetValue("user-agent", out var userAgent), Is.True); + Assert.That(userAgent, Does.StartWith("zerobus-sdk-dotnet/")); + } + + [Test] + public async Task ApplicationName_AppendedToDefaultSdkIdentifier() + { + await using var fixture = await MockServerFixture.StartAsync(); + + fixture.MockServer.InjectResponses(TestTableName, + [ + MockResponses.CreateStreamResponse("test_stream_sdk_identifier_app_name"), + MockResponses.RecordAckResponse(0), + ]); + + using var sdk = ZerobusSdk.CreateBuilder() + .Endpoint(fixture.ServerUrl) + .UnityCatalogUrl("https://mock-uc.com") + .ApplicationName("integration-test") + .DisableTls() + .Build(); + + using var stream = sdk.CreateStreamWithHeadersProvider( + CreateTableProperties(), new TestHeadersProvider(), CreateDefaultOptions()); + + stream.IngestRecord("test record data"u8.ToArray()); + stream.Flush(); + + // The core sends " " and tonic appends its own token. + var observedHeaders = fixture.MockServer.GetLastRequestHeaders(TestTableName); + Assert.That(observedHeaders.TryGetValue("user-agent", out var userAgent), Is.True); + Assert.That(userAgent, Does.StartWith("zerobus-sdk-dotnet/")); + Assert.That(userAgent, Does.Contain(" integration-test")); + } +}