updates from code review on 6.x branch

Author: aditi-khare-mongoDB

src/cmap/connect.ts

@@ -25,7 +25,6 @@ import {
   type ConnectionOptions,
   CryptoConnection
 } from './connection';
-import { addContainerMetadata } from './handshake/client_metadata';
 import {
   MAX_SUPPORTED_SERVER_VERSION,
   MAX_SUPPORTED_WIRE_VERSION,
@@ -197,7 +196,7 @@ export async function prepareHandshakeDocument(
   const options = authContext.options;
   const compressors = options.compressors ? options.compressors : [];
   const { serverApi } = authContext.connection;
-  const clientMetadata = await addContainerMetadata(options.metadata);
+  const clientMetadata = await options.extendedMetadata;
 
   const handshakeDoc: HandshakeDocument = {
     [serverApi?.version ? 'hello' : LEGACY_HELLO_COMMAND]: 1,

src/cmap/connection.ts

@@ -133,6 +133,8 @@ export interface ConnectionOptions
   socketTimeoutMS?: number;
   cancellationToken?: CancellationToken;
   metadata: ClientMetadata;
+  /** @internal */
+  extendedMetadata: Promise<Document>;
 }
 
 /** @internal */

src/cmap/handshake/client_metadata.ts

@@ -157,20 +157,15 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe
   return metadataDocument.toObject() as ClientMetadata;
 }
 
-let isDocker: boolean;
-let dockerPromise: Promise<void>;
+let dockerPromise: Promise<boolean>;
 /** @internal */
 async function getContainerMetadata() {
   const containerMetadata: Record<string, any> = {};
-  if (isDocker == null) {
-    dockerPromise ??= fs.access('/.dockerenv');
-    try {
-      await dockerPromise;
-      isDocker = true;
-    } catch {
-      isDocker = false;
-    }
-  }
+  dockerPromise ??= fs.access('/.dockerenv').then(
+    () => true,
+    () => false
+  );
+  const isDocker = await dockerPromise;
 
   const { KUBERNETES_SERVICE_HOST = '' } = process.env;
   const isKubernetes = KUBERNETES_SERVICE_HOST.length > 0 ? true : false;

src/connection_string.ts

@@ -6,7 +6,7 @@ import { URLSearchParams } from 'url';
 import type { Document } from './bson';
 import { MongoCredentials } from './cmap/auth/mongo_credentials';
 import { AUTH_MECHS_AUTH_SRC_EXTERNAL, AuthMechanism } from './cmap/auth/providers';
-import { makeClientMetadata } from './cmap/handshake/client_metadata';
+import { addContainerMetadata, makeClientMetadata } from './cmap/handshake/client_metadata';
 import { Compressor, type CompressorName } from './cmap/wire_protocol/compression';
 import { Encrypter } from './encrypter';
 import {
@@ -550,6 +550,9 @@ export function parseOptions(
   );
 
   mongoOptions.metadata = makeClientMetadata(mongoOptions);
+  mongoOptions.extendedMetadata = addContainerMetadata(mongoOptions.metadata).catch(() => {
+    /* rejections will be handled later */
+  });
 
   return mongoOptions;
 }

src/mongo_client.ts

@@ -757,6 +757,8 @@ export interface MongoOptions
   writeConcern: WriteConcern;
   dbName: string;
   metadata: ClientMetadata;
+  /** @internal */
+  extendedMetadata: Promise<Document>;
   /**
    * @deprecated This option will be removed in the next major version.
    */

src/sdam/topology.ts

@@ -143,6 +143,7 @@ export interface TopologyOptions extends BSONSerializeOptions, ServerOptions {
   directConnection: boolean;
   loadBalanced: boolean;
   metadata: ClientMetadata;
+  extendedMetadata: Promise<Document>;
   /** MongoDB server API version */
   serverApi?: ServerApi;
   [featureFlag: symbol]: any;

test/integration/connection-monitoring-and-pooling/connection.test.ts

@@ -1,6 +1,7 @@
 import { expect } from 'chai';
 
 import {
+  addContainerMetadata,
   connect,
   Connection,
   type ConnectionOptions,
@@ -36,7 +37,8 @@ describe('Connection', function () {
         const connectOptions: Partial<ConnectionOptions> = {
           connectionType: Connection,
           ...this.configuration.options,
-          metadata: makeClientMetadata({ driverInfo: {} })
+          metadata: makeClientMetadata({ driverInfo: {} }),
+          extendedMetadata: addContainerMetadata(makeClientMetadata({ driverInfo: {} }))
         };
 
         connect(connectOptions as any as ConnectionOptions, (err, conn) => {
@@ -60,7 +62,8 @@ describe('Connection', function () {
           connectionType: Connection,
           monitorCommands: true,
           ...this.configuration.options,
-          metadata: makeClientMetadata({ driverInfo: {} })
+          metadata: makeClientMetadata({ driverInfo: {} }),
+          extendedMetadata: addContainerMetadata(makeClientMetadata({ driverInfo: {} }))
         };
 
         connect(connectOptions as any as ConnectionOptions, (err, conn) => {

test/tools/cmap_spec_runner.ts

@@ -4,6 +4,7 @@ import { clearTimeout, setTimeout } from 'timers';
 import { promisify } from 'util';
 
 import {
+  addContainerMetadata,
   CMAP_EVENTS,
   type Connection,
   ConnectionPool,
@@ -371,6 +372,7 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) {
   }
 
   const metadata = makeClientMetadata({ appName: poolOptions.appName, driverInfo: {} });
+  const extendedMetadata = addContainerMetadata(metadata);
   delete poolOptions.appName;
 
   const operations = test.operations;
@@ -382,7 +384,12 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) {
   const mainThread = threadContext.getThread(MAIN_THREAD_KEY);
   mainThread.start();
 
-  threadContext.createPool({ ...poolOptions, metadata, minPoolSizeCheckFrequencyMS });
+  threadContext.createPool({
+    ...poolOptions,
+    metadata,
+    extendedMetadata,
+    minPoolSizeCheckFrequencyMS
+  });
   // yield control back to the event loop so that the ConnectionPoolCreatedEvent
   // has a chance to be fired before any synchronously-emitted events from
   // the queued operations

test/unit/cmap/connect.test.ts

@@ -2,6 +2,7 @@ import { expect } from 'chai';
 import { promisify } from 'util';
 
 import {
+  addContainerMetadata,
   CancellationToken,
   type ClientMetadata,
   connect,
@@ -24,6 +25,7 @@ const CONNECT_DEFAULTS = {
   generation: 1,
   monitorCommands: false,
   metadata: {} as ClientMetadata,
+  extendedMetadata: addContainerMetadata({} as ClientMetadata),
   loadBalanced: false
 };
 
@@ -212,13 +214,17 @@ describe('Connect Tests', function () {
       const cachedEnv = process.env;
 
       context('when only kubernetes is present', () => {
-        const authContext = {
-          connection: {},
-          options: { ...CONNECT_DEFAULTS }
-        };
+        let authContext;
 
         beforeEach(() => {
           process.env.KUBERNETES_SERVICE_HOST = 'I exist';
+          authContext = {
+            connection: {},
+            options: {
+              ...CONNECT_DEFAULTS,
+              extendedMetadata: addContainerMetadata({} as ClientMetadata)
+            }
+          };
         });
 
         afterEach(() => {
@@ -227,6 +233,7 @@ describe('Connect Tests', function () {
           } else {
             delete process.env.KUBERNETES_SERVICE_HOST;
           }
+          authContext = {};
         });
 
         it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => {
@@ -238,16 +245,36 @@ describe('Connect Tests', function () {
           const handshakeDocument = await prepareHandshakeDocument(authContext);
           expect(handshakeDocument.client.env).to.not.have.property('name');
         });
+
+        context('when 512 byte size limit is exceeded', async () => {
+          it(`should not 'env' property in client`, async () => {
+            // make metadata = 507 bytes, so it takes up entire LimitedSizeDocument
+            const longAppName = 's'.repeat(493);
+            const longAuthContext = {
+              connection: {},
+              options: {
+                ...CONNECT_DEFAULTS,
+                extendedMetadata: addContainerMetadata({ appName: longAppName })
+              }
+            };
+            const handshakeDocument = await prepareHandshakeDocument(longAuthContext);
+            expect(handshakeDocument.client).to.not.have.property('env');
+          });
+        });
       });
 
       context('when kubernetes and FAAS are both present', () => {
-        const authContext = {
-          connection: {},
-          options: { ...CONNECT_DEFAULTS, metadata: { env: { name: 'aws.lambda' } } }
-        };
+        let authContext;
 
         beforeEach(() => {
           process.env.KUBERNETES_SERVICE_HOST = 'I exist';
+          authContext = {
+            connection: {},
+            options: {
+              ...CONNECT_DEFAULTS,
+              extendedMetadata: addContainerMetadata({ env: { name: 'aws.lambda' } })
+            }
+          };
         });
 
         afterEach(() => {
@@ -256,6 +283,7 @@ describe('Connect Tests', function () {
           } else {
             delete process.env.KUBERNETES_SERVICE_HOST;
           }
+          authContext = {};
         });
 
         it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => {
@@ -267,6 +295,26 @@ describe('Connect Tests', function () {
           const handshakeDocument = await prepareHandshakeDocument(authContext);
           expect(handshakeDocument.client.env.name).to.equal('aws.lambda');
         });
+
+        context('when 512 byte size limit is exceeded', async () => {
+          it(`should not have 'container' property in client.env`, async () => {
+            // make metadata = 507 bytes, so it takes up entire LimitedSizeDocument
+            const longAppName = 's'.repeat(447);
+            const longAuthContext = {
+              connection: {},
+              options: {
+                ...CONNECT_DEFAULTS,
+                extendedMetadata: {
+                  appName: longAppName,
+                  env: { name: 'aws.lambda' }
+                } as unknown as Promise<Document>
+              }
+            };
+            const handshakeDocument = await prepareHandshakeDocument(longAuthContext);
+            expect(handshakeDocument.client.env.name).to.equal('aws.lambda');
+            expect(handshakeDocument.client.env).to.not.have.property('container');
+          });
+        });
       });
 
       context('when container nor FAAS env is not present (empty string case)', () => {

test/unit/cmap/connection_pool.test.js

@@ -22,6 +22,9 @@ describe('Connection Pool', function () {
         },
         s: {
           authProviders: new MongoClientAuthProviders()
+        },
+        options: {
+          extendedMetadata: {}
         }
       }
     }