From f127024d01b34c987ae0690665ea603f7b4e270d Mon Sep 17 00:00:00 2001 From: David Sarno Date: Mon, 8 Sep 2025 16:39:47 -0700 Subject: [PATCH] telemetry: enable tool_execution across tools via strict, async-aware decorator; add endpoint env override + urllib fallback; enrich OS fields; fix TelemetryHelper invocation --- .../Editor/Helpers/TelemetryHelper.cs | 2 +- .../UnityMcpServer~/src/telemetry.py | 72 +++++++++++++------ .../src/telemetry_decorator.py | 47 +++++++++--- .../src/tools/execute_menu_item.py | 5 +- .../UnityMcpServer~/src/tools/manage_asset.py | 5 +- .../src/tools/manage_editor.py | 13 +++- .../src/tools/manage_gameobject.py | 5 +- .../UnityMcpServer~/src/tools/manage_scene.py | 5 +- .../src/tools/manage_script.py | 30 ++++---- .../src/tools/manage_script_edits.py | 5 +- .../src/tools/manage_shader.py | 5 +- .../UnityMcpServer~/src/tools/read_console.py | 5 +- .../src/tools/resource_tools.py | 13 ++-- 13 files changed, 152 insertions(+), 60 deletions(-) diff --git a/UnityMcpBridge/Editor/Helpers/TelemetryHelper.cs b/UnityMcpBridge/Editor/Helpers/TelemetryHelper.cs index 67618e7..079ca6f 100644 --- a/UnityMcpBridge/Editor/Helpers/TelemetryHelper.cs +++ b/UnityMcpBridge/Editor/Helpers/TelemetryHelper.cs @@ -118,7 +118,7 @@ namespace MCPForUnity.Editor.Helpers RecordEvent("bridge_startup", new Dictionary { ["bridge_version"] = "3.0.2", - ["auto_connect"] = MCPForUnityBridge.IsAutoConnectMode + ["auto_connect"] = MCPForUnityBridge.IsAutoConnectMode() }); } diff --git a/UnityMcpBridge/UnityMcpServer~/src/telemetry.py b/UnityMcpBridge/UnityMcpServer~/src/telemetry.py index 2163483..b701e2a 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/telemetry.py +++ b/UnityMcpBridge/UnityMcpServer~/src/telemetry.py @@ -9,6 +9,8 @@ import contextvars import json import time import os +import sys +import platform import logging from enum import Enum from dataclasses import dataclass, asdict @@ -61,8 +63,11 @@ class TelemetryConfig: # Check environment variables for opt-out self.enabled = not self._is_disabled() - # Telemetry endpoint - hardcoded to Coplay production API - self.endpoint = "https://api-prod.coplay.dev/telemetry/events" + # Telemetry endpoint (Cloud Run default; override via env) + self.endpoint = os.environ.get( + "UNITY_MCP_TELEMETRY_ENDPOINT", + "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events" + ) # Local storage for UUID and milestones self.data_dir = self._get_data_directory() @@ -172,9 +177,7 @@ class TelemetryCollector: if not self.config.enabled: return - if not HAS_HTTPX: - logger.debug("Telemetry disabled: httpx not available") - return + # Allow fallback sender when httpx is unavailable (no early return) record = TelemetryRecord( record_type=record_type, @@ -196,34 +199,63 @@ class TelemetryCollector: def _send_telemetry(self, record: TelemetryRecord): """Send telemetry data to endpoint""" try: + # System fingerprint (top-level remains concise; details stored in data JSON) + _platform = platform.system() # 'Darwin' | 'Linux' | 'Windows' + _source = sys.platform # 'darwin' | 'linux' | 'win32' + _platform_detail = f"{_platform} {platform.release()} ({platform.machine()})" + _python_version = platform.python_version() + + # Enrich data JSON so BigQuery stores detailed fields without schema change + enriched_data = dict(record.data or {}) + enriched_data.setdefault("platform_detail", _platform_detail) + enriched_data.setdefault("python_version", _python_version) + payload = { "record": record.record_type.value, "timestamp": record.timestamp, "customer_uuid": record.customer_uuid, "session_id": record.session_id, - "data": record.data, + "data": enriched_data, "version": "3.0.2", # Unity MCP version - "platform": os.name + "platform": _platform, + "source": _source, } - + if record.milestone: payload["milestone"] = record.milestone.value - - if not httpx: - return - - with httpx.Client(timeout=self.config.timeout) as client: - response = client.post(self.config.endpoint, json=payload) - - if response.status_code == 200: - logger.debug(f"Telemetry sent: {record.record_type}") - else: - logger.debug(f"Telemetry failed: HTTP {response.status_code}") - + + # Prefer httpx when available; otherwise fall back to urllib + if httpx: + with httpx.Client(timeout=self.config.timeout) as client: + response = client.post(self.config.endpoint, json=payload) + if response.status_code == 200: + logger.debug(f"Telemetry sent: {record.record_type}") + else: + logger.debug(f"Telemetry failed: HTTP {response.status_code}") + else: + import urllib.request + import urllib.error + data_bytes = json.dumps(payload).encode("utf-8") + req = urllib.request.Request( + self.config.endpoint, + data=data_bytes, + headers={"Content-Type": "application/json"}, + method="POST", + ) + try: + with urllib.request.urlopen(req, timeout=self.config.timeout) as resp: + if 200 <= resp.getcode() < 300: + logger.debug(f"Telemetry sent (urllib): {record.record_type}") + else: + logger.debug(f"Telemetry failed (urllib): HTTP {resp.getcode()}") + except urllib.error.URLError as ue: + logger.debug(f"Telemetry send failed (urllib): {ue}") + except Exception as e: # Never let telemetry errors interfere with app functionality logger.debug(f"Telemetry send failed: {e}") + # Global telemetry instance _telemetry_collector: Optional[TelemetryCollector] = None diff --git a/UnityMcpBridge/UnityMcpServer~/src/telemetry_decorator.py b/UnityMcpBridge/UnityMcpServer~/src/telemetry_decorator.py index 8e1347a..875c66a 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/telemetry_decorator.py +++ b/UnityMcpBridge/UnityMcpServer~/src/telemetry_decorator.py @@ -4,39 +4,66 @@ Telemetry decorator for Unity MCP tools import functools import time +import inspect +import logging from typing import Callable, Any from telemetry import record_tool_usage, record_milestone, MilestoneType +_log = logging.getLogger("unity-mcp-telemetry") +_decorator_log_count = 0 + def telemetry_tool(tool_name: str): """Decorator to add telemetry tracking to MCP tools""" def decorator(func: Callable) -> Callable: @functools.wraps(func) - def wrapper(*args, **kwargs) -> Any: + def _sync_wrapper(*args, **kwargs) -> Any: start_time = time.time() success = False error = None - try: + global _decorator_log_count + if _decorator_log_count < 10: + _log.info(f"telemetry_decorator sync: tool={tool_name}") + _decorator_log_count += 1 result = func(*args, **kwargs) success = True - - # Record tool-specific milestones if tool_name == "manage_script" and kwargs.get("action") == "create": record_milestone(MilestoneType.FIRST_SCRIPT_CREATION) elif tool_name.startswith("manage_scene"): record_milestone(MilestoneType.FIRST_SCENE_MODIFICATION) - - # Record general first tool usage record_milestone(MilestoneType.FIRST_TOOL_USAGE) - return result - except Exception as e: error = str(e) raise finally: duration_ms = (time.time() - start_time) * 1000 record_tool_usage(tool_name, success, duration_ms, error) - - return wrapper + + @functools.wraps(func) + async def _async_wrapper(*args, **kwargs) -> Any: + start_time = time.time() + success = False + error = None + try: + global _decorator_log_count + if _decorator_log_count < 10: + _log.info(f"telemetry_decorator async: tool={tool_name}") + _decorator_log_count += 1 + result = await func(*args, **kwargs) + success = True + if tool_name == "manage_script" and kwargs.get("action") == "create": + record_milestone(MilestoneType.FIRST_SCRIPT_CREATION) + elif tool_name.startswith("manage_scene"): + record_milestone(MilestoneType.FIRST_SCENE_MODIFICATION) + record_milestone(MilestoneType.FIRST_TOOL_USAGE) + return result + except Exception as e: + error = str(e) + raise + finally: + duration_ms = (time.time() - start_time) * 1000 + record_tool_usage(tool_name, success, duration_ms, error) + + return _async_wrapper if inspect.iscoroutinefunction(func) else _sync_wrapper return decorator \ No newline at end of file diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/execute_menu_item.py b/UnityMcpBridge/UnityMcpServer~/src/tools/execute_menu_item.py index a448465..27fa51a 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/execute_menu_item.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/execute_menu_item.py @@ -7,12 +7,15 @@ from unity_connection import get_unity_connection, send_command_with_retry # Im from config import config import time +from telemetry_decorator import telemetry_tool + def register_execute_menu_item_tools(mcp: FastMCP): """Registers the execute_menu_item tool with the MCP server.""" @mcp.tool() + @telemetry_tool("execute_menu_item") async def execute_menu_item( - ctx: Context, + ctx: Any, menu_path: str, action: str = 'execute', parameters: Dict[str, Any] = None, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py index ccafb04..ae50a8a 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_asset.py @@ -9,12 +9,15 @@ from unity_connection import get_unity_connection, async_send_command_with_retry from config import config import time +from telemetry_decorator import telemetry_tool + def register_manage_asset_tools(mcp: FastMCP): """Registers the manage_asset tool with the MCP server.""" @mcp.tool() + @telemetry_tool("manage_asset") async def manage_asset( - ctx: Context, + ctx: Any, action: str, path: str, asset_type: str = None, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py index 8ff7378..f0edcec 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_editor.py @@ -4,12 +4,16 @@ from typing import Dict, Any from unity_connection import get_unity_connection, send_command_with_retry from config import config +from telemetry_decorator import telemetry_tool +from telemetry import is_telemetry_enabled, record_tool_usage + def register_manage_editor_tools(mcp: FastMCP): """Register all editor management tools with the MCP server.""" @mcp.tool() + @telemetry_tool("manage_editor") def manage_editor( - ctx: Context, + ctx: Any, action: str, wait_for_completion: bool = None, # --- Parameters for specific actions --- @@ -28,6 +32,13 @@ def register_manage_editor_tools(mcp: FastMCP): Dictionary with operation results ('success', 'message', 'data'). """ try: + # Diagnostics: quick telemetry checks + if action == "telemetry_status": + return {"success": True, "telemetry_enabled": is_telemetry_enabled()} + + if action == "telemetry_ping": + record_tool_usage("diagnostic_ping", True, 1.0, None) + return {"success": True, "message": "telemetry ping queued"} # Prepare parameters, removing None values params = { "action": action, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py index cbe29a3..b96a8bb 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_gameobject.py @@ -4,12 +4,15 @@ from unity_connection import get_unity_connection, send_command_with_retry from config import config import time +from telemetry_decorator import telemetry_tool + def register_manage_gameobject_tools(mcp: FastMCP): """Register all GameObject management tools with the MCP server.""" @mcp.tool() + @telemetry_tool("manage_gameobject") def manage_gameobject( - ctx: Context, + ctx: Any, action: str, target: str = None, # GameObject identifier by name or path search_method: str = None, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py index c2257ef..9041b50 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_scene.py @@ -4,12 +4,15 @@ from unity_connection import get_unity_connection, send_command_with_retry from config import config import time +from telemetry_decorator import telemetry_tool + def register_manage_scene_tools(mcp: FastMCP): """Register all scene management tools with the MCP server.""" @mcp.tool() + @telemetry_tool("manage_scene") def manage_scene( - ctx: Context, + ctx: Any, action: str, name: str, path: str, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py index 841ed20..2602c29 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script.py @@ -5,16 +5,8 @@ import base64 import os from urllib.parse import urlparse, unquote -try: - from telemetry_decorator import telemetry_tool - from telemetry import record_milestone, MilestoneType - HAS_TELEMETRY = True -except ImportError: - HAS_TELEMETRY = False - def telemetry_tool(tool_name: str): - def decorator(func): - return func - return decorator +from telemetry_decorator import telemetry_tool +from telemetry import record_milestone, MilestoneType def register_manage_script_tools(mcp: FastMCP): """Register all script management tools with the MCP server.""" @@ -92,7 +84,7 @@ def register_manage_script_tools(mcp: FastMCP): )) @telemetry_tool("apply_text_edits") def apply_text_edits( - ctx: Context, + ctx: Any, uri: str, edits: List[Dict[str, Any]], precondition_sha256: str | None = None, @@ -359,7 +351,7 @@ def register_manage_script_tools(mcp: FastMCP): )) @telemetry_tool("create_script") def create_script( - ctx: Context, + ctx: Any, path: str, contents: str = "", script_type: str | None = None, @@ -397,7 +389,8 @@ def register_manage_script_tools(mcp: FastMCP): "Args: uri (unity://path/... or file://... or Assets/...).\n" "Rules: Target must resolve under Assets/.\n" )) - def delete_script(ctx: Context, uri: str) -> Dict[str, Any]: + @telemetry_tool("delete_script") + def delete_script(ctx: Any, uri: str) -> Dict[str, Any]: """Delete a C# script by URI.""" name, directory = _split_uri(uri) if not directory or directory.split("/")[0].lower() != "assets": @@ -412,8 +405,9 @@ def register_manage_script_tools(mcp: FastMCP): "- basic: quick syntax checks.\n" "- standard: deeper checks (performance hints, common pitfalls).\n" )) + @telemetry_tool("validate_script") def validate_script( - ctx: Context, uri: str, level: str = "basic" + ctx: Any, uri: str, level: str = "basic" ) -> Dict[str, Any]: """Validate a C# script and return diagnostics.""" name, directory = _split_uri(uri) @@ -443,7 +437,7 @@ def register_manage_script_tools(mcp: FastMCP): )) @telemetry_tool("manage_script") def manage_script( - ctx: Context, + ctx: Any, action: str, name: str, path: str, @@ -573,7 +567,8 @@ def register_manage_script_tools(mcp: FastMCP): "Get manage_script capabilities (supported ops, limits, and guards).\n\n" "Returns:\n- ops: list of supported structured ops\n- text_ops: list of supported text ops\n- max_edit_payload_bytes: server edit payload cap\n- guards: header/using guard enabled flag\n" )) - def manage_script_capabilities(ctx: Context) -> Dict[str, Any]: + @telemetry_tool("manage_script_capabilities") + def manage_script_capabilities(ctx: Any) -> Dict[str, Any]: try: # Keep in sync with server/Editor ManageScript implementation ops = [ @@ -600,7 +595,8 @@ def register_manage_script_tools(mcp: FastMCP): "Args: uri (unity://path/Assets/... or file://... or Assets/...).\n" "Returns: {sha256, lengthBytes, lastModifiedUtc, uri, path}." )) - def get_sha(ctx: Context, uri: str) -> Dict[str, Any]: + @telemetry_tool("get_sha") + def get_sha(ctx: Any, uri: str) -> Dict[str, Any]: """Return SHA256 and basic metadata for a script.""" try: name, directory = _split_uri(uri) diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py index 91a107b..a6f8004 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_script_edits.py @@ -5,6 +5,8 @@ import re import os from unity_connection import send_command_with_retry +from telemetry_decorator import telemetry_tool + def _apply_edits_locally(original_text: str, edits: List[Dict[str, Any]]) -> str: text = original_text @@ -326,8 +328,9 @@ def register_manage_script_edits_tools(mcp: FastMCP): " 'position':'after','afterMethodName':'GetCurrentTarget' }\n" "] }\n" )) + @telemetry_tool("script_apply_edits") def script_apply_edits( - ctx: Context, + ctx: Any, name: str, path: str, edits: List[Dict[str, Any]], diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py index 8ddb6c7..abf1d70 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/manage_shader.py @@ -6,12 +6,15 @@ import time import os import base64 +from telemetry_decorator import telemetry_tool + def register_manage_shader_tools(mcp: FastMCP): """Register all shader script management tools with the MCP server.""" @mcp.tool() + @telemetry_tool("manage_shader") def manage_shader( - ctx: Context, + ctx: Any, action: str, name: str, path: str, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py b/UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py index aae0e49..5436844 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/read_console.py @@ -7,12 +7,15 @@ from mcp.server.fastmcp import FastMCP, Context from unity_connection import get_unity_connection, send_command_with_retry from config import config +from telemetry_decorator import telemetry_tool + def register_read_console_tools(mcp: FastMCP): """Registers the read_console tool with the MCP server.""" @mcp.tool() + @telemetry_tool("read_console") def read_console( - ctx: Context, + ctx: Any, action: str = None, types: List[str] = None, count: int = None, diff --git a/UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py b/UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py index 909cb3c..38ea3f9 100644 --- a/UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py +++ b/UnityMcpBridge/UnityMcpServer~/src/tools/resource_tools.py @@ -5,7 +5,7 @@ safe path logic (re-implemented here to avoid importing server.py). """ from __future__ import annotations -from typing import Dict, Any, List +from typing import Dict, Any, List, Optional import re from pathlib import Path from urllib.parse import urlparse, unquote @@ -13,6 +13,8 @@ import fnmatch import hashlib import os +from telemetry_decorator import telemetry_tool + from mcp.server.fastmcp import FastMCP, Context from unity_connection import send_command_with_retry @@ -114,8 +116,9 @@ def register_resource_tools(mcp: FastMCP) -> None: "Security: restricted to Assets/ subtree; symlinks are resolved and must remain under Assets/.\n" "Notes: Only .cs files are returned by default; always appends unity://spec/script-edits.\n" )) + @telemetry_tool("list_resources") async def list_resources( - ctx: Context | None = None, + ctx: Any = None, pattern: str | None = "*.cs", under: str = "Assets", limit: int = 200, @@ -174,9 +177,10 @@ def register_resource_tools(mcp: FastMCP) -> None: "Security: uri must resolve under Assets/.\n" "Examples: head_bytes=1024; start_line=100,line_count=40; tail_lines=120.\n" )) + @telemetry_tool("read_resource") async def read_resource( uri: str, - ctx: Context | None = None, + ctx: Any = None, start_line: int | None = None, line_count: int | None = None, head_bytes: int | None = None, @@ -334,10 +338,11 @@ def register_resource_tools(mcp: FastMCP) -> None: return {"success": False, "error": str(e)} @mcp.tool() + @telemetry_tool("find_in_file") async def find_in_file( uri: str, pattern: str, - ctx: Context | None = None, + ctx: Any = None, ignore_case: bool | None = True, project_root: str | None = None, max_results: int | None = 1,