Skip to content

Commit 0262b49

Browse files
authored
Fix Socket telemetry outerloop test failures (#45170)
Based purely on code inspection, since I couldn't repro the failures happening in the lab, I believe what's happening is we're not outputting the right events if the connect ends up completing so fast that it's treated as a synchronous completion. The fix is to move the relevant tracing to be done when the work completes, regardless of the completion mode. I was able to simulate at least one set of failures by delaying the calling thread before it reaches a particular point, and this fixes that issue, so even if it's not fixing all known problems (hopefully it is), it's at least fixing some.
1 parent e08358f commit 0262b49

File tree

1 file changed

+9
-3
lines changed

1 file changed

+9
-3
lines changed

src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -813,19 +813,25 @@ caughtException is OperationCanceledException ||
813813
}
814814

815815
// Complete the operation.
816-
if (SocketsTelemetry.Log.IsEnabled() && !_disableTelemetry) LogBytesTransferEvents(_connectSocket?.SocketType, SocketAsyncOperation.Connect, internalArgs.BytesTransferred);
816+
if (SocketsTelemetry.Log.IsEnabled() && !_disableTelemetry)
817+
{
818+
LogBytesTransferEvents(_connectSocket?.SocketType, SocketAsyncOperation.Connect, internalArgs.BytesTransferred);
819+
AfterConnectAcceptTelemetry();
820+
}
817821
Complete();
818822

819-
// If the caller is treating this operation as pending, own the completion.
823+
// Clean up after our temporary arguments.
820824
internalArgs.Dispose();
825+
826+
// If the caller is treating this operation as pending, own the completion.
821827
if (!internalArgs.ReachedCoordinationPointFirst())
822828
{
823829
// Regardless of _flowExecutionContext, context will have been flown through this async method, as that's part
824830
// of what async methods do. As such, we're already on whatever ExecutionContext is the right one to invoke
825831
// the completion callback. This method may have even mutated the ExecutionContext, in which case for telemetry
826832
// we need those mutations to be surfaced as part of this callback, so that logging performed here sees those
827833
// mutations (e.g. to the current Activity).
828-
OnCompletedInternal();
834+
OnCompleted(this);
829835
}
830836
}
831837
}

0 commit comments

Comments
 (0)