Merge pull request #9 from xiaomi7732/dev/saars/optimize-feclient-as-…

…singleton Dev/saars/optimize feclient as singleton
Azure · Dec 11, 2024 · 1ce0a92 · 1ce0a92
2 parents fcaa604 + 681fa75
commit 1ce0a92
Show file tree

Hide file tree

Showing 6 changed files with 26 additions and 40 deletions.
diff --git a/ServiceProfiler b/ServiceProfiler
diff --git a/...pe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/Orchestrations/RemoteSettingsService.cs b/...pe.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<UserConfigurationBase> userConfigurationOptions,
-        ILogger<RemoteSettingsService> logger) : base(frontendClientFactory, userConfigurationOptions, logger)
+        ILogger<RemoteSettingsService> logger) : base(frontendClient, userConfigurationOptions, logger)
     {
     }
 
-    protected override IDisposable EnterInternalZone()
-    {
-        return SuppressInstrumentationScope.Begin();
-    }
+    protected override IDisposable EnterInternalZone() => SuppressInstrumentationScope.Begin();
 }
diff --git a/...r.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/ServiceCollectionExtensions.cs b/...r.EventPipe.Otel/Azure.Monitor.OpenTelemetry.Profiler.Core/ServiceCollectionExtensions.cs
@@ -78,7 +78,7 @@ public static IServiceCollection AddServiceProfilerCore(this IServiceCollection
         services.AddSingleton<IServiceProfilerProvider, OpenTelemetryProfilerProvider>();
 
         // Client
-        services.AddSingleton<IProfilerFrontendClientFactory, ProfilerFrontendClientFactory>();
+        services.AddSingleton(p => ActivatorUtilities.CreateInstance<ProfilerFrontendClientFactory>(p).CreateProfilerFrontendClient());
 
         // Token
         services.AddSingleton<IAuthTokenProvider, AuthTokenProvider>();

diff --git a/...Microsoft.ApplicationInsights.Profiler.Shared/Orchestrations/RemoteSettingsServiceBase.cs b/...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<bool> _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<SettingsContract>? SettingsUpdated;
 
     public RemoteSettingsServiceBase(
-        IProfilerFrontendClientFactory frontendClientFactory,
+        IProfilerFrontendClient frontendClient,
         IOptions<UserConfigurationBase> userConfigurationOptions,
         ILogger<RemoteSettingsServiceBase> 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<bool>();
         _frequency = _userConfiguration.ConfigurationUpdateFrequency;
         _standaloneMode = _userConfiguration.StandaloneMode;
@@ -50,12 +50,12 @@ public async Task<bool> 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;
         }
     }
@@ -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)
@@ -102,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
         {

diff --git a/...plicationInsights.Profiler.Shared/Services.Abstractions/IProfilerFrontendClientFactory.cs b/...plicationInsights.Profiler.Shared/Services.Abstractions/IProfilerFrontendClientFactory.cs
diff --git a/...l/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs b/...l/Microsoft.ApplicationInsights.Profiler.Shared/Services/ProfilerFrontendClientFactory.cs
@@ -15,7 +15,11 @@
 
 namespace Microsoft.ApplicationInsights.Profiler.Shared.Services;
 
-internal class ProfilerFrontendClientFactory : IProfilerFrontendClientFactory
+/// <summary>
+/// This helper function simplifies the registration process for the <see cref="IProfilerFrontendClient" />.
+/// Please do NOT inject this directly. Instead, inject the <see cref="IProfilerFrontendClient"/>.
+/// </summary>
+internal class ProfilerFrontendClientFactory
 {
     private readonly IServiceProvider _serviceProvider;
     private readonly IServiceProfilerContext _serviceProfilerContext;