-
Notifications
You must be signed in to change notification settings - Fork 83
feat(clp-package)!: Add profile setting inside CLPConfig. #1488
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 29 commits
752ab1a
29436b6
4d19c81
b5bdcf1
02b29fb
6f65035
c2be1f6
a677177
cc85f93
9810d31
b013429
a33a9c1
99d134b
61e8332
ab6ee4f
cbb32de
8447f12
159e645
4080df6
7df8e09
4089eb0
11641e2
9437246
ada4073
e94d2cc
8beeda2
e6dfbea
5501b7f
555592f
24d8213
3ec8d9f
e9cba89
344b018
1d9a7be
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,260 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Profiling utilities for CLP query execution performance analysis. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| This module provides lightweight profiling decorators using pyinstrument. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Profile outputs include: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - HTML files with interactive flame graphs and call trees. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - Text summaries showing call hierarchy and timing. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import datetime | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import functools | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import inspect | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from collections.abc import Callable | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pathlib import Path | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Any, TypeVar | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from pyinstrument import Profiler | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from clp_py_utils.clp_logging import get_logger | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger = get_logger("profiler") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| F = TypeVar("F", bound=Callable[..., Any]) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| PROFILING_INTERVAL_SECONDS = 0.001 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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. 🧹 Nitpick | 🔵 Trivial Make sampling interval configurable via env. Allows tuning overhead without code changes. -PROFILING_INTERVAL_SECONDS = 0.001
+PROFILING_INTERVAL_SECONDS = float(os.getenv("CLP_PROFILING_INTERVAL_SECONDS", "0.001"))📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def profile( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| section_name: str | None = None, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job_id_param: str = "job_id", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task_id_param: str = "task_id", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> Callable[[F], F]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Profiles function execution as decorator with automatic context extraction. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Output files are written to $CLP_LOGS_DIR/profiles/ (e.g., clp-package/var/log/query_worker/ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiles/). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param section_name: Override for profile section name. If None, uses function name. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param job_id_param: Parameter name to extract job_id from (default: "job_id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Can use dot notation for attributes, e.g., "job.id". | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param task_id_param: Parameter name to extract task_id from (default: "task_id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Can use dot notation for attributes, e.g., "task.id". | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :return: Decorated function with profiling capabilities. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def decorator(func: F) -> F: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name = section_name or func.__name__ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| is_async = inspect.iscoroutinefunction(func) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if is_async: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @functools.wraps(func) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| async def async_wrapper(*args: Any, **kwargs: Any) -> Any: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not _is_profiling_enabled(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return await func(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Profiling enabled: extract context and profile execution | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job_id, task_id = _extract_context_from_args( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func, args, kwargs, job_id_param, task_id_param | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler = Profiler(interval=PROFILING_INTERVAL_SECONDS) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler.start() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except RuntimeError as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Skip profiling this function to avoid conflicts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "already a profiler running" in str(e): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"Skipping nested profiling for {name} " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"(parent profiler already active)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return await func(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = await func(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler.stop() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _save_profile(profiler, name, job_id, task_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return async_wrapper # type: ignore | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| @functools.wraps(func) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def sync_wrapper(*args: Any, **kwargs: Any) -> Any: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not _is_profiling_enabled(): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return func(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Profiling enabled: extract context and profile execution | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job_id, task_id = _extract_context_from_args( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func, args, kwargs, job_id_param, task_id_param | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler = Profiler(interval=PROFILING_INTERVAL_SECONDS) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler.start() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except RuntimeError as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Skip profiling this function to avoid conflicts | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if "already a profiler running" in str(e): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"Skipping nested profiling for {name} (parent profiler already active)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return func(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| raise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| result = func(*args, **kwargs) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return result | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler.stop() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _save_profile(profiler, name, job_id, task_id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return sync_wrapper # type: ignore | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return decorator | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _extract_context_from_args( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| func: Callable, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args: tuple, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| kwargs: dict, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job_id_param: str = "job_id", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task_id_param: str = "task_id", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> tuple[str, str]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Extracts job_id and task_id from function arguments. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param func: The function being profiled. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param args: Positional arguments passed to the function. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param kwargs: Keyword arguments passed to the function. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param job_id_param: Name/path of the parameter containing job_id (default: "job_id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param task_id_param: Name/path of the parameter containing task_id (default: "task_id"). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :return: tuple of (job_id, task_id) as strings. Empty strings if not found. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job_id = "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task_id = "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Get function signature | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sig = inspect.signature(func) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| param_names = list(sig.parameters.keys()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def extract_value(param_spec: str) -> str: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Extract value from parameter, supporting dot notation for attributes.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not param_spec: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Split on '.' to handle attribute access | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| parts = param_spec.split(".") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| param_name = parts[0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Find the parameter value | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if param_name in kwargs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value = kwargs[param_name] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| elif param_name in param_names: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| idx = param_names.index(param_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if idx < len(args): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value = args[idx] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if value is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Navigate through attributes if dot notation was used | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for attr_name in parts[1:]: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if hasattr(value, attr_name): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| value = getattr(value, attr_name) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return str(value) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Extract job_id and task_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| job_id = extract_value(job_id_param) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| task_id = extract_value(task_id_param) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug(f"Failed to extract context from {func.__name__}: {e}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return job_id, task_id | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _is_profiling_enabled() -> bool: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Checks if profiling is enabled. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :return: If `CLP_ENABLE_PROFILING` environment variable is set to `true`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profile_enabled = os.getenv("CLP_ENABLE_PROFILING") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
sitaowang1998 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if profile_enabled is not None and profile_enabled.lower() == "true": | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return True | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _save_profile( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| profiler: Profiler, section_name: str, job_id: str = "", task_id: str = "" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) -> None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Saves profiler output to HTML and text formats. Generates .html and .txt files. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param profiler: The pyinstrument Profiler object containing profiling data. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param section_name: Name identifying this profiling section. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param job_id: Optional job identifier for filename. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| :param task_id: Optional task identifier for filename. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Get the session for logging | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| session = profiler.last_session | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if not session: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.debug(f"No profiling session for {section_name}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| duration = session.duration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| sample_count = session.sample_count | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S_%f") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filename_parts = [section_name] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if job_id: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filename_parts.append(f"job{job_id}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if task_id: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filename_parts.append(f"task{task_id}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| filename_parts.append(timestamp) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| base_filename = "_".join(filename_parts) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+219
to
+229
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. 🧹 Nitpick | 🔵 Trivial Prefer timezone-aware timestamps for filenames. Ensures clarity across hosts and logs. - timestamp = datetime.datetime.now().strftime("%Y%m%d_%H%M%S_%f")
+ timestamp = datetime.datetime.now(datetime.timezone.utc).strftime("%Y%m%d_%H%M%S_%fZ")Confirm pyinstrument 5.1.x exposes session.sample_count (line 217) as used here. 📝 Committable suggestion
Suggested change
🧰 Tools🪛 Ruff (0.14.1)219-219: (DTZ005) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| output_dir = Path(os.getenv("CLP_LOGS_DIR")) / "profiles" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| output_dir.mkdir(exist_ok=True, parents=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Save HTML with interactive visualization | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| html_path = output_dir / f"{base_filename}.html" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open(html_path, "w", encoding="utf-8") as f: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(profiler.output_html()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Save human-readable text summary with call hierarchy | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| txt_path = output_dir / f"{base_filename}.txt" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with open(txt_path, "w", encoding="utf-8") as f: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Header | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+230
to
+241
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. Guard against missing CLP_LOGS_DIR (prevents crash). Path(None) will raise when CLP_LOGS_DIR is unset. Provide a fallback and warn. - output_dir = Path(os.getenv("CLP_LOGS_DIR")) / "profiles"
- output_dir.mkdir(exist_ok=True, parents=True)
+ logs_dir_env = os.getenv("CLP_LOGS_DIR")
+ base_dir = Path(logs_dir_env) if logs_dir_env else Path.cwd()
+ if not logs_dir_env:
+ logger.warning("CLP_LOGS_DIR not set; writing profiles under %s/profiles", base_dir)
+ output_dir = base_dir / "profiles"
+ output_dir.mkdir(exist_ok=True, parents=True)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write("=" * 80 + "\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(f"CLP Query Profiling Report (pyinstrument)\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(f"Section: {section_name}\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if job_id: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(f"Job ID: {job_id}\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if task_id: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(f"Task ID: {task_id}\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(f"Timestamp: {timestamp}\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write("=" * 80 + "\n\n") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f.write(profiler.output_text(unicode=True, color=False)) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.info( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"Profile saved: {section_name} " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"(duration={duration:.6f}s, samples={sample_count}) " | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| f"HTML={html_path}, TXT={txt_path}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| except Exception as e: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| logger.error(f"Failed to save profile for {section_name}: {e}", exc_info=True) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ dependencies = [ | |||||
| "mariadb>=1.0.11,<1.1.dev0", | ||||||
| "mysql-connector-python>=9.4.0", | ||||||
| "pydantic>=2.12.3", | ||||||
| "pyinstrument>=5.1.1", | ||||||
|
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. 🧹 Nitpick | 🔵 Trivial Pin pyinstrument to major and consider optionality. To avoid unexpected breaks on future major releases, pin the upper bound. If you want to keep profiling optional, pair this with a lazy import (see profiling_utils note). Apply: - "pyinstrument>=5.1.1",
+ "pyinstrument>=5.1.1,<6",📝 Committable suggestion
Suggested change
|
||||||
| "python-Levenshtein>=0.27.1", | ||||||
| "PyYAML>=6.0.3", | ||||||
| "result>=0.17.0", | ||||||
|
|
||||||
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.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
Add field description for clarity and ensure env plumbing is present.
Document the new config knob.
class QueryWorker(BaseModel): logging_level: LoggingLevel = "INFO" - enable_profiling: bool = False + enable_profiling: bool = Field( + False, + description="Enable pyinstrument runtime profiling for query worker tasks." + )Run to confirm the plumbing from config -> env -> container:
🏁 Script executed:
Length of output: 2231
Add field description for documentation clarity.
The config → env plumbing is already correctly implemented and verified. Consider adding a Field description to document the profiling knob:
[optional_refactor_recommended]
class QueryWorker(BaseModel): logging_level: LoggingLevel = "INFO" - enable_profiling: bool = False + enable_profiling: bool = Field( + False, + description="Enable pyinstrument runtime profiling for query worker tasks." + )📝 Committable suggestion
🤖 Prompt for AI Agents