Skip to content

Commit

Permalink
Use IHttpClientFactory (#578)
Browse files Browse the repository at this point in the history 
### Motivation and Context
To reduce the latency between for requests, we can re-use connections.

### Description
Use IHttpClientFactory to pool and re-use underlying
HttpClientMessageHandler instances.

### Contribution Checklist
- [ ] The code builds clean without any errors or warnings
- [ ] The PR follows the [Contribution
Guidelines](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/chat-copilot/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [ ] All unit tests pass, and I have added new tests where possible
- [ ] I didn't break anyone 😄
  • Loading branch information
@glahaye
glahaye authored Nov 7, 2023
1 parent 39edfcc commit 342a928
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 24 deletions.
10 changes: 5 additions & 5 deletions webapi/Controllers/ChatController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ namespace CopilotChat.WebApi.Controllers;
public class ChatController : ControllerBase, IDisposable
{
private readonly ILogger<ChatController> _logger;
private readonly IHttpClientFactory _httpClientFactory;
private readonly List<IDisposable> _disposables;
private readonly ITelemetryService _telemetryService;
private readonly ServiceOptions _serviceOptions;
Expand All @@ -58,12 +59,14 @@ public class ChatController : ControllerBase, IDisposable

public ChatController(
ILogger<ChatController> logger,
IHttpClientFactory httpClientFactory,
ITelemetryService telemetryService,
IOptions<ServiceOptions> serviceOptions,
IOptions<PlannerOptions> plannerOptions,
IDictionary<string, Plugin> plugins)
{
this._logger = logger;
this._httpClientFactory = httpClientFactory;
this._telemetryService = telemetryService;
this._disposables = new List<IDisposable>();
this._serviceOptions = serviceOptions.Value;
Expand Down Expand Up @@ -335,15 +338,12 @@ await planner.Kernel.ImportAIPluginAsync(
var requiresAuth = !plugin.AuthType.Equals("none", StringComparison.OrdinalIgnoreCase);
BearerAuthenticationProvider authenticationProvider = new(() => Task.FromResult(PluginAuthValue));

HttpClient httpClient = new();
httpClient.Timeout = TimeSpan.FromSeconds(this._plannerOptions.PluginTimeoutLimitInS);

await planner.Kernel.ImportAIPluginAsync(
$"{plugin.NameForModel}Plugin",
PluginUtils.GetPluginManifestUri(plugin.ManifestDomain),
new OpenApiSkillExecutionParameters
{
HttpClient = httpClient,
HttpClient = this._httpClientFactory.CreateClient("Plugin"),
IgnoreNonCompliantErrors = true,
AuthCallback = requiresAuth ? authenticationProvider.AuthenticateRequestAsync : null
});
Expand Down Expand Up @@ -395,7 +395,7 @@ await planner.Kernel.ImportAIPluginAsync(
PluginUtils.GetPluginManifestUri(plugin.ManifestDomain),
new OpenApiSkillExecutionParameters
{
HttpClient = new HttpClient(),
HttpClient = this._httpClientFactory.CreateClient("Plugin"),
IgnoreNonCompliantErrors = true,
AuthCallback = authenticationProvider.AuthenticateRequestAsync
});
Expand Down
5 changes: 4 additions & 1 deletion webapi/Controllers/PluginController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,18 @@ public class PluginController : ControllerBase
{
private const string PluginStateChanged = "PluginStateChanged";
private readonly ILogger<PluginController> _logger;
private readonly IHttpClientFactory _httpClientFactory;
private readonly IDictionary<string, Plugin> _availablePlugins;
private readonly ChatSessionRepository _sessionRepository;

public PluginController(
ILogger<PluginController> logger,
IHttpClientFactory httpClientFactory,
IDictionary<string, Plugin> availablePlugins,
ChatSessionRepository sessionRepository)
{
this._logger = logger;
this._httpClientFactory = httpClientFactory;
this._availablePlugins = availablePlugins;
this._sessionRepository = sessionRepository;
}
Expand All @@ -54,7 +57,7 @@ public async Task<IActionResult> GetPluginManifest([FromQuery] Uri manifestDomai
// Need to set the user agent to avoid 403s from some sites.
request.Headers.Add("User-Agent", Telemetry.HttpUserAgent);

using HttpClient client = new();
using HttpClient client = this._httpClientFactory.CreateClient("Plugin");
var response = await client.SendAsync(request);
if (!response.IsSuccessStatusCode)
{
Expand Down
19 changes: 11 additions & 8 deletions webapi/Controllers/SpeechTokenController.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
// Copyright (c) Microsoft. All rights reserved.

using System;
using System.Net;
using System.Net.Http;
using System.Threading.Tasks;
Expand All @@ -23,11 +22,15 @@ private sealed class TokenResult
}

private readonly ILogger<SpeechTokenController> _logger;
private readonly IHttpClientFactory _httpClientFactory;
private readonly AzureSpeechOptions _options;

public SpeechTokenController(IOptions<AzureSpeechOptions> options, ILogger<SpeechTokenController> logger)
public SpeechTokenController(IOptions<AzureSpeechOptions> options,
ILogger<SpeechTokenController> logger,
IHttpClientFactory httpClientFactory)
{
this._logger = logger;
this._httpClientFactory = httpClientFactory;
this._options = options.Value;
}

Expand Down Expand Up @@ -55,16 +58,16 @@ public async Task<ActionResult<SpeechTokenResponse>> GetAsync()

private async Task<TokenResult> FetchTokenAsync(string fetchUri, string subscriptionKey)
{
// TODO: get the HttpClient from the DI container
using var client = new HttpClient();
client.DefaultRequestHeaders.Add("Ocp-Apim-Subscription-Key", subscriptionKey);
UriBuilder uriBuilder = new(fetchUri);
using var client = this._httpClientFactory.CreateClient();

var result = await client.PostAsync(uriBuilder.Uri, null);
using var request = new HttpRequestMessage(HttpMethod.Post, fetchUri);
request.Headers.Add("Ocp-Apim-Subscription-Key", subscriptionKey);

var result = await client.SendAsync(request);
if (result.IsSuccessStatusCode)
{
var response = result.EnsureSuccessStatusCode();
this._logger.LogDebug("Token Uri: {0}", uriBuilder.Uri.AbsoluteUri);
this._logger.LogDebug("Token Uri: {0}", fetchUri);
string token = await result.Content.ReadAsStringAsync();
return new TokenResult { Token = token, ResponseCode = response.StatusCode };
}
Expand Down
3 changes: 2 additions & 1 deletion webapi/Extensions/SemanticKernelExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
using System;
using System.IO;
using System.Linq;
using System.Net.Http;
using System.Reflection;
using System.Threading.Tasks;
using CopilotChat.WebApi.Hubs;
Expand Down Expand Up @@ -169,7 +170,7 @@ public static IKernel RegisterChatSkill(this IKernel kernel, IServiceProvider sp

private static void InitializeKernelProvider(this WebApplicationBuilder builder)
{
builder.Services.AddSingleton(sp => new SemanticKernelProvider(sp, builder.Configuration));
builder.Services.AddSingleton(sp => new SemanticKernelProvider(sp, builder.Configuration, sp.GetRequiredService<IHttpClientFactory>()));
}

/// <summary>
Expand Down
9 changes: 9 additions & 0 deletions webapi/Program.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

using System;
using System.Diagnostics;
using System.Globalization;
using System.Linq;
using System.Text.Json;
using System.Threading.Tasks;
Expand Down Expand Up @@ -68,6 +69,14 @@ public static async Task Main(string[] args)

TelemetryDebugWriter.IsTracingDisabled = Debugger.IsAttached;

// Add named HTTP clients for IHttpClientFactory
builder.Services.AddHttpClient();
builder.Services.AddHttpClient("Plugin", httpClient =>
{
int timeout = int.Parse(builder.Configuration["Planner:PluginTimeoutLimitInS"] ?? "100", CultureInfo.InvariantCulture);
httpClient.Timeout = TimeSpan.FromSeconds(timeout);
});

// Add in the rest of the services.
builder.Services
.AddMaintenanceServices()
Expand Down
30 changes: 21 additions & 9 deletions webapi/Services/SemanticKernelProvider.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@ public sealed class SemanticKernelProvider

private readonly IServiceProvider _serviceProvider;
private readonly IConfiguration _configuration;
private readonly IHttpClientFactory _httpClientFactory;

public SemanticKernelProvider(IServiceProvider serviceProvider, IConfiguration configuration)
public SemanticKernelProvider(IServiceProvider serviceProvider, IConfiguration configuration, IHttpClientFactory httpClientFactory)
{
this._serviceProvider = serviceProvider;
this._configuration = configuration;
this._httpClientFactory = httpClientFactory;
}

/// <summary>
Expand Down Expand Up @@ -83,11 +85,15 @@ private KernelBuilder WithCompletionBackend(KernelBuilder kernelBuilder)
case string x when x.Equals("AzureOpenAI", StringComparison.OrdinalIgnoreCase):
case string y when y.Equals("AzureOpenAIText", StringComparison.OrdinalIgnoreCase):
var azureAIOptions = memoryOptions.GetServiceConfig<AzureOpenAIConfig>(this._configuration, "AzureOpenAIText");
return kernelBuilder.WithAzureChatCompletionService(azureAIOptions.Deployment, azureAIOptions.Endpoint, azureAIOptions.APIKey);
#pragma warning disable CA2000 // Dispose objects before losing scope - No need to dispose of HttpClient instances from IHttpClientFactory
return kernelBuilder.WithAzureChatCompletionService(azureAIOptions.Deployment, azureAIOptions.Endpoint, azureAIOptions.APIKey,
httpClient: this._httpClientFactory.CreateClient());

case string x when x.Equals("OpenAI", StringComparison.OrdinalIgnoreCase):
var openAIOptions = memoryOptions.GetServiceConfig<OpenAIConfig>(this._configuration, "OpenAI");
return kernelBuilder.WithOpenAIChatCompletionService(openAIOptions.TextModel, openAIOptions.APIKey);
return kernelBuilder.WithOpenAIChatCompletionService(openAIOptions.TextModel, openAIOptions.APIKey,
httpClient: this._httpClientFactory.CreateClient());
#pragma warning restore CA2000 // Dispose objects before losing scope

default:
throw new ArgumentException($"Invalid {nameof(memoryOptions.TextGeneratorType)} value in 'SemanticMemory' settings.");
Expand All @@ -107,12 +113,15 @@ private KernelBuilder WithPlannerBackend(KernelBuilder kernelBuilder)
case string x when x.Equals("AzureOpenAI", StringComparison.OrdinalIgnoreCase):
case string y when y.Equals("AzureOpenAIText", StringComparison.OrdinalIgnoreCase):
var azureAIOptions = memoryOptions.GetServiceConfig<AzureOpenAIConfig>(this._configuration, "AzureOpenAIText");
return kernelBuilder.WithAzureChatCompletionService(plannerOptions.Model, azureAIOptions.Endpoint, azureAIOptions.APIKey);
#pragma warning disable CA2000 // Dispose objects before losing scope - No need to dispose of HttpClient instances from IHttpClientFactory
return kernelBuilder.WithAzureChatCompletionService(plannerOptions.Model, azureAIOptions.Endpoint, azureAIOptions.APIKey,
httpClient: this._httpClientFactory.CreateClient());

case string x when x.Equals("OpenAI", StringComparison.OrdinalIgnoreCase):
var openAIOptions = memoryOptions.GetServiceConfig<OpenAIConfig>(this._configuration, "OpenAI");
return kernelBuilder.WithOpenAIChatCompletionService(plannerOptions.Model, openAIOptions.APIKey);

return kernelBuilder.WithOpenAIChatCompletionService(plannerOptions.Model, openAIOptions.APIKey,
httpClient: this._httpClientFactory.CreateClient());
#pragma warning restore CA2000 // Dispose objects before losing scope
default:
throw new ArgumentException($"Invalid {nameof(memoryOptions.TextGeneratorType)} value in 'SemanticMemory' settings.");
}
Expand All @@ -130,12 +139,15 @@ private KernelBuilder WithEmbeddingBackend(KernelBuilder kernelBuilder)
case string x when x.Equals("AzureOpenAI", StringComparison.OrdinalIgnoreCase):
case string y when y.Equals("AzureOpenAIEmbedding", StringComparison.OrdinalIgnoreCase):
var azureAIOptions = memoryOptions.GetServiceConfig<AzureOpenAIConfig>(this._configuration, "AzureOpenAIEmbedding");
return kernelBuilder.WithAzureTextEmbeddingGenerationService(azureAIOptions.Deployment, azureAIOptions.Endpoint, azureAIOptions.APIKey);
#pragma warning disable CA2000 // Dispose objects before losing scope - No need to dispose of HttpClient instances from IHttpClientFactory
return kernelBuilder.WithAzureTextEmbeddingGenerationService(azureAIOptions.Deployment, azureAIOptions.Endpoint, azureAIOptions.APIKey,
httpClient: this._httpClientFactory.CreateClient());

case string x when x.Equals("OpenAI", StringComparison.OrdinalIgnoreCase):
var openAIOptions = memoryOptions.GetServiceConfig<OpenAIConfig>(this._configuration, "OpenAI");
return kernelBuilder.WithOpenAITextEmbeddingGenerationService(openAIOptions.EmbeddingModel, openAIOptions.APIKey);

return kernelBuilder.WithOpenAITextEmbeddingGenerationService(openAIOptions.EmbeddingModel, openAIOptions.APIKey,
httpClient: this._httpClientFactory.CreateClient());
#pragma warning restore CA2000 // Dispose objects before losing scope
default:
throw new ArgumentException($"Invalid {nameof(memoryOptions.Retrieval.EmbeddingGeneratorType)} value in 'SemanticMemory' settings.");
}
Expand Down

0 comments on commit 342a928

Please sign in to comment.