Skip to content

Commit 0ebb99c

Browse files
committed
fetching IsToolingConsoleJsonLogEntry earlier to break the debugging deadlock (#9486)
* fetching IsToolingConsoleJsonLogEntry earlier to break the debugging deadlock * PR fixes * Fixing unit test failure
1 parent 202e77f commit 0ebb99c

File tree

11 files changed

+41
-36
lines changed

11 files changed

+41
-36
lines changed

src/WebJobs.Script/Workers/Http/HttpWorkerProcess.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@ internal HttpWorkerProcess(string workerId,
3131
IWorkerConsoleLogSource consoleLogSource,
3232
IEnvironment environment,
3333
IMetricsLogger metricsLogger,
34-
IServiceProvider serviceProvider)
35-
: base(eventManager, processRegistry, workerProcessLogger, consoleLogSource, metricsLogger, serviceProvider, httpWorkerOptions.Description.UseStdErrorStreamForErrorsOnly)
34+
IServiceProvider serviceProvider,
35+
ILoggerFactory loggerFactory)
36+
: base(eventManager, processRegistry, workerProcessLogger, consoleLogSource, metricsLogger, serviceProvider, loggerFactory, httpWorkerOptions.Description.UseStdErrorStreamForErrorsOnly)
3637
{
3738
_processFactory = processFactory;
3839
_eventManager = eventManager;

src/WebJobs.Script/Workers/Http/HttpWorkerProcessFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ public HttpWorkerProcessFactory(IScriptEventManager eventManager,
4141
public IWorkerProcess Create(string workerId, string scriptRootPath, HttpWorkerOptions httpWorkerOptions)
4242
{
4343
ILogger workerProcessLogger = _loggerFactory.CreateLogger($"Worker.HttpWorkerProcess.{workerId}");
44-
return new HttpWorkerProcess(workerId, scriptRootPath, httpWorkerOptions, _eventManager, _workerProcessFactory, _processRegistry, workerProcessLogger, _consoleLogSource, _environment, _metricsLogger, _serviceProvider);
44+
return new HttpWorkerProcess(workerId, scriptRootPath, httpWorkerOptions, _eventManager, _workerProcessFactory, _processRegistry, workerProcessLogger, _consoleLogSource, _environment, _metricsLogger, _serviceProvider, _loggerFactory);
4545
}
4646
}
4747
}

src/WebJobs.Script/Workers/ProcessManagement/WorkerProcess.cs

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,14 @@ internal abstract class WorkerProcess : IWorkerProcess, IDisposable
2626
private readonly int processExitTimeoutInMilliseconds = 1000;
2727
private readonly IServiceProvider _serviceProvider;
2828
private readonly IDisposable _eventSubscription;
29+
private readonly Lazy<ILogger> _toolingConsoleJsonLoggerLazy;
2930

3031
private bool _useStdErrorStreamForErrorsOnly;
3132
private Queue<string> _processStdErrDataQueue = new Queue<string>(3);
3233
private IHostProcessMonitor _processMonitor;
3334
private object _syncLock = new object();
3435

