Conversation
Implements a new Cache panel that tracks cache operations during requests: - Automatic patching of Redis and pymemcache clients - Tracks GET, SET, DELETE, INCR, DECR, MGET, MSET operations - Records hit/miss status, duration, and backend type - Provides aggregate statistics (hit rate, avg latency) - Server-Timing header support for browser DevTools - Both Redis and memcached are optional dependencies 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements a new Profiling panel for request performance analysis: - Dual backend support: cProfile (stdlib) and pyinstrument (optional) - Configurable top functions limit and sort order - Tracks function calls, total time, and cumulative time - Call tree visualization for pyinstrument backend - Server-Timing header for profiling overhead metrics - Automatic fallback from pyinstrument to cProfile if unavailable 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements a new Templates panel for template rendering analysis: - Support for Jinja2 and Mako template engines (optional deps) - Tracks template name, render time, and context variables - Transparent method patching during request lifecycle - Aggregate statistics: total renders, total time, engines used - Server-Timing header for template rendering metrics - Clean patch/unpatch lifecycle per request 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements a new Headers panel for comprehensive header analysis: - Categorizes headers: authentication, content, caching, CORS, security, custom - Parses Authorization headers with token redaction for security - Decodes Basic auth (shows username, redacts password) - Analyzes Cookie headers (count and names) - Security header detection with missing header recommendations - Cache-Control directive parsing and analysis - Passive panel - no request/response hooks needed 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements a new Settings panel for configuration introspection: - Displays debug toolbar configuration - Shows environment variables with automatic sensitive value redaction - Exposes Python runtime settings (debug mode, optimize level, sys.path) - Supports custom application settings injection - Configurable sensitive key patterns (PASSWORD, SECRET, KEY, TOKEN, etc.) - Case-insensitive pattern matching for security - Nested dictionary support for sensitive value detection 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements Phase 10 of the debug toolbar development by adding 5 new panels (Cache, Profiling, Templates, Headers, and Settings) to enhance debugging capabilities. The implementation follows the established Panel base class pattern and includes comprehensive test coverage with 135+ new tests.
- Adds tracking for cache operations (Redis/memcached), request profiling (cProfile/pyinstrument), template rendering (Jinja2/Mako), HTTP headers analysis, and application settings
- Implements Server-Timing header support for all new panels
- Includes working examples demonstrating each panel's functionality
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/debug_toolbar/core/panels/cache.py | Implements CachePanel with Redis/memcached operation tracking via method patching |
| src/debug_toolbar/core/panels/profiling.py | Implements ProfilingPanel with dual backend support (cProfile/pyinstrument) |
| src/debug_toolbar/core/panels/templates.py | Implements TemplatesPanel with Jinja2/Mako template rendering tracking |
| src/debug_toolbar/core/panels/headers.py | Implements HeadersPanel with HTTP header analysis and security validation |
| src/debug_toolbar/core/panels/settings.py | Implements SettingsPanel with sensitive data redaction for configuration display |
| src/debug_toolbar/core/panels/init.py | Updates exports to include all new panels |
| tests/unit/test_cache_panel.py | Comprehensive unit tests for CachePanel (160+ assertions) |
| tests/unit/test_profiling_panel.py | Unit tests for ProfilingPanel covering both backends |
| tests/unit/test_templates_panel.py | Unit tests for TemplatesPanel with engine patching tests |
| tests/unit/test_headers_panel.py | Unit tests for HeadersPanel including security analysis |
| tests/unit/test_settings_panel.py | Unit tests for SettingsPanel with redaction validation |
| tests/integration/test_settings_panel_integration.py | Integration tests for SettingsPanel with toolbar |
| examples/*.py | Working examples demonstrating each panel's usage |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address PR review feedback: - Standardize panel_id values to match existing panel naming convention (e.g., "CachePanel", "HeadersPanel", "SettingsPanel", "TemplatesPanel") - Fix integration test that wasn't properly testing custom settings - Update all related unit tests for new panel_id values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Responses to Review CommentsComment 1 & 2 (FBT001 - Boolean Parameters in cache.py)The Comment 3 (Integration Test Issue)✅ Fixed in commit 665820e - The test was indeed creating a panel with one toolbar but then using it in a different context. Simplified the test to properly instantiate the panel with the toolbar it uses. Comment 4 (panel_id Naming Convention)✅ Fixed in commit 665820e - Standardized all new panel_id values to use PascalCase with "Panel" suffix to match existing panels:
Updated all related unit and integration tests accordingly. |
- Use ContextVar for async-safe tracker storage (race condition fix) - Add warning about private method usage in example - Remove unused CachePanel import from example - Remove unused Jinja2Environment import - Add explanatory comment to except pass in test fixture 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
JacobCoffee
left a comment
JacobCoffee
left a comment
There was a problem hiding this comment.
PR Comment Responses
Comments 1, 2, 7 (FBT001 - boolean params): The check_hit parameter comes from tuple values in methods_to_patch dict. Refactoring to keyword-only would require restructuring the data model for marginal benefit in this internal helper. Noqa is appropriate here.
Comment 3 (integration test): ✅ Fixed in 665820e
Comments 4, 5, 6, 10 (panel_id naming): ✅ Fixed in 665820e - standardized to PascalCase
Comment 8 (ContextVar race condition): ✅ Fixed in c1c9834 - now uses contextvars.ContextVar
Comment 9 (private method in example): ✅ Fixed in c1c9834 - added warning docstring
Comment 11 (unused CachePanel import): ✅ Fixed in c1c9834
Comments 12, 13, 14 (unused imports): ✅ Fixed in c1c9834 - removed Jinja2Environment
Comment 15 (except pass comment): ✅ Fixed in c1c9834
- litestar_basic: Add Headers, Settings, Templates panels with Jinja2 demos - litestar_advanced_alchemy: Add Headers, Settings, Profiling panels - Update about page to list all available panels 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add thread locks to cache panel patching to prevent race conditions - Add thread locks to templates panel patching for Jinja2/Mako - Use ContextVar instead of global for async-safe cache tracker - Fix broad exception handler in headers.py to catch specific exceptions - Add warning log when profiler enablement fails - Fix except clause in profiling test to catch specific exceptions - Update cache panel demo to use public track_operation API - Remove unused Jinja2Environment import 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- CachePanel: "cache" -> "CachePanel" - HeadersPanel: "headers" -> "HeadersPanel" - TemplatesPanel: "templates" -> "TemplatesPanel" These were missed in the previous naming fix commit. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add warning log when panel import fails (helps debug config issues) - Add tests to verify string-based extra_panels work correctly - Test that invalid import paths log warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Refactored TemplatesPanel to use module-level ContextVar approach for tracking template renders. This ensures proper isolation when multiple requests are processed concurrently in parallel tests. - Changed from instance-level patching to module-level singleton patches - Active tracker stored in ContextVar for async-safe request isolation - Simplified TemplateRenderTracker class (removed patch methods) - Updated tests to reflect new ContextVar-based architecture - Added test for parallel tracker isolation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def _patch_jinja2() -> None: | ||
| """Patch Jinja2 template rendering to track renders via ContextVar.""" | ||
| global _original_jinja2_render, _jinja2_patched # noqa: PLW0603 | ||
|
|
||
| try: | ||
| from jinja2 import Template as Jinja2Template | ||
| except ImportError: | ||
| return | ||
|
|
||
| with _patch_lock: | ||
| if _jinja2_patched: | ||
| return | ||
|
|
||
| _original_jinja2_render = Jinja2Template.render | ||
|
|
||
| def tracked_render(template_self: Jinja2Template, *args: Any, **kwargs: Any) -> str: | ||
| tracker = _active_tracker.get() | ||
|
|
||
| start_time = time.perf_counter() | ||
|
|
||
| context_keys = None | ||
| if args and isinstance(args[0], dict): | ||
| context_keys = list(args[0].keys()) | ||
| elif kwargs: | ||
| context_keys = list(kwargs.keys()) | ||
|
|
||
| result = _original_jinja2_render(template_self, *args, **kwargs) | ||
|
|
||
| render_time = time.perf_counter() - start_time | ||
|
|
||
| if tracker is not None: | ||
| template_name = getattr(template_self, "name", None) or getattr(template_self, "filename", "<string>") | ||
| tracker.track_render( | ||
| template_name=template_name, | ||
| engine="jinja2", | ||
| render_time=render_time, | ||
| context_keys=context_keys, | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| Jinja2Template.render = tracked_render # type: ignore[method-assign] | ||
| _jinja2_patched = True |
There was a problem hiding this comment.
Similar to the cache panel, the monkey-patching of Jinja2 and Mako Template.render methods modifies global class objects. While ContextVar is used for tracking isolation, the patching itself is global and permanent after first use. This could cause issues:
- The original render method is never restored (unlike cache panel which has unpatch methods)
- All template renders globally will incur tracking overhead even when no tracker is active
- Multiple TemplatesPanel instances will all use the same patched methods
Consider implementing unpatch functionality in process_response or clearly documenting that this is a one-time global modification.
| def _process_custom_settings(self, settings: dict[str, Any] | None) -> dict[str, Any] | None: | ||
| """Process custom settings and redact sensitive values. | ||
|
|
||
| Args: | ||
| settings: Custom settings dictionary. | ||
|
|
||
| Returns: | ||
| Processed settings with sensitive values redacted, or None. | ||
| """ | ||
| if settings is None: | ||
| return None | ||
|
|
||
| processed = {} | ||
| for key, value in settings.items(): | ||
| if isinstance(value, dict): | ||
| processed[key] = self._process_custom_settings(value) | ||
| else: | ||
| processed[key] = self._redact_sensitive_value(key, value) | ||
|
|
||
| return processed |
There was a problem hiding this comment.
The recursion in _process_custom_settings lacks a maximum depth check, which could lead to a RecursionError for deeply nested dictionaries. Consider adding a depth parameter with a maximum limit to prevent stack overflow.
Example fix:
def _process_custom_settings(self, settings: dict[str, Any] | None, depth: int = 0, max_depth: int = 20) -> dict[str, Any] | None:
if settings is None or depth >= max_depth:
return None
processed = {}
for key, value in settings.items():
if isinstance(value, dict):
processed[key] = self._process_custom_settings(value, depth + 1, max_depth)
else:
processed[key] = self._redact_sensitive_value(key, value)
return processed| functions: list[dict[str, Any]] = [] | ||
|
|
||
| def collect_frames(frame: Any, depth: int = 0) -> None: | ||
| if depth > MAX_RECURSION_DEPTH: |
There was a problem hiding this comment.
The recursion in collect_frames within _extract_pyinstrument_functions checks depth > MAX_RECURSION_DEPTH, but this will still allow 101 levels of recursion before stopping (MAX_RECURSION_DEPTH is 100). The check should be >= to properly enforce the limit at 100 levels.
| if depth > MAX_RECURSION_DEPTH: | |
| if depth >= MAX_RECURSION_DEPTH: |
|
|
||
| async def process_request(self, context: RequestContext) -> None: | ||
| """Start profiling at request start.""" | ||
| import time |
There was a problem hiding this comment.
[nitpick] The time module is imported inside the process_request method instead of at the module level. This is unnecessary since time is a standard library module with no import overhead. Consider moving this import to the top of the file for better code organization and consistency with other module imports.
Suggested change:
# At the top of the file (after other imports)
import time|
|
||
| async def process_response(self, context: RequestContext) -> None: | ||
| """Stop profiling at response completion.""" | ||
| import time |
There was a problem hiding this comment.
[nitpick] The time module is imported inside the process_response method instead of at the module level. This is unnecessary since time is a standard library module with no import overhead. Consider moving this import to the top of the file for better code organization and consistency. The same time module is already imported in process_request, so consolidating the import at the module level makes sense.
| def _patch_redis(self) -> None: | ||
| """Patch Redis client methods to track operations.""" | ||
| try: | ||
| import redis # type: ignore[import-untyped] | ||
| except ImportError: | ||
| return | ||
|
|
||
| if hasattr(redis.Redis, "_debug_toolbar_patched"): | ||
| return | ||
|
|
||
| methods_to_patch = { | ||
| "get": ("GET", True), | ||
| "set": ("SET", False), | ||
| "delete": ("DELETE", False), | ||
| "mget": ("MGET", True), | ||
| "mset": ("MSET", False), | ||
| "incr": ("INCR", False), | ||
| "decr": ("DECR", False), | ||
| "exists": ("EXISTS", True), | ||
| "expire": ("EXPIRE", False), | ||
| "setex": ("SET", False), | ||
| "setnx": ("SET", False), | ||
| "getset": ("GET", True), | ||
| "hget": ("GET", True), | ||
| "hset": ("SET", False), | ||
| "hdel": ("DELETE", False), | ||
| "sadd": ("SET", False), | ||
| "srem": ("DELETE", False), | ||
| "lpush": ("SET", False), | ||
| "rpush": ("SET", False), | ||
| "lpop": ("GET", True), | ||
| "rpop": ("GET", True), | ||
| } | ||
|
|
||
| for method_name, (operation, is_read) in methods_to_patch.items(): | ||
| original_method = getattr(redis.Redis, method_name, None) | ||
| if original_method is None: | ||
| continue | ||
|
|
||
| self._original_redis_methods[method_name] = original_method | ||
|
|
||
| def create_wrapper( | ||
| orig_method: Any, | ||
| op: CacheOperation, | ||
| check_hit: bool, # noqa: FBT001 | ||
| ) -> Any: | ||
| def wrapper(self_redis: Any, *args: Any, **kwargs: Any) -> Any: | ||
| start = time.perf_counter() | ||
| result = orig_method(self_redis, *args, **kwargs) | ||
| duration = time.perf_counter() - start | ||
|
|
||
| key = args[0] if args else kwargs.get("name", "unknown") | ||
| hit = None | ||
| if check_hit: | ||
| hit = result is not None | ||
|
|
||
| tracker = _get_tracker() | ||
| if tracker: | ||
| tracker._record_operation( # noqa: SLF001 | ||
| operation=op, | ||
| key=key, | ||
| hit=hit, | ||
| duration=duration, | ||
| backend="redis", | ||
| ) | ||
|
|
||
| return result | ||
|
|
||
| return wrapper | ||
|
|
||
| setattr( | ||
| redis.Redis, | ||
| method_name, | ||
| create_wrapper(original_method, operation, is_read), # type: ignore[arg-type] | ||
| ) | ||
|
|
||
| redis.Redis._debug_toolbar_patched = True # type: ignore[attr-defined] # noqa: SLF001 | ||
|
|
There was a problem hiding this comment.
The monkey-patching of Redis and memcached client methods modifies global class objects which could cause issues in multi-threaded environments or when multiple CacheTracker instances are used concurrently. While there's a _patch_lock to protect the patching itself, once patched, all Redis/memcached operations globally will record to whichever tracker is active via _get_tracker().
Potential issues:
- If stop_tracking() is called while another request is still using the patched methods, tracking will stop for both
- Multiple panels/trackers could interfere with each other
Consider adding guards or documentation about thread safety limitations, or implement a more robust patching strategy that doesn't rely on global state.
| try: | ||
| from mako.template import Template as MakoTemplate | ||
| except ImportError: | ||
| MakoTemplate = Any # type: ignore[misc] |
There was a problem hiding this comment.
Import of 'MakoTemplate' is not used.
| try: | |
| from mako.template import Template as MakoTemplate | |
| except ImportError: | |
| MakoTemplate = Any # type: ignore[misc] |
Summary
Implements Phase 10 of the debug toolbar development - adding 5 new panels for enhanced debugging capabilities:
New Panels
Features
examples/directoryTest Results
Test Plan
make ci- all checks passmake test- 240 tests passmake type-check🤖 Generated with Claude Code