Skip to content
Draft
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
68 changes: 68 additions & 0 deletions csharp/src/DatabricksConnection.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,29 @@ internal class DatabricksConnection : SparkHttpConnection

// Telemetry
private IConnectionTelemetry _telemetry = NoOpConnectionTelemetry.Instance;

// Issue #477: long-lived "lifetime" Activity that wraps every per-call activity
// emitted on this connection. Opened at the end of the constructor (after
// ValidateProperties succeeds) so it becomes Activity.Current for the synchronous
// Database.Connect caller — that way OpenAsync().Wait(), ApplyServerSidePropertiesAsync().Wait(),
// and every subsequent call (executes, metadata, dispose) automatically chains
// underneath via Activity.Current. Stopped in Dispose AFTER base.Dispose runs
// DisposeClient so the close-session RPC also lands under this root.
//
// Without this wrapper every top-level driver call is its own root TraceId — the
// MemoryStress baseline in #477 shows 12,958 TraceIds across one run, 95% of which
// are single-span throwaway ReadNextRecordBatchAsync roots, breaking trace-tree
// visualization on fleet dashboards.
//
// STATIC ActivitySource is intentional: the per-instance source held by
// <see cref="ActivityTrace"/> is disposed inside <c>TracingConnection.Dispose</c>
// BEFORE <see cref="Dispose(bool)"/> returns, which would silently drop the
// <c>Stop()</c> for our lifetime Activity (no listener notification once the
// source is disposed). A process-wide source survives connection disposal and
// keeps notifying listeners.
private static readonly ActivitySource s_lifetimeActivitySource = new ActivitySource("AdbcDrivers.Databricks");
private Activity? _lifetimeActivity;
private bool _lifetimeActivityStopped;
// Stopwatch covering the connection lifetime; started at construction and used to
// measure session-open latency for the CREATE_SESSION telemetry event. The wall-clock
// time between construction and HandleOpenSessionResponse encompasses transport setup
Expand Down Expand Up @@ -157,8 +180,30 @@ internal DatabricksConnection(
Lz4BufferPool = lz4BufferPool ?? System.Buffers.ArrayPool<byte>.Create(maxArrayLength: 4 * 1024 * 1024, maxArraysPerBucket: 10);

ValidateProperties();

// Issue #477: open the connection-lifetime Activity now, AFTER ValidateProperties
// succeeds, so a throw from validation doesn't leak a half-open Activity. The
// call site (DatabricksDatabase.Connect) runs synchronously, so starting the
// Activity here makes it Activity.Current for the same async context that
// immediately invokes OpenAsync().Wait() and ApplyServerSidePropertiesAsync().Wait();
// those per-call activities therefore chain underneath via Activity.Current with
// no extra plumbing. Kind=Internal because this is a self-traced scope, not an RPC.
_lifetimeActivity = s_lifetimeActivitySource.StartActivity(
"DatabricksConnection.Lifetime",
ActivityKind.Internal);
}

/// <summary>
/// The long-lived <see cref="Activity"/> opened in the constructor that wraps
/// every per-call activity emitted on this connection. Exposed (internal) so
/// <see cref="DatabricksStatement"/> can pass <see cref="Activity.Context"/>
/// explicitly as the parent context to its own lifetime Activity — that guards
/// against the case where the Statement is constructed on a different async
/// context than where the Connection was opened (so <see cref="Activity.Current"/>
/// would not see this Activity).
/// </summary>
internal Activity? LifetimeActivity => _lifetimeActivity;

private void LogConnectionProperties(Activity? activity)
{
if (activity == null) return;
Expand Down Expand Up @@ -1100,13 +1145,36 @@ protected override void Dispose(bool disposing)
// This is synchronous because Dispose() cannot be async; we block on
// GetAwaiter().GetResult(), which is acceptable in Dispose.
DisposeTelemetryAsync().GetAwaiter().GetResult();

// Issue #477: stop the connection-lifetime Activity LAST, after
// base.Dispose has already run the TCloseSessionReq RPC (which emits
// HiveServer2Connection.DisposeClient as a child of this Activity) and
// after DELETE_SESSION telemetry has flushed. Stopping it earlier would
// re-orphan DisposeClient back to a root span — exactly the case
// ConcurrentDisposeTests flags in #477. Stop is idempotent across
// repeated Dispose() calls.
StopLifetimeActivity();
}
return;
}

base.Dispose(disposing);
}

