chore: add telemetry to setup script MCP-460 by nirinchev · Pull Request #1085 · mongodb-js/mongodb-mcp-server

nirinchev · 2026-04-21T11:42:08Z

Proposed changes

Adds setup telemetry. Additionally, modifies the telemetry service constructor to decouple it from the session and userConfig objects. Backwards compatibility is preserved by adding an overload for Telemetry.create.

Telemetry events for the setup script are emitted at every step indicating success/failure while ensuring we don't leak any PII.

Made-with: Cursor # Conflicts: # src/telemetry/telemetry.ts # src/web.ts

Copilot

Pull request overview

This PR introduces telemetry for the interactive setup wizard and refactors the telemetry pipeline to be configurable without requiring a full Session, improving reuse across setup/runtime and updating tests and public API reports accordingly.

Changes:

Add SetupTelemetry to emit structured, step-based setup events (with stable setup_session_id and accumulated context).
Refactor Telemetry.create to support a new TelemetryConfig object (keeping the legacy overload deprecated) and update call sites/tests.
Improve secret redaction in setup error formatting by redacting against globally-registered secrets.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/unit/telemetry.test.ts	Updates unit tests to use `TelemetryConfig`-based construction and host-supplied common properties.
tests/unit/setup/setupTelemetry.test.ts	Adds unit coverage for the new `SetupTelemetry` event semantics and flush behavior.
tests/unit/resources/common/debug.test.ts	Updates test wiring to new `Telemetry.create({ ... })` signature.
tests/integration/ui/mcpUIFeature.test.ts	Updates telemetry wiring for integration test server/session setup.
tests/integration/tools/mongodb/mongodbTool.test.ts	Updates telemetry wiring for tool integration tests.
tests/integration/telemetry.test.ts	Simplifies integration test by constructing telemetry directly from config.
tests/integration/server.test.ts	Updates server integration test telemetry wiring to new constructor.
tests/integration/helpers.ts	Updates shared integration test harness to construct telemetry via `TelemetryConfig`.
src/web.ts	Updates public exports to include `TelemetryConfig` as a type export.
src/transports/base.ts	Switches transport runner telemetry initialization to `TelemetryConfig` with dynamic `getCommonProperties()`.
src/telemetry/types.ts	Adds setup telemetry types (`SetupCommand`, `SetupEventProperties`, `SetupEvent`).
src/telemetry/telemetry.ts	Introduces `TelemetryConfig`, adds new `create(config)` overload, and deprecates legacy overload.
src/setup/setupTelemetry.ts	Adds the `SetupTelemetry` implementation and helpers (context accumulation, step durations, terminal events).
src/setup/setupMcpServer.ts	Wires setup wizard steps to `SetupTelemetry` and adds redaction registration for setup secrets.
src/setup/setupAiToolsUtils.ts	Redacts formatted error messages using globally-registered secrets.
src/lib.ts	Exports `TelemetryConfig` type from the package entrypoint.
src/index.ts	Simplifies setup execution path to call `runSetup(config)` directly.
src/common/session.ts	Makes `Session.logger` readonly (reflected in API reports).
api-extractor/reports/web.public.api.md	Updates public API report for telemetry overloads and new `TelemetryConfig`.
api-extractor/reports/mongodb-mcp-server.public.api.md	Updates public API report for telemetry overloads and new `TelemetryConfig`.

Comments suppressed due to low confidence (1)

src/telemetry/telemetry.ts:158

In the Telemetry.create(...) implementation, the destructured parameter default eventCache = EventCache.getInstance() is evaluated even when callers use the new TelemetryConfig overload (because the parameter is always destructured). This adds an unnecessary side effect/overhead and makes EventCache.getInstance harder to reason about in tests. Consider only resolving the legacy defaults when the legacy overload is actually used (e.g., branch on userConfig/deviceId before applying defaults).

        {
            commonProperties = {},
            eventCache = EventCache.getInstance(),
        }: {
            commonProperties?: Partial<CommonProperties>;

coveralls · 2026-04-23T10:37:35Z

Coverage Report for CI Build 24830273274

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage decreased (-0.03%) to 81.594%

Details

Coverage decreased (-0.03%) from the base build.
Patch coverage: 16 uncovered changes across 2 files (59 of 75 lines covered, 78.67%).
2 coverage regressions across 1 file.

Uncovered Changes

File	Changed	Covered	%
src/setup/setupTelemetry.ts	49	35	71.43%
src/telemetry/telemetry.ts	24	22	91.67%

Coverage Regressions

2 previously-covered lines in 1 file lost coverage.

File	Lines Losing Coverage	Coverage
src/tools/mongodb/mongodbTool.ts	2	88.68%

Coverage Stats


Relevant Lines:	3631
Covered Lines:	3153
Line Coverage:	86.84%
Relevant Branches:	2280
Covered Branches:	1670
Branch Coverage:	73.25%
Branches in Coverage %:	Yes
Coverage Strength:	170.83 hits per line

💛 - Coveralls

gagik · 2026-04-23T11:46:25Z

+    browser: z.ZodOptional<z.ZodUnion<readonly [z.ZodLiteral<false>, z.ZodString]>>;
+}, z.core.$strip>;
+
+// Warnings were encountered during analysis:


