-
Notifications
You must be signed in to change notification settings - Fork 2.1k
OpenAI.HttpClient to Azure.HttpClient mapper for test instrumentation #47416
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
89496d3
eca457f
954ba7d
4df3dba
385e9b5
0957fd2
d113cc2
224e3fb
672a6df
d4505e6
bf085da
226c2c0
7c95643
867d0d6
6bf38b7
7c1b414
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ | |
|
|
||
| import com.azure.ai.agents.implementation.AgentsClientImpl; | ||
| import com.azure.ai.agents.implementation.TokenUtils; | ||
| import com.azure.ai.agents.implementation.http.HttpClientHelper; | ||
| import com.azure.ai.agents.implementation.http.PolicyDecoratingHttpClient; | ||
| import com.azure.core.annotation.Generated; | ||
| import com.azure.core.annotation.ServiceClientBuilder; | ||
| import com.azure.core.client.traits.ConfigurationTrait; | ||
|
|
@@ -43,6 +45,7 @@ | |
| import com.openai.credential.BearerTokenCredential; | ||
| import java.time.Duration; | ||
| import java.util.ArrayList; | ||
| import java.util.Collections; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
|
|
@@ -328,7 +331,12 @@ private HttpPipeline createHttpPipeline() { | |
| * @return an instance of ConversationsAsyncClient. | ||
| */ | ||
| public ConversationsAsyncClient buildConversationsAsyncClient() { | ||
| return new ConversationsAsyncClient(getOpenAIAsyncClientBuilder().build()); | ||
| HttpClient decoratedHttpClient = getOpenAIHttpClient(); | ||
| return new ConversationsAsyncClient(getOpenAIAsyncClientBuilder().build().withOptions(optionBuilder -> { | ||
| if (decoratedHttpClient != null) { | ||
| optionBuilder.httpClient(HttpClientHelper.httpClientMapper(decoratedHttpClient)); | ||
| } | ||
| })); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -337,7 +345,12 @@ public ConversationsAsyncClient buildConversationsAsyncClient() { | |
| * @return an instance of ConversationsClient. | ||
| */ | ||
| public ConversationsClient buildConversationsClient() { | ||
| return new ConversationsClient(getOpenAIClientBuilder().build()); | ||
| HttpClient decoratedHttpClient = getOpenAIHttpClient(); | ||
| return new ConversationsClient(getOpenAIClientBuilder().build().withOptions(optionBuilder -> { | ||
| if (decoratedHttpClient != null) { | ||
| optionBuilder.httpClient(HttpClientHelper.httpClientMapper(decoratedHttpClient)); | ||
| } | ||
| })); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -346,8 +359,12 @@ public ConversationsClient buildConversationsClient() { | |
| * @return an instance of ResponsesClient | ||
| */ | ||
| public ResponsesClient buildResponsesClient() { | ||
| OpenAIOkHttpClient.Builder builder = getOpenAIClientBuilder(); | ||
| return new ResponsesClient(builder.build()); | ||
| HttpClient decoratedHttpClient = getOpenAIHttpClient(); | ||
| return new ResponsesClient(getOpenAIClientBuilder().build().withOptions(optionBuilder -> { | ||
| if (decoratedHttpClient != null) { | ||
| optionBuilder.httpClient(HttpClientHelper.httpClientMapper(decoratedHttpClient)); | ||
| } | ||
| })); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -356,7 +373,12 @@ public ResponsesClient buildResponsesClient() { | |
| * @return an instance of ResponsesAsyncClient | ||
| */ | ||
| public ResponsesAsyncClient buildResponsesAsyncClient() { | ||
| return new ResponsesAsyncClient(getOpenAIAsyncClientBuilder().build()); | ||
| HttpClient decoratedHttpClient = getOpenAIHttpClient(); | ||
| return new ResponsesAsyncClient(getOpenAIAsyncClientBuilder().build().withOptions(optionBuilder -> { | ||
| if (decoratedHttpClient != null) { | ||
| optionBuilder.httpClient(HttpClientHelper.httpClientMapper(decoratedHttpClient)); | ||
| } | ||
| })); | ||
| } | ||
|
|
||
| private OpenAIOkHttpClient.Builder getOpenAIClientBuilder() { | ||
|
|
@@ -396,6 +418,36 @@ private String getUserAgent() { | |
| return UserAgentUtil.toUserAgentString(applicationId, sdkName, sdkVersion, configuration); | ||
| } | ||
|
|
||
| private HttpClient getOpenAIHttpClient() { | ||
| if (this.httpClient == null) { | ||
| return null; | ||
| } | ||
| List<HttpPipelinePolicy> orderedPolicies = getOrderedCustomPolicies(); | ||
|
||
| if (orderedPolicies.isEmpty()) { | ||
|
||
| return this.httpClient; | ||
| } | ||
jpalvarezl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return new PolicyDecoratingHttpClient(this.httpClient, orderedPolicies); | ||
| } | ||
|
|
||
| private List<HttpPipelinePolicy> getOrderedCustomPolicies() { | ||
| if (this.pipelinePolicies.isEmpty()) { | ||
| return Collections.emptyList(); | ||
| } | ||
| List<HttpPipelinePolicy> orderedPolicies = new ArrayList<>(); | ||
| this.pipelinePolicies.stream() | ||
| .filter(policy -> pipelinePosition(policy) == HttpPipelinePosition.PER_CALL) | ||
| .forEach(orderedPolicies::add); | ||
| this.pipelinePolicies.stream() | ||
| .filter(policy -> pipelinePosition(policy) == HttpPipelinePosition.PER_RETRY) | ||
| .forEach(orderedPolicies::add); | ||
| return orderedPolicies; | ||
| } | ||
|
|
||
| private static HttpPipelinePosition pipelinePosition(HttpPipelinePolicy policy) { | ||
| HttpPipelinePosition position = policy.getPipelinePosition(); | ||
| return position == null ? HttpPipelinePosition.PER_RETRY : position; | ||
| } | ||
|
|
||
| private static final ClientLogger LOGGER = new ClientLogger(AgentsClientBuilder.class); | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.azure.ai.agents.implementation.http; | ||
|
|
||
| import com.azure.core.http.HttpHeader; | ||
| import com.azure.core.http.HttpHeaders; | ||
| import com.azure.core.util.logging.ClientLogger; | ||
| import com.openai.core.http.Headers; | ||
| import com.openai.core.http.HttpResponse; | ||
|
|
||
| import java.io.IOException; | ||
| import java.io.InputStream; | ||
| import java.io.UncheckedIOException; | ||
|
|
||
| /** | ||
| * Adapter that exposes an Azure {@link com.azure.core.http.HttpResponse} as an OpenAI {@link HttpResponse}. This keeps | ||
| * the translation logic encapsulated so response handling elsewhere can remain framework agnostic. | ||
| */ | ||
| final class AzureHttpResponseAdapter implements HttpResponse { | ||
|
|
||
| private static final ClientLogger LOGGER = new ClientLogger(AzureHttpResponseAdapter.class); | ||
|
|
||
| private final com.azure.core.http.HttpResponse azureResponse; | ||
| private final Headers headers; | ||
| private final InputStream bodyStream; | ||
|
|
||
| /** | ||
| * Creates a new adapter instance for the provided Azure response. | ||
| * | ||
| * @param azureResponse Response returned by the Azure pipeline. | ||
| */ | ||
| AzureHttpResponseAdapter(com.azure.core.http.HttpResponse azureResponse) { | ||
| this.azureResponse = azureResponse; | ||
| this.headers = toOpenAiHeaders(azureResponse.getHeaders()); | ||
|
||
| this.bodyStream = azureResponse.getBodyAsBinaryData().toStream(); | ||
jpalvarezl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| } | ||
|
|
||
| @Override | ||
| public int statusCode() { | ||
| return azureResponse.getStatusCode(); | ||
| } | ||
|
|
||
| @Override | ||
| public Headers headers() { | ||
| return headers; | ||
| } | ||
|
|
||
| @Override | ||
| public InputStream body() { | ||
| return bodyStream; | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| try { | ||
| bodyStream.close(); | ||
| } catch (IOException ex) { | ||
| throw LOGGER.logExceptionAsWarning(new UncheckedIOException("Failed to close response body stream", ex)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Copies headers from the Azure response into the immutable OpenAI {@link Headers} collection. | ||
| */ | ||
| private static Headers toOpenAiHeaders(HttpHeaders httpHeaders) { | ||
| Headers.Builder builder = Headers.builder(); | ||
| for (HttpHeader header : httpHeaders) { | ||
| builder.put(header.getName(), header.getValuesList()); | ||
| } | ||
| return builder.build(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,191 @@ | ||
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
|
|
||
| package com.azure.ai.agents.implementation.http; | ||
|
|
||
| import com.azure.core.http.HttpHeaderName; | ||
| import com.azure.core.http.HttpHeaders; | ||
| import com.azure.core.util.BinaryData; | ||
| import com.azure.core.util.Context; | ||
| import com.azure.core.util.CoreUtils; | ||
| import com.azure.core.util.logging.ClientLogger; | ||
| import com.openai.core.RequestOptions; | ||
| import com.openai.core.http.Headers; | ||
| import com.openai.core.http.HttpRequest; | ||
| import com.openai.core.http.HttpRequestBody; | ||
| import com.openai.core.http.HttpResponse; | ||
|
|
||
| import java.io.ByteArrayOutputStream; | ||
| import java.io.IOException; | ||
| import java.io.UncheckedIOException; | ||
| import java.util.List; | ||
| import java.util.Objects; | ||
| import java.util.concurrent.CompletableFuture; | ||
|
|
||
| /** | ||
| * Utility entry point that adapts an Azure {@link com.azure.core.http.HttpClient} so it can be consumed by | ||
| * the OpenAI SDK generated clients. The helper performs request/response translation so that existing Azure | ||
| * pipelines, diagnostics, and retry policies can be reused without exposing the Azure HTTP primitives to | ||
| * callers that only understand the OpenAI surface area. | ||
| */ | ||
| public final class HttpClientHelper { | ||
|
|
||
| private static final ClientLogger LOGGER = new ClientLogger(HttpClientHelper.class); | ||
|
|
||
| private HttpClientHelper() { | ||
| } | ||
|
|
||
| /** | ||
| * Wraps the given Azure {@link com.azure.core.http.HttpClient} with an implementation of the OpenAI | ||
| * {@link com.openai.core.http.HttpClient} interface. All requests and responses are converted on the fly. | ||
| * | ||
| * @param azureHttpClient The Azure HTTP client that should execute requests. | ||
| * @return A bridge client that honors the OpenAI interface but delegates execution to the Azure pipeline. | ||
| */ | ||
| public static com.openai.core.http.HttpClient httpClientMapper(com.azure.core.http.HttpClient azureHttpClient) { | ||
jpalvarezl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return new HttpClientWrapper(azureHttpClient); | ||
| } | ||
|
|
||
| private static final class HttpClientWrapper implements com.openai.core.http.HttpClient { | ||
|
|
||
| private static final HttpHeaderName CONTENT_TYPE = HttpHeaderName.CONTENT_TYPE; | ||
jpalvarezl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
|
|
||
| private final com.azure.core.http.HttpClient azureHttpClient; | ||
|
|
||
| private HttpClientWrapper(com.azure.core.http.HttpClient azureHttpClient) { | ||
| this.azureHttpClient = Objects.requireNonNull(azureHttpClient, "'azureHttpClient' cannot be null."); | ||
| } | ||
|
|
||
| @Override | ||
| public void close() { | ||
| // no-op | ||
| } | ||
|
|
||
| @Override | ||
| public HttpResponse execute(HttpRequest request) { | ||
| return execute(request, RequestOptions.none()); | ||
| } | ||
|
|
||
| @Override | ||
| public HttpResponse execute(HttpRequest request, RequestOptions requestOptions) { | ||
| Objects.requireNonNull(request, "request"); | ||
| Objects.requireNonNull(requestOptions, "requestOptions"); | ||
|
|
||
| com.azure.core.http.HttpRequest azureRequest = buildAzureRequest(request); | ||
| com.azure.core.http.HttpResponse azureResponse = this.azureHttpClient.sendSync(azureRequest, Context.NONE); | ||
jpalvarezl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| return new AzureHttpResponseAdapter(azureResponse); | ||
| } | ||
|
|
||
| @Override | ||
| public CompletableFuture<HttpResponse> executeAsync(HttpRequest request) { | ||
| return executeAsync(request, RequestOptions.none()); | ||
| } | ||
|
|
||
| @Override | ||
| public CompletableFuture<HttpResponse> executeAsync(HttpRequest request, RequestOptions requestOptions) { | ||
| Objects.requireNonNull(request, "request"); | ||
| Objects.requireNonNull(requestOptions, "requestOptions"); | ||
|
|
||
| final com.azure.core.http.HttpRequest azureRequest; | ||
| try { | ||
| azureRequest = buildAzureRequest(request); | ||
| } catch (RuntimeException runtimeException) { | ||
| return failedFuture(runtimeException); | ||
| } | ||
|
|
||
| return this.azureHttpClient.send(azureRequest) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When an exception is thrown, we should map that as well to OpenAI exception types. |
||
| .map(AzureHttpResponseAdapter::new) | ||
| .map(response -> (HttpResponse) response) | ||
jpalvarezl marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| .toFuture(); | ||
| } | ||
|
|
||
| /** | ||
| * Converts the OpenAI request metadata and body into an Azure {@link com.azure.core.http.HttpRequest}. | ||
| */ | ||
| private static com.azure.core.http.HttpRequest buildAzureRequest(HttpRequest request) { | ||
| HttpRequestBody requestBody = request.body(); | ||
| String contentType = requestBody != null ? requestBody.contentType() : null; | ||
| BinaryData bodyData = null; | ||
|
|
||
| if (requestBody != null) { | ||
| try { | ||
| bodyData = toBinaryData(requestBody); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This requires reading the request body twice. We should consider adding additional methods to OpenAI's |
||
| } finally { | ||
| closeQuietly(requestBody); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the source request body didn't close cleanly, then we should bubble that exception up as that indicates some issue with the underlying data source. |
||
| } | ||
| } | ||
|
|
||
| HttpHeaders headers = toAzureHeaders(request.headers()); | ||
| if (!CoreUtils.isNullOrEmpty(contentType) && headers.getValue(CONTENT_TYPE) == null) { | ||
| headers.set(CONTENT_TYPE, contentType); | ||
| } | ||
|
|
||
| com.azure.core.http.HttpRequest azureRequest | ||
| = new com.azure.core.http.HttpRequest(com.azure.core.http.HttpMethod.valueOf(request.method().name()), | ||
| OpenAiRequestUrlBuilder.buildUrl(request), headers); | ||
|
|
||
| if (bodyData != null) { | ||
| azureRequest.setBody(bodyData); | ||
| } | ||
|
|
||
| return azureRequest; | ||
| } | ||
|
|
||
| /** | ||
| * Copies OpenAI headers into an {@link HttpHeaders} instance so the Azure pipeline can process them. | ||
| */ | ||
| private static HttpHeaders toAzureHeaders(Headers sourceHeaders) { | ||
| HttpHeaders target = new HttpHeaders(); | ||
| sourceHeaders.names().forEach(name -> { | ||
| List<String> values = sourceHeaders.values(name); | ||
| HttpHeaderName headerName = HttpHeaderName.fromString(name); | ||
| if (values.isEmpty()) { | ||
| target.set(headerName, ""); | ||
| } else { | ||
| target.set(headerName, values); | ||
| } | ||
| }); | ||
| return target; | ||
| } | ||
|
Comment on lines
+143
to
+155
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here - this results in looping through each header twice. Instead, updating the |
||
|
|
||
| /** | ||
| * Buffers the OpenAI {@link HttpRequestBody} into {@link BinaryData} so it can be attached to the Azure | ||
| * request. The body is consumed exactly once and closed afterwards. | ||
| */ | ||
| private static BinaryData toBinaryData(HttpRequestBody requestBody) { | ||
| if (requestBody == null) { | ||
| return null; | ||
| } | ||
|
|
||
| try (ByteArrayOutputStream outputStream = new ByteArrayOutputStream()) { | ||
| requestBody.writeTo(outputStream); | ||
| return BinaryData.fromBytes(outputStream.toByteArray()); | ||
| } catch (IOException e) { | ||
| throw LOGGER.logExceptionAsError(new UncheckedIOException("Failed to buffer request body", e)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Closes the OpenAI request body while suppressing any exceptions (to avoid masking the root cause). | ||
| */ | ||
| private static void closeQuietly(HttpRequestBody body) { | ||
| if (body == null) { | ||
| return; | ||
| } | ||
| try { | ||
| body.close(); | ||
| } catch (Exception ignored) { | ||
| // no-op | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Creates a failed {@link CompletableFuture} for callers that require a future even when conversion fails. | ||
| */ | ||
| private static <T> CompletableFuture<T> failedFuture(Throwable throwable) { | ||
| CompletableFuture<T> future = new CompletableFuture<>(); | ||
| future.completeExceptionally(throwable); | ||
| return future; | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will switch from OpenAI's HTTP client to Azure's HTTP client when there is a custom policy. Should we always use the Azure HTTP client to avoid different behaviors/HTTP clients? It would be good to have a consistent behavior regardless of how the client builder was configured. Or, we should clearly document that setting custom policy will result in a different HTTP client being used.