From 11bdfae547c92d5a84560d0ae499e17402320547 Mon Sep 17 00:00:00 2001 From: Long Do Thanh Date: Sat, 15 Jul 2023 13:14:29 +0100 Subject: [PATCH 1/3] Update --- src/Abstractions/Option/MonitoringOption.cs | 5 ---- .../Internal/ActivityMetricsSender.cs | 30 +++++-------------- 2 files changed, 8 insertions(+), 27 deletions(-) diff --git a/src/Abstractions/Option/MonitoringOption.cs b/src/Abstractions/Option/MonitoringOption.cs index 991b4897f..90893f663 100644 --- a/src/Abstractions/Option/MonitoringOption.cs +++ b/src/Abstractions/Option/MonitoringOption.cs @@ -12,10 +12,5 @@ public class MonitoringOption /// Path to the setting /// public static string MonitoringPath = "Monitoring"; - - /// - /// Setting to determine whether using Histogram ar Counter for metrics - /// - public bool UseHistogramForActivityMonitoring { get; set; } } } diff --git a/src/Activities/Internal/ActivityMetricsSender.cs b/src/Activities/Internal/ActivityMetricsSender.cs index b6936403e..5b9da7894 100644 --- a/src/Activities/Internal/ActivityMetricsSender.cs +++ b/src/Activities/Internal/ActivityMetricsSender.cs @@ -18,31 +18,25 @@ namespace Microsoft.Omex.Extensions.Activities internal sealed class ActivityMetricsSender : IActivitiesEventSender, IDisposable { private readonly Meter m_meter; - private readonly Counter m_activityCounter; - private readonly Counter m_healthCheckActivityCounter; - private readonly Histogram m_activityHistogram; - private readonly Histogram m_healthCheckActivityHistogram; + private readonly Histogram m_activityHistogram; + private readonly Histogram m_healthCheckActivityHistogram; private readonly IExecutionContext m_context; private readonly IHostEnvironment m_hostEnvironment; - private readonly IOptions m_monitoringOption; private readonly ArrayPool> m_arrayPool; - public ActivityMetricsSender(IExecutionContext executionContext, IHostEnvironment hostEnvironment, IOptions monitoringOption) + public ActivityMetricsSender(IExecutionContext executionContext, IHostEnvironment hostEnvironment) { m_context = executionContext; m_hostEnvironment = hostEnvironment; - m_monitoringOption = monitoringOption; m_meter = new Meter("Microsoft.Omex.Activities", "1.0.0"); - m_activityCounter = m_meter.CreateCounter("Activities"); - m_healthCheckActivityCounter = m_meter.CreateCounter("HealthCheckActivities"); - m_activityHistogram = m_meter.CreateHistogram("Activities"); - m_healthCheckActivityHistogram = m_meter.CreateHistogram("HealthCheckActivities"); + m_activityHistogram = m_meter.CreateHistogram("Activities"); + m_healthCheckActivityHistogram = m_meter.CreateHistogram("HealthCheckActivities"); m_arrayPool = ArrayPool>.Create(); } public void SendActivityMetric(Activity activity) { - double durationMs = activity.Duration.TotalMilliseconds; + long durationMs = Convert.ToInt64(activity.Duration.TotalMilliseconds); int tagsCount = s_customTags.Length + activity.TagObjects.Count() + activity.Baggage.Count(); @@ -67,17 +61,9 @@ public void SendActivityMetric(Activity activity) ReadOnlySpan> tagsSpan = MemoryExtensions.AsSpan(tags, 0, tagsCount); - Histogram histogram = activity.IsHealthCheck() ? m_healthCheckActivityHistogram : m_activityHistogram; - Counter counter = activity.IsHealthCheck() ? m_healthCheckActivityCounter : m_activityCounter; + Histogram histogram = activity.IsHealthCheck() ? m_healthCheckActivityHistogram : m_activityHistogram; - if (m_monitoringOption.Value.UseHistogramForActivityMonitoring) - { - histogram.Record(durationMs, tagsSpan); - } - else - { - counter.Add(durationMs, tagsSpan); - } + histogram.Record(durationMs, tagsSpan); m_arrayPool.Return(tags, clearArray: true); } From 37e68dfc20730b0f9a08b11a41e37df97b85d559 Mon Sep 17 00:00:00 2001 From: Long Do Thanh Date: Wed, 20 Sep 2023 16:20:54 +0100 Subject: [PATCH 2/3] commit 1 --- src/Hosting.Services/HostBuilderExtensions.cs | 6 +++--- src/Hosting/ServiceCollectionExtensions.cs | 6 +++--- src/Logging/InitializationLogger.cs | 2 +- src/Logging/ServiceCollectionExtensions.cs | 21 ++++++++++++++----- .../HostBuilderExtensionsTests.cs | 2 +- .../HostBuilderExtensionsTests.cs | 2 +- .../ServiceCollectionExtensionsTests.cs | 2 +- .../ServiceCollectionTests.cs | 8 +++---- 8 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/Hosting.Services/HostBuilderExtensions.cs b/src/Hosting.Services/HostBuilderExtensions.cs index 9990d5444..2e6521b27 100644 --- a/src/Hosting.Services/HostBuilderExtensions.cs +++ b/src/Hosting.Services/HostBuilderExtensions.cs @@ -41,7 +41,7 @@ public static IHost BuildStatefulService( /// /// Registering Dependency Injection classes that will provide Service Fabric specific information for logging /// - public static IServiceCollection AddOmexServiceFabricDependencies(this IServiceCollection collection) + public static IServiceCollection AddOmexServiceFabricDependencies(this IServiceCollection collection, HostBuilderContext? hostBuilderContext) where TContext : ServiceContext { bool isStatefulService = typeof(StatefulServiceContext).IsAssignableFrom(typeof(TContext)); @@ -60,7 +60,7 @@ public static IServiceCollection AddOmexServiceFabricDependencies(this collection.TryAddSingleton(); collection.TryAddSingleton(); - return collection.AddOmexServices(); + return collection.AddOmexServices(hostBuilderContext); } private static IHost BuildServiceFabricService( @@ -92,7 +92,7 @@ private static IHost BuildServiceFabricService( { options.ServiceTypeName = serviceName; }) - .AddOmexServiceFabricDependencies() + .AddOmexServiceFabricDependencies(context) .AddSingleton() .AddHostedService(); }) diff --git a/src/Hosting/ServiceCollectionExtensions.cs b/src/Hosting/ServiceCollectionExtensions.cs index 44e1d8cb2..5ec78878a 100644 --- a/src/Hosting/ServiceCollectionExtensions.cs +++ b/src/Hosting/ServiceCollectionExtensions.cs @@ -19,14 +19,14 @@ public static class ServiceCollectionExtensions /// public static IHostBuilder AddOmexServices(this IHostBuilder builder) => builder - .ConfigureServices((context, collection) => collection.AddOmexServices()); + .ConfigureServices((context, collection) => collection.AddOmexServices(context)); /// /// Add Omex Logging and ActivitySource dependencies /// - public static IServiceCollection AddOmexServices(this IServiceCollection collection) => + public static IServiceCollection AddOmexServices(this IServiceCollection collection, HostBuilderContext? hostBuilderContext) => collection - .AddOmexLogging() + .AddOmexLogging(hostBuilderContext) .AddOmexActivitySource(); /// diff --git a/src/Logging/InitializationLogger.cs b/src/Logging/InitializationLogger.cs index 5f82e8dc4..180487ad0 100644 --- a/src/Logging/InitializationLogger.cs +++ b/src/Logging/InitializationLogger.cs @@ -29,7 +29,7 @@ private static ILoggingBuilder LoadInitializationLogger(this ILoggingBuilder bui builder.AddConsole(); } - builder.AddOmexLogging(); + builder.AddOmexLogging(null!); // Because this method cannot access HostBuilder's configuration. return builder; } diff --git a/src/Logging/ServiceCollectionExtensions.cs b/src/Logging/ServiceCollectionExtensions.cs index b9bdefbc9..37bf4d2b2 100644 --- a/src/Logging/ServiceCollectionExtensions.cs +++ b/src/Logging/ServiceCollectionExtensions.cs @@ -1,8 +1,10 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Omex.Extensions.Abstractions.Activities.Processing; using Microsoft.Omex.Extensions.Abstractions.ExecutionContext; @@ -29,9 +31,10 @@ public static IServiceCollection AddOmexServiceContext(this ISe /// Adds Omex event logger to the factory /// /// The extension method argument - public static ILoggingBuilder AddOmexLogging(this ILoggingBuilder builder) + /// HostBuilderContext to access Configuration + public static ILoggingBuilder AddOmexLogging(this ILoggingBuilder builder, HostBuilderContext? context) { - builder.Services.AddOmexLogging(); + builder.Services.AddOmexLogging(context); return builder; } @@ -39,13 +42,21 @@ public static ILoggingBuilder AddOmexLogging(this ILoggingBuilder builder) /// Adds Omex event logger to the factory /// /// The extension method argument + /// HostBuilderContext to access Configuration /// The so that additional calls can be chained - public static IServiceCollection AddOmexLogging(this IServiceCollection serviceCollection) + public static IServiceCollection AddOmexLogging(this IServiceCollection serviceCollection, HostBuilderContext? context) { - serviceCollection.AddLogging(); - serviceCollection.TryAddTransient(); serviceCollection.TryAddTransient(); + serviceCollection.AddLogging(); + + const string settingName = "Monitoring:EventSourceLoggingEnabled"; + bool isEventSourceLoggingEnabled = bool.Parse(context?.Configuration.GetSection(settingName).Get() ?? "true"); + if (!isEventSourceLoggingEnabled) + { + return serviceCollection; + } + serviceCollection.TryAddTransient(); serviceCollection.TryAddSingleton(_ => OmexLogEventSource.Instance); diff --git a/tests/Hosting.Services.UnitTests/HostBuilderExtensionsTests.cs b/tests/Hosting.Services.UnitTests/HostBuilderExtensionsTests.cs index 3cc04aa94..40b81b2b1 100644 --- a/tests/Hosting.Services.UnitTests/HostBuilderExtensionsTests.cs +++ b/tests/Hosting.Services.UnitTests/HostBuilderExtensionsTests.cs @@ -30,7 +30,7 @@ public void AddOmexServiceFabricDependencies_TypesRegistered(Type typeToResolver void CheckTypeRegistration() where TContext : ServiceContext { object? obj = new ServiceCollection() - .AddOmexServiceFabricDependencies() + .AddOmexServiceFabricDependencies(null!) .AddSingleton(new Mock().Object) .BuildServiceProvider() .GetService(typeToResolver); diff --git a/tests/Hosting.UnitTests/HostBuilderExtensionsTests.cs b/tests/Hosting.UnitTests/HostBuilderExtensionsTests.cs index 005af3e48..1af119ce9 100644 --- a/tests/Hosting.UnitTests/HostBuilderExtensionsTests.cs +++ b/tests/Hosting.UnitTests/HostBuilderExtensionsTests.cs @@ -24,7 +24,7 @@ public void AddOmexServices_TypesRegistered(Type type) object? collectionObj = new ServiceCollection() .AddSingleton(new ConfigurationBuilder().Build()) // Added IConfiguration because one of the dependency depends on IOptions which in turn depends on IConfiguration .AddSingleton(new HostingEnvironment()) - .AddOmexServices() + .AddOmexServices(null!) .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, diff --git a/tests/Logging.UnitTests/Scrubbing/ServiceCollectionExtensionsTests.cs b/tests/Logging.UnitTests/Scrubbing/ServiceCollectionExtensionsTests.cs index 0f622ef52..4908dda05 100644 --- a/tests/Logging.UnitTests/Scrubbing/ServiceCollectionExtensionsTests.cs +++ b/tests/Logging.UnitTests/Scrubbing/ServiceCollectionExtensionsTests.cs @@ -143,7 +143,7 @@ public void AddRegexLogScrubbingRule_Scrubs(string input, string expected) private static ILogScrubbingRule[] GetTypeRegistrations(IServiceCollection collection) { IEnumerable objects = collection - .AddOmexLogging() + .AddOmexLogging(null!) .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, diff --git a/tests/Logging.UnitTests/ServiceCollectionTests.cs b/tests/Logging.UnitTests/ServiceCollectionTests.cs index d5cb96989..426e61564 100644 --- a/tests/Logging.UnitTests/ServiceCollectionTests.cs +++ b/tests/Logging.UnitTests/ServiceCollectionTests.cs @@ -25,7 +25,7 @@ public void AddOmexServiceContext_OverridesContextType() { IServiceCollection collection = new ServiceCollection() .AddOmexServiceContext() - .AddOmexLogging(); + .AddOmexLogging(null!); IServiceContext context = ValidateTypeRegistration(collection); @@ -37,14 +37,14 @@ public void AddOmexServiceContext_OverridesContextType() [TestMethod] public void AddOmexLoggerOnServiceCollection_RegistersLogger() { - IServiceCollection collection = new ServiceCollection().AddOmexLogging(); + IServiceCollection collection = new ServiceCollection().AddOmexLogging(null!); ValidateTypeRegistration>(collection); } [TestMethod] public void AddOmexLoggerOnLogBuilder_RegistersLogger() { - ILoggingBuilder builder = new MockLoggingBuilder().AddOmexLogging(); + ILoggingBuilder builder = new MockLoggingBuilder().AddOmexLogging(null!); ValidateTypeRegistration>(builder.Services); } @@ -52,7 +52,7 @@ private T ValidateTypeRegistration(IServiceCollection collection) where T : class { T obj = collection - .AddOmexLogging() + .AddOmexLogging(null!) .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, From e1240f9c80eea892650ed5c0668fce5a3ae8a252 Mon Sep 17 00:00:00 2001 From: Long Do Thanh Date: Fri, 29 Sep 2023 14:32:53 +0100 Subject: [PATCH 3/3] Add Unit test --- src/Logging/ServiceCollectionExtensions.cs | 2 +- .../ServiceCollectionTests.cs | 78 +++++++++++++++++++ 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/Logging/ServiceCollectionExtensions.cs b/src/Logging/ServiceCollectionExtensions.cs index 37bf4d2b2..bd56e7209 100644 --- a/src/Logging/ServiceCollectionExtensions.cs +++ b/src/Logging/ServiceCollectionExtensions.cs @@ -50,7 +50,7 @@ public static IServiceCollection AddOmexLogging(this IServiceCollection serviceC serviceCollection.TryAddTransient(); serviceCollection.AddLogging(); - const string settingName = "Monitoring:EventSourceLoggingEnabled"; + const string settingName = "Monitoring:OmexLoggingEnabled"; bool isEventSourceLoggingEnabled = bool.Parse(context?.Configuration.GetSection(settingName).Get() ?? "true"); if (!isEventSourceLoggingEnabled) { diff --git a/tests/Logging.UnitTests/ServiceCollectionTests.cs b/tests/Logging.UnitTests/ServiceCollectionTests.cs index 426e61564..886ed20b4 100644 --- a/tests/Logging.UnitTests/ServiceCollectionTests.cs +++ b/tests/Logging.UnitTests/ServiceCollectionTests.cs @@ -2,8 +2,12 @@ // Licensed under the MIT license. using System; +using System.Collections.Generic; +using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; +using Microsoft.Omex.Extensions.Logging.Scrubbing; using Microsoft.VisualStudio.TestTools.UnitTesting; namespace Microsoft.Omex.Extensions.Logging.UnitTests @@ -64,6 +68,80 @@ private T ValidateTypeRegistration(IServiceCollection collection) return obj; } + [TestMethod] + public void AddOmexLoggerOnLogBuilder_ContainHostBuilder_SettingOmexLoggingEnabled_False_OmexLoggerNotRegistered() + { + HostBuilderContext hostBuilderContext = new(new Dictionary()); + IConfigurationRoot configuration = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + { "Monitoring:OmexLoggingEnabled", "false"}, + }) + .Build(); + + hostBuilderContext.Configuration = configuration; + IServiceCollection collection = new ServiceCollection() + .AddOmexServiceContext() + .AddOmexLogging(hostBuilderContext); + + Assert.ThrowsException(() => + { + OmexLogEventSource logEventSource = collection + .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }) + .GetRequiredService(); + }); + + Assert.ThrowsException(() => + { + ILogEventSender eventSender = collection + .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }) + .GetRequiredService(); + }); + + Assert.ThrowsException(() => + { + ILoggerProvider omexLoggerProvider = collection + .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }) + .GetRequiredService(); + }); + } + + [TestMethod] + [DataTestMethod] + [DataRow(true)] + [DataRow(null!)] + public void AddOmexLoggerOnLogBuilder_ContainHostBuilder_SettingOmexLoggingEnabled_TrueOrMissing_OmexLoggerRegistered(bool? isEnabled) + { + HostBuilderContext hostBuilderContext = new(new Dictionary()); + IConfigurationRoot configuration = new ConfigurationBuilder() + .AddInMemoryCollection(isEnabled == null ? new Dictionary() : new Dictionary + { + { "Monitoring:OmexLoggingEnabled", isEnabled.ToString()}, + }) + .Build(); + + hostBuilderContext.Configuration = configuration; + IServiceCollection collection = new ServiceCollection() + .AddOmexServiceContext() + .AddOmexLogging(hostBuilderContext); + + OmexLogEventSource logEventSource = collection + .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }) + .GetRequiredService(); + + ILogEventSender eventSender = collection + .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }) + .GetRequiredService(); + + ILoggerProvider omexLoggerProvider = collection + .BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }) + .GetRequiredService(); + + Assert.IsInstanceOfType(logEventSource); + Assert.IsInstanceOfType(eventSender); + Assert.IsInstanceOfType(omexLoggerProvider); + } + private class MockLoggingBuilder : ILoggingBuilder { public IServiceCollection Services { get; } = new ServiceCollection();