35-
internal WorkerProcess(IScriptEventManager eventManager, IProcessRegistry processRegistry, ILogger workerProcessLogger, IWorkerConsoleLogSource consoleLogSource, IMetricsLogger metricsLogger, IServiceProvider serviceProvider, bool useStdErrStreamForErrorsOnly = false)
36+
internal WorkerProcess(IScriptEventManager eventManager, IProcessRegistry processRegistry, ILogger workerProcessLogger, IWorkerConsoleLogSource consoleLogSource, IMetricsLogger metricsLogger, IServiceProvider serviceProvider, ILoggerFactory loggerFactory, bool useStdErrStreamForErrorsOnly = false)
3637
{
3738
_processRegistry = processRegistry;
3839
_workerProcessLogger = workerProcessLogger;
@@ -41,6 +42,7 @@ internal WorkerProcess(IScriptEventManager eventManager, IProcessRegistry proces
4142
_metricsLogger = metricsLogger;
4243
_useStdErrorStreamForErrorsOnly = useStdErrStreamForErrorsOnly;
4344
_serviceProvider = serviceProvider;
45+
_toolingConsoleJsonLoggerLazy = new Lazy<ILogger>(() => loggerFactory.CreateLogger(WorkerConstants.ToolingConsoleLogCategoryName), isThreadSafe: true);
4446

4547
// We subscribe to host start events so we can handle the restart that occurs
4648
// on host specialization.
@@ -193,10 +195,16 @@ internal void BuildAndLogConsoleLog(string msg, LogLevel level)
193195
Message = msg,
194196
Level = level
195197
};
198+
196199
if (WorkerProcessUtilities.IsConsoleLog(msg))
197200
{
198201
_workerProcessLogger?.Log(level, WorkerProcessUtilities.RemoveLogPrefix(msg));
199202
}
203+
else if (WorkerProcessUtilities.IsToolingConsoleJsonLogEntry(msg))
204+
{
205+
// log with the message prefix as coretools expects it.
206+
_toolingConsoleJsonLoggerLazy.Value.Log(level, msg);
207+
}
200208
else
201209
{
202210
_consoleLogSource?.Log(consoleLog);

src/WebJobs.Script/Workers/ProcessManagement/WorkerProcessUtilities.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,9 +39,9 @@ public static string RemoveLogPrefix(string msg)
3939
return Regex.Replace(msg, WorkerConstants.LanguageWorkerConsoleLogPrefix, string.Empty, RegexOptions.IgnoreCase);
4040
}
4141

42-
public static bool IsToolingConsoleJsonLogEntry(ConsoleLog consoleLog)
42+
public static bool IsToolingConsoleJsonLogEntry(string msg)
4343
{
44-
return consoleLog.Message.StartsWith(WorkerConstants.ToolingConsoleLogPrefix, StringComparison.OrdinalIgnoreCase);
44+
return msg.StartsWith(WorkerConstants.ToolingConsoleLogPrefix, StringComparison.OrdinalIgnoreCase);
4545
}
4646
}
4747
}

src/WebJobs.Script/Workers/Rpc/RpcWorkerProcess.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ internal RpcWorkerProcess(string runtime,
3838
IMetricsLogger metricsLogger,
3939
IServiceProvider serviceProvider,
4040
IOptions<FunctionsHostingConfigOptions> hostingConfigOptions,
41-
IEnvironment environment)
42-
: base(eventManager, processRegistry, workerProcessLogger, consoleLogSource, metricsLogger, serviceProvider, rpcWorkerConfig.Description.UseStdErrorStreamForErrorsOnly)
41+
IEnvironment environment,
42+
ILoggerFactory loggerFactory)
43+
: base(eventManager, processRegistry, workerProcessLogger, consoleLogSource, metricsLogger, serviceProvider, loggerFactory, rpcWorkerConfig.Description.UseStdErrorStreamForErrorsOnly)
4344
{
4445
_runtime = runtime;
4546
_processFactory = processFactory;

src/WebJobs.Script/Workers/Rpc/RpcWorkerProcessFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ public RpcWorkerProcessFactory(IRpcServer rpcServer,
4949
public IWorkerProcess Create(string workerId, string runtime, string scriptRootPath, RpcWorkerConfig workerConfig)
5050
{
5151
ILogger workerProcessLogger = _loggerFactory.CreateLogger($"Worker.rpcWorkerProcess.{runtime}.{workerId}");
52-
return new RpcWorkerProcess(runtime, workerId, scriptRootPath, _rpcServer.Uri, workerConfig, _eventManager, _workerProcessFactory, _processRegistry, workerProcessLogger, _consoleLogSource, _metricsLogger, _serviceProvider, _hostingConfigOptions, _environment);
52+
return new RpcWorkerProcess(runtime, workerId, scriptRootPath, _rpcServer.Uri, workerConfig, _eventManager, _workerProcessFactory, _processRegistry, workerProcessLogger, _consoleLogSource, _metricsLogger, _serviceProvider, _hostingConfigOptions, _environment, _loggerFactory);
5353
}
5454
}
5555
}

