Skip to content

Commit e3a4bc0

Browse files
committed
fix: Segment requires either userId OR anonymousId, but not both When userId is available, use it; otherwise use anonymousId
1 parent dd5291e commit e3a4bc0

File tree

3 files changed

+42
-14
lines changed

3 files changed

+42
-14
lines changed

src/mcp/server.ts

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
* Model Context Protocol (MCP) server for Apify Actors
33
*/
44

5-
import * as crypto from 'node:crypto';
6-
75
import type { Client } from '@modelcontextprotocol/sdk/client/index.js';
86
import { Server } from '@modelcontextprotocol/sdk/server/index.js';
97
import type { Transport } from '@modelcontextprotocol/sdk/shared/transport.js';
@@ -761,13 +759,13 @@ Please verify the tool name and ensure the tool is properly registered.`;
761759
* Calculates execution time, sets final status, and sends the telemetry event.
762760
*
763761
* @param telemetryData - Telemetry data to finalize and track (null if telemetry is disabled)
764-
* @param userId - Apify user ID
762+
* @param userId - Apify user ID (string or null if not available)
765763
* @param startTime - Timestamp when the tool call started
766764
* @param toolStatus - Final status of the tool call ('succeeded', 'failed', or 'aborted')
767765
*/
768766
private finalizeAndTrackTelemetry(
769767
telemetryData: ToolCallTelemetryProperties | null,
770-
userId: string,
768+
userId: string | null,
771769
startTime: number,
772770
toolStatus: 'succeeded' | 'failed' | 'aborted',
773771
): void {
@@ -789,9 +787,9 @@ Please verify the tool name and ensure the tool is properly registered.`;
789787
*/
790788
private async prepareTelemetryData(
791789
tool: HelperTool | ActorTool | ActorMcpTool, mcpSessionId: string | undefined, apifyToken: string,
792-
): Promise<{ telemetryData: ToolCallTelemetryProperties | null; userId: string }> {
790+
): Promise<{ telemetryData: ToolCallTelemetryProperties | null; userId: string | null }> {
793791
if (this.options.telemetry?.enabled !== true) {
794-
return { telemetryData: null, userId: crypto.randomUUID() };
792+
return { telemetryData: null, userId: null };
795793
}
796794

797795
const toolFullName = tool.type === 'actor' ? tool.actorFullName : tool.name;
@@ -813,11 +811,6 @@ Please verify the tool name and ensure the tool is properly registered.`;
813811
userId = await getUserIdFromTokenCached(apifyToken, apifyClient);
814812
log.debug('Telemetry: fetched userId', { userId });
815813
}
816-
if (!userId) {
817-
userId = crypto.randomUUID();
818-
log.debug('Telemetry: using random userId', { userId });
819-
}
820-
821814
const capabilities = this.options.initializeRequestData?.params?.capabilities;
822815
const params = this.options.initializeRequestData?.params as InitializeRequest['params'];
823816
const telemetryData: ToolCallTelemetryProperties = {

src/telemetry.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import * as crypto from 'node:crypto';
2+
13
import { Analytics } from '@segment/analytics-node';
24

35
import log from '@apify/log';
@@ -57,12 +59,14 @@ export function getOrInitAnalyticsClient(): Analytics | null {
5759

5860
/**
5961
* Tracks a tool call event to Segment.
62+
* Segment requires either userId OR anonymousId, but not both
63+
* When userId is available, use it; otherwise use anonymousId
6064
*
6165
* @param userId - Apify user ID (null if not available)
6266
* @param properties - Event properties for the tool call
6367
*/
6468
export function trackToolCall(
65-
userId: string,
69+
userId: string | null,
6670
properties: ToolCallTelemetryProperties,
6771
): void {
6872
const client = getOrInitAnalyticsClient();
@@ -73,7 +77,7 @@ export function trackToolCall(
7377

7478
try {
7579
client.track({
76-
userId,
80+
...(userId ? { userId } : { anonymousId: crypto.randomUUID() }),
7781
event: SEGMENT_EVENTS.TOOL_CALL,
7882
properties,
7983
});

tests/unit/telemetry.test.ts

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ describe('telemetry', () => {
1515
vi.clearAllMocks();
1616
});
1717

18-
it('should send correct payload structure to Segment', () => {
18+
it('should send correct payload structure to Segment with userId', () => {
1919
const userId = 'test-user-123';
2020
const properties = {
2121
app: 'mcp' as const,
@@ -53,4 +53,35 @@ describe('telemetry', () => {
5353
},
5454
});
5555
});
56+
57+
it('should use anonymousId when userId is null', () => {
58+
const properties = {
59+
app: 'mcp' as const,
60+
app_version: '0.5.6',
61+
mcp_client_name: 'test-client',
62+
mcp_client_version: '1.0.0',
63+
mcp_protocol_version: '2024-11-05',
64+
mcp_client_capabilities: '{}',
65+
mcp_session_id: 'session-123',
66+
transport_type: 'stdio',
67+
tool_name: 'test-tool',
68+
tool_status: 'succeeded' as const,
69+
tool_exec_time_ms: 100,
70+
tool_call_number: 1,
71+
};
72+
73+
trackToolCall(null, properties);
74+
75+
expect(mockTrack).toHaveBeenCalledTimes(1);
76+
const callArgs = mockTrack.mock.calls[0][0];
77+
78+
// Should have anonymousId but not userId
79+
expect(callArgs).toHaveProperty('anonymousId');
80+
expect(callArgs.anonymousId).toBeDefined();
81+
expect(typeof callArgs.anonymousId).toBe('string');
82+
expect(callArgs.anonymousId.length).toBeGreaterThan(0);
83+
expect(callArgs).not.toHaveProperty('userId');
84+
expect(callArgs.event).toBe('MCP Tool Call');
85+
expect(callArgs.properties).toEqual(properties);
86+
});
5687
});

0 commit comments

Comments
 (0)