-
Notifications
You must be signed in to change notification settings - Fork 152
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
Added ETW events #322
Added ETW events #322
Conversation
Codecov Report
@@ Coverage Diff @@
## v2.1 #322 +/- ##
==========================================
- Coverage 89.64% 87.43% -2.21%
==========================================
Files 31 32 +1
Lines 1951 2046 +95
Branches 369 390 +21
==========================================
+ Hits 1749 1789 +40
- Misses 156 209 +53
- Partials 46 48 +2
Continue to review full report at Codecov.
|
src/StreamJsonRpc/JsonRpc.cs
Outdated
|
||
if (JsonRpcEventSource.Instance.IsEnabled()) | ||
{ | ||
JsonRpcEventSource.Instance.SendRequestStop($"ResponseDetails: {responseDetails}; RequestDetails: Id: \"{request.Id}\", Method: \"{request.Method}\", Arguments: \"{JsonRpcEventSource.GetArgumentsString(request.Arguments)}\""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some reason, Duration_Msec is not getting captured for most of the SendRequestStop events. For such events, no ActivityId shows up in PerfView.
In case of WinForms designer, we spawn a .NetCore process from devenv. Not sure if framework mismatch is the issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's probably because of missing attribute properties. See #322 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tried but it didn't help.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking much better.
/azp run vs-streamjsonrpc |
Commenter does not have sufficient privileges for PR 322 in repo microsoft/vs-streamjsonrpc |
47a39ad
to
959bdc7
Compare
I took the liberty of squashing and rebasing your commits from master to v2.1 so that we can check this in for 16.3. |
0a6751b
to
bd10c27
Compare
By providing method names and args as separate parameters instead of a pre-formatted string, ETW recognizes it as distinct data, allowing us to filter and sort on it, etc.
bd10c27
to
a0ba0d7
Compare
Thanks Andrew for making the changes. Looks very good now. |
Update .NET SDK and xunit.runner.visualstudio
A sample PerfView log:
Call stacks getting captured with the events: