Skip to content

Commit fb4c911

Browse files
kshyjukshyju
authored
Fixes circular dependency when resolving LinuxContainerLegionMetricsPublisher (#10991)
* Avoid circular dependency when resolving LinuxContainerLegionMetricsPublisher. * Revert launchsettings.json chane. * Adding a placeholder mode host startup test for Linux consumption on legion * Update test to verify SKU specific services are resolved from DI container. * Resolve conflict in release notes * Switch to Type check instead of type name check, in the test. * release notes insanity fix.
1 parent c00aad6 commit fb4c911

File tree

9 files changed

+102
-61
lines changed

9 files changed

+102
-61
lines changed

‎release_notes.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,4 +4,5 @@
44
- My change description (#PR)
55
-->
66
- Improved memory metrics reporting using CGroup data for Linux consumption (#10968)
7-
- Fixing GrpcWorkerChannel concurrency bug (#10998)
7+
- Fixing GrpcWorkerChannel concurrency bug (#10998)
8+
- Avoid circular dependency when resolving LinuxContainerLegionMetricsPublisher. (#10991)

‎src/WebJobs.Script.WebHost/Metrics/LinuxContainerLegionMetricsPublisher.cs

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
using Microsoft.Azure.WebJobs.Script.Config;
1111
using Microsoft.Azure.WebJobs.Script.Diagnostics;
1212
using Microsoft.Azure.WebJobs.Script.WebHost.Configuration;
13+
using Microsoft.Extensions.DependencyInjection;
1314
using Microsoft.Extensions.Logging;
1415
using Microsoft.Extensions.Options;
1516

@@ -26,7 +27,7 @@ public sealed class LinuxContainerLegionMetricsPublisher : IMetricsPublisher, ID
2627
private readonly IDisposable _hostingConfigOptionsOnChangeListener;
2728
private readonly IEnvironment _environment;
2829
private readonly ILogger _logger;
29-
private readonly IScriptHostManager _scriptHostManager;
30+
private readonly IServiceProvider _serviceProvider;
3031
private readonly string _containerName;
3132
private readonly IOptionsMonitor<FunctionsHostingConfigOptions> _hostingConfigOptions;
3233

@@ -40,15 +41,15 @@ public sealed class LinuxContainerLegionMetricsPublisher : IMetricsPublisher, ID
4041

4142
public LinuxContainerLegionMetricsPublisher(IEnvironment environment, IOptionsMonitor<StandbyOptions> standbyOptions,
4243
IOptions<LinuxConsumptionLegionMetricsPublisherOptions> options, ILogger<LinuxContainerLegionMetricsPublisher> logger,
43-
IFileSystem fileSystem, ILinuxConsumptionMetricsTracker metricsTracker, IScriptHostManager scriptHostManager,
44+
IFileSystem fileSystem, ILinuxConsumptionMetricsTracker metricsTracker, IServiceProvider serviceProvider,
4445
IOptionsMonitor<FunctionsHostingConfigOptions> functionsHostingConfigOptions,
4546
int? metricsPublishIntervalMS = null)
4647
{
4748
_standbyOptions = standbyOptions ?? throw new ArgumentNullException(nameof(standbyOptions));
4849
_environment = environment ?? throw new ArgumentNullException(nameof(environment));
4950
_logger = logger ?? throw new ArgumentNullException(nameof(logger));
5051
_metricsTracker = metricsTracker ?? throw new ArgumentNullException(nameof(metricsTracker));
51-
_scriptHostManager = scriptHostManager ?? throw new ArgumentNullException(nameof(scriptHostManager));
52+
_serviceProvider = serviceProvider ?? throw new ArgumentNullException(nameof(serviceProvider));
5253
_hostingConfigOptions = functionsHostingConfigOptions ?? throw new ArgumentNullException(nameof(functionsHostingConfigOptions));
5354
_containerName = options.Value.ContainerName;
5455

@@ -75,21 +76,7 @@ public LinuxContainerLegionMetricsPublisher(IEnvironment environment, IOptionsMo
7576
_hostingConfigOptionsOnChangeListener = _hostingConfigOptions.OnChange(OnHostingConfigOptionsChanged);
7677
}
7778

78-
public IMetricsLogger MetricsLogger
79-
{
80-
get
81-
{
82-
if (_metricsLogger == null)
83-
{
84-
if (!Utility.TryGetHostService<IMetricsLogger>(_scriptHostManager, out _metricsLogger))
85-
{
86-
throw new InvalidOperationException($"Unable to resolve {nameof(IMetricsLogger)} service.");
87-
}
88-
}
89-
90-
return _metricsLogger;
91-
}
92-
}
79+
private IMetricsLogger MetricsLogger => _metricsLogger ??= _serviceProvider.GetRequiredService<IMetricsLogger>();
9380

9481
private void OnHostingConfigOptionsChanged(FunctionsHostingConfigOptions newOptions)
9582
{

‎src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright (c) .NET Foundation. All rights reserved.
22
// Licensed under the MIT License. See License.txt in the project root for license information.
33

4+
using System;
45
using System.IO.Abstractions;
56
using System.Net.Http;
67
using System.Runtime.InteropServices;
@@ -304,9 +305,9 @@ private static void AddLinuxContainerServices(this IServiceCollection services)
304305
var logger = s.GetService<ILogger<LinuxContainerLegionMetricsPublisher>>();
305306
var metricsTracker = s.GetService<ILinuxConsumptionMetricsTracker>();
306307
var standbyOptions = s.GetService<IOptionsMonitor<StandbyOptions>>();
307-
var scriptHostManager = s.GetService<IScriptHostManager>();
308+
var serviceProvider = s.GetService<IServiceProvider>();
308309
var hostingConfigOptions = s.GetService<IOptionsMonitor<FunctionsHostingConfigOptions>>();
309-
return new LinuxContainerLegionMetricsPublisher(environment, standbyOptions, options, logger, new FileSystem(), metricsTracker, scriptHostManager, hostingConfigOptions);
310+
return new LinuxContainerLegionMetricsPublisher(environment, standbyOptions, options, logger, new FileSystem(), metricsTracker, serviceProvider, hostingConfigOptions);
310311
}
311312
else if (environment.IsFlexConsumptionSku())
312313
{

‎test/WebJobs.Script.Tests.Integration/Host/StandbyManager/StandbyManagerE2ETestBase.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using System.Web.Http;
1313
using Microsoft.AspNetCore.Hosting;
1414
using Microsoft.AspNetCore.TestHost;
15+
using Microsoft.Azure.Functions.Platform.Metrics.LinuxConsumption;
1516
using Microsoft.Azure.WebJobs.Extensions.Http;
1617
using Microsoft.Azure.WebJobs.Host.Executors;
1718
using Microsoft.Azure.WebJobs.Script.Diagnostics;
@@ -73,6 +74,7 @@ protected async Task<IWebHostBuilder> CreateWebHostBuilderAsync(string testDirNa
7374
environment.SetEnvironmentVariable(EnvironmentSettingNames.AzureWebsiteName, websiteSiteName);
7475
}
7576

77+
var testMetricsTracker = new TestMetricsTracker();
7678
var webHostBuilder = Program.CreateWebHostBuilder()
7779
.ConfigureAppConfiguration(c =>
7880
{
@@ -102,6 +104,7 @@ protected async Task<IWebHostBuilder> CreateWebHostBuilderAsync(string testDirNa
102104

103105
c.AddSingleton<IEnvironment>(_ => environment);
104106
c.AddSingleton<IMetricsLogger>(_ => _metricsLogger);
107+
c.AddSingleton<ILinuxConsumptionMetricsTracker>(_ => testMetricsTracker);
105108
})
106109
.ConfigureScriptHostLogging(b =>
107110
{
@@ -121,7 +124,7 @@ protected async Task<IWebHostBuilder> CreateWebHostBuilderAsync(string testDirNa
121124
return webHostBuilder;
122125
}
123126

124-
protected async Task InitializeTestHostAsync(string testDirName, IEnvironment environment, string websiteSiteName = TestSiteName)
127+
protected async Task<IWebHost> InitializeTestHostAsync(string testDirName, IEnvironment environment, string websiteSiteName = TestSiteName)
125128
{
126129
var webHostBuilder = await CreateWebHostBuilderAsync(testDirName, environment, websiteSiteName);
127130
_httpServer = new TestServer(webHostBuilder);
@@ -135,6 +138,8 @@ protected async Task InitializeTestHostAsync(string testDirName, IEnvironment en
135138
Assert.NotNull(traces.Single(p => p.FormattedMessage.StartsWith("Host is in standby mode")));
136139

137140
_expectedHostId = await _httpServer.Host.Services.GetService<IHostIdProvider>().GetHostIdAsync(CancellationToken.None);
141+
142+
return _httpServer.Host;
138143
}
139144

140145

‎test/WebJobs.Script.Tests.Integration/Host/StandbyManager/StandbyManagerE2ETests_Linux.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using Microsoft.Azure.WebJobs.Script.Tests.Integration.Diagnostics;
1313
using Microsoft.Azure.WebJobs.Script.WebHost;
1414
using Microsoft.Azure.WebJobs.Script.WebHost.Authentication;
15+
using Microsoft.Azure.WebJobs.Script.WebHost.Metrics;
1516
using Microsoft.Azure.WebJobs.Script.WebHost.Models;
1617
using Microsoft.Azure.WebJobs.Script.WebHost.Security;
1718
using Microsoft.Azure.WebJobs.Script.Workers;
@@ -31,6 +32,44 @@ namespace Microsoft.Azure.WebJobs.Script.Tests
3132
[Trait(TestTraits.Group, TestTraits.StandbyModeTestsLinux)]
3233
public class StandbyManagerE2ETests_Linux : StandbyManagerE2ETestBase
3334
{
35+
[Fact]
36+
public async Task StandbyModeE2E_LinuxConsumptionOnLegion()
37+
{
38+
var vars = new Dictionary<string, string>
39+
{
40+
{ EnvironmentSettingNames.AzureWebsitePlaceholderMode, "1" },
41+
{ EnvironmentSettingNames.ContainerName, "testContainer" },
42+
{ EnvironmentSettingNames.AzureWebsiteHostName, "testapp.azurewebsites.net" },
43+
{ EnvironmentSettingNames.AzureWebsiteName, "TestApp" },
44+
{ EnvironmentSettingNames.ContainerEncryptionKey, Convert.ToBase64String(TestHelpers.GenerateKeyBytes()) },
45+
{ EnvironmentSettingNames.AzureWebsiteContainerReady, null },
46+
{ EnvironmentSettingNames.AzureWebsiteSku, "Dynamic" },
47+
{ EnvironmentSettingNames.AzureWebsiteZipDeployment, null },
48+
{ EnvironmentSettingNames.AzureWebJobsFeatureFlags, ScriptConstants.FeatureFlagEnableProxies },
49+
{ "AzureWebEncryptionKey", "0F75CA46E7EBDD39E4CA6B074D1F9A5972B849A55F91A248" },
50+
{ EnvironmentSettingNames.FunctionsTimeZone, "Europe/Berlin" },
51+
{ EnvironmentSettingNames.FunctionWorkerRuntime, "dotnet-isolated" },
52+
{ EnvironmentSettingNames.LegionServiceHost, "1"}
53+
};
54+
55+
var environment = new TestEnvironment(vars);
56+
57+
Assert.True(environment.IsLinuxConsumptionOnLegion());
58+
Assert.False(environment.IsFlexConsumptionSku());
59+
Assert.True(environment.IsAnyLinuxConsumption());
60+
61+
var webHost = await InitializeTestHostAsync("Linux", environment);
62+
63+
// verify the expected startup logs
64+
var logEntries = _loggerProvider.GetAllLogMessages().Where(p => p.FormattedMessage != null).ToList();
65+
Assert.True(logEntries.Any(m => m.Category == "Host.Startup" && m.FormattedMessage.Contains("Host is in standby mode")));
66+
Assert.True(logEntries.Any(cat => cat.Category == "Microsoft.Azure.WebJobs.Script.WebHost.StandbyManager"));
67+
Assert.True(logEntries.Any(cat => cat.Category == "Host.Triggers.Warmup"));
68+
69+
// verify that the expected (legion specific) implementations are resolved
70+
Assert.Equal(typeof(LinuxContainerLegionMetricsPublisher), webHost.Services.GetRequiredService<IMetricsPublisher>().GetType());
71+
}
72+
3473
[Fact]
3574
public async Task StandbyModeE2E_LinuxContainer()
3675
{

‎test/WebJobs.Script.Tests.Shared/TestHelpers.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ public static async Task Await(Func<Task<bool>> condition, int timeout = 60 * 10
103103
}
104104
throw new ApplicationException(error);
105105
}
106-
}
107106
}
107+
}
108108

109109
public static async Task RetryFailedTest(Func<Task> test, int retries, ITestOutputHelper output = null)
110110
{

‎test/WebJobs.Script.Tests.Shared/TestMetricsTracker.cs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the MIT License. See License.txt in the project root for license information.
3+
4+
using System;
5+
using System.Collections.Generic;
6+
using Microsoft.Azure.Functions.Platform.Metrics.LinuxConsumption;
7+
8+
namespace Microsoft.Azure.WebJobs.Script.Tests
9+
{
10+
public class TestMetricsTracker : ILinuxConsumptionMetricsTracker
11+
{
12+
public event EventHandler<DiagnosticEventArgs> OnDiagnosticEvent;
13+
14+
public List<FunctionActivity> FunctionActivities { get; } = new List<FunctionActivity>();
15+
16+
public List<MemoryActivity> MemoryActivities { get; } = new List<MemoryActivity>();
17+
18+
public Queue<LinuxConsumptionMetrics> MetricsQueue { get; } = new Queue<LinuxConsumptionMetrics>();
19+
20+
public void AddFunctionActivity(FunctionActivity activity)
21+
{
22+
FunctionActivities.Add(activity);
23+
}
24+
25+
public void AddMemoryActivity(MemoryActivity activity)
26+
{
27+
MemoryActivities.Add(activity);
28+
}
29+
30+
public bool TryGetMetrics(out LinuxConsumptionMetrics metrics)
31+
{
32+
return MetricsQueue.TryDequeue(out metrics);
33+
}
34+
35+
public void LogEvent(string eventName)
36+
{
37+
OnDiagnosticEvent?.Invoke(this, new DiagnosticEventArgs(eventName));
38+
}
39+
}
40+
}

‎test/WebJobs.Script.Tests.Shared/WebJobs.Script.Tests.Shared.projitems

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
<Compile Include="$(MSBuildThisFileDirectory)TestFunctionMetadataManager.cs" />
2525
<Compile Include="$(MSBuildThisFileDirectory)TestHostBuilderExtensions.cs" />
2626
<Compile Include="$(MSBuildThisFileDirectory)TestHostExtensions.cs" />
27+
<Compile Include="$(MSBuildThisFileDirectory)TestMetricsTracker.cs" />
2728
<Compile Include="$(MSBuildThisFileDirectory)TestOutputHelperLogger.cs" />
2829
<Compile Include="$(MSBuildThisFileDirectory)TestOutputHelperLoggerProvider.cs" />
2930
<Compile Include="$(MSBuildThisFileDirectory)TestMetricsLogger.cs" />

‎test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs

Lines changed: 5 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// Licensed under the MIT License. See License.txt in the project root for license information.
33

44
using System;
5-
using System.Collections.Generic;
65
using System.IO;
76
using System.IO.Abstractions;
87
using System.Linq;
@@ -18,7 +17,6 @@
1817
using Microsoft.WebJobs.Script.Tests;
1918
using Newtonsoft.Json;
2019
using Xunit;
21-
using static Microsoft.Azure.WebJobs.Script.Tests.TestHelpers;
2220

2321
namespace Microsoft.Azure.WebJobs.Script.Tests.Metrics
2422
{
@@ -33,7 +31,7 @@ public class LinuxContainerLegionMetricsPublisherTests
3331
private readonly Random _random = new Random();
3432
private readonly TestLogger<LinuxContainerLegionMetricsPublisher> _logger;
3533
private readonly TestMetricsLogger _testMetricsLogger;
36-
private readonly IScriptHostManager _scriptHostManager;
34+
private readonly IServiceProvider _serviceProvider;
3735

3836
private IOptions<LinuxConsumptionLegionMetricsPublisherOptions> _options;
3937
private StandbyOptions _standbyOptions;
@@ -45,7 +43,7 @@ public LinuxContainerLegionMetricsPublisherTests()
4543
_environment = new TestEnvironment();
4644
_logger = new TestLogger<LinuxContainerLegionMetricsPublisher>();
4745
_testMetricsLogger = new TestMetricsLogger();
48-
_scriptHostManager = new TestScriptHostManager(_testMetricsLogger);
46+
_serviceProvider = new TestScriptHostManager(_testMetricsLogger);
4947

5048
_environment.SetEnvironmentVariable(EnvironmentSettingNames.FunctionsMetricsPublishPath, _metricsFilePath);
5149

@@ -65,7 +63,7 @@ private LinuxContainerLegionMetricsPublisher CreatePublisher(int? metricsPublish
6563

6664
var testHostingConfigOptionsMonitor = new TestOptionsMonitor<FunctionsHostingConfigOptions>(new FunctionsHostingConfigOptions());
6765

68-
return new LinuxContainerLegionMetricsPublisher(_environment, _standbyOptionsMonitor, _options, _logger, new FileSystem(), _testMetricsTracker, _scriptHostManager, testHostingConfigOptionsMonitor, metricsPublishInterval);
66+
return new LinuxContainerLegionMetricsPublisher(_environment, _standbyOptionsMonitor, _options, _logger, new FileSystem(), _testMetricsTracker, _serviceProvider, testHostingConfigOptionsMonitor, metricsPublishInterval);
6967
}
7068

7169
[Fact]
@@ -240,37 +238,6 @@ private static FileInfo[] GetMetricsFilesSafe(string path)
240238
return new FileInfo[0];
241239
}
242240

243-
private class TestMetricsTracker : ILinuxConsumptionMetricsTracker
244-
{
245-
public event EventHandler<DiagnosticEventArgs> OnDiagnosticEvent;
246-
247-
public List<FunctionActivity> FunctionActivities { get; } = new List<FunctionActivity>();
248-
249-
public List<MemoryActivity> MemoryActivities { get; } = new List<MemoryActivity>();
250-
251-
public Queue<LinuxConsumptionMetrics> MetricsQueue { get; } = new Queue<LinuxConsumptionMetrics>();
252-
253-
public void AddFunctionActivity(FunctionActivity activity)
254-
{
255-
FunctionActivities.Add(activity);
256-
}
257-
258-
public void AddMemoryActivity(MemoryActivity activity)
259-
{
260-
MemoryActivities.Add(activity);
261-
}
262-
263-
public bool TryGetMetrics(out LinuxConsumptionMetrics metrics)
264-
{
265-
return MetricsQueue.TryDequeue(out metrics);
266-
}
267-
268-
public void LogEvent(string eventName)
269-
{
270-
OnDiagnosticEvent?.Invoke(this, new DiagnosticEventArgs(eventName));
271-
}
272-
}
273-
274241
private class TestScriptHostManager : IServiceProvider, IScriptHostManager
275242
{
276243
private readonly IMetricsLogger _metricsLogger;
@@ -280,11 +247,11 @@ public TestScriptHostManager(IMetricsLogger metricsLogger)
280247
_metricsLogger = metricsLogger;
281248
}
282249

283-
#pragma warning disable CS0067
250+
#pragma warning disable CS0067
284251
public event EventHandler HostInitializing;
285252

286253
public event EventHandler<ActiveHostChangedEventArgs> ActiveHostChanged;
287-
#pragma warning restore CS0067
254+
#pragma warning restore CS0067
288255

289256
public ScriptHostState State => throw new NotImplementedException();
290257

0 commit comments

Comments
 (0)