Skip to content
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.

Sign up for GitHub

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

Implement collection rule filters. #882

Merged
merged 5 commits into from
Sep 21, 2021
Merged

Implement collection rule filters. #882

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc21807
Implement collection rule filters.
jander-msft Sep 20, 2021
48fce90
PR Feedback
jander-msft Sep 20, 2021
d0891b2
Disable CommandLineFilterMatchTest on .NET 5 non-Windows
jander-msft Sep 21, 2021
d557f53
Expand exclusion to .NET 5+ on non-Windows
jander-msft Sep 21, 2021
2fc9465
Move IProcessInfo creation to ApplyRules method.
jander-msft Sep 21, 2021
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
142 changes: 4 additions & 138 deletions src/Microsoft.Diagnostics.Monitoring.WebApi/DiagnosticServices.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,29 +2,17 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.Extensions.Options;
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Diagnostics.Monitoring.EventPipe;
using Microsoft.Diagnostics.NETCore.Client;
using Microsoft.Extensions.Options;

namespace Microsoft.Diagnostics.Monitoring.WebApi
{
internal sealed class DiagnosticServices : IDiagnosticServices
{
// The value of the operating system field of the ProcessInfo result when the target process is running
// on a Windows operating system.
private const string ProcessOperatingSystemWindowsValue = "windows";

// The amount of time to wait before cancelling get additional process information (e.g. getting
// the process command line if the IEndpointInfo doesn't provide it).
private static readonly TimeSpan ExtendedProcessInfoTimeout = TimeSpan.FromMilliseconds(1000);

private readonly IEndpointInfoSourceInternal _endpointInfoSource;
private readonly IOptionsMonitor<ProcessFilterOptions> _defaultProcessOptions;

Expand All @@ -42,7 +30,7 @@ public async Task<IEnumerable<IProcessInfo>> GetProcessesAsync(DiagProcessFilter
try
{
using CancellationTokenSource extendedInfoCancellation = CancellationTokenSource.CreateLinkedTokenSource(token);
IList<Task<ProcessInfo>> processInfoTasks = new List<Task<ProcessInfo>>();
IList<Task<IProcessInfo>> processInfoTasks = new List<Task<IProcessInfo>>();
foreach (IEndpointInfo endpointInfo in await _endpointInfoSource.GetEndpointInfoAsync(token))
{
// CONSIDER: Can this processing be pushed into the IEndpointInfoSource implementation and cached
Expand All @@ -51,13 +39,13 @@ public async Task<IEnumerable<IProcessInfo>> GetProcessesAsync(DiagProcessFilter
// - .NET Core 3.1 processes, which require issuing a brief event pipe session to get the process commmand
// line information and parse out the process name
// - Caching entrypoint information (when that becomes available).
processInfoTasks.Add(ProcessInfo.FromEndpointInfoAsync(endpointInfo, extendedInfoCancellation.Token));
processInfoTasks.Add(ProcessInfoImpl.FromEndpointInfoAsync(endpointInfo, extendedInfoCancellation.Token));
}

// FromEndpointInfoAsync can fill in the command line for .NET Core 3.1 processes by invoking the
// event pipe and capturing the ProcessInfo event. Timebox this operation with the cancellation token
// so that getting the process list does not take a long time or wait indefinitely.
extendedInfoCancellation.CancelAfter(ExtendedProcessInfoTimeout);
extendedInfoCancellation.CancelAfter(ProcessInfoImpl.ExtendedProcessInfoTimeout);

await Task.WhenAll(processInfoTasks);

Expand Down Expand Up @@ -110,127 +98,5 @@ private async Task<IProcessInfo> GetProcessAsync(DiagProcessFilter processFilter
throw new ArgumentException(Strings.ErrorMessage_MultipleTargetProcesses);
}
}

/// <summary>
/// We want to make sure we destroy files we finish streaming.
/// We want to make sure that we stream out files since we compress on the fly; the size cannot be known upfront.
/// CONSIDER The above implies knowledge of how the file is used by the rest api.
/// </summary>
private sealed class AutoDeleteFileStream : FileStream
{
public AutoDeleteFileStream(string path) : base(path, FileMode.Open, FileAccess.ReadWrite, FileShare.ReadWrite | FileShare.Delete,
bufferSize: 4096, FileOptions.DeleteOnClose)
{
}

public override bool CanSeek => false;
}

private sealed class ProcessInfo : IProcessInfo
{
// String returned for a process field when its value could not be retrieved. This is the same
// value that is returned by the runtime when it could not determine the value for each of those fields.
private static readonly string ProcessFieldUnknownValue = "unknown";

public ProcessInfo(
IEndpointInfo endpointInfo,
string commandLine,
string processName)
{
EndpointInfo = endpointInfo;

// The GetProcessInfo command will return "unknown" for values for which it does
// not know the value, such as operating system and process architecture if the
// process is running on one that is not predefined. Mimic the same behavior here
// when the extra process information was not provided.
CommandLine = commandLine ?? ProcessFieldUnknownValue;
ProcessName = processName ?? ProcessFieldUnknownValue;
}

public static async Task<ProcessInfo> FromEndpointInfoAsync(IEndpointInfo endpointInfo)
{
using CancellationTokenSource extendedInfoCancellation = new CancellationTokenSource(ExtendedProcessInfoTimeout);
return await FromEndpointInfoAsync(endpointInfo, extendedInfoCancellation.Token);
}

// Creates a ProcessInfo object from the IEndpointInfo. Attempts to get the command line using event pipe
// if the endpoint information doesn't provide it. The cancelation token can be used to timebox this fallback
// mechansim.
public static async Task<ProcessInfo> FromEndpointInfoAsync(IEndpointInfo endpointInfo, CancellationToken extendedInfoCancellationToken)
{
if (null == endpointInfo)
{
throw new ArgumentNullException(nameof(endpointInfo));
}

var client = new DiagnosticsClient(endpointInfo.Endpoint);

string commandLine = endpointInfo.CommandLine;
if (string.IsNullOrEmpty(commandLine))
{
try
{
var infoSettings = new EventProcessInfoPipelineSettings
{
Duration = Timeout.InfiniteTimeSpan,
};

await using var pipeline = new EventProcessInfoPipeline(client, infoSettings,
(cmdLine, token) => { commandLine = cmdLine; return Task.CompletedTask; });

await pipeline.RunAsync(extendedInfoCancellationToken);
}
catch
{
}
}

string processName = null;
if (!string.IsNullOrEmpty(commandLine))
{
// Get the process name from the command line
bool isWindowsProcess = false;
if (string.IsNullOrEmpty(endpointInfo.OperatingSystem))
{
// If operating system is null, the process is likely .NET Core 3.1 (which doesn't have the GetProcessInfo command).
// Since the underlying diagnostic communication channel used by the .NET runtime requires that the diagnostic process
// must be running on the same type of operating system as the target process (e.g. dotnet-monitor must be running on Windows
// if the target process is running on Windows), then checking the local operating system should be a sufficient heuristic
// to determine the operating system of the target process.
isWindowsProcess = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
}
else
{
isWindowsProcess = ProcessOperatingSystemWindowsValue.Equals(endpointInfo.OperatingSystem, StringComparison.OrdinalIgnoreCase);
}

string processPath = CommandLineHelper.ExtractExecutablePath(commandLine, isWindowsProcess);
if (!string.IsNullOrEmpty(processPath))
{
processName = Path.GetFileName(processPath);
if (isWindowsProcess)
{
// Remove the extension on Windows to match the behavior of Process.ProcessName
processName = Path.GetFileNameWithoutExtension(processName);
}
}
}

return new ProcessInfo(
endpointInfo,
commandLine,
processName);
}

public IEndpointInfo EndpointInfo { get; }

public string CommandLine { get; }

public string OperatingSystem => EndpointInfo.OperatingSystem ?? ProcessFieldUnknownValue;

public string ProcessArchitecture => EndpointInfo.ProcessArchitecture ?? ProcessFieldUnknownValue;

public string ProcessName { get; }
}
}
}
131 changes: 131 additions & 0 deletions src/Microsoft.Diagnostics.Monitoring.WebApi/ProcessInfoImpl.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using Microsoft.Diagnostics.Monitoring.EventPipe;
using Microsoft.Diagnostics.NETCore.Client;
using System;
using System.IO;
using System.Runtime.InteropServices;
using System.Threading;
using System.Threading.Tasks;

namespace Microsoft.Diagnostics.Monitoring.WebApi
{
/// Named "ProcessInfoImpl" to disambiguate from Microsoft.Diagnostics.NETCore.client ProcessInfo
/// class returned from issuing GetProcesssInfo command on diagnostic pipe.
jander-msft marked this conversation as resolved.
Show resolved Hide resolved
internal sealed class ProcessInfoImpl : IProcessInfo
{
// The amount of time to wait before cancelling get additional process information (e.g. getting
// the process command line if the IEndpointInfo doesn't provide it).
public static readonly TimeSpan ExtendedProcessInfoTimeout = TimeSpan.FromMilliseconds(1000);

// String returned for a process field when its value could not be retrieved. This is the same
// value that is returned by the runtime when it could not determine the value for each of those fields.
private static readonly string ProcessFieldUnknownValue = "unknown";

// The value of the operating system field of the ProcessInfo result when the target process is running
// on a Windows operating system.
private const string ProcessOperatingSystemWindowsValue = "windows";

public ProcessInfoImpl(
IEndpointInfo endpointInfo,
string commandLine,
string processName)
{
EndpointInfo = endpointInfo;

// The GetProcessInfo command will return "unknown" for values for which it does
// not know the value, such as operating system and process architecture if the
// process is running on one that is not predefined. Mimic the same behavior here
// when the extra process information was not provided.
CommandLine = commandLine ?? ProcessFieldUnknownValue;
ProcessName = processName ?? ProcessFieldUnknownValue;
}

public static async Task<IProcessInfo> FromEndpointInfoAsync(IEndpointInfo endpointInfo)
{
using CancellationTokenSource extendedInfoCancellation = new(ExtendedProcessInfoTimeout);
return await FromEndpointInfoAsync(endpointInfo, extendedInfoCancellation.Token);
}

// Creates a ProcessInfo object from the IEndpointInfo. Attempts to get the command line using event pipe
jander-msft marked this conversation as resolved.
Show resolved Hide resolved
// if the endpoint information doesn't provide it. The cancelation token can be used to timebox this fallback
// mechansim.
jander-msft marked this conversation as resolved.
Show resolved Hide resolved
public static async Task<IProcessInfo> FromEndpointInfoAsync(IEndpointInfo endpointInfo, CancellationToken extendedInfoCancellationToken)
{
if (null == endpointInfo)
{
throw new ArgumentNullException(nameof(endpointInfo));
}

DiagnosticsClient client = new(endpointInfo.Endpoint);

string commandLine = endpointInfo.CommandLine;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's odd. No, I'm not aware of an issue like this. This is only happening while the .net5 app is suspended?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm disabling the affected test for .NET 5 on non-Windows and logged an issue to track and investigate this later: #885

Copy link
Member Author

@jander-msft jander-msft Sep 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@jander-msft jander-msft Sep 21, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (string.IsNullOrEmpty(commandLine))
{
try
{
EventProcessInfoPipelineSettings infoSettings = new()
{
Duration = Timeout.InfiniteTimeSpan,
};

await using EventProcessInfoPipeline pipeline = new(client, infoSettings,
(cmdLine, token) => { commandLine = cmdLine; return Task.CompletedTask; });

await pipeline.RunAsync(extendedInfoCancellationToken);
}
catch
{
}
}

string processName = null;
if (!string.IsNullOrEmpty(commandLine))
{
// Get the process name from the command line
bool isWindowsProcess = false;
if (string.IsNullOrEmpty(endpointInfo.OperatingSystem))
{
// If operating system is null, the process is likely .NET Core 3.1 (which doesn't have the GetProcessInfo command).
// Since the underlying diagnostic communication channel used by the .NET runtime requires that the diagnostic process
// must be running on the same type of operating system as the target process (e.g. dotnet-monitor must be running on Windows
// if the target process is running on Windows), then checking the local operating system should be a sufficient heuristic
// to determine the operating system of the target process.
isWindowsProcess = RuntimeInformation.IsOSPlatform(OSPlatform.Windows);
}
else
{
isWindowsProcess = ProcessOperatingSystemWindowsValue.Equals(endpointInfo.OperatingSystem, StringComparison.OrdinalIgnoreCase);
}

string processPath = CommandLineHelper.ExtractExecutablePath(commandLine, isWindowsProcess);
if (!string.IsNullOrEmpty(processPath))
{
processName = Path.GetFileName(processPath);
if (isWindowsProcess)
{
// Remove the extension on Windows to match the behavior of Process.ProcessName
processName = Path.GetFileNameWithoutExtension(processName);
}
}
}

return new ProcessInfoImpl(
endpointInfo,
commandLine,
processName);
}

public IEndpointInfo EndpointInfo { get; }

public string CommandLine { get; }

public string OperatingSystem => EndpointInfo.OperatingSystem ?? ProcessFieldUnknownValue;

public string ProcessArchitecture => EndpointInfo.ProcessArchitecture ?? ProcessFieldUnknownValue;

public string ProcessName { get; }
}
}
5 changes: 5 additions & 0 deletions src/Tests/Microsoft.Diagnostics.Monitoring.TestCommon/DotNetHost.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// See the LICENSE file in the project root for more information.

using System;
using System.IO;
using System.Runtime.InteropServices;

namespace Microsoft.Diagnostics.Monitoring.TestCommon
Expand All @@ -17,6 +18,10 @@ public partial class DotNetHost
public static Version RuntimeVersion =>
s_runtimeVersionLazy.Value;

public static string HostExeNameWithoutExtension => RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
Path.GetFileNameWithoutExtension(HostExePath) :
Path.GetFileName(HostExePath);

public static string HostExePath = RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ?
@"..\..\..\..\..\.dotnet\dotnet.exe" :
"../../../../../.dotnet/dotnet";
Expand Down
Loading