/// <summary>
/// Idempotently stops the connection-lifetime Activity (#477). Called from
/// <see cref="Dispose(bool)"/>; calling it from a finalizer / unmanaged-only path
/// is intentionally a no-op because Activity stop is not async-signal-safe and we
/// already accept that GC'd Activity objects leak (their state is GC-tracked).
/// </summary>
private void StopLifetimeActivity()
{
if (_lifetimeActivityStopped) return;
_lifetimeActivityStopped = true;
_lifetimeActivity?.Stop();
_lifetimeActivity = null;
}

/// <summary>
/// Emits the DELETE_SESSION telemetry event when a session was previously opened.
/// <paramref name="elapsedMs"/> is the measured duration of <c>base.Dispose</c>,
Expand Down
51 changes: 51 additions & 0 deletions csharp/src/DatabricksStatement.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,28 @@ internal class DatabricksStatement : SparkStatement, IHiveServer2Statement
private readonly Stopwatch _statementLifetimeStopwatch = Stopwatch.StartNew();
private bool _closeStatementTelemetryEmitted;

// Issue #477: long-lived "lifetime" Activity that wraps every per-call activity
// emitted on behalf of this statement. Parented explicitly to the connection's
// own lifetime Activity (see DatabricksConnection.LifetimeActivity) so the
// hierarchy is robust even when the Statement is constructed on a different
// async context than where the Connection was opened — in that case
// Activity.Current here would NOT see the connection's lifetime Activity.
// Stopped in Dispose AFTER the statement's CLOSE_STATEMENT telemetry has been
// emitted so any close-side spans (DatabricksCompositeReader.Dispose, the
// CloseStatement RPC) chain underneath this scope.
//
// STATIC ActivitySource is intentional: the per-instance source held by
// <see cref="ActivityTrace"/> is shared with the connection and disposed
// inside <c>TracingConnection.Dispose</c> BEFORE <see cref="DatabricksConnection.Dispose"/>
// returns. Listeners registered against that source stop receiving
// notifications once it's disposed, so a Stop() called from the statement
// Dispose path (which might run alongside or after connection disposal in
// pooled scenarios) would silently drop. A process-wide static source
// survives both lifecycles.
private static readonly ActivitySource s_lifetimeActivitySource = new ActivitySource("AdbcDrivers.Databricks");
private Activity? _lifetimeActivity;
private bool _lifetimeActivityStopped;

// Populated by DatabricksCompositeReader.Dispose when it issues TCloseOperationReq
// to the server. Lets us report the actual close-RPC latency in the CLOSE_STATEMENT
// telemetry event emitted at statement Dispose, instead of a meaningless 0.
Expand Down Expand Up @@ -127,6 +149,21 @@ public DatabricksStatement(DatabricksConnection connection)
{
SetOption(ApacheParameters.PollTimeMilliseconds, DatabricksConstants.DefaultAsyncExecPollIntervalMs.ToString());
}

// Issue #477: open the statement-lifetime Activity here. Pass the connection's
// lifetime Activity Context explicitly as the parent so the hierarchy is
// robust against cross-async-context construction — Activity.Current is
// AsyncLocal, so if CreateStatement is invoked on a continuation that didn't
// observe the connection-open context, Activity.Current would NOT see
// ConnectionLifetime. The default ActivityContext (when LifetimeActivity is
// null, e.g. no listener was attached when the connection was opened) is
// "no parent", which still lets the statement-lifetime Activity be its own
// root and parent all per-call activities under one TraceId.
var connectionLifetimeContext = connection.LifetimeActivity?.Context ?? default;
_lifetimeActivity = s_lifetimeActivitySource.StartActivity(
"DatabricksStatement.Lifetime",
ActivityKind.Internal,
connectionLifetimeContext);
}

private StatementTelemetryContext? CreateTelemetryContext(Telemetry.Proto.Statement.Types.Type statementType)
Expand Down Expand Up @@ -1346,6 +1383,20 @@ protected override void Dispose(bool disposing)
}
}
base.Dispose(disposing);

// Issue #477: stop the statement-lifetime Activity LAST so any spans emitted
// during base.Dispose (e.g. the per-statement CloseStatement RPC, the
// composite-reader's CloseOperation, etc.) chain underneath this scope.
// Idempotent across repeated Dispose() calls.
if (disposing)
{
if (!_lifetimeActivityStopped)
{
_lifetimeActivityStopped = true;
_lifetimeActivity?.Stop();
_lifetimeActivity = null;
}
}
}

/// <inheritdoc/>
Expand Down
Loading
Loading