src/WebJobs.Script/Workers/WorkerConsoleLogService.cs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ namespace Microsoft.Azure.WebJobs.Script.Workers
1313
internal class WorkerConsoleLogService : IHostedService, IDisposable
1414
{
1515
private readonly ILogger _logger;
16-
private readonly Lazy<ILogger> _toolingConsoleJsonLoggerLazy;
1716
private readonly IWorkerConsoleLogSource _source;
1817
private readonly CancellationTokenSource _cts = new CancellationTokenSource();
1918
private Task _processingTask;
@@ -28,14 +27,12 @@ public WorkerConsoleLogService(ILoggerFactory loggerFactory, IWorkerConsoleLogSo
2827

2928
_source = consoleLogSource ?? throw new ArgumentNullException(nameof(consoleLogSource));
3029
_logger = loggerFactory.CreateLogger(WorkerConstants.ConsoleLogCategoryName);
31-
_toolingConsoleJsonLoggerLazy = new Lazy<ILogger>(() => loggerFactory.CreateLogger(WorkerConstants.ToolingConsoleLogCategoryName), isThreadSafe: true);
3230
}
3331

34-
internal WorkerConsoleLogService(ILogger logger, Lazy<ILogger> toolingConsoleJsonLoggerLazy, IWorkerConsoleLogSource consoleLogSource)
32+
internal WorkerConsoleLogService(ILogger logger, IWorkerConsoleLogSource consoleLogSource)
3533
{
3634
_source = consoleLogSource ?? throw new ArgumentNullException(nameof(consoleLogSource));
3735
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
38-
_toolingConsoleJsonLoggerLazy = toolingConsoleJsonLoggerLazy ?? throw new ArgumentNullException(nameof(toolingConsoleJsonLoggerLazy));
3936
}
4037

4138
public Task StartAsync(CancellationToken cancellationToken)
@@ -62,16 +59,7 @@ internal async Task ProcessLogs()
6259
while (await source.OutputAvailableAsync(_cts.Token))
6360
{
6461
var consoleLog = await source.ReceiveAsync();
65-
66-
if (WorkerProcessUtilities.IsToolingConsoleJsonLogEntry(consoleLog))
67-
{
68-
// log with the message prefix as coretools expects it.
69-
_toolingConsoleJsonLoggerLazy.Value.Log(consoleLog.Level, consoleLog.Message);
70-
}
71-
else
72-
{
73-
_logger.Log(consoleLog.Level, consoleLog.Message);
74-
}
62+
_logger.Log(consoleLog.Level, consoleLog.Message);
7563
}
7664
}
7765
catch (OperationCanceledException)

