Skip to content

Commit 70f5f27

Browse files
committed
Add fallback dev insights string
1 parent abbbb96 commit 70f5f27

6 files changed

Lines changed: 208 additions & 38 deletions

File tree

packages/b2c-dx-mcp/README.md

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,11 @@ When `--config` is not provided, the MCP server searches upward from `~/` for a
523523
524524
## Telemetry
525525

526-
The MCP server collects anonymous usage telemetry to help improve the developer experience. Telemetry is enabled by default and can be disabled by setting the `SFCC_TELEMETRY` environment variable to `false`.
526+
The MCP server collects anonymous usage telemetry to help improve the developer experience.
527+
528+
**Development mode**: Telemetry is automatically disabled when `NODE_ENV=development` (set by `bin/dev.js`), so local development and testing won't pollute production data.
529+
530+
**Production**: Telemetry is enabled by default for published releases. To disable, set `SFCC_TELEMETRY=false`.
527531

528532
### What We Collect
529533

@@ -538,22 +542,6 @@ The MCP server collects anonymous usage telemetry to help improve the developer
538542
- **No tool arguments**: No input parameters or output results from tool calls
539543
- **No file contents**: No source code, configuration files, or project data
540544

541-
### Disabling Telemetry
542-
543-
Set the `SFCC_TELEMETRY` environment variable to `false`:
544-
545-
```json
546-
{
547-
"mcpServers": {
548-
"b2c-dx": {
549-
"command": "/path/to/packages/b2c-dx-mcp/bin/dev.js",
550-
"args": ["--toolsets", "all"],
551-
"env": { "SFCC_TELEMETRY": "false" }
552-
}
553-
}
554-
}
555-
```
556-
557545
## License
558546

559547
Apache-2.0

packages/b2c-dx-mcp/bin/dev.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
* For full license text, see the license.txt file in the repo root or http://www.apache.org/licenses/LICENSE-2.0
66
*/
77

