diff --git a/cli/planoai/trace_cmd.py b/cli/planoai/trace_cmd.py index 8f3dd348..42538434 100644 --- a/cli/planoai/trace_cmd.py +++ b/cli/planoai/trace_cmd.py @@ -6,6 +6,7 @@ import subprocess import sys import threading import time +from http import HTTPStatus from collections import OrderedDict from concurrent import futures from dataclasses import dataclass @@ -38,7 +39,7 @@ class TraceListenerBindError(RuntimeError): def _trace_listener_bind_error_message(address: str) -> str: return ( f"Failed to start OTLP listener on {address}: address is already in use.\n" - "Stop the process using that port or run `planoai trace listen --port `." + "Stop the process using that port or run `planoai trace listen`." ) @@ -60,13 +61,13 @@ class TraceSummary: return dt.astimezone().strftime("%Y-%m-%d %H:%M:%S") -def _is_port_in_use(port: int) -> bool: - """Check whether a local TCP listener is accepting connections on a port.""" +def _is_port_in_use(host: str, port: int) -> bool: + """Check whether a TCP listener is accepting connections on host:port.""" import socket with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: s.settimeout(0.2) - return s.connect_ex(("127.0.0.1", port)) == 0 + return s.connect_ex((host, port)) == 0 def _get_listener_pid() -> int | None: @@ -546,7 +547,7 @@ def _start_trace_listener(host: str, grpc_port: int) -> None: console = Console() # Check if the requested port is already in use. - if _is_port_in_use(grpc_port): + if _is_port_in_use(host, grpc_port): existing_pid = _get_listener_pid() if existing_pid: # If the process PID is known, inform user that our listener is already running. @@ -578,7 +579,7 @@ def _start_trace_listener(host: str, grpc_port: int) -> None: # In the parent process: wait briefly for the background process to bind the port. time.sleep(0.5) # Give child process time to start and bind to the port. - if _is_port_in_use(grpc_port): + if _is_port_in_use(host, grpc_port): # Success: the trace listener started and bound the port. console.print() console.print( @@ -666,30 +667,14 @@ def _error_description(status_code: str) -> str: """Return a developer-friendly description of the error.""" code = int(status_code) if status_code.isdigit() else 0 - if code == 500: - return "Internal Server Error" - elif code == 502: - return "Bad Gateway" - elif code == 503: - return "Service Unavailable" - elif code == 504: - return "Gateway Timeout" - elif code >= 500: - return "Server Error" - elif code == 429: - return "Rate Limit Exceeded" - elif code == 404: - return "Not Found" - elif code == 403: - return "Forbidden - Access Denied" - elif code == 401: - return "Unauthorized - Auth Required" - elif code == 400: - return "Bad Request" - elif code >= 400: - return "Client Error" - else: + if code < 400: return "Error" + try: + return HTTPStatus(code).phrase + except ValueError: + if code >= 500: + return "Server Error" + return "Client Error" def _detect_error(span: dict[str, Any]) -> tuple[bool, str, str]: @@ -1097,14 +1082,6 @@ def _run_trace_show( @click.option("--limit", type=int, default=None, help="Limit results.") @click.option("--since", default=None, help="Look back window (e.g. 5m, 2h, 1d).") @click.option("--json", "json_out", is_flag=True, help="Output raw JSON.") -@click.option("--host", default="0.0.0.0", show_default=True) -@click.option( - "--port", - type=int, - default=DEFAULT_GRPC_PORT, - show_default=True, - help="gRPC port for receiving OTLP traces when target is 'listen'.", -) @click.option( "--verbose", "-v", @@ -1122,8 +1099,6 @@ def trace( limit, since, json_out, - host, - port, verbose, ): """Trace requests from the local OTLP listener.""" @@ -1142,12 +1117,9 @@ def trace( ) if target == "listen" and not has_show_options: - _start_trace_listener(host, port) + _start_trace_listener("0.0.0.0", DEFAULT_GRPC_PORT) return - if (host != "0.0.0.0" or port != DEFAULT_GRPC_PORT) and target != "listen": - raise click.ClickException("--host/--port are only valid with target 'listen'.") - if target in ("stop", "down") and not has_show_options: console = Console() if _stop_background_listener(): diff --git a/cli/planoai/trace_listener_runtime.py b/cli/planoai/trace_listener_runtime.py index 5ddeb6ee..cca65289 100644 --- a/cli/planoai/trace_listener_runtime.py +++ b/cli/planoai/trace_listener_runtime.py @@ -5,10 +5,13 @@ Trace listener process runtime utilities. import os import signal import time +import logging from collections.abc import Callable # Canonical PID file used by `planoai trace listen/down`. TRACE_LISTENER_PID_PATH = os.path.expanduser("~/.plano/run/trace_listener.pid") +TRACE_LISTENER_LOG_PATH = os.path.expanduser("~/.plano/run/trace_listener.log") +LOGGER = logging.getLogger(__name__) def write_listener_pid(pid: int) -> None: @@ -40,6 +43,10 @@ def get_listener_pid() -> int | None: return pid except (ValueError, ProcessLookupError, OSError): # Stale or malformed PID file: clean it up to prevent repeated confusion. + LOGGER.warning( + "Removing stale or malformed trace listener PID file at %s", + TRACE_LISTENER_PID_PATH, + ) remove_listener_pid() return None @@ -92,14 +99,26 @@ def daemonize_and_run(run_forever: Callable[[], None]) -> int | None: # the daemon cannot reacquire a controlling terminal. os.setsid() - # Redirect stdin/stdout/stderr to /dev/null so daemon is terminal-independent. - # This prevents broken pipe errors and ensures no output leaks to the parent terminal. - devnull = os.open(os.devnull, os.O_RDWR) - os.dup2(devnull, 0) # stdin - os.dup2(devnull, 1) # stdout - os.dup2(devnull, 2) # stderr - if devnull > 2: - os.close(devnull) + # Redirect stdin to /dev/null and stdout/stderr to a persistent log file. + # This keeps the daemon terminal-independent while preserving diagnostics. + os.makedirs(os.path.dirname(TRACE_LISTENER_LOG_PATH), exist_ok=True) + devnull_in = os.open(os.devnull, os.O_RDONLY) + try: + log_fd = os.open( + TRACE_LISTENER_LOG_PATH, + os.O_WRONLY | os.O_CREAT | os.O_APPEND, + 0o644, + ) + except OSError: + # If logging cannot be initialized, keep running with output discarded. + log_fd = os.open(os.devnull, os.O_WRONLY) + os.dup2(devnull_in, 0) # stdin + os.dup2(log_fd, 1) # stdout + os.dup2(log_fd, 2) # stderr + if devnull_in > 2: + os.close(devnull_in) + if log_fd > 2: + os.close(log_fd) # Run the daemon main loop (expected to block until process termination). run_forever() diff --git a/cli/test/test_trace_cmd.py b/cli/test/test_trace_cmd.py index 64fb4f68..fdcf8c3c 100644 --- a/cli/test/test_trace_cmd.py +++ b/cli/test/test_trace_cmd.py @@ -89,12 +89,10 @@ def test_start_trace_server_raises_bind_error(monkeypatch): trace_cmd._start_trace_server("0.0.0.0", 4317) assert "already in use" in str(excinfo.value) - assert "planoai trace listen --port" in str(excinfo.value) + assert "planoai trace listen" in str(excinfo.value) -def test_trace_listen_starts_listener_with_custom_bind_after_target( - runner, monkeypatch -): +def test_trace_listen_starts_listener_with_defaults(runner, monkeypatch): seen = {} def fake_start(host: str, port: int) -> None: @@ -103,27 +101,10 @@ def test_trace_listen_starts_listener_with_custom_bind_after_target( monkeypatch.setattr(trace_cmd, "_start_trace_listener", fake_start) - result = runner.invoke(trace, ["listen", "--host", "127.0.0.1", "--port", "9876"]) + result = runner.invoke(trace, ["listen"]) assert result.exit_code == 0, result.output - assert seen == {"host": "127.0.0.1", "port": 9876} - - -def test_trace_listen_starts_listener_with_custom_bind_before_target( - runner, monkeypatch -): - seen = {} - - def fake_start(host: str, port: int) -> None: - seen["host"] = host - seen["port"] = port - - monkeypatch.setattr(trace_cmd, "_start_trace_listener", fake_start) - - result = runner.invoke(trace, ["--host", "127.0.0.1", "--port", "9876", "listen"]) - - assert result.exit_code == 0, result.output - assert seen == {"host": "127.0.0.1", "port": 9876} + assert seen == {"host": "0.0.0.0", "port": trace_cmd.DEFAULT_GRPC_PORT} def test_trace_down_prints_success_when_listener_stopped(runner, monkeypatch): @@ -144,15 +125,6 @@ def test_trace_down_prints_no_listener_when_not_running(runner, monkeypatch): assert "No background trace listener running" in result.output -def test_trace_host_port_requires_listen_target(runner): - result = runner.invoke(trace, ["--host", "127.0.0.1", "any"]) - - assert result.exit_code != 0 - assert "--host/--port are only valid with target 'listen'." in _plain_output( - result.output - ) - - def test_trace_default_target_uses_last_and_builds_first_trace( runner, monkeypatch, traces ): diff --git a/cli/uv.lock b/cli/uv.lock index 5f18604b..45ccf82e 100644 --- a/cli/uv.lock +++ b/cli/uv.lock @@ -337,7 +337,7 @@ wheels = [ [[package]] name = "planoai" -version = "0.4.6" +version = "0.4.7" source = { editable = "." } dependencies = [ { name = "click" },