-
Notifications
You must be signed in to change notification settings - Fork 4.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix System.Net.Sockets telemetry #42188
Conversation
Taken from c4c9a2f99b7e339388199086d3014041abccc21e
Tagging subscribers to this area: @dotnet/ncl |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
CC @josalem for |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs
Outdated
Show resolved
Hide resolved
Changes RE: #42128 look good to me 👍 |
/azp run runtime-libraries-coreclr outerloop |
This comment has been minimized.
This comment has been minimized.
a619308
to
98ec512
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
98ec512
to
d0d849f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
src/libraries/Common/tests/System/Diagnostics/Tracing/TestEventListener.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketsTelemetry.cs
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Sockets/tests/FunctionalTests/TelemetryTest.cs
Outdated
Show resolved
Hide resolved
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
/backport to release/5.0-rc2 |
Started backporting to release/5.0-rc2: https://github.com/dotnet/runtime/actions/runs/272073672 |
@MihaZupan backporting to release/5.0-rc2 failed, the patch most likely resulted in conflicts: $ git am --3way --ignore-whitespace --keep-non-patch changes.patch
Applying: Correct NameResolutionTelemetry logic
Applying: Enable listening to multiple sources with TestEventListener
Applying: Workaround EventWrittenEventArgs bug when the EventArgs are stored
Applying: Correct System.Net.Sockets Telemetry
Applying: Avoid using value tuple in TestEventListener
Applying: Remove unnecessary argument to OnCompletedInternal
Applying: Remove redundant Telemetry.IsEnabled check
Applying: Log Connect/Accept start before the initial context capture
Applying: Use SocketHelperBase in Accept tests
Applying: Avoid duplicate events for BeginConnect without ConnextEx support
Applying: Enable Sync Socket tests
Applying: Revert unrelated SocketPal change
Applying: Log the correct ErrorCode in case of socket disposal
Applying: Add more info on TelemetryTest timeout
Applying: Add PlatformSpecific attribute to ConnectFailure test
Applying: Add missing BeginConnect AfterConnect call on sync failure
error: sha1 information is lacking or useless (src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs).
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0016 Add missing BeginConnect AfterConnect call on sync failure
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Error: The process '/usr/bin/git' failed with exit code 128 Please backport manually! |
Includes:
System.Net.NameResolution
telemetry fix outlined by @ManickaP in Socket.Connect(Start|Stop) activity ID missmatch on Windows #41670 (comment) - Stop wasn't called in the same scope as startTestEventListener
to allow for listening to multipleEventSource
s (enabling activity tracking in tests)EventWrittenEventArgs
bug (EventSource EventWrittenEventArgs.ActivityId isn't always set #42128) in test code onlySystem.Net.Security
telemetryBefore, we were logging Start events after capturing the execution context and calling Stop before restoring it.
The PR creates a common point
OnCompletedInternal
that is called when the execution context is restored from both sync and async completions.Fixes #41670