8+
// Set NODE_ENV to development if not already set (disables production telemetry)
9+
process.env.NODE_ENV = process.env.NODE_ENV || 'development';
10+
811
// Load .env file if present (Node.js native support)
912
try {
1013
process.loadEnvFile();

packages/b2c-dx-mcp/src/config.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,28 @@
1212
const DEFAULT_APPLICATION_INSIGHTS_CONNECTION_STRING =
1313
'InstrumentationKey=6fcc215f-0b11-4864-ad5c-3945ae19e2f3;IngestionEndpoint=https://eastus-8.in.applicationinsights.azure.com/;LiveEndpoint=https://eastus.livediagnostics.monitor.azure.com/;ApplicationId=a60f17ec-265a-4dfc-b8df-03a64695697d';
1414

15+
/**
16+
* Determines if telemetry should be enabled based on environment.
17+
* Telemetry is disabled in development mode to avoid polluting production data.
18+
*/
19+
function getConnectionString(): string | undefined {
20+
// Allow explicit override via env var
21+
if (process.env.B2C_DX_MCP_APP_INSIGHTS_CONNECTION_STRING) {
22+
return process.env.B2C_DX_MCP_APP_INSIGHTS_CONNECTION_STRING;
23+
}
24+
25+
// Disable telemetry in development mode
26+
if (process.env.NODE_ENV === 'development') {
27+
return undefined;
28+
}
29+
30+
return DEFAULT_APPLICATION_INSIGHTS_CONNECTION_STRING;
31+
}
32+
1533
/**
1634
* Application Insights connection string for telemetry.
1735
*
18-
* Can be overridden via the `B2C_DX_MCP_APP_INSIGHTS_CONNECTION_STRING` environment variable for testing.
36+
* Returns undefined in development mode (NODE_ENV=development) to disable telemetry.
37+
* Can be overridden via the `B2C_DX_MCP_APP_INSIGHTS_CONNECTION_STRING` environment variable.
1938
*/
20-
export const APPLICATION_INSIGHTS_CONNECTION_STRING =
21-
process.env.B2C_DX_MCP_APP_INSIGHTS_CONNECTION_STRING ?? DEFAULT_APPLICATION_INSIGHTS_CONNECTION_STRING;
39+
export const APPLICATION_INSIGHTS_CONNECTION_STRING = getConnectionString();

packages/b2c-dx-mcp/test/config.test.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,33 @@ import {APPLICATION_INSIGHTS_CONNECTION_STRING} from '../src/config.js';
99

1010
describe('config', () => {
1111
describe('APPLICATION_INSIGHTS_CONNECTION_STRING', () => {
12-
it('should export a non-empty connection string', () => {
13-
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.be.a('string');
14-
expect(APPLICATION_INSIGHTS_CONNECTION_STRING.length).to.be.greaterThan(0);
15-
});
12+
// The connection string behavior depends on NODE_ENV:
13+
// - In development mode (NODE_ENV=development): undefined (telemetry disabled)
14+
// - In production mode: returns the connection string
1615

17-
it('should contain InstrumentationKey', () => {
18-
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.include('InstrumentationKey=');
16+
it('should be undefined in development mode or a valid connection string in production', () => {
17+
if (process.env.NODE_ENV === 'development') {
18+
// In development mode, telemetry is disabled
19+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.be.undefined;
20+
} else {
21+
// In production mode, should have a valid connection string
22+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.be.a('string');
23+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING!.length).to.be.greaterThan(0);
24+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.include('InstrumentationKey=');
25+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.include('IngestionEndpoint=');
26+
}
1927
});
2028

21-
it('should contain IngestionEndpoint', () => {
22-
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.include('IngestionEndpoint=');
29+
it('should respect NODE_ENV for telemetry gating', () => {
30+
// This test documents the expected behavior:
31+
// Tests run without NODE_ENV=development set, so we get the production string
32+
// bin/dev.js sets NODE_ENV=development, so local dev gets undefined
33+
const isDevelopment = process.env.NODE_ENV === 'development';
34+
if (isDevelopment) {
35+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.be.undefined;
36+
} else {
37+
expect(APPLICATION_INSIGHTS_CONNECTION_STRING).to.not.be.undefined;
38+
}
2339
});
2440
});
2541
});

packages/b2c-dx-mcp/test/server.test.ts

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,23 @@ const paramHandler = async (args: Record<string, unknown>) => ({
4343
content: [{type: 'text' as const, text: `Hello ${args.name}`}],
4444
});
4545

46+
// Telemetry test handlers
47+
const successHandler = async () => ({content: [{type: 'text' as const, text: 'success'}]});
48+
const errorResultHandler = async () => ({
49+
content: [{type: 'text' as const, text: 'error occurred'}],
50+
isError: true,
51+
});
52+
const throwingHandler = async (): Promise<{content: Array<{type: 'text'; text: string}>}> => {
53+
throw new Error('Tool execution failed');
54+
};
55+
const delayMs = 50;
56+
const slowHandler = async () => {
57+
await new Promise<void>((resolve) => {
58+
setTimeout(resolve, delayMs);
59+
});
60+
return {content: [{type: 'text' as const, text: 'done'}]};
61+
};
62+
4663
describe('B2CDxMcpServer', () => {
4764
describe('constructor', () => {
4865
it('should create server with name and version', () => {
@@ -132,4 +149,121 @@ describe('B2CDxMcpServer', () => {
132149
expect(server).to.be.instanceOf(B2CDxMcpServer);
133150
});
134151
});
152+
153+
describe('TOOL_CALLED telemetry', () => {
154+
let mockTelemetry: MockTelemetry;
155+
let capturedHandler: ((args: Record<string, unknown>, extra: unknown) => Promise<unknown>) | null;
156+
157+
beforeEach(() => {
158+
mockTelemetry = new MockTelemetry();
159+
capturedHandler = null;
160+
});
161+
162+
/**
163+
* Creates a server that captures the wrapped handler for testing.
164+
* This allows us to test the telemetry wrapper logic without going through
165+
* the full MCP protocol.
166+
*/
167+
function createServerWithHandlerCapture(): B2CDxMcpServer {
168+
const server = new B2CDxMcpServer(
169+
{name: 'test-server', version: '1.0.0'},
170+
{telemetry: mockTelemetry as unknown as Telemetry},
171+
);
172+
173+
// Override registerTool to capture the wrapped handler
174+
const originalRegisterTool = server.registerTool.bind(server);
175+
server.registerTool = (name, config, handler) => {
176+
capturedHandler = handler as (args: Record<string, unknown>, extra: unknown) => Promise<unknown>;
177+
return originalRegisterTool(name, config, handler);
178+
};
179+
180+
return server;
181+
}
182+
183+
it('should send TOOL_CALLED event on successful tool execution', async () => {
184+
const server = createServerWithHandlerCapture();
185+
186+
server.addTool('test_tool', 'Test tool', {}, successHandler);
187+
188+
// Call the captured wrapped handler
189+
expect(capturedHandler).to.not.be.null;
190+
await capturedHandler!({}, {});
191+
192+
// Verify telemetry event
193+
expect(mockTelemetry.events).to.have.lengthOf(1);
194+
expect(mockTelemetry.events[0].name).to.equal('TOOL_CALLED');
195+
expect(mockTelemetry.events[0].attributes.toolName).to.equal('test_tool');
196+
expect(mockTelemetry.events[0].attributes.isError).to.equal(false);
197+
expect(mockTelemetry.events[0].attributes.runTimeMs).to.be.a('number');
198+
});
199+
200+
it('should send TOOL_CALLED event with isError=true when tool returns error', async () => {
201+
const server = createServerWithHandlerCapture();
202+
203+
server.addTool('error_tool', 'Error tool', {}, errorResultHandler);
204+
205+
await capturedHandler!({}, {});
206+
207+
expect(mockTelemetry.events).to.have.lengthOf(1);
208+
expect(mockTelemetry.events[0].name).to.equal('TOOL_CALLED');
209+
expect(mockTelemetry.events[0].attributes.toolName).to.equal('error_tool');
210+
expect(mockTelemetry.events[0].attributes.isError).to.equal(true);
211+
});
212+
213+
it('should send TOOL_CALLED event with isError=true when tool throws', async () => {
214+
const server = createServerWithHandlerCapture();
215+
216+
server.addTool('throwing_tool', 'Throwing tool', {}, throwingHandler);
217+
218+
try {
219+
await capturedHandler!({}, {});
220+
expect.fail('Should have thrown');
221+
} catch {
222+
// Expected
223+
}
224+
225+
expect(mockTelemetry.events).to.have.lengthOf(1);
226+
expect(mockTelemetry.events[0].name).to.equal('TOOL_CALLED');
227+
expect(mockTelemetry.events[0].attributes.toolName).to.equal('throwing_tool');
228+
expect(mockTelemetry.events[0].attributes.isError).to.equal(true);
229+
});
230+
231+
it('should measure runTimeMs accurately', async () => {
232+
const server = createServerWithHandlerCapture();
233+
234+
server.addTool('slow_tool', 'Slow tool', {}, slowHandler);
235+
236+
await capturedHandler!({}, {});
237+
238+
const runTimeMs = mockTelemetry.events[0].attributes.runTimeMs as number;
239+
// Allow some tolerance for timing
240+
expect(runTimeMs).to.be.at.least(delayMs - 10);
241+
expect(runTimeMs).to.be.at.most(delayMs + 50);
242+
});
243+
244+
it('should not send events when telemetry is disabled', async () => {
245+
// Create server without telemetry
246+
const server = new B2CDxMcpServer({name: 'test-server', version: '1.0.0'});
247+
248+
let handlerCalled = false;
249+
const handler = async () => {
250+
handlerCalled = true;
251+
return {content: [{type: 'text' as const, text: 'ok'}]};
252+
};
253+
254+
// Override registerTool to capture handler
255+
let noTelemetryHandler: ((args: Record<string, unknown>, extra: unknown) => Promise<unknown>) | null = null;
256+
const originalRegisterTool = server.registerTool.bind(server);
257+
server.registerTool = (name, config, h) => {
258+
noTelemetryHandler = h as (args: Record<string, unknown>, extra: unknown) => Promise<unknown>;
259+
return originalRegisterTool(name, config, h);
260+
};
261+
262+
server.addTool('no_telemetry_tool', 'No telemetry', {}, handler);
263+
264+
// Should not throw even without telemetry
265+
await noTelemetryHandler!({}, {});
266+
expect(handlerCalled).to.be.true;
267+
});
268+
});
135269
});

packages/b2c-dx-mcp/test/utils/index.test.ts

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,21 +9,32 @@ import {loadAppInsightsKey} from '../../src/utils/index.js';
99

1010
describe('utils', () => {
1111
describe('loadAppInsightsKey', () => {
12-
it('should return a connection string', () => {
13-
const key = loadAppInsightsKey();
14-
expect(key).to.be.a('string');
15-
});
12+
// In development mode (NODE_ENV=development), returns undefined to disable telemetry
13+
// In production mode, returns the connection string
1614

17-
it('should return a string containing InstrumentationKey', () => {
15+
it('should return undefined in development mode or a valid connection string in production', () => {
1816
const key = loadAppInsightsKey();
19-
expect(key).to.include('InstrumentationKey=');
17+
18+
if (process.env.NODE_ENV === 'development') {
19+
expect(key).to.be.undefined;
20+
} else {
21+
expect(key).to.be.a('string');
22+
expect(key).to.include('InstrumentationKey=');
23+
expect(key).to.include('IngestionEndpoint=');
24+
}
2025
});
2126

22-
it('should return a valid Application Insights connection string format', () => {
27+
it('should gate telemetry based on NODE_ENV', () => {
2328
const key = loadAppInsightsKey();
24-
// Connection string should have key=value pairs separated by semicolons
25-
expect(key).to.match(/InstrumentationKey=[^;]+/);
26-
expect(key).to.match(/IngestionEndpoint=[^;]+/);
29+
const isDevelopment = process.env.NODE_ENV === 'development';
30+
31+
if (isDevelopment) {
32+
// Development mode: telemetry disabled
33+
expect(key).to.be.undefined;
34+
} else {
35+
// Production mode: telemetry enabled
36+
expect(key).to.not.be.undefined;
37+
}
2738
});
2839
});
2940
});

0 commit comments

Comments
 (0)