should this file be added to prettier ignore? is this why this is reformatted?

No idea what caused this reformat 😬 It looks like it's already ignored.

Actually, as far as I can tell, it was formatted according to the prettier rules. The current version is regenerated and not prettier-formatted, which seems to be causing the difference.

blva · 2026-04-24T19:08:04Z

+            command: "completed",
+            ai_tool: "claudeDesktop",
+            is_read_only: "false",
+            has_docker: "false",


we should use the same structure where possible so this should be is_container_env

blva · 2026-04-24T19:08:25Z

+        expect(completed.properties).toMatchObject({
+            command: "completed",
+            ai_tool: "claudeDesktop",
+            is_read_only: "false",


this should be read_only_mode

blva · 2026-04-24T19:09:09Z

+
+        const completed = mock.emitted.at(-1)!;
+        expect(completed.properties).toMatchObject({
+            command: "completed",


nit: we don't need to use command to define this sort of events - we should design the events to fit the scenario you are tracking here - I think this warrants a discussion w/ Analytics about the expected shape for the events

a better design would be to create a command setup and then you can define the states as a stage or status property

blva · 2026-04-24T19:10:03Z

+    has_docker?: TelemetryBoolSet;
+
+    /** Whether the current OS/architecture is supported by the MCP server. */
+    platform_supported?: TelemetryBoolSet;


why not send the platform value as well? platform is the value, and we have that in telemetry too

blva · 2026-04-24T19:10:20Z

+    node_version_ok?: TelemetryBoolSet;
+
+    /** Whether the user supplied a MongoDB connection string. */
+    connection_string_provided?: TelemetryBoolSet;


Suggested change

connection_string_provided?: TelemetryBoolSet;

config_connection_string?: TelemetryBoolSet;

blva · 2026-04-24T19:13:42Z

+ * accumulated context known up to that point so each event is independently
+ * queryable.
+ */
+export type SetupEventProperties = {


I left a few in-line comments, but overall I suggest that we reuse the properties where possible, I don't think it makes sense and add new ones where needed, right?

you can probably reuse the commonBaseProperties and extract the common properties that you want to capture in the events here

blva · 2026-04-24T19:19:07Z

+            deviceId: this.deviceId,
+            apiClient: session.apiClient,
+            keychain: session.keychain,
+            enabled: userConfig.telemetry !== "disabled",


nit: I'd instead validate it is equal to enabled

nirinchev added 4 commits April 17, 2026 16:51

wip

5057318

unreviewed wip

a3ccfbf

Merge remote-tracking branch 'origin/main' into ni/setup-telemetry

3fbff94

Made-with: Cursor # Conflicts: # src/telemetry/telemetry.ts # src/web.ts

clean up

f23fce7

github-actions Bot added the type: chore label Apr 21, 2026

nirinchev added 4 commits April 21, 2026 13:47

regenerate api contracts

ecb656e

clean up tests

cc95e2b

fix tests

a3b83f3

Merge branch 'main' into ni/setup-telemetry

7a865bd

nirinchev requested a review from gagik April 23, 2026 10:15

nirinchev marked this pull request as ready for review April 23, 2026 10:15

nirinchev requested a review from a team as a code owner April 23, 2026 10:15

Copilot AI review requested due to automatic review settings April 23, 2026 10:15

Copilot started reviewing on behalf of nirinchev April 23, 2026 10:15 View session

Copilot AI reviewed Apr 23, 2026

View reviewed changes

Comment thread src/setup/setupMcpServer.ts

Comment thread src/setup/setupMcpServer.ts Outdated

Comment thread src/setup/setupMcpServer.ts Outdated

address copilot review

8494902

nirinchev changed the title ~~chore: add telemetry to setup script~~ chore: add telemetry to setup script MCP-460 Apr 23, 2026

nirinchev requested a review from blva April 23, 2026 11:27

gagik reviewed Apr 23, 2026

View reviewed changes

jeroenvervaeke approved these changes Apr 23, 2026

View reviewed changes

blva reviewed Apr 24, 2026

View reviewed changes

blva requested changes Apr 24, 2026

View reviewed changes

blva reviewed Apr 24, 2026

View reviewed changes

	connection_string_provided?: TelemetryBoolSet;
	config_connection_string?: TelemetryBoolSet;

Conversation

nirinchev commented Apr 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coveralls commented Apr 23, 2026

Coverage Report for CI Build 24830273274

Coverage decreased (-0.03%) to 81.594%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nirinchev Apr 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

nirinchev commented Apr 21, 2026 •

edited

Loading

nirinchev Apr 23, 2026 •

edited

Loading