[core-http(s)] Reuse DefaultHttpClient between ServiceClient instances (

#12966)

Today `ServiceClient` ends up creating a new instance of `DefaultHttpClient` each time one is not passed in via `ServiceClientOptions`. Since it is unusual for a custom `HttpClient` to be passed in, this means we recreate the `HttpClient` each time a someone creates a convenience client that wraps a generated client.

Normally, this isn't a big deal since clients are heavy enough to be cached/re-used by customer code. However, a KeyVault customer ran into a scenario where they had to recreate `CryptographyClient` many times (i.e. 5 times per second) and they noticed some pretty bad performance characteristics (out of memory, out of sockets, etc.)

While there is still more we can do with improving `CryptographyClient` to perhaps not need to be constructed so often (or making it cache its own copy of `KeyVaultClient` internally), this PR aims to solve the biggest issue with the above scenario: many copies of `DefaultHttpClient` each allocating NodeJS `http.Agent`s that then are tied to real OS resources that take a while to cleanup.

Another related note is that the Storage packages already do this today in their custom Pipeline implementation: https://github.com/Azure/azure-sdk-for-js/blob/master/sdk/storage/storage-blob/src/utils/cache.ts

sdk/core/core-client/src/httpClientCache.ts

@@ -0,0 +1,14 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { HttpsClient, DefaultHttpsClient } from "@azure/core-https";
+
+let cachedHttpsClient: HttpsClient | undefined;
+
+export function getCachedDefaultHttpsClient(): HttpsClient {
+  if (!cachedHttpsClient) {
+    cachedHttpsClient = new DefaultHttpsClient();
+  }
+
+  return cachedHttpsClient;
+}

sdk/core/core-client/src/serviceClient.ts

@@ -3,7 +3,6 @@
 
 import { TokenCredential } from "@azure/core-auth";
 import {
-  DefaultHttpsClient,
   HttpsClient,
   PipelineRequest,
   PipelineResponse,
@@ -29,6 +28,7 @@ import { isPrimitiveType } from "./utils";
 import { deserializationPolicy, DeserializationPolicyOptions } from "./deserializationPolicy";
 import { URL } from "./url";
 import { serializationPolicy, serializationPolicyOptions } from "./serializationPolicy";
+import { getCachedDefaultHttpsClient } from "./httpClientCache";
 
 /**
  * Options to be provided while creating the client.
@@ -104,7 +104,7 @@ export class ServiceClient {
   constructor(options: ServiceClientOptions = {}) {
     this._requestContentType = options.requestContentType;
     this._baseUri = options.baseUri;
-    this._httpsClient = options.httpsClient || new DefaultHttpsClient();
+    this._httpsClient = options.httpsClient || getCachedDefaultHttpsClient();
     const credentialScopes = getCredentialScopes(options);
     this._pipeline =
       options.pipeline ||

sdk/core/core-client/test/serviceClient.spec.ts

@@ -26,6 +26,7 @@ import {
 import { getOperationArgumentValueFromParameter } from "../src/operationHelpers";
 import { deserializationPolicy } from "../src/deserializationPolicy";
 import { TokenCredential } from "@azure/core-auth";
+import { getCachedDefaultHttpsClient } from "../src/httpClientCache";
 
 describe("ServiceClient", function() {
   describe("Auth scopes", () => {
@@ -801,6 +802,11 @@ describe("ServiceClient", function() {
       assert.strictEqual(ex.details.message, "InvalidResourceNameBody");
     }
   });
+
+  it("should re-use the common instance of DefaultHttpClient", function() {
+    const client = new ServiceClient();
+    assert.strictEqual((client as any)._httpsClient, getCachedDefaultHttpsClient());
+  });
 });
 
 async function testSendOperationRequest(

sdk/core/core-http/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Release History
 
+## 1.2.2 (Unreleased)
+
+- Cache the default `HttpClient` used when one isn't passed in to `ServiceClient`. This means that for most packages we will only create a single `HttpClient`, which will improve platform resource usage by reducing overhead. [PR 12966](https://github.com/Azure/azure-sdk-for-js/pull/12966)
+
 ## 1.2.1 (2020-12-09)
 
 - Support custom auth scopes. Scope is a mechanism in OAuth 2.0 to limit an application's access to a user's account [PR 12752](https://github.com/Azure/azure-sdk-for-js/pull/12752)

sdk/core/core-http/src/httpClientCache.ts

@@ -0,0 +1,15 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+import { HttpClient } from "./httpClient";
+import { DefaultHttpClient } from "./defaultHttpClient";
+
+let cachedHttpClient: HttpClient | undefined;
+
+export function getCachedDefaultHttpClient(): HttpClient {
+  if (!cachedHttpClient) {
+    cachedHttpClient = new DefaultHttpClient();
+  }
+
+  return cachedHttpClient;
+}

sdk/core/core-http/src/serviceClient.ts

@@ -2,7 +2,6 @@
 // Licensed under the MIT license.
 
 import { TokenCredential, isTokenCredential } from "@azure/core-auth";
-import { DefaultHttpClient } from "./defaultHttpClient";
 import { HttpClient } from "./httpClient";
 import { HttpOperationResponse, RestResponse } from "./httpOperationResponse";
 import { HttpPipelineLogger } from "./httpPipelineLogger";
@@ -62,6 +61,7 @@ import { disableResponseDecompressionPolicy } from "./policies/disableResponseDe
 import { ndJsonPolicy } from "./policies/ndJsonPolicy";
 import { XML_ATTRKEY, SerializerOptions, XML_CHARKEY } from "./util/serializer.common";
 import { URL } from "./url";
+import { getCachedDefaultHttpClient } from "./httpClientCache";
 
 /**
  * Options to configure a proxy for outgoing requests (Node.js only).
@@ -197,7 +197,7 @@ export class ServiceClient {
     }
 
     this._withCredentials = options.withCredentials || false;
-    this._httpClient = options.httpClient || new DefaultHttpClient();
+    this._httpClient = options.httpClient || getCachedDefaultHttpClient();
     this._requestPolicyOptions = new RequestPolicyOptions(options.httpPipelineLogger);
 
     let requestPolicyFactories: RequestPolicyFactory[];

sdk/core/core-http/test/serviceClientTests.ts

@@ -30,6 +30,7 @@ import {
 } from "../src/coreHttp";
 import { ParameterPath } from "../src/operationParameter";
 import * as Mappers from "./testMappers";
+import { getCachedDefaultHttpClient } from "../src/httpClientCache";
 
 describe("ServiceClient", function() {
   describe("Auth scopes", () => {
@@ -1993,6 +1994,11 @@ describe("ServiceClient", function() {
       assert.strictEqual(ex.details.message, "InvalidResourceNameBody");
     }
   });
+
+  it("should re-use the common instance of DefaultHttpClient", function() {
+    const client = new ServiceClient();
+    assert.strictEqual((client as any)._httpClient, getCachedDefaultHttpClient());
+  });
 });
 
 function stringToByteArray(str: string): Uint8Array {