telemetry: prefer config with env override; validate scheme; robust load\n\n- TelemetryConfig reads config.telemetry_enabled/endpoint, env can override\n- Validate endpoint scheme; revalidate on send\n- Split UUID/milestones load error handling\n- Add tests for config precedence, scheme validation, UUID preservation\n- validate_script: optional include_diagnostics with documented behavior
parent
7f0527f708
commit
f6a5568865
|
|
@ -17,6 +17,7 @@ from urllib.parse import urlparse
|
||||||
from dataclasses import dataclass, asdict
|
from dataclasses import dataclass, asdict
|
||||||
from typing import Optional, Dict, Any, List
|
from typing import Optional, Dict, Any, List
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
import importlib
|
||||||
|
|
||||||
try:
|
try:
|
||||||
import httpx
|
import httpx
|
||||||
|
|
@ -61,11 +62,29 @@ class TelemetryRecord:
|
||||||
class TelemetryConfig:
|
class TelemetryConfig:
|
||||||
"""Telemetry configuration"""
|
"""Telemetry configuration"""
|
||||||
def __init__(self):
|
def __init__(self):
|
||||||
# Check environment variables for opt-out
|
# Prefer config file, then allow env overrides
|
||||||
self.enabled = not self._is_disabled()
|
server_config = None
|
||||||
|
for modname in (
|
||||||
|
"UnityMcpBridge.UnityMcpServer~.src.config",
|
||||||
|
"UnityMcpBridge.UnityMcpServer.src.config",
|
||||||
|
"src.config",
|
||||||
|
"config",
|
||||||
|
):
|
||||||
|
try:
|
||||||
|
mod = importlib.import_module(modname)
|
||||||
|
server_config = getattr(mod, "config", None)
|
||||||
|
if server_config is not None:
|
||||||
|
break
|
||||||
|
except Exception:
|
||||||
|
continue
|
||||||
|
|
||||||
|
# Determine enabled flag: config -> env DISABLE_* opt-out
|
||||||
|
cfg_enabled = True if server_config is None else bool(getattr(server_config, "telemetry_enabled", True))
|
||||||
|
self.enabled = cfg_enabled and not self._is_disabled()
|
||||||
|
|
||||||
# Telemetry endpoint (Cloud Run default; override via env)
|
# Telemetry endpoint (Cloud Run default; override via env)
|
||||||
default_ep = "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events"
|
cfg_default = None if server_config is None else getattr(server_config, "telemetry_endpoint", None)
|
||||||
|
default_ep = cfg_default or "https://unity-mcp-telemetry-375728817078.us-central1.run.app/telemetry/events"
|
||||||
self.default_endpoint = default_ep
|
self.default_endpoint = default_ep
|
||||||
self.endpoint = self._validated_endpoint(
|
self.endpoint = self._validated_endpoint(
|
||||||
os.environ.get("UNITY_MCP_TELEMETRY_ENDPOINT", default_ep),
|
os.environ.get("UNITY_MCP_TELEMETRY_ENDPOINT", default_ep),
|
||||||
|
|
@ -142,20 +161,31 @@ class TelemetryCollector:
|
||||||
|
|
||||||
def _load_persistent_data(self):
|
def _load_persistent_data(self):
|
||||||
"""Load UUID and milestones from disk"""
|
"""Load UUID and milestones from disk"""
|
||||||
|
# Load customer UUID
|
||||||
try:
|
try:
|
||||||
# Load customer UUID
|
|
||||||
if self.config.uuid_file.exists():
|
if self.config.uuid_file.exists():
|
||||||
self._customer_uuid = self.config.uuid_file.read_text().strip()
|
self._customer_uuid = self.config.uuid_file.read_text(encoding="utf-8").strip() or str(uuid.uuid4())
|
||||||
else:
|
else:
|
||||||
self._customer_uuid = str(uuid.uuid4())
|
self._customer_uuid = str(uuid.uuid4())
|
||||||
self.config.uuid_file.write_text(self._customer_uuid)
|
try:
|
||||||
|
self.config.uuid_file.write_text(self._customer_uuid, encoding="utf-8")
|
||||||
# Load milestones
|
if os.name == "posix":
|
||||||
if self.config.milestones_file.exists():
|
os.chmod(self.config.uuid_file, 0o600)
|
||||||
self._milestones = json.loads(self.config.milestones_file.read_text())
|
except OSError as e:
|
||||||
except Exception as e:
|
logger.debug(f"Failed to persist customer UUID: {e}", exc_info=True)
|
||||||
logger.warning(f"Failed to load telemetry data: {e}")
|
except OSError as e:
|
||||||
|
logger.debug(f"Failed to load customer UUID: {e}", exc_info=True)
|
||||||
self._customer_uuid = str(uuid.uuid4())
|
self._customer_uuid = str(uuid.uuid4())
|
||||||
|
|
||||||
|
# Load milestones (failure here must not affect UUID)
|
||||||
|
try:
|
||||||
|
if self.config.milestones_file.exists():
|
||||||
|
content = self.config.milestones_file.read_text(encoding="utf-8")
|
||||||
|
self._milestones = json.loads(content) or {}
|
||||||
|
if not isinstance(self._milestones, dict):
|
||||||
|
self._milestones = {}
|
||||||
|
except (OSError, json.JSONDecodeError, ValueError) as e:
|
||||||
|
logger.debug(f"Failed to load milestones: {e}", exc_info=True)
|
||||||
self._milestones = {}
|
self._milestones = {}
|
||||||
|
|
||||||
def _save_milestones(self):
|
def _save_milestones(self):
|
||||||
|
|
|
||||||
|
|
@ -409,13 +409,14 @@ def register_manage_script_tools(mcp: FastMCP):
|
||||||
|
|
||||||
@mcp.tool(description=(
|
@mcp.tool(description=(
|
||||||
"Validate a C# script and return diagnostics.\n\n"
|
"Validate a C# script and return diagnostics.\n\n"
|
||||||
"Args: uri, level=('basic'|'standard').\n"
|
"Args: uri, level=('basic'|'standard'), include_diagnostics (bool, optional).\n"
|
||||||
"- basic: quick syntax checks.\n"
|
"- basic: quick syntax checks.\n"
|
||||||
"- standard: deeper checks (performance hints, common pitfalls).\n"
|
"- standard: deeper checks (performance hints, common pitfalls).\n"
|
||||||
|
"- include_diagnostics: when true, returns full diagnostics and summary; default returns counts only.\n"
|
||||||
))
|
))
|
||||||
@telemetry_tool("validate_script")
|
@telemetry_tool("validate_script")
|
||||||
def validate_script(
|
def validate_script(
|
||||||
ctx: Context, uri: str, level: str = "basic"
|
ctx: Context, uri: str, level: str = "basic", include_diagnostics: bool = False
|
||||||
) -> Dict[str, Any]:
|
) -> Dict[str, Any]:
|
||||||
"""Validate a C# script and return diagnostics."""
|
"""Validate a C# script and return diagnostics."""
|
||||||
name, directory = _split_uri(uri)
|
name, directory = _split_uri(uri)
|
||||||
|
|
@ -431,9 +432,11 @@ def register_manage_script_tools(mcp: FastMCP):
|
||||||
}
|
}
|
||||||
resp = send_command_with_retry("manage_script", params)
|
resp = send_command_with_retry("manage_script", params)
|
||||||
if isinstance(resp, dict) and resp.get("success"):
|
if isinstance(resp, dict) and resp.get("success"):
|
||||||
diags = resp.get("data", {}).get("diagnostics", [])
|
diags = resp.get("data", {}).get("diagnostics", []) or []
|
||||||
warnings = sum(1 for d in diags if str(d.get("severity", "")).lower() in ("warning",))
|
warnings = sum(1 for d in diags if str(d.get("severity", "")).lower() == "warning")
|
||||||
errors = sum(1 for d in diags if str(d.get("severity", "")).lower() in ("error", "fatal"))
|
errors = sum(1 for d in diags if str(d.get("severity", "")).lower() in ("error", "fatal"))
|
||||||
|
if include_diagnostics:
|
||||||
|
return {"success": True, "data": {"diagnostics": diags, "summary": {"warnings": warnings, "errors": errors}}}
|
||||||
return {"success": True, "data": {"warnings": warnings, "errors": errors}}
|
return {"success": True, "data": {"warnings": warnings, "errors": errors}}
|
||||||
return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}
|
return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}
|
||||||
|
|
||||||
|
|
@ -573,7 +576,6 @@ def register_manage_script_tools(mcp: FastMCP):
|
||||||
|
|
||||||
@mcp.tool(description=(
|
@mcp.tool(description=(
|
||||||
"Get manage_script capabilities (supported ops, limits, and guards).\n\n"
|
"Get manage_script capabilities (supported ops, limits, and guards).\n\n"
|
||||||
"Args:\n- random_string: required parameter (any string value)\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"
|
"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"
|
||||||
))
|
))
|
||||||
@telemetry_tool("manage_script_capabilities")
|
@telemetry_tool("manage_script_capabilities")
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,56 @@
|
||||||
|
import os
|
||||||
|
import importlib
|
||||||
|
|
||||||
|
def test_endpoint_rejects_non_http(tmp_path, monkeypatch):
|
||||||
|
# Point data dir to temp to avoid touching real files
|
||||||
|
monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
|
||||||
|
monkeypatch.setenv("UNITY_MCP_TELEMETRY_ENDPOINT", "file:///etc/passwd")
|
||||||
|
|
||||||
|
telemetry = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.telemetry")
|
||||||
|
importlib.reload(telemetry)
|
||||||
|
|
||||||
|
tc = telemetry.TelemetryCollector()
|
||||||
|
# Should have fallen back to default endpoint
|
||||||
|
assert tc.config.endpoint == tc.config.default_endpoint
|
||||||
|
|
||||||
|
def test_config_preferred_then_env_override(tmp_path, monkeypatch):
|
||||||
|
# Simulate config telemetry endpoint
|
||||||
|
monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
|
||||||
|
monkeypatch.delenv("UNITY_MCP_TELEMETRY_ENDPOINT", raising=False)
|
||||||
|
|
||||||
|
# Patch config.telemetry_endpoint via import mocking
|
||||||
|
import importlib
|
||||||
|
cfg_mod = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.config")
|
||||||
|
old_endpoint = cfg_mod.config.telemetry_endpoint
|
||||||
|
cfg_mod.config.telemetry_endpoint = "https://example.com/telemetry"
|
||||||
|
try:
|
||||||
|
telemetry = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.telemetry")
|
||||||
|
importlib.reload(telemetry)
|
||||||
|
tc = telemetry.TelemetryCollector()
|
||||||
|
assert tc.config.endpoint == "https://example.com/telemetry"
|
||||||
|
|
||||||
|
# Env should override config
|
||||||
|
monkeypatch.setenv("UNITY_MCP_TELEMETRY_ENDPOINT", "https://override.example/ep")
|
||||||
|
importlib.reload(telemetry)
|
||||||
|
tc2 = telemetry.TelemetryCollector()
|
||||||
|
assert tc2.config.endpoint == "https://override.example/ep"
|
||||||
|
finally:
|
||||||
|
cfg_mod.config.telemetry_endpoint = old_endpoint
|
||||||
|
|
||||||
|
def test_uuid_preserved_on_malformed_milestones(tmp_path, monkeypatch):
|
||||||
|
monkeypatch.setenv("XDG_DATA_HOME", str(tmp_path))
|
||||||
|
|
||||||
|
telemetry = importlib.import_module("UnityMcpBridge.UnityMcpServer~.src.telemetry")
|
||||||
|
importlib.reload(telemetry)
|
||||||
|
|
||||||
|
tc1 = telemetry.TelemetryCollector()
|
||||||
|
first_uuid = tc1._customer_uuid
|
||||||
|
|
||||||
|
# Write malformed milestones
|
||||||
|
tc1.config.milestones_file.write_text("{not-json}", encoding="utf-8")
|
||||||
|
|
||||||
|
# Reload collector; UUID should remain same despite bad milestones
|
||||||
|
importlib.reload(telemetry)
|
||||||
|
tc2 = telemetry.TelemetryCollector()
|
||||||
|
assert tc2._customer_uuid == first_uuid
|
||||||
|
|
||||||
Loading…
Reference in New Issue