From a480c940435e8f91ef0ba69e059e1bef0967bef4 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 15:31:41 +0100 Subject: [PATCH 1/8] MCP-2: add is_container_env to telemetry --- src/index.ts | 2 +- src/telemetry/telemetry.ts | 78 ++++++++++++++++++++--------- src/telemetry/types.ts | 1 + tests/integration/helpers.ts | 2 +- tests/integration/telemetry.test.ts | 14 +++--- tests/unit/telemetry.test.ts | 36 ++++++++----- 6 files changed, 89 insertions(+), 44 deletions(-) diff --git a/src/index.ts b/src/index.ts index 8f5738cf..a46aa34c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,7 +20,7 @@ try { version: packageInfo.version, }); - const telemetry = Telemetry.create(session, config); + const telemetry = Telemetry.create({ session, userConfig: config }); const server = new Server({ mcpServer, diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index ccf0eb41..cba0d99a 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -7,6 +7,7 @@ import { MACHINE_METADATA } from "./constants.js"; import { EventCache } from "./eventCache.js"; import nodeMachineId from "node-machine-id"; import { getDeviceId } from "@mongodb-js/device-id"; +import fs from "fs/promises"; type EventResult = { success: boolean; @@ -15,38 +16,36 @@ type EventResult = { export const DEVICE_ID_TIMEOUT = 3000; +export type TelemetryOptions = { + session: Session; + userConfig: UserConfig; + commonProperties?: CommonProperties; + eventCache?: EventCache; + getRawMachineId?: () => Promise; +}; + export class Telemetry { private isBufferingEvents: boolean = true; /** Resolves when the device ID is retrieved or timeout occurs */ public deviceIdPromise: Promise | undefined; private deviceIdAbortController = new AbortController(); - private eventCache: EventCache; - private getRawMachineId: () => Promise; private constructor( private readonly session: Session, private readonly userConfig: UserConfig, private readonly commonProperties: CommonProperties, - { eventCache, getRawMachineId }: { eventCache: EventCache; getRawMachineId: () => Promise } - ) { - this.eventCache = eventCache; - this.getRawMachineId = getRawMachineId; - } - - static create( - session: Session, - userConfig: UserConfig, - { - commonProperties = { ...MACHINE_METADATA }, - eventCache = EventCache.getInstance(), - getRawMachineId = () => nodeMachineId.machineId(true), - }: { - eventCache?: EventCache; - getRawMachineId?: () => Promise; - commonProperties?: CommonProperties; - } = {} - ): Telemetry { - const instance = new Telemetry(session, userConfig, commonProperties, { eventCache, getRawMachineId }); + private readonly eventCache: EventCache, + private readonly getRawMachineId: () => Promise + ) {} + + static create({ session, userConfig, commonProperties, eventCache, getRawMachineId }: TelemetryOptions): Telemetry { + const instance = new Telemetry( + session, + userConfig, + commonProperties || { ...MACHINE_METADATA }, + eventCache || EventCache.getInstance(), + getRawMachineId || (() => nodeMachineId.machineId(true)) + ); void instance.start(); return instance; @@ -106,7 +105,7 @@ export class Telemetry { * Gets the common properties for events * @returns Object containing common properties for all events */ - public getCommonProperties(): CommonProperties { + public async getCommonProperties(): Promise { return { ...this.commonProperties, mcp_client_version: this.session.agentRunner?.version, @@ -114,9 +113,39 @@ export class Telemetry { session_id: this.session.sessionId, config_atlas_auth: this.session.apiClient.hasCredentials() ? "true" : "false", config_connection_string: this.userConfig.connectionString ? "true" : "false", + is_container_env: (await this.isContainerized()) ? "true" : "false", }; } + private async fileExists(filePath: string): Promise { + try { + await fs.stat(filePath); + return true; // File exists + } catch (e: unknown) { + if ( + e instanceof Error && + ( + e as Error & { + code: string; + } + ).code === "ENOENT" + ) { + return false; // File does not exist + } + throw e; // Re-throw unexpected errors + } + } + + private async isContainerized(): Promise { + for (const file of ["/.dockerenv", "/run/.containerenv", "/var/run/.containerenv"]) { + const fileExists = await this.fileExists(file); + if (fileExists) { + return true; + } + } + return !!process.env.container; + } + /** * Checks if telemetry is currently enabled * This is a method rather than a constant to capture runtime config changes @@ -177,10 +206,11 @@ export class Telemetry { */ private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise { try { + const commonProperties = await this.getCommonProperties(); await client.sendEvents( events.map((event) => ({ ...event, - properties: { ...this.getCommonProperties(), ...event.properties }, + properties: { ...commonProperties, ...event.properties }, })) ); return { success: true }; diff --git a/src/telemetry/types.ts b/src/telemetry/types.ts index d77cc010..05ce8f3f 100644 --- a/src/telemetry/types.ts +++ b/src/telemetry/types.ts @@ -71,4 +71,5 @@ export type CommonProperties = { config_atlas_auth?: TelemetryBoolSet; config_connection_string?: TelemetryBoolSet; session_id?: string; + is_container_env?: TelemetryBoolSet; } & CommonStaticProperties; diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index e407e250..9283f4ac 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -67,7 +67,7 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati userConfig.telemetry = "disabled"; - const telemetry = Telemetry.create(session, userConfig); + const telemetry = Telemetry.create({ session, userConfig }); mcpServer = new Server({ session, diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 522c1154..9757a363 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -10,19 +10,21 @@ describe("Telemetry", () => { const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex"); - const telemetry = Telemetry.create( - new Session({ + const telemetry = Telemetry.create({ + session: new Session({ apiBaseUrl: "", }), - config - ); + userConfig: config, + }); - expect(telemetry.getCommonProperties().device_id).toBe(undefined); + const commonProperties = await telemetry.getCommonProperties(); + + expect(commonProperties.device_id).toBe(undefined); expect(telemetry["isBufferingEvents"]).toBe(true); await telemetry.deviceIdPromise; - expect(telemetry.getCommonProperties().device_id).toBe(actualHashedId); + expect(commonProperties.device_id).toBe(actualHashedId); expect(telemetry["isBufferingEvents"]).toBe(false); }); }); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index c1ae28ea..0a226868 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -55,7 +55,7 @@ describe("Telemetry", () => { } // Helper function to verify mock calls to reduce duplication - function verifyMockCalls({ + async function verifyMockCalls({ sendEventsCalls = 0, clearEventsCalls = 0, appendEventsCalls = 0, @@ -77,11 +77,13 @@ describe("Telemetry", () => { expect(appendEvents.length).toBe(appendEventsCalls); if (sendEventsCalledWith) { + const commonProperties = await telemetry.getCommonProperties(); + expect(sendEvents[0]?.[0]).toEqual( sendEventsCalledWith.map((event) => ({ ...event, properties: { - ...telemetry.getCommonProperties(), + ...commonProperties, ...event.properties, }, })) @@ -125,7 +127,9 @@ describe("Telemetry", () => { setAgentRunner: jest.fn().mockResolvedValue(undefined), } as unknown as Session; - telemetry = Telemetry.create(session, config, { + telemetry = Telemetry.create({ + session, + userConfig: config, eventCache: mockEventCache, getRawMachineId: () => Promise.resolve(machineId), }); @@ -140,7 +144,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls({ + await verifyMockCalls({ sendEventsCalls: 1, clearEventsCalls: 1, sendEventsCalledWith: [testEvent], @@ -154,7 +158,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls({ + await verifyMockCalls({ sendEventsCalls: 1, appendEventsCalls: 1, appendEventsCalledWith: [testEvent], @@ -177,7 +181,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([newEvent]); - verifyMockCalls({ + await verifyMockCalls({ sendEventsCalls: 1, clearEventsCalls: 1, sendEventsCalledWith: [cachedEvent, newEvent], @@ -185,7 +189,7 @@ describe("Telemetry", () => { }); it("should correctly add common properties to events", () => { - const commonProps = telemetry.getCommonProperties(); + const commonProps = await telemetry.getCommonProperties(); // Use explicit type assertion const expectedProps: Record = { @@ -212,7 +216,9 @@ describe("Telemetry", () => { }); it("should successfully resolve the machine ID", async () => { - telemetry = Telemetry.create(session, config, { + telemetry = Telemetry.create({ + session, + userConfig: config, getRawMachineId: () => Promise.resolve(machineId), }); @@ -228,7 +234,9 @@ describe("Telemetry", () => { it("should handle machine ID resolution failure", async () => { const loggerSpy = jest.spyOn(logger, "debug"); - telemetry = Telemetry.create(session, config, { + telemetry = Telemetry.create({ + session, + userConfig: config, getRawMachineId: () => Promise.reject(new Error("Failed to get device ID")), }); @@ -250,7 +258,11 @@ describe("Telemetry", () => { it("should timeout if machine ID resolution takes too long", async () => { const loggerSpy = jest.spyOn(logger, "debug"); - telemetry = Telemetry.create(session, config, { getRawMachineId: () => new Promise(() => {}) }); + telemetry = Telemetry.create({ + session, + userConfig: config, + getRawMachineId: () => new Promise(() => {}), + }); expect(telemetry["isBufferingEvents"]).toBe(true); expect(telemetry.getCommonProperties().device_id).toBe(undefined); @@ -290,7 +302,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls(); + await verifyMockCalls(); }); }); @@ -315,7 +327,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls(); + await verifyMockCalls(); }); }); }); From 126994cd09e60cc5e7b9248336529cec53b0ea81 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 16:09:54 +0100 Subject: [PATCH 2/8] fix: tests --- src/telemetry/telemetry.ts | 7 +++---- tests/integration/telemetry.test.ts | 6 ++---- tests/unit/telemetry.test.ts | 20 ++++++++------------ 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index cba0d99a..578420b9 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -74,6 +74,7 @@ export class Telemetry { }); this.commonProperties.device_id = await this.deviceIdPromise; + this.commonProperties.is_container_env = (await this.isContainerized()) ? "true" : "false"; this.isBufferingEvents = false; } @@ -105,7 +106,7 @@ export class Telemetry { * Gets the common properties for events * @returns Object containing common properties for all events */ - public async getCommonProperties(): Promise { + public getCommonProperties(): CommonProperties { return { ...this.commonProperties, mcp_client_version: this.session.agentRunner?.version, @@ -113,7 +114,6 @@ export class Telemetry { session_id: this.session.sessionId, config_atlas_auth: this.session.apiClient.hasCredentials() ? "true" : "false", config_connection_string: this.userConfig.connectionString ? "true" : "false", - is_container_env: (await this.isContainerized()) ? "true" : "false", }; } @@ -206,11 +206,10 @@ export class Telemetry { */ private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise { try { - const commonProperties = await this.getCommonProperties(); await client.sendEvents( events.map((event) => ({ ...event, - properties: { ...commonProperties, ...event.properties }, + properties: { ...this.getCommonProperties(), ...event.properties }, })) ); return { success: true }; diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 9757a363..0330fb2b 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -17,14 +17,12 @@ describe("Telemetry", () => { userConfig: config, }); - const commonProperties = await telemetry.getCommonProperties(); - - expect(commonProperties.device_id).toBe(undefined); + expect(telemetry.getCommonProperties().device_id).toBe(undefined); expect(telemetry["isBufferingEvents"]).toBe(true); await telemetry.deviceIdPromise; - expect(commonProperties.device_id).toBe(actualHashedId); + expect(telemetry.getCommonProperties().device_id).toBe(actualHashedId); expect(telemetry["isBufferingEvents"]).toBe(false); }); }); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 0a226868..dd49ef2a 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -55,7 +55,7 @@ describe("Telemetry", () => { } // Helper function to verify mock calls to reduce duplication - async function verifyMockCalls({ + function verifyMockCalls({ sendEventsCalls = 0, clearEventsCalls = 0, appendEventsCalls = 0, @@ -77,13 +77,11 @@ describe("Telemetry", () => { expect(appendEvents.length).toBe(appendEventsCalls); if (sendEventsCalledWith) { - const commonProperties = await telemetry.getCommonProperties(); - expect(sendEvents[0]?.[0]).toEqual( sendEventsCalledWith.map((event) => ({ ...event, properties: { - ...commonProperties, + ...telemetry.getCommonProperties(), ...event.properties, }, })) @@ -144,7 +142,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - await verifyMockCalls({ + verifyMockCalls({ sendEventsCalls: 1, clearEventsCalls: 1, sendEventsCalledWith: [testEvent], @@ -158,7 +156,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - await verifyMockCalls({ + verifyMockCalls({ sendEventsCalls: 1, appendEventsCalls: 1, appendEventsCalledWith: [testEvent], @@ -181,7 +179,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([newEvent]); - await verifyMockCalls({ + verifyMockCalls({ sendEventsCalls: 1, clearEventsCalls: 1, sendEventsCalledWith: [cachedEvent, newEvent], @@ -189,8 +187,6 @@ describe("Telemetry", () => { }); it("should correctly add common properties to events", () => { - const commonProps = await telemetry.getCommonProperties(); - // Use explicit type assertion const expectedProps: Record = { mcp_client_version: "1.0.0", @@ -201,7 +197,7 @@ describe("Telemetry", () => { device_id: hashedMachineId, }; - expect(commonProps).toMatchObject(expectedProps); + expect(telemetry.getCommonProperties()).toMatchObject(expectedProps); }); describe("machine ID resolution", () => { @@ -302,7 +298,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - await verifyMockCalls(); + verifyMockCalls(); }); }); @@ -327,7 +323,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - await verifyMockCalls(); + verifyMockCalls(); }); }); }); From bf204bb0737e67ea539802f94883499bd24b48ce Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 17:23:58 +0100 Subject: [PATCH 3/8] fix: tests --- src/index.ts | 2 +- src/logger.ts | 1 + src/telemetry/telemetry.ts | 110 +++++++++++++++++----------- tests/integration/helpers.ts | 2 +- tests/integration/telemetry.test.ts | 7 +- tests/unit/telemetry.test.ts | 4 + 6 files changed, 76 insertions(+), 50 deletions(-) diff --git a/src/index.ts b/src/index.ts index a46aa34c..8f5738cf 100644 --- a/src/index.ts +++ b/src/index.ts @@ -20,7 +20,7 @@ try { version: packageInfo.version, }); - const telemetry = Telemetry.create({ session, userConfig: config }); + const telemetry = Telemetry.create(session, config); const server = new Server({ mcpServer, diff --git a/src/logger.ts b/src/logger.ts index 8157324b..7adf1263 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -25,6 +25,7 @@ export const LogId = { telemetryMetadataError: mongoLogId(1_002_005), telemetryDeviceIdFailure: mongoLogId(1_002_006), telemetryDeviceIdTimeout: mongoLogId(1_002_007), + telemetryContainerEnvFailure: mongoLogId(1_002_008), toolExecute: mongoLogId(1_003_001), toolExecuteFailure: mongoLogId(1_003_002), diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 578420b9..1a43e76e 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -8,6 +8,7 @@ import { EventCache } from "./eventCache.js"; import nodeMachineId from "node-machine-id"; import { getDeviceId } from "@mongodb-js/device-id"; import fs from "fs/promises"; +import { get } from "http"; type EventResult = { success: boolean; @@ -22,30 +23,74 @@ export type TelemetryOptions = { commonProperties?: CommonProperties; eventCache?: EventCache; getRawMachineId?: () => Promise; + getContainerEnv?: () => Promise; }; +async function fileExists(filePath: string): Promise { + try { + await fs.stat(filePath); + return true; // File exists + } catch (e: unknown) { + if ( + e instanceof Error && + ( + e as Error & { + code: string; + } + ).code === "ENOENT" + ) { + return false; // File does not exist + } + throw e; // Re-throw unexpected errors + } +} + +async function isContainerized(): Promise { + for (const file of ["/.dockerenv", "/run/.containerenv", "/var/run/.containerenv"]) { + const exists = await fileExists(file); + if (exists) { + return true; + } + } + return !!process.env.container; +} + export class Telemetry { private isBufferingEvents: boolean = true; /** Resolves when the device ID is retrieved or timeout occurs */ public deviceIdPromise: Promise | undefined; private deviceIdAbortController = new AbortController(); + private eventCache: EventCache; + private getRawMachineId: () => Promise; + private getContainerEnv: () => Promise; private constructor( private readonly session: Session, private readonly userConfig: UserConfig, private readonly commonProperties: CommonProperties, - private readonly eventCache: EventCache, - private readonly getRawMachineId: () => Promise - ) {} - - static create({ session, userConfig, commonProperties, eventCache, getRawMachineId }: TelemetryOptions): Telemetry { - const instance = new Telemetry( - session, - userConfig, - commonProperties || { ...MACHINE_METADATA }, - eventCache || EventCache.getInstance(), - getRawMachineId || (() => nodeMachineId.machineId(true)) - ); + { eventCache, getRawMachineId, getContainerEnv }: { eventCache: EventCache; getRawMachineId: () => Promise; getContainerEnv: () => Promise } + ) { + this.eventCache = eventCache; + this.getRawMachineId = getRawMachineId; + this.getContainerEnv = getContainerEnv; + } + + static create( + session: Session, + userConfig: UserConfig, + { + commonProperties = { ...MACHINE_METADATA }, + eventCache = EventCache.getInstance(), + getRawMachineId = () => nodeMachineId.machineId(true), + getContainerEnv = isContainerized, + }: { + commonProperties?: CommonProperties; + eventCache?: EventCache; + getRawMachineId?: () => Promise; + getContainerEnv?: () => Promise; + } = {} + ): Telemetry { + const instance = new Telemetry(session, userConfig, commonProperties, { eventCache, getRawMachineId, getContainerEnv }); void instance.start(); return instance; @@ -73,9 +118,17 @@ export class Telemetry { abortSignal: this.deviceIdAbortController.signal, }); + // try { + // this.commonProperties.is_container_env = (await this.getContainerEnv()) ? "true" : "false"; + // } catch (error: unknown) { + // logger.info( + // LogId.telemetryContainerEnvFailure, + // "telemetry", + // `Failed trying to check if is in container environment ${error as string}` + // ); + // } this.commonProperties.device_id = await this.deviceIdPromise; - this.commonProperties.is_container_env = (await this.isContainerized()) ? "true" : "false"; - + this.isBufferingEvents = false; } @@ -117,35 +170,6 @@ export class Telemetry { }; } - private async fileExists(filePath: string): Promise { - try { - await fs.stat(filePath); - return true; // File exists - } catch (e: unknown) { - if ( - e instanceof Error && - ( - e as Error & { - code: string; - } - ).code === "ENOENT" - ) { - return false; // File does not exist - } - throw e; // Re-throw unexpected errors - } - } - - private async isContainerized(): Promise { - for (const file of ["/.dockerenv", "/run/.containerenv", "/var/run/.containerenv"]) { - const fileExists = await this.fileExists(file); - if (fileExists) { - return true; - } - } - return !!process.env.container; - } - /** * Checks if telemetry is currently enabled * This is a method rather than a constant to capture runtime config changes diff --git a/tests/integration/helpers.ts b/tests/integration/helpers.ts index 9283f4ac..e407e250 100644 --- a/tests/integration/helpers.ts +++ b/tests/integration/helpers.ts @@ -67,7 +67,7 @@ export function setupIntegrationTest(getUserConfig: () => UserConfig): Integrati userConfig.telemetry = "disabled"; - const telemetry = Telemetry.create({ session, userConfig }); + const telemetry = Telemetry.create(session, userConfig); mcpServer = new Server({ session, diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 0330fb2b..808babf6 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -10,11 +10,8 @@ describe("Telemetry", () => { const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex"); - const telemetry = Telemetry.create({ - session: new Session({ - apiBaseUrl: "", - }), - userConfig: config, + const telemetry = Telemetry.create(new Session({ apiBaseUrl: "" }), config, { + getContainerEnv: () => Promise.resolve(false), }); expect(telemetry.getCommonProperties().device_id).toBe(undefined); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index dd49ef2a..e378fed3 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -130,6 +130,7 @@ describe("Telemetry", () => { userConfig: config, eventCache: mockEventCache, getRawMachineId: () => Promise.resolve(machineId), + getContainerEnv: () => Promise.resolve(false), }); config.telemetry = "enabled"; @@ -216,6 +217,7 @@ describe("Telemetry", () => { session, userConfig: config, getRawMachineId: () => Promise.resolve(machineId), + getContainerEnv: () => Promise.resolve(false), }); expect(telemetry["isBufferingEvents"]).toBe(true); @@ -234,6 +236,7 @@ describe("Telemetry", () => { session, userConfig: config, getRawMachineId: () => Promise.reject(new Error("Failed to get device ID")), + getContainerEnv: () => Promise.resolve(false), }); expect(telemetry["isBufferingEvents"]).toBe(true); @@ -258,6 +261,7 @@ describe("Telemetry", () => { session, userConfig: config, getRawMachineId: () => new Promise(() => {}), + getContainerEnv: () => Promise.resolve(false), }); expect(telemetry["isBufferingEvents"]).toBe(true); From 3d91b40a26ad5cfa27af4c4a22a449c04509e747 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 18:46:41 +0100 Subject: [PATCH 4/8] fix: tests --- src/telemetry/telemetry.ts | 63 +++++++++------ tests/integration/telemetry.test.ts | 13 ++- tests/unit/telemetry.test.ts | 121 +++++++++++++--------------- 3 files changed, 99 insertions(+), 98 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 1a43e76e..85fa6e5a 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -8,15 +8,12 @@ import { EventCache } from "./eventCache.js"; import nodeMachineId from "node-machine-id"; import { getDeviceId } from "@mongodb-js/device-id"; import fs from "fs/promises"; -import { get } from "http"; type EventResult = { success: boolean; error?: Error; }; -export const DEVICE_ID_TIMEOUT = 3000; - export type TelemetryOptions = { session: Session; userConfig: UserConfig; @@ -56,9 +53,10 @@ async function isContainerized(): Promise { } export class Telemetry { - private isBufferingEvents: boolean = true; /** Resolves when the device ID is retrieved or timeout occurs */ + private bufferingEvents: number = 2; public deviceIdPromise: Promise | undefined; + public containerEnvPromise: Promise | undefined; private deviceIdAbortController = new AbortController(); private eventCache: EventCache; private getRawMachineId: () => Promise; @@ -68,7 +66,15 @@ export class Telemetry { private readonly session: Session, private readonly userConfig: UserConfig, private readonly commonProperties: CommonProperties, - { eventCache, getRawMachineId, getContainerEnv }: { eventCache: EventCache; getRawMachineId: () => Promise; getContainerEnv: () => Promise } + { + eventCache, + getRawMachineId, + getContainerEnv, + }: { + eventCache: EventCache; + getRawMachineId: () => Promise; + getContainerEnv: () => Promise; + } ) { this.eventCache = eventCache; this.getRawMachineId = getRawMachineId; @@ -90,16 +96,21 @@ export class Telemetry { getContainerEnv?: () => Promise; } = {} ): Telemetry { - const instance = new Telemetry(session, userConfig, commonProperties, { eventCache, getRawMachineId, getContainerEnv }); + const instance = new Telemetry(session, userConfig, commonProperties, { + eventCache, + getRawMachineId, + getContainerEnv, + }); - void instance.start(); + instance.start(); return instance; } - private async start(): Promise { + private start(): void { if (!this.isTelemetryEnabled()) { return; } + this.deviceIdPromise = getDeviceId({ getMachineId: () => this.getRawMachineId(), onError: (reason, error) => { @@ -116,25 +127,16 @@ export class Telemetry { } }, abortSignal: this.deviceIdAbortController.signal, + }).finally(() => { + this.bufferingEvents--; + }); + this.containerEnvPromise = this.getContainerEnv().finally(() => { + this.bufferingEvents--; }); - - // try { - // this.commonProperties.is_container_env = (await this.getContainerEnv()) ? "true" : "false"; - // } catch (error: unknown) { - // logger.info( - // LogId.telemetryContainerEnvFailure, - // "telemetry", - // `Failed trying to check if is in container environment ${error as string}` - // ); - // } - this.commonProperties.device_id = await this.deviceIdPromise; - - this.isBufferingEvents = false; } public async close(): Promise { this.deviceIdAbortController.abort(); - this.isBufferingEvents = false; await this.emitEvents(this.eventCache.getEvents()); } @@ -170,6 +172,14 @@ export class Telemetry { }; } + public async getAsyncCommonProperties(): Promise { + return { + ...this.getCommonProperties(), + is_container_env: (await this.containerEnvPromise) ? "true" : "false", + device_id: await this.deviceIdPromise, + }; + } + /** * Checks if telemetry is currently enabled * This is a method rather than a constant to capture runtime config changes @@ -187,12 +197,16 @@ export class Telemetry { return !doNotTrack; } + public isBufferingEvents(): boolean { + return this.bufferingEvents > 0; + } + /** * Attempts to emit events through authenticated and unauthenticated clients * Falls back to caching if both attempts fail */ private async emit(events: BaseEvent[]): Promise { - if (this.isBufferingEvents) { + if (this.isBufferingEvents()) { this.eventCache.appendEvents(events); return; } @@ -230,10 +244,11 @@ export class Telemetry { */ private async sendEvents(client: ApiClient, events: BaseEvent[]): Promise { try { + const commonProperties = await this.getAsyncCommonProperties(); await client.sendEvents( events.map((event) => ({ ...event, - properties: { ...this.getCommonProperties(), ...event.properties }, + properties: { ...commonProperties, ...event.properties }, })) ); return { success: true }; diff --git a/tests/integration/telemetry.test.ts b/tests/integration/telemetry.test.ts index 808babf6..9b283c91 100644 --- a/tests/integration/telemetry.test.ts +++ b/tests/integration/telemetry.test.ts @@ -7,19 +7,16 @@ import nodeMachineId from "node-machine-id"; describe("Telemetry", () => { it("should resolve the actual machine ID", async () => { const actualId: string = await nodeMachineId.machineId(true); - const actualHashedId = createHmac("sha256", actualId.toUpperCase()).update("atlascli").digest("hex"); const telemetry = Telemetry.create(new Session({ apiBaseUrl: "" }), config, { - getContainerEnv: () => Promise.resolve(false), + getContainerEnv: () => new Promise((resolve) => resolve(false)), }); - expect(telemetry.getCommonProperties().device_id).toBe(undefined); - expect(telemetry["isBufferingEvents"]).toBe(true); - - await telemetry.deviceIdPromise; + expect(telemetry.isBufferingEvents()).toBe(true); + const commonProps = await telemetry.getAsyncCommonProperties(); - expect(telemetry.getCommonProperties().device_id).toBe(actualHashedId); - expect(telemetry["isBufferingEvents"]).toBe(false); + expect(commonProps.device_id).toBe(actualHashedId); + expect(telemetry.isBufferingEvents()).toBe(false); }); }); diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index e378fed3..0035dae9 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -1,6 +1,6 @@ import { ApiClient } from "../../src/common/atlas/apiClient.js"; import { Session } from "../../src/session.js"; -import { DEVICE_ID_TIMEOUT, Telemetry } from "../../src/telemetry/telemetry.js"; +import { Telemetry } from "../../src/telemetry/telemetry.js"; import { BaseEvent, TelemetryResult } from "../../src/telemetry/types.js"; import { EventCache } from "../../src/telemetry/eventCache.js"; import { config } from "../../src/config.js"; @@ -55,7 +55,7 @@ describe("Telemetry", () => { } // Helper function to verify mock calls to reduce duplication - function verifyMockCalls({ + async function verifyMockCalls({ sendEventsCalls = 0, clearEventsCalls = 0, appendEventsCalls = 0, @@ -77,11 +77,13 @@ describe("Telemetry", () => { expect(appendEvents.length).toBe(appendEventsCalls); if (sendEventsCalledWith) { + const commonProps = await telemetry.getAsyncCommonProperties(); + expect(sendEvents[0]?.[0]).toEqual( sendEventsCalledWith.map((event) => ({ ...event, properties: { - ...telemetry.getCommonProperties(), + ...commonProps, ...event.properties, }, })) @@ -125,12 +127,9 @@ describe("Telemetry", () => { setAgentRunner: jest.fn().mockResolvedValue(undefined), } as unknown as Session; - telemetry = Telemetry.create({ - session, - userConfig: config, + telemetry = Telemetry.create(session, config, { eventCache: mockEventCache, getRawMachineId: () => Promise.resolve(machineId), - getContainerEnv: () => Promise.resolve(false), }); config.telemetry = "enabled"; @@ -143,7 +142,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls({ + await verifyMockCalls({ sendEventsCalls: 1, clearEventsCalls: 1, sendEventsCalledWith: [testEvent], @@ -157,7 +156,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls({ + await verifyMockCalls({ sendEventsCalls: 1, appendEventsCalls: 1, appendEventsCalledWith: [testEvent], @@ -187,7 +186,7 @@ describe("Telemetry", () => { }); }); - it("should correctly add common properties to events", () => { + it("should correctly add common properties to events", async () => { // Use explicit type assertion const expectedProps: Record = { mcp_client_version: "1.0.0", @@ -198,7 +197,9 @@ describe("Telemetry", () => { device_id: hashedMachineId, }; - expect(telemetry.getCommonProperties()).toMatchObject(expectedProps); + const commonProps = await telemetry.getAsyncCommonProperties(); + + expect(commonProps).toMatchObject(expectedProps); }); describe("machine ID resolution", () => { @@ -213,39 +214,27 @@ describe("Telemetry", () => { }); it("should successfully resolve the machine ID", async () => { - telemetry = Telemetry.create({ - session, - userConfig: config, + telemetry = Telemetry.create(session, config, { getRawMachineId: () => Promise.resolve(machineId), - getContainerEnv: () => Promise.resolve(false), }); - expect(telemetry["isBufferingEvents"]).toBe(true); - expect(telemetry.getCommonProperties().device_id).toBe(undefined); - - await telemetry.deviceIdPromise; - - expect(telemetry["isBufferingEvents"]).toBe(false); - expect(telemetry.getCommonProperties().device_id).toBe(hashedMachineId); + expect(telemetry.isBufferingEvents()).toBe(true); + const commonProps = await telemetry.getAsyncCommonProperties(); + expect(telemetry.isBufferingEvents()).toBe(false); + expect(commonProps.device_id).toBe(hashedMachineId); }); it("should handle machine ID resolution failure", async () => { const loggerSpy = jest.spyOn(logger, "debug"); - telemetry = Telemetry.create({ - session, - userConfig: config, + telemetry = Telemetry.create(session, config, { getRawMachineId: () => Promise.reject(new Error("Failed to get device ID")), - getContainerEnv: () => Promise.resolve(false), }); - expect(telemetry["isBufferingEvents"]).toBe(true); - expect(telemetry.getCommonProperties().device_id).toBe(undefined); - - await telemetry.deviceIdPromise; - - expect(telemetry["isBufferingEvents"]).toBe(false); - expect(telemetry.getCommonProperties().device_id).toBe("unknown"); + expect(telemetry.isBufferingEvents()).toBe(true); + const commonProps = await telemetry.getAsyncCommonProperties(); + expect(telemetry.isBufferingEvents()).toBe(false); + expect(commonProps.device_id).toBe("unknown"); expect(loggerSpy).toHaveBeenCalledWith( LogId.telemetryDeviceIdFailure, @@ -254,37 +243,37 @@ describe("Telemetry", () => { ); }); - it("should timeout if machine ID resolution takes too long", async () => { - const loggerSpy = jest.spyOn(logger, "debug"); - - telemetry = Telemetry.create({ - session, - userConfig: config, - getRawMachineId: () => new Promise(() => {}), - getContainerEnv: () => Promise.resolve(false), - }); - - expect(telemetry["isBufferingEvents"]).toBe(true); - expect(telemetry.getCommonProperties().device_id).toBe(undefined); - - jest.advanceTimersByTime(DEVICE_ID_TIMEOUT / 2); - - // Make sure the timeout doesn't happen prematurely. - expect(telemetry["isBufferingEvents"]).toBe(true); - expect(telemetry.getCommonProperties().device_id).toBe(undefined); - - jest.advanceTimersByTime(DEVICE_ID_TIMEOUT); - - await telemetry.deviceIdPromise; - - expect(telemetry.getCommonProperties().device_id).toBe("unknown"); - expect(telemetry["isBufferingEvents"]).toBe(false); - expect(loggerSpy).toHaveBeenCalledWith( - LogId.telemetryDeviceIdTimeout, - "telemetry", - "Device ID retrieval timed out" - ); - }); + // it("should timeout if machine ID resolution takes too long", async () => { + // const DEVICE_ID_TIMEOUT = 3000; + // const loggerSpy = jest.spyOn(logger, "debug"); + + // telemetry = Telemetry.create(session, config, { + // getRawMachineId: () => new Promise(() => {}), + // getContainerEnv: () => Promise.resolve(false), + // }); + // console.log("DEBUG 1"); + // expect(telemetry.isBufferingEvents()).toBe(true); + // const commonProps = await telemetry.getAsyncCommonProperties(); + // console.log("DEBUG 2", commonProps); + // expect(telemetry.isBufferingEvents()).toBe(true); + // expect(commonProps.device_id).toBe(undefined); + // console.log("DEBUG 3"); + // jest.advanceTimersByTime(DEVICE_ID_TIMEOUT / 2); + // console.log("DEBUG 3", commonProps); + // // Make sure the timeout doesn't happen prematurely. + // expect(telemetry.isBufferingEvents()).toBe(true); + // expect(commonProps.device_id).toBe(undefined); + + // jest.advanceTimersByTime(DEVICE_ID_TIMEOUT); + // console.log("DEBUG 4", commonProps); + // expect(commonProps.device_id).toBe("unknown"); + // expect(telemetry.isBufferingEvents()).toBe(false); + // expect(loggerSpy).toHaveBeenCalledWith( + // LogId.telemetryDeviceIdTimeout, + // "telemetry", + // "Device ID retrieval timed out" + // ); + // }); }); }); @@ -302,7 +291,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls(); + await verifyMockCalls(); }); }); @@ -327,7 +316,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([testEvent]); - verifyMockCalls(); + await verifyMockCalls(); }); }); }); From fe646494879a2bd82e4343786e253e7e9c18663d Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 18:56:37 +0100 Subject: [PATCH 5/8] fix: tests --- tests/unit/telemetry.test.ts | 52 +++++++++++++++--------------------- 1 file changed, 21 insertions(+), 31 deletions(-) diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 0035dae9..47d87b9a 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -130,6 +130,7 @@ describe("Telemetry", () => { telemetry = Telemetry.create(session, config, { eventCache: mockEventCache, getRawMachineId: () => Promise.resolve(machineId), + getContainerEnv: () => Promise.resolve(false), }); config.telemetry = "enabled"; @@ -216,6 +217,7 @@ describe("Telemetry", () => { it("should successfully resolve the machine ID", async () => { telemetry = Telemetry.create(session, config, { getRawMachineId: () => Promise.resolve(machineId), + getContainerEnv: () => Promise.resolve(false), }); expect(telemetry.isBufferingEvents()).toBe(true); @@ -229,6 +231,7 @@ describe("Telemetry", () => { telemetry = Telemetry.create(session, config, { getRawMachineId: () => Promise.reject(new Error("Failed to get device ID")), + getContainerEnv: () => Promise.resolve(false), }); expect(telemetry.isBufferingEvents()).toBe(true); @@ -243,37 +246,24 @@ describe("Telemetry", () => { ); }); - // it("should timeout if machine ID resolution takes too long", async () => { - // const DEVICE_ID_TIMEOUT = 3000; - // const loggerSpy = jest.spyOn(logger, "debug"); - - // telemetry = Telemetry.create(session, config, { - // getRawMachineId: () => new Promise(() => {}), - // getContainerEnv: () => Promise.resolve(false), - // }); - // console.log("DEBUG 1"); - // expect(telemetry.isBufferingEvents()).toBe(true); - // const commonProps = await telemetry.getAsyncCommonProperties(); - // console.log("DEBUG 2", commonProps); - // expect(telemetry.isBufferingEvents()).toBe(true); - // expect(commonProps.device_id).toBe(undefined); - // console.log("DEBUG 3"); - // jest.advanceTimersByTime(DEVICE_ID_TIMEOUT / 2); - // console.log("DEBUG 3", commonProps); - // // Make sure the timeout doesn't happen prematurely. - // expect(telemetry.isBufferingEvents()).toBe(true); - // expect(commonProps.device_id).toBe(undefined); - - // jest.advanceTimersByTime(DEVICE_ID_TIMEOUT); - // console.log("DEBUG 4", commonProps); - // expect(commonProps.device_id).toBe("unknown"); - // expect(telemetry.isBufferingEvents()).toBe(false); - // expect(loggerSpy).toHaveBeenCalledWith( - // LogId.telemetryDeviceIdTimeout, - // "telemetry", - // "Device ID retrieval timed out" - // ); - // }); + it("should timeout if machine ID resolution takes too long", async () => { + const DEVICE_ID_TIMEOUT = 3000; + const loggerSpy = jest.spyOn(logger, "debug"); + + telemetry = Telemetry.create(session, config, { + getRawMachineId: () => new Promise(() => {}), + getContainerEnv: () => Promise.resolve(false), + }); + expect(telemetry.isBufferingEvents()).toBe(true); + jest.advanceTimersByTime(DEVICE_ID_TIMEOUT); + const commonProps = await telemetry.getAsyncCommonProperties(); + expect(commonProps.device_id).toBe("unknown"); + expect(loggerSpy).toHaveBeenCalledWith( + LogId.telemetryDeviceIdTimeout, + "telemetry", + "Device ID retrieval timed out" + ); + }); }); }); From f73c2087c26a1de8b2611fa285011252092e0d77 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 18:57:33 +0100 Subject: [PATCH 6/8] fix: cleanup --- src/logger.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/logger.ts b/src/logger.ts index 7adf1263..8157324b 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -25,7 +25,6 @@ export const LogId = { telemetryMetadataError: mongoLogId(1_002_005), telemetryDeviceIdFailure: mongoLogId(1_002_006), telemetryDeviceIdTimeout: mongoLogId(1_002_007), - telemetryContainerEnvFailure: mongoLogId(1_002_008), toolExecute: mongoLogId(1_003_001), toolExecuteFailure: mongoLogId(1_003_002), From 671bbeb7717e6173cc5f12d46d37802e0765381c Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 19:01:01 +0100 Subject: [PATCH 7/8] fix: check --- tests/unit/telemetry.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/telemetry.test.ts b/tests/unit/telemetry.test.ts index 47d87b9a..252973d1 100644 --- a/tests/unit/telemetry.test.ts +++ b/tests/unit/telemetry.test.ts @@ -180,7 +180,7 @@ describe("Telemetry", () => { await telemetry.emitEvents([newEvent]); - verifyMockCalls({ + await verifyMockCalls({ sendEventsCalls: 1, clearEventsCalls: 1, sendEventsCalledWith: [cachedEvent, newEvent], From c2a1246cf3d2ebe1f0163868f25673c1d04f92c5 Mon Sep 17 00:00:00 2001 From: Filipe Constantinov Menezes Date: Fri, 13 Jun 2025 19:03:44 +0100 Subject: [PATCH 8/8] fix: cleanup --- src/telemetry/telemetry.ts | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/telemetry/telemetry.ts b/src/telemetry/telemetry.ts index 85fa6e5a..e9aa3ba2 100644 --- a/src/telemetry/telemetry.ts +++ b/src/telemetry/telemetry.ts @@ -14,15 +14,6 @@ type EventResult = { error?: Error; }; -export type TelemetryOptions = { - session: Session; - userConfig: UserConfig; - commonProperties?: CommonProperties; - eventCache?: EventCache; - getRawMachineId?: () => Promise; - getContainerEnv?: () => Promise; -}; - async function fileExists(filePath: string): Promise { try { await fs.stat(filePath);