Implement collection rule filters. #882
Conversation
Refactor ProcessInfo class to be separate.
...t.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/MonitorCollectRunner.CollectionRule.cs
Outdated
Show resolved
Hide resolved
...t.Diagnostics.Monitoring.Tool.FunctionalTests/Runners/MonitorCollectRunner.CollectionRule.cs
Show resolved
Hide resolved
|
|
||
| DiagnosticsClient client = new(endpointInfo.Endpoint); | ||
|
|
||
| string commandLine = endpointInfo.CommandLine; |
There was a problem hiding this comment.
Looks like that if a .NET 5.0 runtime is suspended and a GetProcessInfo command is issued over the diagnostic pipe, then the CommandLine only contains the process name on Unix operating systems, e.g.:
/__w/1/s/.dotnet/dotnet
If the GetProcessInfo command is issued AFTER the runtime is resumed, then it returns the property command line, e.g.:
/__w/1/s/.dotnet/dotnet /__w/1/s/artifacts/bin/Microsoft.Diagnostics.Monitoring.UnitTestApp/Release/net5.0/Microsoft.Diagnostics.Monitoring.UnitTestApp.dll AsyncWait
From what I can tell, this does not repro on .NET 6 when running these newly added tests.
@josalem, are you aware of any issues in regard to this for .NET 5? This is an issue for dotnet-monitor because command line is one of the usable criteria for filtering collection rules.
A potential workaround is to force run the EventProcessInfoPipeline when we know dotnet-monitor hasn't resumed the runtime yet and it is running on .NET 5.
There was a problem hiding this comment.
That's odd. No, I'm not aware of an issue like this. This is only happening while the .net5 app is suspended?
There was a problem hiding this comment.
Correct. .NET 5, non-Windows, suspended. This build demonstrates the symptoms: https://dev.azure.com/dnceng/public/_build/results?buildId=1372672&view=results
I can produce another build with more verbosing logging that demonstrates the issue precisely. I can also try adjusting the GetProcessInfoTests in the dotnet/diagnostics repo to demonstrate it more simply, but that's running on .NET 6 (which this does not repro), if I change them back to .NET 5.
There was a problem hiding this comment.
I'm disabling the affected test for .NET 5 on non-Windows and logged an issue to track and investigate this later: #885
There was a problem hiding this comment.
After disabling the test for .NET 5 on non-Windows, it started failing on .NET 6 for non-Windows. Also, I've updated the GetProcessInfoTests (dotnet/diagnostics#2603) in dotnet/diagnostics to check that the values before and after are consistent, which seems to be the case. So this is looking more like a dotnet-monitor bug than a runtime bug. I'm going to keep investigating this and update this thread with new info when its available.
There was a problem hiding this comment.
Okay, now I have nearly definitive proof that the runtime (likely in both .NET 5 and .NET 6) only reports the process name for the command line field on the ProcessInfo reponse on non-Windows platforms when the runtime is suspended. See this build, which is the same as the changes in dotnet/diagnostics#2603, except that it writes out the command line at the end of the test and intentionally fails the test. It shows that is has a value of something like /__w/1/s/.dotnet/dotnet. This is occurring before and shortly after resuming the runtime. So this is likely another race condition where the command line arguments aren't available until sometime in the future after resuming the runtime. This does not repro on Windows.
src/Tools/dotnet-monitor/CollectionRules/CollectionRuleService.cs
Outdated
Show resolved
Hide resolved
This change filters collection rules based on the filters specified in configuration. It includes a refactor of the DiagnosticServices+ProcessInfo class to a separate ProcessInfoImpl class as well as a few basic tests to verify that the rules are able to be conditionally applied.
closes #870