Skip to content

Commit

Permalink
Merged PR 742682: Revert 'Log plugin process stderr if plugin start f…
Browse files Browse the repository at this point in the history 
…ails'

Log plugin process stderr if plugin start fails

Reverts !739622
  • Loading branch information
@benwitmanmsft
benwitmanmsft authored and Oleksii Kononenko committed Oct 2, 2023
1 parent 02f45fb commit d64275b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 27 deletions.
44 changes: 21 additions & 23 deletions Public/Src/Utilities/Plugin/PluginFactory.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,51 +7,38 @@
using System.Threading;
using System.Threading.Tasks;
using BuildXL.Utilities.Core;
using BuildXL.Utilities.Instrumentation.Common;

namespace BuildXL.Plugin
{
/// <nodoc />
public class PluginFactory: IPluginFactory
{
/// <nodoc />
private readonly LoggingContext m_loggingContext;

/// <nodoc />
public PluginFactory(LoggingContext loggingContext)
{
m_loggingContext = loggingContext;
}
public static PluginFactory Instance = new PluginFactory();

private const string StartPluginProcessArgumentsForamt = "--ipcmoniker {0} --logdir {1} --logVerbose {2}";

private IPlugin PluginCreation_RunInProcess(PluginCreationArgument argument)
private readonly Func<PluginCreationArgument, IPlugin> m_pluginRunInProcessCreationFunc = argument =>
{
Tracing.Logger.Log.PluginManagerLogMessage(m_loggingContext, $"PluginFactory starting plugin {argument.PluginPath} with args " +
$"{string.Format(StartPluginProcessArgumentsForamt, argument.ConnectionOption.IpcMoniker, argument.ConnectionOption.LogDir, argument.ConnectionOption.LogVerbose)}");

var processStartInfo = new ProcessStartInfo(argument.PluginPath)
{
Arguments = string.Format(StartPluginProcessArgumentsForamt, argument.ConnectionOption.IpcMoniker, argument.ConnectionOption.LogDir, argument.ConnectionOption.LogVerbose),
UseShellExecute = false,
RedirectStandardOutput = false,
RedirectStandardError = true,
};
string id = argument.PluginId;
var process = new Process();
process.StartInfo = processStartInfo;
bool success = process.Start();

Tracing.Logger.Log.PluginManagerLogMessage(m_loggingContext, $"PluginFactory start result for plugin {argument.PluginPath}: {success}");
process.Start();
var client = argument.CreatePluginClientFunc.Invoke(argument.ConnectionOption);
var plugin = new Plugin(id, argument.PluginPath, process, client);
return plugin;
}
};

private IPlugin PluginCreation_RunInThread(PluginCreationArgument argument)
private readonly Func<PluginCreationArgument, IPlugin> m_pluginRunInThreadCreationFunc = argument =>
{
Contract.RequiresNotNull(argument, "argument can't be null");
Contract.RequiresNotNull(argument.RunInPluginThreadAction, "runInpluginThead can't be null");
Expand All @@ -65,14 +52,14 @@ private IPlugin PluginCreation_RunInThread(PluginCreationArgument argument)
var client = argument.CreatePluginClientFunc.Invoke(argument.ConnectionOption);
var plugin = new Plugin(argument.PluginId, argument.PluginPath, pluginTask, cancellationTokenSource, client);
return plugin;
}
};

/// <nodoc />
public IPlugin CreatePlugin(PluginCreationArgument pluginCreationArgument)
{
return pluginCreationArgument.RunInSeparateProcess ?
PluginCreation_RunInProcess(pluginCreationArgument) :
PluginCreation_RunInThread(pluginCreationArgument);
m_pluginRunInProcessCreationFunc.Invoke(pluginCreationArgument) :
m_pluginRunInThreadCreationFunc.Invoke(pluginCreationArgument);
}

/// <nodoc />
Expand All @@ -91,8 +78,6 @@ public async Task<Possible<IPlugin>> CreatePluginAsync(PluginCreationArgument pl
}
else
{
string stderr = await plugin.PluginProcess.StandardError.ReadToEndAsync();
Tracing.Logger.Log.PluginManagerLogMessage(m_loggingContext, $"Plugin process stderr:\n{stderr}");
plugin.PluginProcess.Kill();
return pluginCreationResult.Failure;
}
Expand All @@ -108,5 +93,18 @@ public string CreatePluginId()
{
return Guid.NewGuid().ToString();
}

/// <nodoc />
public static void SetGrpcPluginClientBasedOnMessageType(IPlugin plugin, PluginMessageType messageType)
{
//switch(messageType)
//{
// case PluginMessageType.ParseLogMessage:
// plugin.SetLogParsePluginClient();
// break;
// default:
// break;
//}
}
}
}
6 changes: 2 additions & 4 deletions Public/Src/Utilities/Plugin/PluginManager.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ public class PluginManager
{
private readonly ConcurrentDictionary<string, Task<Possible<IPlugin>>> m_plugins;
private readonly PluginHandlers m_pluginHandlers;
private readonly PluginFactory m_pluginFactory;
private bool m_isDisposed = false;
private readonly LoggingContext m_loggingContext;
private readonly IReadOnlyList<string> m_pluginPaths;
Expand Down Expand Up @@ -74,7 +73,6 @@ public PluginManager(LoggingContext loggingContext, string logDirectory, IEnumer
{
m_plugins = new ConcurrentDictionary<string, Task<Possible<IPlugin>>>();
m_pluginHandlers = new PluginHandlers();
m_pluginFactory = new PluginFactory(loggingContext);
m_pluginStopTaskSource = TaskSourceSlim.Create<Unit>();
m_loggingContext = loggingContext;
m_logDirectory = logDirectory;
Expand Down Expand Up @@ -180,7 +178,7 @@ private async Task<Possible<IPlugin>> CreatePluginAsync(PluginCreationArgument p
}

var sw = Stopwatch.StartNew();
var result = await m_pluginFactory.CreatePluginAsync(pluginCreationArgument);
var result = await PluginFactory.Instance.CreatePluginAsync(pluginCreationArgument);

Interlocked.Add(ref m_pluginLoadingTime, sw.ElapsedMilliseconds);

Expand Down Expand Up @@ -252,7 +250,7 @@ public async Task<Possible<ProcessResultMessageResponse>> ProcessResultAsync(str

private PluginCreationArgument GetPluginArgument(string pluginPath, bool runInSeparateProcess)
{
var pluginId = m_pluginFactory.CreatePluginId();
var pluginId = PluginFactory.Instance.CreatePluginId();
return new PluginCreationArgument()
{
PluginPath = pluginPath,
Expand Down

0 comments on commit d64275b

Please sign in to comment.