-
Notifications
You must be signed in to change notification settings - Fork 836
feat(telemetry): integrate telemetry tracking across interactive and … #1798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -653,6 +653,9 @@ def _find_slash_command(self, name: str) -> SlashCommand[Any] | None: | |
|
|
||
| def _make_skill_runner(self, skill: Skill) -> Callable[[KimiSoul, str], None | Awaitable[None]]: | ||
| async def _run_skill(soul: KimiSoul, args: str, *, _skill: Skill = skill) -> None: | ||
| from kimi_cli.telemetry import track | ||
|
|
||
| track("kimi_skill_invoked", skill_name=_skill.name) | ||
| skill_text = await read_skill_text(_skill) | ||
| if skill_text is None: | ||
| wire_send( | ||
|
|
@@ -683,17 +686,30 @@ async def _agent_loop(self) -> TurnOutcome: | |
| wire_send(MCPLoadingBegin()) | ||
| try: | ||
| await self.wait_for_background_mcp_loading() | ||
| # Track MCP connection result | ||
| if loading: | ||
| from kimi_cli.telemetry import track as _track_mcp | ||
|
|
||
| mcp_snap = self._mcp_status_snapshot() | ||
| if mcp_snap: | ||
| if mcp_snap.connected > 0: | ||
| _track_mcp("kimi_mcp_connected", server_count=mcp_snap.connected) | ||
| _failed = mcp_snap.total - mcp_snap.connected | ||
| if _failed > 0: | ||
| _track_mcp("kimi_mcp_failed") | ||
| finally: | ||
| if loading: | ||
| wire_send(StatusUpdate(mcp_status=self._mcp_status_snapshot())) | ||
| wire_send(MCPLoadingEnd()) | ||
|
|
||
| step_no = 0 | ||
| self._current_step_no = 0 | ||
| while True: | ||
| step_no += 1 | ||
| if step_no > self._loop_control.max_steps_per_turn: | ||
| raise MaxStepsReached(self._loop_control.max_steps_per_turn) | ||
|
|
||
| self._current_step_no = step_no | ||
| wire_send(StepBegin(n=step_no)) | ||
| back_to_the_future: BackToTheFuture | None = None | ||
| step_outcome: StepOutcome | None = None | ||
|
|
@@ -735,6 +751,23 @@ async def _agent_loop(self) -> TurnOutcome: | |
| request_id=req_id, | ||
| ) | ||
| wire_send(StepInterrupted()) | ||
| # Track API/step errors | ||
| from kimi_cli.telemetry import track | ||
|
|
||
| error_type = "other" | ||
| if isinstance(e, APIStatusError): | ||
| status = getattr(e, "status_code", getattr(e, "status", 0)) | ||
| if status == 429: | ||
| error_type = "rate_limit" | ||
| elif status in (401, 403): | ||
| error_type = "auth" | ||
| else: | ||
| error_type = "api" | ||
| elif isinstance(e, APIConnectionError): | ||
| error_type = "network" | ||
| elif isinstance(e, (APITimeoutError, TimeoutError)): | ||
| error_type = "timeout" | ||
| track("kimi_api_error", error_type=error_type) | ||
| # --- StopFailure hook --- | ||
| from kimi_cli.hooks import events as _hook_events | ||
|
|
||
|
|
@@ -1185,6 +1218,10 @@ def ralph_loop( | |
| return FlowRunner(flow, max_moves=max_moves) | ||
|
|
||
| async def run(self, soul: KimiSoul, args: str) -> None: | ||
| if self._name: | ||
| from kimi_cli.telemetry import track | ||
|
|
||
| track("kimi_flow_invoked", flow_name=self._name) | ||
| if args.strip(): | ||
| command = f"/{FLOW_COMMAND_PREFIX}{self._name}" if self._name else "/flow" | ||
| logger.warning("Agent flow {command} ignores args: {args}", command=command, args=args) | ||
|
Comment on lines
+1221
to
1227
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 kimi_flow_invoked tracked even when flow does not execute due to args validation failure In (Refers to lines 1221-1228) Was this helpful? React with 👍 or 👎 to provide feedback. |
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Telemetry code inside fail-open try/except can silently discard hook results
The telemetry tracking code (import,
any()call,track()) was inserted inside thetryblock that was originally designed to catch only_execute_hooks()errors and fail open. If any of these new statements raise (e.g.,_sink.accept()encounters an unexpected error), the already-computedresultsare silently discarded and replaced with[]. This changes the error-handling semantics: previously only hook-execution failures caused fail-open, now telemetry failures also cause it. For security-critical hooks likePreToolUsethat block dangerous operations, a telemetry failure would silently bypass the block.Original vs new code structure
Original:
New code places
track()between result computation and return, still inside the same try/except:Was this helpful? React with 👍 or 👎 to provide feedback.