From ce58e89a4019648388a84c2e4de8a8de0dfed908 Mon Sep 17 00:00:00 2001 From: Saar Shen Date: Tue, 10 Dec 2024 15:51:30 -0800 Subject: [PATCH 1/3] Remove ProfilerFrontendClientFactory --- ServiceProfiler | 2 +- .../Orchestrations/RemoteSettingsService.cs | 11 +++---- .../ServiceCollectionExtensions.cs | 2 +- .../RemoteSettingsServiceBase.cs | 29 +++++++------------ .../IProfilerFrontendClientFactory.cs | 8 ----- .../Services/ProfilerFrontendClientFactory.cs | 7 ++++- 6 files changed, 23 insertions(+), 36 deletions(-) delete mode 100644 src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services.Abstractions/IProfilerFrontendClientFactory.cs diff --git a/ServiceProfiler b/ServiceProfiler index 9a4d9fd..9e50dc8 160000 --- a/ServiceProfiler +++ b/ServiceProfiler @@ -1 +1 @@ -Subproject commit 9a4d9fd59c15ac56fe10c9c67903de01e3870941 +Subproject commit 9e50dc8e070248b10bd689da3dc13967103ee234 diff --git a/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/Orchestrations/RemoteSettingsService.cs b/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/Orchestrations/RemoteSettingsService.cs index 29e4a1c..fa13f76 100644 --- a/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/Orchestrations/RemoteSettingsService.cs +++ b/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/Orchestrations/RemoteSettingsService.cs @@ -4,9 +4,9 @@ using Microsoft.ApplicationInsights.Profiler.Shared.Contracts; using Microsoft.ApplicationInsights.Profiler.Shared.Orchestrations; -using Microsoft.ApplicationInsights.Profiler.Shared.Services.Abstractions; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; +using Microsoft.ServiceProfiler.Agent.FrontendClient; using OpenTelemetry; namespace Azure.Monitor.OpenTelemetry.Profiler.Core.Orchestrations; @@ -15,14 +15,11 @@ namespace Azure.Monitor.OpenTelemetry.Profiler.Core.Orchestrations; internal sealed class RemoteSettingsService : RemoteSettingsServiceBase { public RemoteSettingsService( - IProfilerFrontendClientFactory frontendClientFactory, + IProfilerFrontendClient frontendClient, IOptions userConfigurationOptions, - ILogger logger) : base(frontendClientFactory, userConfigurationOptions, logger) + ILogger logger) : base(frontendClient, userConfigurationOptions, logger) { } - protected override IDisposable EnterInternalZone() - { - return SuppressInstrumentationScope.Begin(); - } + protected override IDisposable EnterInternalZone() => SuppressInstrumentationScope.Begin(); } diff --git a/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/ServiceCollectionExtensions.cs b/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/ServiceCollectionExtensions.cs index 5f9e09a..841b84c 100644 --- a/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/ServiceCollectionExtensions.cs +++ b/src/ServiceProfiler.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/ServiceCollectionExtensions.cs @@ -78,7 +78,7 @@ public static IServiceCollection AddServiceProfilerCore(this IServiceCollection services.AddSingleton(); // Client - services.AddSingleton(); + services.AddSingleton(p => ActivatorUtilities.CreateInstance(p).CreateProfilerFrontendClient()); // Token services.AddSingleton(); diff --git a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs index 744d612..0d95c73 100644 --- a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs +++ b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs @@ -3,11 +3,11 @@ //----------------------------------------------------------------------------- using Microsoft.ApplicationInsights.Profiler.Shared.Contracts; -using Microsoft.ApplicationInsights.Profiler.Shared.Services.Abstractions; using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Microsoft.ServiceProfiler.Agent.Exceptions; +using Microsoft.ServiceProfiler.Agent.FrontendClient; using Microsoft.ServiceProfiler.Contract.Agent.Profiler; using Microsoft.ServiceProfiler.Orchestration; using System; @@ -21,7 +21,7 @@ internal abstract class RemoteSettingsServiceBase : BackgroundService, IProfiler private readonly ILogger _logger; private readonly TaskCompletionSource _taskCompletionSource; private readonly UserConfigurationBase _userConfiguration; - private readonly IProfilerFrontendClientFactory _frontendClientFactory; + private readonly IProfilerFrontendClient _frontendClient; private readonly TimeSpan _frequency; private readonly bool _standaloneMode; private readonly bool _isDisabled; @@ -31,13 +31,13 @@ internal abstract class RemoteSettingsServiceBase : BackgroundService, IProfiler public event Action? SettingsUpdated; public RemoteSettingsServiceBase( - IProfilerFrontendClientFactory frontendClientFactory, + IProfilerFrontendClient frontendClient, IOptions userConfigurationOptions, ILogger logger) { _logger = logger ?? throw new ArgumentNullException(nameof(logger)); _userConfiguration = userConfigurationOptions.Value ?? throw new ArgumentNullException(nameof(userConfigurationOptions)); - _frontendClientFactory = frontendClientFactory ?? throw new ArgumentNullException(nameof(frontendClientFactory)); + _frontendClient = frontendClient ?? throw new ArgumentNullException(nameof(frontendClient)); _taskCompletionSource = new TaskCompletionSource(); _frequency = _userConfiguration.ConfigurationUpdateFrequency; _standaloneMode = _userConfiguration.StandaloneMode; @@ -67,24 +67,17 @@ private async Task FetchRemoteSettingsAsync(CancellationToken cancellationToken) using IDisposable internalZoneHandler = EnterInternalZone(); try { - if (_frontendClientFactory != null) + _logger.LogTrace("Fetching remote settings."); + SettingsContract newContract = await _frontendClient.GetProfilerSettingsAsync(cancellationToken).ConfigureAwait(false); + if (newContract != null) { - _logger.LogTrace("Fetching remote settings."); - SettingsContract newContract = await _frontendClientFactory.CreateProfilerFrontendClient().GetProfilerSettingsAsync(cancellationToken).ConfigureAwait(false); - if (newContract != null) - { - _logger.LogTrace("Remote settings fetched."); - // New settings coming. - OnSettingsUpdated(newContract); - } - else - { - _logger.LogTrace("No settings contract fetched."); - } + _logger.LogTrace("Remote settings fetched."); + // New settings coming. + OnSettingsUpdated(newContract); } else { - _logger.LogTrace("{0} is null. Indicating it is disposed.", nameof(_frontendClientFactory)); + _logger.LogTrace("No settings contract fetched."); } } catch (InstrumentationKeyInvalidException ikie) diff --git a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services.Abstractions/IProfilerFrontendClientFactory.cs b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services.Abstractions/IProfilerFrontendClientFactory.cs deleted file mode 100644 index 73e6835..0000000 --- a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services.Abstractions/IProfilerFrontendClientFactory.cs +++ /dev/null @@ -1,8 +0,0 @@ -using Microsoft.ServiceProfiler.Agent.FrontendClient; - -namespace Microsoft.ApplicationInsights.Profiler.Shared.Services.Abstractions; - -internal interface IProfilerFrontendClientFactory -{ - IProfilerFrontendClient CreateProfilerFrontendClient(); -} diff --git a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs index b5ae762..98c339e 100644 --- a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs +++ b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs @@ -15,7 +15,12 @@ namespace Microsoft.ApplicationInsights.Profiler.Shared.Services; -internal class ProfilerFrontendClientFactory : IProfilerFrontendClientFactory +/// +/// This is a helper creator for the frontend client that makes the registering +/// of the easier. Please do NOT inject this directly. +/// Injecting instead. +/// +internal class ProfilerFrontendClientFactory { private readonly IServiceProvider _serviceProvider; private readonly IServiceProfilerContext _serviceProfilerContext; From 439f408533efa04515cb3233966be9c07aeedaca Mon Sep 17 00:00:00 2001 From: Saar Shen Date: Tue, 10 Dec 2024 16:00:36 -0800 Subject: [PATCH 2/3] Tweak some logging --- .../Orchestrations/RemoteSettingsServiceBase.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs index 0d95c73..e19ab56 100644 --- a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs +++ b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs @@ -50,12 +50,12 @@ public async Task WaitForInitializedAsync(TimeSpan timeout) if (completed == _taskCompletionSource.Task) { // Initialize done. - _logger.LogTrace("Initial remote settings fetched within given time: {0}.", timeout); + _logger.LogTrace("Initial remote settings fetched within given time: {timeout}.", timeout); return true; } else { - _logger.LogDebug("Remote settings fetch timed out. Timeout settings: {0}", timeout); + _logger.LogDebug("Remote settings fetch timed out. Timeout settings: {timeout}", timeout); return false; } } @@ -95,8 +95,8 @@ private async Task FetchRemoteSettingsAsync(CancellationToken cancellationToken) #pragma warning restore CA1031 // Only to allow for getting the value for the next iteration. The exception will be logged. { // Move on for the next iteration. - _logger.LogDebug("Unexpected error contacting service profiler service endpoint for settings. Details: {0}", ex); - _logger.LogTrace(ex.ToString()); + _logger.LogDebug("Unexpected error contacting service profiler service endpoint for settings. Details: {details}", ex); + _logger.LogTrace("Error with trace: {error}", ex.ToString()); } finally { From 681fa752a4a6223a21f2a394aa7c145720a0e464 Mon Sep 17 00:00:00 2001 From: Saar Shen Date: Tue, 10 Dec 2024 16:09:32 -0800 Subject: [PATCH 3/3] Update the comment a bit --- .../Services/ProfilerFrontendClientFactory.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs index 98c339e..bee4694 100644 --- a/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs +++ b/src/ServiceProfiler.EventPipe.Otel/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs @@ -16,9 +16,8 @@ namespace Microsoft.ApplicationInsights.Profiler.Shared.Services; /// -/// This is a helper creator for the frontend client that makes the registering -/// of the easier. Please do NOT inject this directly. -/// Injecting instead. +/// This helper function simplifies the registration process for the . +/// Please do NOT inject this directly. Instead, inject the . /// internal class ProfilerFrontendClientFactory {