From bdfe28ff36e538cd70e9345983f5d48d076b5114 Mon Sep 17 00:00:00 2001 From: Harsh Jain Date: Mon, 3 Feb 2025 13:22:04 +0530 Subject: [PATCH] Remove Cyclic Dependency between metrics logger and publisher --- .../LinuxContainerLegionMetricsPublisher.cs | 18 ++++++-- .../WebHostServiceCollectionExtensions.cs | 4 +- ...nuxContainerLegionMetricsPublisherTests.cs | 41 ++++++++++++++++++- 3 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/WebJobs.Script.WebHost/Metrics/LinuxContainerLegionMetricsPublisher.cs b/src/WebJobs.Script.WebHost/Metrics/LinuxContainerLegionMetricsPublisher.cs index f0bf03b4bd..987ce7d7a8 100644 --- a/src/WebJobs.Script.WebHost/Metrics/LinuxContainerLegionMetricsPublisher.cs +++ b/src/WebJobs.Script.WebHost/Metrics/LinuxContainerLegionMetricsPublisher.cs @@ -23,7 +23,7 @@ public class LinuxContainerLegionMetricsPublisher : IMetricsPublisher, IDisposab private readonly IDisposable _standbyOptionsOnChangeSubscription; private readonly IEnvironment _environment; private readonly ILogger _logger; - //private readonly IMetricsLogger _metricsLogger; + private readonly IMetricsLogger _metricsLogger; private readonly string _containerName; private TimeSpan _metricPublishInterval; @@ -32,13 +32,16 @@ public class LinuxContainerLegionMetricsPublisher : IMetricsPublisher, IDisposab private Timer _metricsPublisherTimer; private bool _initialized = false; - public LinuxContainerLegionMetricsPublisher(IEnvironment environment, IOptionsMonitor standbyOptions, ILogger logger, IFileSystem fileSystem, ILinuxConsumptionMetricsTracker metricsTracker, int? metricsPublishIntervalMS = null) + public LinuxContainerLegionMetricsPublisher(IEnvironment environment, IOptionsMonitor standbyOptions, ILogger logger, IFileSystem fileSystem, ILinuxConsumptionMetricsTracker metricsTracker, IScriptHostManager scriptHostManager, int? metricsPublishIntervalMS = null) { _standbyOptions = standbyOptions ?? throw new ArgumentNullException(nameof(standbyOptions)); _environment = environment ?? throw new ArgumentNullException(nameof(environment)); _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _metricsTracker = metricsTracker ?? throw new ArgumentNullException(nameof(metricsTracker)); - //_metricsLogger = metricsLogger ?? throw new ArgumentNullException(nameof(metricsLogger)); + if (scriptHostManager == null || !Utility.TryGetHostService(scriptHostManager, out _metricsLogger)) + { + throw new InvalidOperationException("Unable to get IMetricsLogger Service."); + } _containerName = _environment.GetEnvironmentVariable(EnvironmentSettingNames.ContainerName); _metricPublishInterval = TimeSpan.FromMilliseconds(metricsPublishIntervalMS ?? 30 * 1000); @@ -51,6 +54,8 @@ public LinuxContainerLegionMetricsPublisher(IEnvironment environment, IOptionsMo _processMonitorTimer = new Timer(OnProcessMonitorTimer, null, Timeout.Infinite, Timeout.Infinite); _metricsPublisherTimer = new Timer(OnFunctionMetricsPublishTimer, null, Timeout.Infinite, Timeout.Infinite); + _metricsTracker.OnDiagnosticEvent += OnMetricsDiagnosticEvent; + if (_standbyOptions.CurrentValue.InStandbyMode) { _standbyOptionsOnChangeSubscription = _standbyOptions.OnChange(o => OnStandbyOptionsChange()); @@ -198,6 +203,11 @@ private void SetTimerInterval(Timer timer, TimeSpan dueTime) } } + private void OnMetricsDiagnosticEvent(object sender, DiagnosticEventArgs e) + { + _metricsLogger.LogEvent(e.EventName); + } + public void OnFunctionStarted(string functionName, string invocationId) { // nothing to do @@ -215,6 +225,8 @@ public void Dispose() _metricsPublisherTimer?.Dispose(); _metricsPublisherTimer = null; + + _metricsTracker.OnDiagnosticEvent -= OnMetricsDiagnosticEvent; } internal class Metrics diff --git a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs index 61fb9f5940..25ea7eee7e 100644 --- a/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs +++ b/src/WebJobs.Script.WebHost/WebHostServiceCollectionExtensions.cs @@ -291,8 +291,8 @@ private static void AddLinuxContainerServices(this IServiceCollection services) var logger = s.GetService>(); var metricsTracker = s.GetService(); var standbyOptions = s.GetService>(); - //var metricsLogger = s.GetService(); - return new LinuxContainerLegionMetricsPublisher(environment, standbyOptions, logger, new FileSystem(), metricsTracker); + var scriptHostManager = s.GetService(); + return new LinuxContainerLegionMetricsPublisher(environment, standbyOptions, logger, new FileSystem(), metricsTracker, scriptHostManager); } else if (environment.IsFlexConsumptionSku()) { diff --git a/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs b/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs index f4afd750c9..c53457f1eb 100644 --- a/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs +++ b/test/WebJobs.Script.Tests/Metrics/LinuxContainerLegionMetricsPublisherTests.cs @@ -6,13 +6,16 @@ using System.IO; using System.IO.Abstractions; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.Azure.Functions.Platform.Metrics.LinuxConsumption; +using Microsoft.Azure.WebJobs.Script.Diagnostics; using Microsoft.Azure.WebJobs.Script.WebHost; using Microsoft.Azure.WebJobs.Script.WebHost.Metrics; using Microsoft.WebJobs.Script.Tests; using Newtonsoft.Json; using Xunit; +using static Microsoft.Azure.WebJobs.Script.Tests.TestHelpers; namespace Microsoft.Azure.WebJobs.Script.Tests.Metrics { @@ -27,6 +30,7 @@ public class LinuxContainerLegionMetricsPublisherTests private readonly Random _random = new Random(); private readonly TestLogger _logger; private readonly TestMetricsLogger _testMetricsLogger; + private readonly IScriptHostManager _scriptHostManager; private StandbyOptions _standbyOptions; private TestOptionsMonitor _standbyOptionsMonitor; @@ -37,6 +41,7 @@ public LinuxContainerLegionMetricsPublisherTests() _environment = new TestEnvironment(); _logger = new TestLogger(); _testMetricsLogger = new TestMetricsLogger(); + _scriptHostManager = new TestScriptHostManager(_testMetricsLogger); _environment.SetEnvironmentVariable(EnvironmentSettingNames.FunctionsMetricsPublishPath, _metricsFilePath); @@ -49,7 +54,7 @@ private LinuxContainerLegionMetricsPublisher CreatePublisher(bool inStandbyMode _standbyOptions = new StandbyOptions { InStandbyMode = inStandbyMode }; _standbyOptionsMonitor = new TestOptionsMonitor(_standbyOptions); - return new LinuxContainerLegionMetricsPublisher(_environment, _standbyOptionsMonitor, _logger, new FileSystem(), _testMetricsLogger, _testMetricsTracker, metricsPublishInterval); + return new LinuxContainerLegionMetricsPublisher(_environment, _standbyOptionsMonitor, _logger, new FileSystem(), _testMetricsTracker, _scriptHostManager, metricsPublishInterval); } [Fact] @@ -254,5 +259,39 @@ public void LogEvent(string eventName) OnDiagnosticEvent?.Invoke(this, new DiagnosticEventArgs(eventName)); } } + + private class TestScriptHostManager : IServiceProvider, IScriptHostManager + { + private readonly IMetricsLogger _metricsLogger; + + public TestScriptHostManager(IMetricsLogger metricsLogger) + { + _metricsLogger = metricsLogger; + } + + public ScriptHostState State => throw new NotImplementedException(); + + public Exception LastError => throw new NotImplementedException(); + + public event EventHandler HostInitializing; + + public event EventHandler ActiveHostChanged; + + + public object GetService(Type serviceType) + { + if (serviceType == typeof(IMetricsLogger)) + { + return _metricsLogger; + } + + throw new NotImplementedException(); + } + + public Task RestartHostAsync(CancellationToken cancellationToken = default) + { + throw new NotImplementedException(); + } + } } } \ No newline at end of file