From f35fd91e11f2483188b306843a97ca19be3e6373 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 27 Feb 2024 17:47:07 -0500 Subject: [PATCH 01/14] EOD commit --- src/cmap/connect.ts | 6 +-- src/cmap/connection.ts | 4 +- src/cmap/connection_pool.ts | 4 +- src/cmap/handshake/client_metadata.ts | 57 ++++++++++++++++++++++++++- src/connection_string.ts | 2 +- src/mongo_client.ts | 9 ++++- src/sdam/topology.ts | 3 +- 7 files changed, 74 insertions(+), 11 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 54b00a7031b..51b528d1a77 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -25,7 +25,7 @@ import { type ConnectionOptions, CryptoConnection } from './connection'; -import type { ClientMetadata } from './handshake/client_metadata'; +import { type ClientMetadata, addAllEnvClientMetadata } from './handshake/client_metadata'; import { MAX_SUPPORTED_SERVER_VERSION, MAX_SUPPORTED_WIRE_VERSION, @@ -200,11 +200,11 @@ export async function prepareHandshakeDocument( const options = authContext.options; const compressors = options.compressors ? options.compressors : []; const { serverApi } = authContext.connection; - + const clientMetadata = await addAllEnvClientMetadata(options.internalMetadata); const handshakeDoc: HandshakeDocument = { [serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1, helloOk: true, - client: options.metadata, + client: clientMetadata.toObject(), compression: compressors }; diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index e33b4f835f6..2d5cdacf69e 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -57,7 +57,7 @@ import { type WriteProtocolMessageType } from './commands'; import type { Stream } from './connect'; -import type { ClientMetadata } from './handshake/client_metadata'; +import type { ClientMetadata, LimitedSizeDocument } from './handshake/client_metadata'; import { StreamDescription, type StreamDescriptionOptions } from './stream_description'; import { type CompressorName, decompressResponse } from './wire_protocol/compression'; import { onData } from './wire_protocol/on_data'; @@ -118,6 +118,8 @@ export interface ConnectionOptions socketTimeoutMS?: number; cancellationToken?: CancellationToken; metadata: ClientMetadata; + /** @internal */ + internalMetadata: LimitedSizeDocument; /** @internal */ mongoLogger?: MongoLogger | undefined; } diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 435b66936d5..20c6336e279 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -234,7 +234,7 @@ export class ConnectionPool extends TypedEventEmitter { waitQueueTimeoutMS: options.waitQueueTimeoutMS ?? 0, minPoolSizeCheckFrequencyMS: options.minPoolSizeCheckFrequencyMS ?? 100, autoEncrypter: options.autoEncrypter, - metadata: options.metadata + metadata: options.metadata, }); if (this.options.minPoolSize > this.options.maxPoolSize) { @@ -634,7 +634,7 @@ export class ConnectionPool extends TypedEventEmitter { generation: this[kGeneration], cancellationToken: this[kCancellationToken], mongoLogger: this.mongoLogger, - authProviders: this[kServer].topology.client.s.authProviders + authProviders: this[kServer].topology.client.s.authProviders, }; this[kPending]++; diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index fb1ba40b14e..71a0e4d26e2 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -1,3 +1,4 @@ +import { promises as fs } from 'fs'; import * as os from 'os'; import * as process from 'process'; @@ -91,8 +92,14 @@ type MakeClientMetadataOptions = Pick; * 4. Truncate `platform`. -- special we do not truncate this field */ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMetadata { - const metadataDocument = new LimitedSizeDocument(512); + let metadataDocument = new LimitedSizeDocument(512); + metadataDocument = addNonEnvClientMetadata(options, metadataDocument); + metadataDocument = addFAASOnlyEnvClientMetadata(metadataDocument); + return metadataDocument.toObject(); +} + +export function addNonEnvClientMetadata(options: MakeClientMetadataOptions, metadataDocument: LimitedSizeDocument): LimitedSizeDocument { const { appName = '' } = options; // Add app name first, it must be sent if (appName.length > 0) { @@ -142,6 +149,10 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe } } + return metadataDocument; +} + +function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): LimitedSizeDocument { const faasEnv = getFAASEnv(); if (faasEnv != null) { if (!metadataDocument.ifItFitsItSits('env', faasEnv)) { @@ -152,8 +163,50 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe } } } + return metadataDocument; +} - return metadataDocument.toObject(); + +let isDocker: boolean; +export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocument) { + const faasEnv = getFAASEnv(); + + async function getContainerMetadata() { + const containerMetadata: Record = {}; + if (isDocker !== false && isDocker !== true) { + try { + await fs.access('/.dockerenv'); + isDocker = true; + } catch { + isDocker = false; + } + } + const isKubernetes = process.env.KUBERNETES_SERVICE_HOST ? true : false; + + if (isDocker || isKubernetes) { + if (isDocker) { + containerMetadata['runtime'] = 'docker'; + } + if (isKubernetes) { + containerMetadata['orchestrator'] = 'kubernetes'; + } + } + return containerMetadata; + } + + const containerMetadata = await getContainerMetadata(); + const envMetadata = faasEnv ? faasEnv.set('container', containerMetadata) : containerMetadata; + + if (envMetadata != null) { + if (!metadataDocument.ifItFitsItSits('env', envMetadata)) { + for (const key of envMetadata.keys()) { + envMetadata.delete(key); + if (envMetadata.size === 0) break; + if (metadataDocument.ifItFitsItSits('env', envMetadata)) break; + } + } + } + return metadataDocument; } /** diff --git a/src/connection_string.ts b/src/connection_string.ts index fdc4e47ab22..a65c43c22ca 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -5,7 +5,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 { type ClientMetadata, makeClientMetadata } from './cmap/handshake/client_metadata'; import { Compressor, type CompressorName } from './cmap/wire_protocol/compression'; import { Encrypter } from './encrypter'; import { diff --git a/src/mongo_client.ts b/src/mongo_client.ts index be039944a4f..8d11750ea81 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -14,7 +14,7 @@ import { import { AuthMechanism } from './cmap/auth/providers'; import type { LEGAL_TCP_SOCKET_OPTIONS, LEGAL_TLS_SOCKET_OPTIONS } from './cmap/connect'; import type { Connection } from './cmap/connection'; -import type { ClientMetadata } from './cmap/handshake/client_metadata'; +import type { ClientMetadata, LimitedSizeDocument } from './cmap/handshake/client_metadata'; import type { CompressorName } from './cmap/wire_protocol/compression'; import { parseOptions, resolveSRVRecord } from './connection_string'; import { MONGO_CLIENT_EVENTS } from './constants'; @@ -897,4 +897,11 @@ export interface MongoOptions * TODO: NODE-5671 - remove internal flag */ mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable; + + /** + * @internal + * Metadata as BSON bytes. + * Does not contain any environment information. + */ + internalMetadata: LimitedSizeDocument; } diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 400db63870f..1c5942108f7 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -4,7 +4,7 @@ import type { BSONSerializeOptions, Document } from '../bson'; import type { MongoCredentials } from '../cmap/auth/mongo_credentials'; import type { ConnectionEvents, DestroyOptions } from '../cmap/connection'; import type { CloseOptions, ConnectionPoolEvents } from '../cmap/connection_pool'; -import type { ClientMetadata } from '../cmap/handshake/client_metadata'; +import type { ClientMetadata, LimitedSizeDocument } from '../cmap/handshake/client_metadata'; import { DEFAULT_OPTIONS, FEATURE_FLAGS } from '../connection_string'; import { CLOSE, @@ -158,6 +158,7 @@ export interface TopologyOptions extends BSONSerializeOptions, ServerOptions { directConnection: boolean; loadBalanced: boolean; metadata: ClientMetadata; + internalMetadata: LimitedSizeDocument; serverMonitoringMode: ServerMonitoringMode; /** MongoDB server API version */ serverApi?: ServerApi; From 42a83432a775d317d811a7f6c617131f01633322 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 27 Feb 2024 17:49:43 -0500 Subject: [PATCH 02/14] removed extraneous changes --- src/cmap/connection_pool.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 20c6336e279..435b66936d5 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -234,7 +234,7 @@ export class ConnectionPool extends TypedEventEmitter { waitQueueTimeoutMS: options.waitQueueTimeoutMS ?? 0, minPoolSizeCheckFrequencyMS: options.minPoolSizeCheckFrequencyMS ?? 100, autoEncrypter: options.autoEncrypter, - metadata: options.metadata, + metadata: options.metadata }); if (this.options.minPoolSize > this.options.maxPoolSize) { @@ -634,7 +634,7 @@ export class ConnectionPool extends TypedEventEmitter { generation: this[kGeneration], cancellationToken: this[kCancellationToken], mongoLogger: this.mongoLogger, - authProviders: this[kServer].topology.client.s.authProviders, + authProviders: this[kServer].topology.client.s.authProviders }; this[kPending]++; From d16b5473fcfe49f51e79a2b92a6228eeb1d7d07c Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 27 Feb 2024 17:57:27 -0500 Subject: [PATCH 03/14] removed error and added single promise for addContainerMetadata --- src/cmap/handshake/client_metadata.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index 71a0e4d26e2..9dce5580ff5 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -168,14 +168,16 @@ function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): Li let isDocker: boolean; +let dockerPromise: any; export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocument) { const faasEnv = getFAASEnv(); async function getContainerMetadata() { const containerMetadata: Record = {}; - if (isDocker !== false && isDocker !== true) { + if (isDocker == null) { + dockerPromise ??= fs.access('/.dockerenv'); try { - await fs.access('/.dockerenv'); + await dockerPromise; isDocker = true; } catch { isDocker = false; @@ -195,8 +197,8 @@ export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocum } const containerMetadata = await getContainerMetadata(); - const envMetadata = faasEnv ? faasEnv.set('container', containerMetadata) : containerMetadata; - + const envMetadata = faasEnv ?? new Map(); + envMetadata.set('container', containerMetadata); if (envMetadata != null) { if (!metadataDocument.ifItFitsItSits('env', envMetadata)) { for (const key of envMetadata.keys()) { From 7c4ce2737b2e904cf8593d01616a79fce0bfb0d5 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 28 Feb 2024 15:09:29 -0500 Subject: [PATCH 04/14] lint fix lint fix 2 --- src/cmap/connect.ts | 2 +- src/cmap/connection.ts | 4 ++-- src/cmap/handshake/client_metadata.ts | 14 +++++++++----- src/connection_string.ts | 3 ++- src/index.ts | 6 +++++- src/mongo_client.ts | 2 +- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 51b528d1a77..1b51b60c477 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -25,7 +25,7 @@ import { type ConnectionOptions, CryptoConnection } from './connection'; -import { type ClientMetadata, addAllEnvClientMetadata } from './handshake/client_metadata'; +import { addAllEnvClientMetadata, type ClientMetadata } from './handshake/client_metadata'; import { MAX_SUPPORTED_SERVER_VERSION, MAX_SUPPORTED_WIRE_VERSION, diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index 2d5cdacf69e..c368508a81f 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -118,8 +118,8 @@ export interface ConnectionOptions socketTimeoutMS?: number; cancellationToken?: CancellationToken; metadata: ClientMetadata; - /** @internal */ - internalMetadata: LimitedSizeDocument; + /** @internal */ + internalMetadata: LimitedSizeDocument; /** @internal */ mongoLogger?: MongoLogger | undefined; } diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index 9dce5580ff5..0487b3ba612 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -98,8 +98,11 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe return metadataDocument.toObject(); } - -export function addNonEnvClientMetadata(options: MakeClientMetadataOptions, metadataDocument: LimitedSizeDocument): LimitedSizeDocument { +/** @internal */ +export function addNonEnvClientMetadata( + options: MakeClientMetadataOptions, + metadataDocument: LimitedSizeDocument +): LimitedSizeDocument { const { appName = '' } = options; // Add app name first, it must be sent if (appName.length > 0) { @@ -152,6 +155,7 @@ export function addNonEnvClientMetadata(options: MakeClientMetadataOptions, meta return metadataDocument; } +/** @internal */ function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): LimitedSizeDocument { const faasEnv = getFAASEnv(); if (faasEnv != null) { @@ -166,12 +170,12 @@ function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): Li return metadataDocument; } - let isDocker: boolean; let dockerPromise: any; +/** @internal */ export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocument) { const faasEnv = getFAASEnv(); - + async function getContainerMetadata() { const containerMetadata: Record = {}; if (isDocker == null) { @@ -184,7 +188,7 @@ export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocum } } const isKubernetes = process.env.KUBERNETES_SERVICE_HOST ? true : false; - + if (isDocker || isKubernetes) { if (isDocker) { containerMetadata['runtime'] = 'docker'; diff --git a/src/connection_string.ts b/src/connection_string.ts index a65c43c22ca..5da80261874 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -5,7 +5,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 { type ClientMetadata, makeClientMetadata } from './cmap/handshake/client_metadata'; +import { LimitedSizeDocument, addNonEnvClientMetadata, makeClientMetadata } from './cmap/handshake/client_metadata'; import { Compressor, type CompressorName } from './cmap/wire_protocol/compression'; import { Encrypter } from './encrypter'; import { @@ -544,6 +544,7 @@ export function parseOptions( ); mongoOptions.metadata = makeClientMetadata(mongoOptions); + mongoOptions.internalMetadata = addNonEnvClientMetadata(mongoOptions, new LimitedSizeDocument(512)); return mongoOptions; } diff --git a/src/index.ts b/src/index.ts index aae568dd79e..c7bf327419a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -287,7 +287,11 @@ export type { WaitQueueMember, WithConnectionCallback } from './cmap/connection_pool'; -export type { ClientMetadata, ClientMetadataOptions } from './cmap/handshake/client_metadata'; +export type { + ClientMetadata, + ClientMetadataOptions, + LimitedSizeDocument +} from './cmap/handshake/client_metadata'; export type { ConnectionPoolMetrics } from './cmap/metrics'; export type { StreamDescription, StreamDescriptionOptions } from './cmap/stream_description'; export type { CompressorName } from './cmap/wire_protocol/compression'; diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 8d11750ea81..9e058b616b8 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -899,7 +899,7 @@ export interface MongoOptions mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable; /** - * @internal + * @internal * Metadata as BSON bytes. * Does not contain any environment information. */ From 03e5e1379a62f72a891db1dd9242d052a6f7b499 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 28 Feb 2024 17:53:56 -0500 Subject: [PATCH 05/14] added tests added tests - no docker tests removed extraneous export and newline removed extraneous export and newline removed unnecesary helper funcs removed unnecesary let, changed to const added more tests to comply w kickoff - no docker tests still cleared up logic --- src/cmap/connect.ts | 9 ++-- src/cmap/connection.ts | 4 +- src/cmap/handshake/client_metadata.ts | 65 +++++++++++---------------- src/connection_string.ts | 3 +- src/index.ts | 6 +-- src/mongo_client.ts | 9 +--- src/sdam/topology.ts | 3 +- test/unit/cmap/connect.test.ts | 53 +++++++++++++++++++--- 8 files changed, 82 insertions(+), 70 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 1b51b60c477..1d49a0c2287 100644 --- a/src/cmap/connect.ts +++ b/src/cmap/connect.ts @@ -25,7 +25,7 @@ import { type ConnectionOptions, CryptoConnection } from './connection'; -import { addAllEnvClientMetadata, type ClientMetadata } from './handshake/client_metadata'; +import { addContainerMetadata } from './handshake/client_metadata'; import { MAX_SUPPORTED_SERVER_VERSION, MAX_SUPPORTED_WIRE_VERSION, @@ -183,7 +183,7 @@ export interface HandshakeDocument extends Document { ismaster?: boolean; hello?: boolean; helloOk?: boolean; - client: ClientMetadata; + client: Document; compression: string[]; saslSupportedMechs?: string; loadBalanced?: boolean; @@ -200,11 +200,12 @@ export async function prepareHandshakeDocument( const options = authContext.options; const compressors = options.compressors ? options.compressors : []; const { serverApi } = authContext.connection; - const clientMetadata = await addAllEnvClientMetadata(options.internalMetadata); + const clientMetadata = await addContainerMetadata(options.metadata); + const handshakeDoc: HandshakeDocument = { [serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1, helloOk: true, - client: clientMetadata.toObject(), + client: clientMetadata, compression: compressors }; diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index c368508a81f..e33b4f835f6 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -57,7 +57,7 @@ import { type WriteProtocolMessageType } from './commands'; import type { Stream } from './connect'; -import type { ClientMetadata, LimitedSizeDocument } from './handshake/client_metadata'; +import type { ClientMetadata } from './handshake/client_metadata'; import { StreamDescription, type StreamDescriptionOptions } from './stream_description'; import { type CompressorName, decompressResponse } from './wire_protocol/compression'; import { onData } from './wire_protocol/on_data'; @@ -119,8 +119,6 @@ export interface ConnectionOptions cancellationToken?: CancellationToken; metadata: ClientMetadata; /** @internal */ - internalMetadata: LimitedSizeDocument; - /** @internal */ mongoLogger?: MongoLogger | undefined; } diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index 0487b3ba612..45b1d5ddba5 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -2,7 +2,7 @@ import { promises as fs } from 'fs'; import * as os from 'os'; import * as process from 'process'; -import { BSON, Int32 } from '../../bson'; +import { BSON, type Document, Int32 } from '../../bson'; import { MongoInvalidArgumentError } from '../../error'; import type { MongoOptions } from '../../mongo_client'; @@ -72,13 +72,13 @@ export class LimitedSizeDocument { return true; } - toObject(): ClientMetadata { + toObject(): Document { return BSON.deserialize(BSON.serialize(this.document), { promoteLongs: false, promoteBuffers: false, promoteValues: false, useBigInt64: false - }) as ClientMetadata; + }); } } @@ -92,17 +92,8 @@ type MakeClientMetadataOptions = Pick; * 4. Truncate `platform`. -- special we do not truncate this field */ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMetadata { - let metadataDocument = new LimitedSizeDocument(512); - metadataDocument = addNonEnvClientMetadata(options, metadataDocument); - metadataDocument = addFAASOnlyEnvClientMetadata(metadataDocument); - return metadataDocument.toObject(); -} + const metadataDocument = new LimitedSizeDocument(512); -/** @internal */ -export function addNonEnvClientMetadata( - options: MakeClientMetadataOptions, - metadataDocument: LimitedSizeDocument -): LimitedSizeDocument { const { appName = '' } = options; // Add app name first, it must be sent if (appName.length > 0) { @@ -152,11 +143,6 @@ export function addNonEnvClientMetadata( } } - return metadataDocument; -} - -/** @internal */ -function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): LimitedSizeDocument { const faasEnv = getFAASEnv(); if (faasEnv != null) { if (!metadataDocument.ifItFitsItSits('env', faasEnv)) { @@ -167,15 +153,13 @@ function addFAASOnlyEnvClientMetadata(metadataDocument: LimitedSizeDocument): Li } } } - return metadataDocument; + return metadataDocument.toObject() as ClientMetadata; } let isDocker: boolean; let dockerPromise: any; /** @internal */ -export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocument) { - const faasEnv = getFAASEnv(); - +export async function addContainerMetadata(originalMetadata: ClientMetadata) { async function getContainerMetadata() { const containerMetadata: Record = {}; if (isDocker == null) { @@ -189,30 +173,33 @@ export async function addAllEnvClientMetadata(metadataDocument: LimitedSizeDocum } const isKubernetes = process.env.KUBERNETES_SERVICE_HOST ? true : false; - if (isDocker || isKubernetes) { - if (isDocker) { - containerMetadata['runtime'] = 'docker'; - } - if (isKubernetes) { - containerMetadata['orchestrator'] = 'kubernetes'; - } - } + if (isDocker) containerMetadata['runtime'] = 'docker'; + if (isKubernetes) containerMetadata['orchestrator'] = 'kubernetes'; + return containerMetadata; } const containerMetadata = await getContainerMetadata(); - const envMetadata = faasEnv ?? new Map(); - envMetadata.set('container', containerMetadata); - if (envMetadata != null) { - if (!metadataDocument.ifItFitsItSits('env', envMetadata)) { - for (const key of envMetadata.keys()) { - envMetadata.delete(key); - if (envMetadata.size === 0) break; - if (metadataDocument.ifItFitsItSits('env', envMetadata)) break; + if (Object.keys(containerMetadata).length === 0) return originalMetadata; + + const extendedMetadata = new LimitedSizeDocument(512); + const envMetadata = { ...originalMetadata?.env, container: containerMetadata }; + + for (const [key, val] of Object.entries(originalMetadata)) { + if (key !== 'env') { + extendedMetadata.ifItFitsItSits(key, val); + } else { + if (!extendedMetadata.ifItFitsItSits('env', envMetadata)) { + extendedMetadata.ifItFitsItSits('env', val); } } } - return metadataDocument; + + if (!('env' in originalMetadata)) { + extendedMetadata.ifItFitsItSits('env', envMetadata); + } + + return extendedMetadata.toObject(); } /** diff --git a/src/connection_string.ts b/src/connection_string.ts index 5da80261874..fdc4e47ab22 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -5,7 +5,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 { LimitedSizeDocument, addNonEnvClientMetadata, makeClientMetadata } from './cmap/handshake/client_metadata'; +import { makeClientMetadata } from './cmap/handshake/client_metadata'; import { Compressor, type CompressorName } from './cmap/wire_protocol/compression'; import { Encrypter } from './encrypter'; import { @@ -544,7 +544,6 @@ export function parseOptions( ); mongoOptions.metadata = makeClientMetadata(mongoOptions); - mongoOptions.internalMetadata = addNonEnvClientMetadata(mongoOptions, new LimitedSizeDocument(512)); return mongoOptions; } diff --git a/src/index.ts b/src/index.ts index c7bf327419a..aae568dd79e 100644 --- a/src/index.ts +++ b/src/index.ts @@ -287,11 +287,7 @@ export type { WaitQueueMember, WithConnectionCallback } from './cmap/connection_pool'; -export type { - ClientMetadata, - ClientMetadataOptions, - LimitedSizeDocument -} from './cmap/handshake/client_metadata'; +export type { ClientMetadata, ClientMetadataOptions } from './cmap/handshake/client_metadata'; export type { ConnectionPoolMetrics } from './cmap/metrics'; export type { StreamDescription, StreamDescriptionOptions } from './cmap/stream_description'; export type { CompressorName } from './cmap/wire_protocol/compression'; diff --git a/src/mongo_client.ts b/src/mongo_client.ts index 9e058b616b8..be039944a4f 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -14,7 +14,7 @@ import { import { AuthMechanism } from './cmap/auth/providers'; import type { LEGAL_TCP_SOCKET_OPTIONS, LEGAL_TLS_SOCKET_OPTIONS } from './cmap/connect'; import type { Connection } from './cmap/connection'; -import type { ClientMetadata, LimitedSizeDocument } from './cmap/handshake/client_metadata'; +import type { ClientMetadata } from './cmap/handshake/client_metadata'; import type { CompressorName } from './cmap/wire_protocol/compression'; import { parseOptions, resolveSRVRecord } from './connection_string'; import { MONGO_CLIENT_EVENTS } from './constants'; @@ -897,11 +897,4 @@ export interface MongoOptions * TODO: NODE-5671 - remove internal flag */ mongodbLogPath?: 'stderr' | 'stdout' | MongoDBLogWritable; - - /** - * @internal - * Metadata as BSON bytes. - * Does not contain any environment information. - */ - internalMetadata: LimitedSizeDocument; } diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 1c5942108f7..400db63870f 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -4,7 +4,7 @@ import type { BSONSerializeOptions, Document } from '../bson'; import type { MongoCredentials } from '../cmap/auth/mongo_credentials'; import type { ConnectionEvents, DestroyOptions } from '../cmap/connection'; import type { CloseOptions, ConnectionPoolEvents } from '../cmap/connection_pool'; -import type { ClientMetadata, LimitedSizeDocument } from '../cmap/handshake/client_metadata'; +import type { ClientMetadata } from '../cmap/handshake/client_metadata'; import { DEFAULT_OPTIONS, FEATURE_FLAGS } from '../connection_string'; import { CLOSE, @@ -158,7 +158,6 @@ export interface TopologyOptions extends BSONSerializeOptions, ServerOptions { directConnection: boolean; loadBalanced: boolean; metadata: ClientMetadata; - internalMetadata: LimitedSizeDocument; serverMonitoringMode: ServerMonitoringMode; /** MongoDB server API version */ serverApi?: ServerApi; diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 7697c124fbe..72b41d47adb 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -26,7 +26,7 @@ const CONNECT_DEFAULTS = { loadBalanced: false }; -describe('Connect Tests', function () { +describe('Connect Tests', async function () { context('when PLAIN auth enabled', () => { const test: { server?: any; @@ -185,9 +185,48 @@ describe('Connect Tests', function () { expect(error).to.be.instanceOf(MongoNetworkError); }); - context('prepareHandshakeDocument', () => { + context('prepareHandshakeDocument', async () => { + context('when container is present', async () => { + const authContext = { + connection: {}, + options: { ...CONNECT_DEFAULTS } + }; + + context('when only kubernetes is present', async () => { + beforeEach(() => { + process.env.KUBERNETES_SERVICE_HOST = 'I exist'; + }); + + afterEach(() => { + process.env.KUBERNETES_SERVICE_HOST = ''; + }); + + it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client.env.container.orchestrator).to.equal('kubernetes'); + }); + + it(`should not have 'name' property in client.env `, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client.env).to.not.have.property('name'); + }); + }); + }); + + context('when container nor FAAS env is not present', async () => { + const authContext = { + connection: {}, + options: { ...CONNECT_DEFAULTS } + }; + + it(`should not have 'env' property in client`, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client).to.not.have.property('env'); + }); + }); + context('when serverApi.version is present', () => { - const options = {}; + const options = { ...CONNECT_DEFAULTS }; const authContext = { connection: { serverApi: { version: '1' } }, options @@ -200,7 +239,7 @@ describe('Connect Tests', function () { }); context('when serverApi is not present', () => { - const options = {}; + const options = { ...CONNECT_DEFAULTS }; const authContext = { connection: {}, options @@ -216,7 +255,7 @@ describe('Connect Tests', function () { context('when loadBalanced is not set as an option', () => { const authContext = { connection: {}, - options: {} + options: { ...CONNECT_DEFAULTS } }; it('does not set loadBalanced on the handshake document', async () => { @@ -238,7 +277,7 @@ describe('Connect Tests', function () { context('when loadBalanced is set to false', () => { const authContext = { connection: {}, - options: { loadBalanced: false } + options: { ...CONNECT_DEFAULTS, loadBalanced: false } }; it('does not set loadBalanced on the handshake document', async () => { @@ -260,7 +299,7 @@ describe('Connect Tests', function () { context('when loadBalanced is set to true', () => { const authContext = { connection: {}, - options: { loadBalanced: true } + options: { ...CONNECT_DEFAULTS, loadBalanced: true } }; it('sets loadBalanced on the handshake document', async () => { From 4ff472d82555650255ec458b99ea82ca7e424671 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Thu, 29 Feb 2024 18:00:51 -0500 Subject: [PATCH 06/14] primary review changes 0 --- src/cmap/handshake/client_metadata.ts | 49 +++++++++------ test/unit/cmap/connect.test.ts | 88 +++++++++++++++++++++------ 2 files changed, 99 insertions(+), 38 deletions(-) diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index 45b1d5ddba5..9bf5707bbdd 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -157,46 +157,55 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe } let isDocker: boolean; -let dockerPromise: any; +let dockerPromise: Promise; /** @internal */ -export async function addContainerMetadata(originalMetadata: ClientMetadata) { - async function getContainerMetadata() { - const containerMetadata: Record = {}; - if (isDocker == null) { - dockerPromise ??= fs.access('/.dockerenv'); - try { - await dockerPromise; - isDocker = true; - } catch { - isDocker = false; - } +async function getContainerMetadata() { + const containerMetadata: Record = {}; + if (isDocker == null) { + dockerPromise ??= fs.access('/.dockerenv'); + try { + await dockerPromise; + isDocker = true; + } catch { + isDocker = false; } - const isKubernetes = process.env.KUBERNETES_SERVICE_HOST ? true : false; + } - if (isDocker) containerMetadata['runtime'] = 'docker'; - if (isKubernetes) containerMetadata['orchestrator'] = 'kubernetes'; + const { KUBERNETES_SERVICE_HOST = '' } = process.env; + const isKubernetes = KUBERNETES_SERVICE_HOST.length > 0 ? true : false; - return containerMetadata; - } + if (isDocker) containerMetadata.runtime = 'docker'; + if (isKubernetes) containerMetadata.orchestrator = 'kubernetes'; + + return containerMetadata; +} +/** + * @internal + * Re-add each metadata value. + * Attempt to add new env container metadata, but keep old data if it does not fit. + */ +export async function addContainerMetadata(originalMetadata: ClientMetadata) { const containerMetadata = await getContainerMetadata(); if (Object.keys(containerMetadata).length === 0) return originalMetadata; const extendedMetadata = new LimitedSizeDocument(512); - const envMetadata = { ...originalMetadata?.env, container: containerMetadata }; + + const extendedEnvMetadata = { ...originalMetadata?.env, container: containerMetadata }; for (const [key, val] of Object.entries(originalMetadata)) { if (key !== 'env') { extendedMetadata.ifItFitsItSits(key, val); } else { - if (!extendedMetadata.ifItFitsItSits('env', envMetadata)) { + if (!extendedMetadata.ifItFitsItSits('env', extendedEnvMetadata)) { + // add in old data if newer / extended metadata does not fit extendedMetadata.ifItFitsItSits('env', val); } } } if (!('env' in originalMetadata)) { - extendedMetadata.ifItFitsItSits('env', envMetadata); + extendedMetadata.ifItFitsItSits('env', extendedEnvMetadata); } return extendedMetadata.toObject(); diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 72b41d47adb..e1190c7858f 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -26,7 +26,7 @@ const CONNECT_DEFAULTS = { loadBalanced: false }; -describe('Connect Tests', async function () { +describe('Connect Tests', function () { context('when PLAIN auth enabled', () => { const test: { server?: any; @@ -185,20 +185,22 @@ describe('Connect Tests', async function () { expect(error).to.be.instanceOf(MongoNetworkError); }); - context('prepareHandshakeDocument', async () => { - context('when container is present', async () => { - const authContext = { - connection: {}, - options: { ...CONNECT_DEFAULTS } - }; + describe('prepareHandshakeDocument', () => { + describe('client environment (containers and FAAS)', () => { + const cachedEnv = process.env; + + context('when only kubernetes is present', () => { + const authContext = { + connection: {}, + options: { ...CONNECT_DEFAULTS } + }; - context('when only kubernetes is present', async () => { beforeEach(() => { process.env.KUBERNETES_SERVICE_HOST = 'I exist'; }); afterEach(() => { - process.env.KUBERNETES_SERVICE_HOST = ''; + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; }); it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { @@ -211,17 +213,67 @@ describe('Connect Tests', async function () { expect(handshakeDocument.client.env).to.not.have.property('name'); }); }); - }); - context('when container nor FAAS env is not present', async () => { - const authContext = { - connection: {}, - options: { ...CONNECT_DEFAULTS } - }; + context('when kubernetes and FAAS are both present', () => { + const authContext = { + connection: {}, + options: { ...CONNECT_DEFAULTS, metadata: { env: { name: 'aws.lambda' } } } + }; - it(`should not have 'env' property in client`, async () => { - const handshakeDocument = await prepareHandshakeDocument(authContext); - expect(handshakeDocument.client).to.not.have.property('env'); + beforeEach(() => { + process.env.KUBERNETES_SERVICE_HOST = 'I exist'; + }); + + afterEach(() => { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + }); + + it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client.env.container.orchestrator).to.equal('kubernetes'); + }); + + it(`should still have properly set 'name' property in client.env `, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client.env.name).to.equal('aws.lambda'); + }); + }); + + context('when container nor FAAS env is not present (empty string case)', () => { + const authContext = { + connection: {}, + options: { ...CONNECT_DEFAULTS } + }; + + context('when process.env.KUBERNETES_SERVICE_HOST = undefined', () => { + beforeEach(() => { + process.env.KUBERNETES_SERVICE_HOST = ''; + }); + + afterEach(() => { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + }); + + it(`should not have 'env' property in client`, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client).to.not.have.property('env'); + }); + }); + + context('when process.env.KUBERNETES_SERVICE_HOST is an empty string', () => { + beforeEach(() => { + process.env.KUBERNETES_SERVICE_HOST = ''; + }); + + afterEach(() => { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + }); + + it(`should not have 'env' property in client`, async () => { + const handshakeDocument = await prepareHandshakeDocument(authContext); + expect(handshakeDocument.client).to.not.have.property('env'); + }); + }); }); }); From 525703aefbcb5ce0d5c22042ea7b77bc8bcd7c9a Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 1 Mar 2024 13:44:09 -0500 Subject: [PATCH 07/14] testing fix --- test/unit/cmap/connect.test.ts | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index e1190c7858f..ebe3d5a4b5a 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -26,7 +26,7 @@ const CONNECT_DEFAULTS = { loadBalanced: false }; -describe('Connect Tests', function () { +describe.only('Connect Tests', function () { context('when PLAIN auth enabled', () => { const test: { server?: any; @@ -200,7 +200,11 @@ describe('Connect Tests', function () { }); afterEach(() => { - process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + if (cachedEnv.KUBERNETES_SERVICE_HOST) { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + } else { + delete process.env.KUBERNETES_SERVICE_HOST; + } }); it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { @@ -225,7 +229,11 @@ describe('Connect Tests', function () { }); afterEach(() => { - process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + if (cachedEnv.KUBERNETES_SERVICE_HOST) { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + } else { + delete process.env.KUBERNETES_SERVICE_HOST; + } }); it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { @@ -247,11 +255,17 @@ describe('Connect Tests', function () { context('when process.env.KUBERNETES_SERVICE_HOST = undefined', () => { beforeEach(() => { - process.env.KUBERNETES_SERVICE_HOST = ''; + delete process.env.KUBERNETES_SERVICE_HOST; }); afterEach(() => { - process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + afterEach(() => { + if (cachedEnv.KUBERNETES_SERVICE_HOST) { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + } else { + delete process.env.KUBERNETES_SERVICE_HOST; + } + }); }); it(`should not have 'env' property in client`, async () => { @@ -266,7 +280,11 @@ describe('Connect Tests', function () { }); afterEach(() => { - process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + if (cachedEnv.KUBERNETES_SERVICE_HOST) { + process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; + } else { + delete process.env.KUBERNETES_SERVICE_HOST; + } }); it(`should not have 'env' property in client`, async () => { From 91bb3e1918a5e5cfbafcacda3d56c128c96741bd Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 1 Mar 2024 13:45:47 -0500 Subject: [PATCH 08/14] removed stray only --- test/unit/cmap/connect.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index ebe3d5a4b5a..d80ebdc3c09 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -26,7 +26,7 @@ const CONNECT_DEFAULTS = { loadBalanced: false }; -describe.only('Connect Tests', function () { +describe('Connect Tests', function () { context('when PLAIN auth enabled', () => { const test: { server?: any; From f41e3708d50a0207d2a881ca29ed912e50b99446 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Fri, 1 Mar 2024 14:37:46 -0500 Subject: [PATCH 09/14] fix kubernetes delete check: --- test/unit/cmap/connect.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index d80ebdc3c09..be2d08df1fc 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -200,7 +200,7 @@ describe('Connect Tests', function () { }); afterEach(() => { - if (cachedEnv.KUBERNETES_SERVICE_HOST) { + if (cachedEnv.KUBERNETES_SERVICE_HOST != null) { process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; } else { delete process.env.KUBERNETES_SERVICE_HOST; @@ -229,7 +229,7 @@ describe('Connect Tests', function () { }); afterEach(() => { - if (cachedEnv.KUBERNETES_SERVICE_HOST) { + if (cachedEnv.KUBERNETES_SERVICE_HOST != null) { process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; } else { delete process.env.KUBERNETES_SERVICE_HOST; @@ -260,7 +260,7 @@ describe('Connect Tests', function () { afterEach(() => { afterEach(() => { - if (cachedEnv.KUBERNETES_SERVICE_HOST) { + if (cachedEnv.KUBERNETES_SERVICE_HOST != null) { process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; } else { delete process.env.KUBERNETES_SERVICE_HOST; @@ -280,7 +280,7 @@ describe('Connect Tests', function () { }); afterEach(() => { - if (cachedEnv.KUBERNETES_SERVICE_HOST) { + if (cachedEnv.KUBERNETES_SERVICE_HOST != null) { process.env.KUBERNETES_SERVICE_HOST = cachedEnv.KUBERNETES_SERVICE_HOST; } else { delete process.env.KUBERNETES_SERVICE_HOST; From b44e63152d51d00d187977eaef6e188cd6264353 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 4 Mar 2024 12:49:56 -0500 Subject: [PATCH 10/14] added size tests --- test/unit/cmap/connect.test.ts | 38 ++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index be2d08df1fc..2fe6474789a 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -216,6 +216,22 @@ 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 () => { + let longAppName = ''; + // make metadata = 507 bytes, so it takes up entire LimitedSizeDocument + for (let i = 0; i < 493; i++) { + longAppName += 's'; + } + const longAuthContext = { + connection: {}, + options: { ...CONNECT_DEFAULTS, metadata: { appName: longAppName } } + }; + const handshakeDocument = await prepareHandshakeDocument(longAuthContext); + expect(handshakeDocument.client).to.not.have.property('env'); + }); + }); }); context('when kubernetes and FAAS are both present', () => { @@ -245,6 +261,28 @@ 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 () => { + let longAppName = ''; + // make metadata = 507 bytes, so it takes up entire LimitedSizeDocument + for (let i = 0; i < 485; i++) { + longAppName += 's'; + } + const longAuthContext = { + connection: {}, + options: { + ...CONNECT_DEFAULTS, + metadata: { + appName: longAppName, + env: { name: 'aws.lambda' } + } + } + }; + const handshakeDocument = await prepareHandshakeDocument(longAuthContext); + expect(handshakeDocument.client).to.not.have.property('env'); + }); + }); }); context('when container nor FAAS env is not present (empty string case)', () => { From 25d1b441704526824f2637ff363cadd9a4139b00 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Mon, 4 Mar 2024 14:38:02 -0500 Subject: [PATCH 11/14] fixed size tests --- test/unit/cmap/connect.test.ts | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 2fe6474789a..03b943d059c 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -219,11 +219,8 @@ describe('Connect Tests', function () { context('when 512 byte size limit is exceeded', async () => { it(`should not 'env' property in client`, async () => { - let longAppName = ''; // make metadata = 507 bytes, so it takes up entire LimitedSizeDocument - for (let i = 0; i < 493; i++) { - longAppName += 's'; - } + const longAppName = 's'.repeat(493); const longAuthContext = { connection: {}, options: { ...CONNECT_DEFAULTS, metadata: { appName: longAppName } } @@ -264,11 +261,8 @@ describe('Connect Tests', function () { context('when 512 byte size limit is exceeded', async () => { it(`should not have 'container' property in client.env`, async () => { - let longAppName = ''; // make metadata = 507 bytes, so it takes up entire LimitedSizeDocument - for (let i = 0; i < 485; i++) { - longAppName += 's'; - } + const longAppName = 's'.repeat(447); const longAuthContext = { connection: {}, options: { @@ -280,7 +274,8 @@ describe('Connect Tests', function () { } }; const handshakeDocument = await prepareHandshakeDocument(longAuthContext); - expect(handshakeDocument.client).to.not.have.property('env'); + expect(handshakeDocument.client.env.name).to.equal('aws.lambda'); + expect(handshakeDocument.client.env).to.not.have.property('container'); }); }); }); From a9ce7a83973c720b1da3100a581000c3e96275a9 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 5 Mar 2024 14:31:53 -0500 Subject: [PATCH 12/14] shorten cokerPromise code --- src/cmap/handshake/client_metadata.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index 9bf5707bbdd..0a4e869c1a9 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -157,18 +157,16 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe } let isDocker: boolean; -let dockerPromise: Promise; +let dockerPromise: Promise; /** @internal */ async function getContainerMetadata() { const containerMetadata: Record = {}; if (isDocker == null) { - dockerPromise ??= fs.access('/.dockerenv'); - try { - await dockerPromise; - isDocker = true; - } catch { - isDocker = false; - } + dockerPromise ??= fs.access('/.dockerenv').then( + () => true, + () => false + ); + isDocker = await dockerPromise; } const { KUBERNETES_SERVICE_HOST = '' } = process.env; From 8d2c4326ca13dba8666ad7bc2b1bf4f575039211 Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Tue, 5 Mar 2024 18:35:37 -0500 Subject: [PATCH 13/14] Temp commit EOD, added in extendedMetadata client vars fixed failing test fixed failing test --- src/cmap/connect.ts | 3 +- src/cmap/connection.ts | 2 + src/cmap/connection_pool.ts | 3 +- src/connection_string.ts | 3 +- src/mongo_client.ts | 2 + src/sdam/topology.ts | 1 + .../connection.test.ts | 7 +++- test/tools/cmap_spec_runner.ts | 9 ++++- test/unit/cmap/connect.test.ts | 37 +++++++++++++------ test/unit/cmap/connection_pool.test.js | 3 ++ 10 files changed, 51 insertions(+), 19 deletions(-) diff --git a/src/cmap/connect.ts b/src/cmap/connect.ts index 1d49a0c2287..4c91860f1ef 100644 --- a/src/cmap/connect.ts +++ b/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, @@ -200,7 +199,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: Document = await options.extendedMetadata; const handshakeDoc: HandshakeDocument = { [serverApi?.version || options.loadBalanced === true ? 'hello' : LEGACY_HELLO_COMMAND]: 1, diff --git a/src/cmap/connection.ts b/src/cmap/connection.ts index e33b4f835f6..f0e373a825a 100644 --- a/src/cmap/connection.ts +++ b/src/cmap/connection.ts @@ -119,6 +119,8 @@ export interface ConnectionOptions cancellationToken?: CancellationToken; metadata: ClientMetadata; /** @internal */ + extendedMetadata: Promise; + /** @internal */ mongoLogger?: MongoLogger | undefined; } diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 435b66936d5..64b89ee1200 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -233,8 +233,7 @@ export class ConnectionPool extends TypedEventEmitter { maxIdleTimeMS: options.maxIdleTimeMS ?? 0, waitQueueTimeoutMS: options.waitQueueTimeoutMS ?? 0, minPoolSizeCheckFrequencyMS: options.minPoolSizeCheckFrequencyMS ?? 100, - autoEncrypter: options.autoEncrypter, - metadata: options.metadata + autoEncrypter: options.autoEncrypter }); if (this.options.minPoolSize > this.options.maxPoolSize) { diff --git a/src/connection_string.ts b/src/connection_string.ts index fdc4e47ab22..beee5746bea 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -5,7 +5,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 { @@ -544,6 +544,7 @@ export function parseOptions( ); mongoOptions.metadata = makeClientMetadata(mongoOptions); + mongoOptions.extendedMetadata = addContainerMetadata(mongoOptions.metadata); return mongoOptions; } diff --git a/src/mongo_client.ts b/src/mongo_client.ts index be039944a4f..5ab24eee9bf 100644 --- a/src/mongo_client.ts +++ b/src/mongo_client.ts @@ -827,6 +827,8 @@ export interface MongoOptions dbName: string; metadata: ClientMetadata; /** @internal */ + extendedMetadata: Promise; + /** @internal */ autoEncrypter?: AutoEncrypter; proxyHost?: string; proxyPort?: number; diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 400db63870f..68d85657387 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -158,6 +158,7 @@ export interface TopologyOptions extends BSONSerializeOptions, ServerOptions { directConnection: boolean; loadBalanced: boolean; metadata: ClientMetadata; + extendedMetadata: Promise; serverMonitoringMode: ServerMonitoringMode; /** MongoDB server API version */ serverApi?: ServerApi; diff --git a/test/integration/connection-monitoring-and-pooling/connection.test.ts b/test/integration/connection-monitoring-and-pooling/connection.test.ts index a1e8f1f9571..de1e455c665 100644 --- a/test/integration/connection-monitoring-and-pooling/connection.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection.test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; import { + addContainerMetadata, connect, Connection, type ConnectionOptions, @@ -50,7 +51,8 @@ describe('Connection', function () { ...commonConnectOptions, connectionType: Connection, ...this.configuration.options, - metadata: makeClientMetadata({ driverInfo: {} }) + metadata: makeClientMetadata({ driverInfo: {} }), + extendedMetadata: addContainerMetadata(makeClientMetadata({ driverInfo: {} })) }; let conn; @@ -72,7 +74,8 @@ describe('Connection', function () { connectionType: Connection, ...this.configuration.options, monitorCommands: true, - metadata: makeClientMetadata({ driverInfo: {} }) + metadata: makeClientMetadata({ driverInfo: {} }), + extendedMetadata: addContainerMetadata(makeClientMetadata({ driverInfo: {} })) }; let conn; diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 9d0817548f1..a497e6e1923 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/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, @@ -369,6 +370,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; @@ -380,7 +382,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 diff --git a/test/unit/cmap/connect.test.ts b/test/unit/cmap/connect.test.ts index 03b943d059c..65ad159b0b1 100644 --- a/test/unit/cmap/connect.test.ts +++ b/test/unit/cmap/connect.test.ts @@ -1,6 +1,7 @@ import { expect } from 'chai'; import { + addContainerMetadata, CancellationToken, type ClientMetadata, connect, @@ -23,6 +24,7 @@ const CONNECT_DEFAULTS = { generation: 1, monitorCommands: false, metadata: {} as ClientMetadata, + extendedMetadata: addContainerMetadata({} as ClientMetadata), loadBalanced: false }; @@ -190,13 +192,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(() => { @@ -205,6 +211,7 @@ describe('Connect Tests', function () { } else { delete process.env.KUBERNETES_SERVICE_HOST; } + authContext = {}; }); it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { @@ -223,7 +230,10 @@ describe('Connect Tests', function () { const longAppName = 's'.repeat(493); const longAuthContext = { connection: {}, - options: { ...CONNECT_DEFAULTS, metadata: { appName: longAppName } } + options: { + ...CONNECT_DEFAULTS, + extendedMetadata: addContainerMetadata({ appName: longAppName }) + } }; const handshakeDocument = await prepareHandshakeDocument(longAuthContext); expect(handshakeDocument.client).to.not.have.property('env'); @@ -232,13 +242,17 @@ describe('Connect Tests', function () { }); 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(() => { @@ -247,6 +261,7 @@ describe('Connect Tests', function () { } else { delete process.env.KUBERNETES_SERVICE_HOST; } + authContext = {}; }); it(`should include { orchestrator: 'kubernetes'} in client.env.container`, async () => { @@ -267,10 +282,10 @@ describe('Connect Tests', function () { connection: {}, options: { ...CONNECT_DEFAULTS, - metadata: { + extendedMetadata: { appName: longAppName, env: { name: 'aws.lambda' } - } + } as unknown as Promise } }; const handshakeDocument = await prepareHandshakeDocument(longAuthContext); diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index b6e408e3d56..a9ea375c7f5 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -22,6 +22,9 @@ describe('Connection Pool', function () { }, s: { authProviders: new MongoClientAuthProviders() + }, + options: { + extendedMetadata: {} } } } From e33b33d75028188a27c0c11232bd18f22e59ce4e Mon Sep 17 00:00:00 2001 From: Aditi Khare Date: Wed, 6 Mar 2024 13:32:40 -0500 Subject: [PATCH 14/14] bailey's last few comments lint fix --- src/cmap/handshake/client_metadata.ts | 13 +++++-------- src/connection_string.ts | 5 ++++- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/cmap/handshake/client_metadata.ts b/src/cmap/handshake/client_metadata.ts index 0a4e869c1a9..c9589f6e009 100644 --- a/src/cmap/handshake/client_metadata.ts +++ b/src/cmap/handshake/client_metadata.ts @@ -156,18 +156,15 @@ export function makeClientMetadata(options: MakeClientMetadataOptions): ClientMe return metadataDocument.toObject() as ClientMetadata; } -let isDocker: boolean; let dockerPromise: Promise; /** @internal */ async function getContainerMetadata() { const containerMetadata: Record = {}; - if (isDocker == null) { - dockerPromise ??= fs.access('/.dockerenv').then( - () => true, - () => false - ); - isDocker = await dockerPromise; - } + 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; diff --git a/src/connection_string.ts b/src/connection_string.ts index beee5746bea..ea350490a1c 100644 --- a/src/connection_string.ts +++ b/src/connection_string.ts @@ -544,7 +544,10 @@ export function parseOptions( ); mongoOptions.metadata = makeClientMetadata(mongoOptions); - mongoOptions.extendedMetadata = addContainerMetadata(mongoOptions.metadata); + + mongoOptions.extendedMetadata = addContainerMetadata(mongoOptions.metadata).catch(() => { + /* rejections will be handled later */ + }); return mongoOptions; }