Add product analytics - #95
Conversation
Report anonymous SDK usage events — conversation lifecycle, channel type, SDK version — so we can see how the SDK is used in practice. No PII, message content, transcripts, or phone numbers are collected. Set TAC_ANALYTICS_DISABLED=true to opt out. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
A critical extension API break and multiple moderate analytics, shutdown, test, and dependency issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds anonymous, batched Segment analytics across SDK conversation and messaging lifecycles, with opt-out support and shutdown flushing.
Changes:
- Adds analytics tracking, tests, and README documentation.
- Instruments messaging and voice channels.
- Updates dependencies, build configuration, and lockfiles.
File summaries
| File | Summary and review notes |
|---|---|
tsup.config.ts |
Externalizes the analytics dependency. |
tests/analytics.test.ts |
Adds analytics behavior tests. |
README.md |
Documents telemetry and opt-out configuration. |
packages/core/src/lib/tac.ts |
Integrates analytics shutdown. Moderate (3 votes): shutdown is not awaited, so pending events may be dropped. |
packages/core/src/lib/analytics.ts |
Implements Segment telemetry. Moderate (2 votes): ordinary tests may send real events because analytics are not disabled in shared test setup. |
packages/core/src/index.ts |
Exports analytics utilities. |
packages/core/src/channels/whatsapp.ts |
Adapts the messaging response hook. |
packages/core/src/channels/voice.ts |
Tracks voice lifecycle events. Moderate (1 vote): incoming voice prompts do not emit Message Received. |
packages/core/src/channels/sms.ts |
Adapts the messaging response hook. |
packages/core/src/channels/rcs.ts |
Adapts the messaging response hook. |
packages/core/src/channels/messaging.ts |
Centralizes response and inbound tracking. Critical (3 votes): the required abstract hook breaks existing custom subclasses. Moderate (1 vote): missing channel-level success/failure response tracking coverage. Moderate (1 vote): voice prompt tracking is not covered here. |
packages/core/src/channels/chat.ts |
Adapts the messaging response hook. |
packages/core/src/channels/base.ts |
Tracks conversation lifecycle events. |
package.json |
Adds the analytics dependency. Moderate (1 vote): other example lockfiles omit the new dependency. |
package-lock.json |
Locks analytics dependencies. Moderate (2 votes): Segment entries use an inaccessible Twilio Artifactory registry. |
getting_started/examples/openai/package-lock.json |
Updates the example dependency snapshot. |
Review details
Files not reviewed (1)
- getting_started/examples/openai/package-lock.json: Generated file
Suppressed comments (4)
package.json:69
- All getting-started examples use the root package through
file:../../.., so their lockfiles snapshot the root dependency list. Only the OpenAI example lockfile was updated with@segment/analytics-node; for example, the outbound snapshot still omits it. A clean install of those examples can produce a TAC installation whose new analytics import is unresolved. Regenerate the other example lockfiles as well.
"@segment/analytics-node": "^3.1.0",
packages/core/src/channels/messaging.ts:128
- The new analytics test only exercises
trackEvent()directly; no existing channel test verifies that this template method emits exactly oneResponse Sentevent after a successful messaging send and none after a failed send. A regression in any of the four subclass delegations would still pass the suite. Add a channel-level test with the analytics client mocked.
public async sendResponse(
conversationId: ConversationId,
message: string,
metadata?: Record<string, unknown>
): Promise<void> {
await this.doSendResponse(conversationId, message, metadata);
trackEvent('Response Sent', {
packages/core/src/channels/messaging.ts:440
- This is the only
Message Receivedemission, butVoiceChannel.handlePromptMessage()never records an event for a voice prompt. Voice sessions therefore report responses, interrupts, and WebSocket lifecycle events without any inbound-message usage data, despite the PR describing lifecycle analytics across messaging and voice. Add equivalent tracking when the voice prompt is received.
trackEvent('Message Received', {
account_sid: this.config.accountSid,
channel: this.channelType,
conversation_id: conversationId,
});
packages/core/src/channels/voice.ts:856
- Voice responses are instrumented here, but incoming voice prompts never emit
Message Received(the prompt handler only invokes the callback). This leaves the advertised lifecycle analytics incomplete for voice; add the event whenhandlePromptMessage()accepts a prompt, before invoking the user callback.
trackEvent('Response Sent', {
account_sid: this.config.accountSid,
channel: 'voice',
conversation_id: conversationId,
response_type: 'full',
- Files reviewed: 14/16 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| } | ||
|
|
||
| this.channels.clear(); | ||
| shutdownAnalytics().catch(() => {}); |
There was a problem hiding this comment.
Accurate, but leaving as-is. Making shutdown() await the flush changes its signature from void to Promise<void>, and this isn't a major-version change. Losing a few buffered telemetry events on exit is an acceptable trade for keeping the public API stable — the events are best-effort by design.
Point the new dependency entries at the public npm registry, and drop unrelated churn in the example lockfile. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resolves from the public npm registry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keep sendResponse as the overridable extension point on messaging channels and track the event in each channel instead, so subclasses outside this repo keep compiling. Disable telemetry across the test suite so tests never construct a real client or emit fixture events. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Reports anonymous SDK usage events so we can see how the SDK is used in practice. No PII, message content, transcripts, or phone numbers are collected.
Events cover the conversation lifecycle across messaging and voice:
Conversation Started,Conversation Ended,Message Received,Response Sent,Websocket Connected,Websocket Disconnected, andVoice Interrupt. Each carries the account SID, channel, conversation ID, SDK version, and SDK package name.Consumers can opt out with
TAC_ANALYTICS_DISABLED=true, documented in the README.Notes:
trackEventswallows all errors, and a failure to deliver events leaves conversations untouched.TAC.shutdown()flushes anything pending.MessagingChannel.sendResponse()is now a template method delegating todoSendResponse(), soResponse Sentis tracked once for every messaging channel rather than in each subclass.sendResponse()keeps its existing signature and behavior.trackEventandshutdownAnalyticsare exported for framework use and tagged@internal, so they stay out of the published API reference.Type of Change
Checklist
SDK Parity
This is the TypeScript SDK. If this change affects shared functionality, ensure the Python SDK is updated as well.