Reduce misleading telemetry shims in the open build
The open build already treated analytics and tracing as inert, but several empty sink and shutdown modules still made startup and exit paths look like they initialized or flushed telemetry. This trims those dead compatibility layers, updates the surrounding comments to match reality, and adds small regression tests that lock in the inert analytics boundary and disabled transcript sharing behavior. Constraint: Preserve the no-op logEvent/logOTelEvent compatibility surface for existing call sites Constraint: Avoid touching unrelated bridge and session work already in progress in the worktree Rejected: Remove every remaining logEvent/logOTelEvent call site | Too broad for a safe first cleanup pass Rejected: Keep the empty sink/shutdown modules | Continued to mislead future audits and maintenance Confidence: high Scope-risk: narrow Reversibility: clean Directive: Treat remaining analytics and GrowthBook helpers as compatibility surfaces until each call path is individually proven dead Tested: bun test src/services/analytics/index.test.ts src/components/FeedbackSurvey/submitTranscriptShare.test.ts Tested: bun run ./scripts/build.ts Not-tested: bun x tsc --noEmit (repository has pre-existing unrelated type errors)
This commit is contained in:
@@ -6,14 +6,11 @@ import {
|
||||
} from '@ant/claude-for-chrome-mcp'
|
||||
import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
|
||||
import { format } from 'util'
|
||||
import { shutdownDatadog } from '../../services/analytics/datadog.js'
|
||||
import { shutdown1PEventLogging } from '../../services/analytics/firstPartyEventLogger.js'
|
||||
import { getFeatureValue_CACHED_MAY_BE_STALE } from '../../services/analytics/growthbook.js'
|
||||
import {
|
||||
type AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
logEvent,
|
||||
} from '../../services/analytics/index.js'
|
||||
import { initializeAnalyticsSink } from '../../services/analytics/sink.js'
|
||||
import { getClaudeAIOAuthTokens } from '../auth.js'
|
||||
import { enableConfigs, getGlobalConfig, saveGlobalConfig } from '../config.js'
|
||||
import { logForDebugging } from '../debug.js'
|
||||
@@ -225,7 +222,7 @@ export function createChromeContext(
|
||||
} = {}
|
||||
if (metadata) {
|
||||
for (const [key, value] of Object.entries(metadata)) {
|
||||
// Rename 'status' to 'bridge_status' to avoid Datadog's reserved field
|
||||
// Keep the status field namespaced to avoid downstream collisions.
|
||||
const safeKey = key === 'status' ? 'bridge_status' : key
|
||||
if (typeof value === 'boolean' || typeof value === 'number') {
|
||||
safeMetadata[safeKey] = value
|
||||
@@ -247,22 +244,18 @@ export function createChromeContext(
|
||||
|
||||
export async function runClaudeInChromeMcpServer(): Promise<void> {
|
||||
enableConfigs()
|
||||
initializeAnalyticsSink()
|
||||
const context = createChromeContext()
|
||||
|
||||
const server = createClaudeForChromeMcpServer(context)
|
||||
const transport = new StdioServerTransport()
|
||||
|
||||
// Exit when parent process dies (stdin pipe closes).
|
||||
// Flush analytics before exiting so final-batch events (e.g. disconnect) aren't lost.
|
||||
let exiting = false
|
||||
const shutdownAndExit = async (): Promise<void> => {
|
||||
const shutdownAndExit = (): void => {
|
||||
if (exiting) {
|
||||
return
|
||||
}
|
||||
exiting = true
|
||||
await shutdown1PEventLogging()
|
||||
await shutdownDatadog()
|
||||
// eslint-disable-next-line custom-rules/no-process-exit
|
||||
process.exit(0)
|
||||
}
|
||||
|
||||
@@ -6,9 +6,6 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'
|
||||
import { ListToolsRequestSchema } from '@modelcontextprotocol/sdk/types.js'
|
||||
import { homedir } from 'os'
|
||||
|
||||
import { shutdownDatadog } from '../../services/analytics/datadog.js'
|
||||
import { shutdown1PEventLogging } from '../../services/analytics/firstPartyEventLogger.js'
|
||||
import { initializeAnalyticsSink } from '../../services/analytics/sink.js'
|
||||
import { enableConfigs } from '../config.js'
|
||||
import { logForDebugging } from '../debug.js'
|
||||
import { filterAppsForDescription } from './appNames.js'
|
||||
@@ -80,20 +77,18 @@ export async function createComputerUseMcpServerForCli(): Promise<
|
||||
/**
|
||||
* Subprocess entrypoint for `--computer-use-mcp`. Mirror of
|
||||
* `runClaudeInChromeMcpServer` — stdio transport, exit on stdin close,
|
||||
* flush analytics before exit.
|
||||
* and exit promptly when the parent process closes stdin.
|
||||
*/
|
||||
export async function runComputerUseMcpServer(): Promise<void> {
|
||||
enableConfigs()
|
||||
initializeAnalyticsSink()
|
||||
|
||||
const server = await createComputerUseMcpServerForCli()
|
||||
const transport = new StdioServerTransport()
|
||||
|
||||
let exiting = false
|
||||
const shutdownAndExit = async (): Promise<void> => {
|
||||
const shutdownAndExit = (): void => {
|
||||
if (exiting) return
|
||||
exiting = true
|
||||
await Promise.all([shutdown1PEventLogging(), shutdownDatadog()])
|
||||
// eslint-disable-next-line custom-rules/no-process-exit
|
||||
process.exit(0)
|
||||
}
|
||||
|
||||
@@ -202,8 +202,6 @@ function logMCPDebugImpl(serverName: string, message: string): void {
|
||||
* Call this during app startup to attach the error logging backend.
|
||||
* Any errors logged before this is called will be queued and drained.
|
||||
*
|
||||
* Should be called BEFORE initializeAnalyticsSink() in the startup sequence.
|
||||
*
|
||||
* Idempotent: safe to call multiple times (subsequent calls are no-ops).
|
||||
*/
|
||||
export function initializeErrorLogSink(): void {
|
||||
|
||||
@@ -29,8 +29,6 @@ import {
|
||||
supportsTabStatus,
|
||||
wrapForMultiplexer,
|
||||
} from '../ink/termio/osc.js'
|
||||
import { shutdownDatadog } from '../services/analytics/datadog.js'
|
||||
import { shutdown1PEventLogging } from '../services/analytics/firstPartyEventLogger.js'
|
||||
import {
|
||||
type AnalyticsMetadata_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS,
|
||||
logEvent,
|
||||
@@ -41,7 +39,6 @@ import { logForDebugging } from './debug.js'
|
||||
import { logForDiagnosticsNoPII } from './diagLogs.js'
|
||||
import { isEnvTruthy } from './envUtils.js'
|
||||
import { getCurrentSessionTitle, sessionIdExists } from './sessionStorage.js'
|
||||
import { sleep } from './sleep.js'
|
||||
import { profileReport } from './startupProfiler.js'
|
||||
|
||||
/**
|
||||
@@ -413,7 +410,7 @@ export async function gracefulShutdown(
|
||||
|
||||
// Failsafe: guarantee process exits even if cleanup hangs (e.g., MCP connections).
|
||||
// Runs cleanupTerminalModes first so a hung cleanup doesn't leave the terminal dirty.
|
||||
// Budget = max(5s, hook budget + 3.5s headroom for cleanup + analytics flush).
|
||||
// Budget = max(5s, hook budget + 3.5s headroom for remaining cleanup).
|
||||
failsafeTimer = setTimeout(
|
||||
code => {
|
||||
cleanupTerminalModes()
|
||||
@@ -487,7 +484,7 @@ export async function gracefulShutdown(
|
||||
}
|
||||
|
||||
// Signal to inference that this session's cache can be evicted.
|
||||
// Fires before analytics flush so the event makes it to the pipeline.
|
||||
// Emit before the final forced-exit path runs.
|
||||
const lastRequestId = getLastMainRequestId()
|
||||
if (lastRequestId) {
|
||||
logEvent('tengu_cache_eviction_hint', {
|
||||
@@ -498,18 +495,6 @@ export async function gracefulShutdown(
|
||||
})
|
||||
}
|
||||
|
||||
// Flush analytics — capped at 500ms. Previously unbounded: the 1P exporter
|
||||
// awaits all pending axios POSTs (10s each), eating the full failsafe budget.
|
||||
// Lost analytics on slow networks are acceptable; a hanging exit is not.
|
||||
try {
|
||||
await Promise.race([
|
||||
Promise.all([shutdown1PEventLogging(), shutdownDatadog()]),
|
||||
sleep(500),
|
||||
])
|
||||
} catch {
|
||||
// Ignore analytics shutdown errors
|
||||
}
|
||||
|
||||
if (options?.finalMessage) {
|
||||
try {
|
||||
// eslint-disable-next-line custom-rules/no-sync-fs -- must flush before forceExit
|
||||
|
||||
@@ -1,15 +1,12 @@
|
||||
import { initializeAnalyticsSink } from '../services/analytics/sink.js'
|
||||
import { initializeErrorLogSink } from './errorLogSink.js'
|
||||
|
||||
/**
|
||||
* Attach error log and analytics compatibility sinks. Both inits are
|
||||
* idempotent. Called from setup() for the default command; other entrypoints
|
||||
* (subcommands, daemon, bridge) call this directly since they bypass setup().
|
||||
* Attach startup sinks used by all entrypoints. The error-log init is
|
||||
* idempotent, so callers that bypass setup() can safely invoke this too.
|
||||
*
|
||||
* Leaf module — kept out of setup.ts to avoid the setup → commands → bridge
|
||||
* → setup import cycle.
|
||||
*/
|
||||
export function initSinks(): void {
|
||||
initializeErrorLogSink()
|
||||
initializeAnalyticsSink()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user