test/WebJobs.Script.Tests/Workers/Http/HttpWorkerProcessTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public HttpWorkerProcessTests()
3535
Arguments = new WorkerProcessArguments() { ExecutablePath = "test" },
3636
Description = new HttpWorkerDescription()
3737
{
38-
WorkingDirectory = @"c:\testDir"
38+
WorkingDirectory = @"c:\testDir"
3939
}
4040
};
4141
_settingsManager = ScriptSettingsManager.Instance;
@@ -54,7 +54,7 @@ public void CreateWorkerProcess_VerifyEnvVars(string processEnvValue)
5454
{
5555
Assert.Equal(Environment.GetEnvironmentVariable(HttpWorkerConstants.PortEnvVarName), processEnvValue);
5656
}
57-
HttpWorkerProcess httpWorkerProcess = new HttpWorkerProcess(_testWorkerId, _rootScriptPath, _httpWorkerOptions, _mockEventManager.Object, _defaultWorkerProcessFactory, _processRegistry, _testLogger, _languageWorkerConsoleLogSource.Object, new TestEnvironment(), new TestMetricsLogger(), _serviceProviderMock.Object);
57+
HttpWorkerProcess httpWorkerProcess = new HttpWorkerProcess(_testWorkerId, _rootScriptPath, _httpWorkerOptions, _mockEventManager.Object, _defaultWorkerProcessFactory, _processRegistry, _testLogger, _languageWorkerConsoleLogSource.Object, new TestEnvironment(), new TestMetricsLogger(), _serviceProviderMock.Object, new LoggerFactory());
5858
Process childProcess = httpWorkerProcess.CreateWorkerProcess();
5959
Assert.NotNull(childProcess.StartInfo.EnvironmentVariables);
6060
Assert.Equal(childProcess.StartInfo.EnvironmentVariables[HttpWorkerConstants.PortEnvVarName], _workerPort.ToString());
@@ -70,7 +70,7 @@ public void CreateWorkerProcess_LinuxConsumption_AssingnsExecutePermissions_invo
7070
{
7171
TestEnvironment testEnvironment = new TestEnvironment();
7272
testEnvironment.SetEnvironmentVariable(EnvironmentSettingNames.ContainerName, "TestContainer");
73-
var mockHttpWorkerProcess = new HttpWorkerProcess(_testWorkerId, _rootScriptPath, _httpWorkerOptions, _mockEventManager.Object, _defaultWorkerProcessFactory, _processRegistry, _testLogger, _languageWorkerConsoleLogSource.Object, testEnvironment, new TestMetricsLogger(), _serviceProviderMock.Object);
73+
var mockHttpWorkerProcess = new HttpWorkerProcess(_testWorkerId, _rootScriptPath, _httpWorkerOptions, _mockEventManager.Object, _defaultWorkerProcessFactory, _processRegistry, _testLogger, _languageWorkerConsoleLogSource.Object, testEnvironment, new TestMetricsLogger(), _serviceProviderMock.Object, new LoggerFactory());
7474
mockHttpWorkerProcess.CreateWorkerProcess();
7575
// Verify method invocation
7676
var testLogs = _testLogger.GetLogMessages();

test/WebJobs.Script.Tests/Workers/Rpc/RpcWorkerProcessTests.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Microsoft.Azure.WebJobs.Script.Eventing;
1010
using Microsoft.Azure.WebJobs.Script.Workers;
1111
using Microsoft.Azure.WebJobs.Script.Workers.Rpc;
12+
using Microsoft.Extensions.Logging;
1213
using Microsoft.Extensions.Options;
1314
using Moq;
1415
using Xunit;
@@ -57,7 +58,8 @@ public RpcWorkerProcessTests()
5758
new TestMetricsLogger(),
5859
serviceProviderMock.Object,
5960
_functionsHostingConfigOptions,
60-
testEnv);
61+
testEnv,
62+
new LoggerFactory());
6163
}
6264

6365
[Fact]

test/WebJobs.Script.Tests/Workers/TestWorkerProcess.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ namespace Microsoft.Azure.WebJobs.Script.Tests.Workers
1212
{
1313
internal class TestWorkerProcess : WorkerProcess
1414
{
15-
internal TestWorkerProcess(IScriptEventManager eventManager, IProcessRegistry processRegistry, ILogger workerProcessLogger, IWorkerConsoleLogSource consoleLogSource, IMetricsLogger metricsLogger, IServiceProvider serviceProvider, bool useStdErrStreamForErrorsOnly = false)
16-
: base(eventManager, processRegistry, workerProcessLogger, consoleLogSource, metricsLogger, serviceProvider, useStdErrStreamForErrorsOnly)
15+
internal TestWorkerProcess(IScriptEventManager eventManager, IProcessRegistry processRegistry, ILogger workerProcessLogger, IWorkerConsoleLogSource consoleLogSource, IMetricsLogger metricsLogger, IServiceProvider serviceProvider, ILoggerFactory loggerFactory, bool useStdErrStreamForErrorsOnly = false)
16+
: base(eventManager, processRegistry, workerProcessLogger, consoleLogSource, metricsLogger, serviceProvider, loggerFactory, useStdErrStreamForErrorsOnly)
1717
{
1818
}
1919

0 commit comments

Comments
 (0)