mirror of
https://github.com/katanemo/plano.git
synced 2026-06-17 15:25:17 +02:00
adjusting trace command for feedback on PR
This commit is contained in:
parent
0416573c82
commit
1506ac0546
4 changed files with 47 additions and 84 deletions
|
|
@ -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 <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():
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -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
|
||||
):
|
||||
|
|
|
|||
2
cli/uv.lock
generated
2
cli/uv.lock
generated
|
|
@ -337,7 +337,7 @@ wheels = [
|
|||
|
||||
[[package]]
|
||||
name = "planoai"
|
||||
version = "0.4.6"
|
||||
version = "0.4.7"
|
||||
source = { editable = "." }
|
||||
dependencies = [
|
||||
{ name = "click" },
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue