-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Add colorization based on health level for panels and toolbar #2178
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 all commits
85b08f0
427ec23
0c15dde
2f8c3d4
aa96318
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||||||
| from django.utils.functional import classproperty | ||||||||||
|
|
||||||||||
| from debug_toolbar import settings as dt_settings | ||||||||||
| from debug_toolbar.utils import get_name_from_obj | ||||||||||
| from debug_toolbar.utils import HealthLevel, get_name_from_obj | ||||||||||
|
|
||||||||||
|
|
||||||||||
| class Panel: | ||||||||||
|
|
@@ -129,6 +129,19 @@ def scripts(self): | |||||||||
| """ | ||||||||||
| return [] | ||||||||||
|
|
||||||||||
| @property | ||||||||||
| def health_level(self): | ||||||||||
| """ | ||||||||||
| Returns the health level of the panel as a `ToolbarHealthLevel` enum value. | ||||||||||
|
|
||||||||||
| This property is used by the toolbar to determine the overall health status of each panel. | ||||||||||
| The default implementation returns `ToolbarHealthLevel.NONE`, indicating no issues. | ||||||||||
|
Comment on lines
+137
to
+138
Member
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.
Suggested change
|
||||||||||
|
|
||||||||||
| Subclasses should override this property to provide custom health logic, returning | ||||||||||
| `ToolbarHealthLevel.WARNING` or `ToolbarHealthLevel.ERROR` as appropriate based on panel-specific conditions. | ||||||||||
|
Comment on lines
+140
to
+141
Member
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. The enum might get more entries later, you might not want to list them. |
||||||||||
| """ | ||||||||||
| return HealthLevel.NONE | ||||||||||
|
|
||||||||||
| # Panel early initialization | ||||||||||
|
|
||||||||||
| @classmethod | ||||||||||
|
|
||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,7 +3,7 @@ | |||||||||||||||||
| from django.utils.translation import gettext_lazy as _ | ||||||||||||||||||
|
|
||||||||||||||||||
| from debug_toolbar.panels import Panel | ||||||||||||||||||
| from debug_toolbar.utils import is_processable_html_response | ||||||||||||||||||
| from debug_toolbar.utils import HealthLevel, is_processable_html_response | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| class FormParser(HTMLParser): | ||||||||||||||||||
|
|
@@ -92,6 +92,16 @@ def nav_subtitle(self): | |||||||||||||||||
| else: | ||||||||||||||||||
| return "" | ||||||||||||||||||
|
|
||||||||||||||||||
| @property | ||||||||||||||||||
| def health_level(self): | ||||||||||||||||||
| """ | ||||||||||||||||||
| Return the health level of the panel based on the alerts. | ||||||||||||||||||
| """ | ||||||||||||||||||
| if not self.get_stats().get("alerts"): | ||||||||||||||||||
| return HealthLevel.NONE | ||||||||||||||||||
|
|
||||||||||||||||||
| return HealthLevel.CRITICAL | ||||||||||||||||||
|
Comment on lines
+100
to
+103
Member
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. Let's keep it positive ☮️
Suggested change
|
||||||||||||||||||
|
|
||||||||||||||||||
| def add_alert(self, alert): | ||||||||||||||||||
| self.alerts.append(alert) | ||||||||||||||||||
|
|
||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,7 +19,7 @@ | |
| is_select_query, | ||
| reformat_sql, | ||
| ) | ||
| from debug_toolbar.utils import render_stacktrace | ||
| from debug_toolbar.utils import HealthLevel, render_stacktrace | ||
|
|
||
|
|
||
| def get_isolation_level_display(vendor, level): | ||
|
|
@@ -186,6 +186,21 @@ def title(self): | |
| count, | ||
| ) % {"count": count} | ||
|
|
||
| @property | ||
| def health_level(self): | ||
| """ | ||
| Return the health level of the SQL panel. | ||
| This is determined by the number of slow queries recorded. | ||
| """ | ||
|
Comment on lines
+191
to
+194
Member
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. I don't love overwriting docstrings. PEP257 would have you document the behavior of a function, not its implementation. And the behavior usually doesn't change in subclasses, but only its implementation does. |
||
| stats = self.get_stats() | ||
| slow_queries = len([1 for q in stats.get("queries", []) if q.get("is_slow")]) | ||
| if slow_queries > 10: | ||
| return HealthLevel.CRITICAL | ||
| elif slow_queries > 0: | ||
| return HealthLevel.WARNING | ||
|
Comment on lines
+195
to
+200
Member
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. The current implementation is susceptible to the number of queries executed on a page. A mean or median threshold might be more reliable. Also, this is a great usecase for |
||
|
|
||
| return super().health_level | ||
|
|
||
| template = "debug_toolbar/panels/sql.html" | ||
|
|
||
| @classmethod | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,13 @@ | |
| --djdt-button-border-color: var(--djdt-table-border-color); | ||
| --djdt-pre-border-color: var(--djdt-table-border-color); | ||
| --djdt-raw-border-color: var(--djdt-table-border-color); | ||
|
|
||
| --djdt-health-background-color-1: #4b3f1b; | ||
| --djdt-health-color-1: #ffe761; | ||
| --djdt-health-border-color-1: #ffcc00; | ||
| --djdt-health-background-color-2: #5a2327; | ||
| --djdt-health-color-2: #ffb3b3; | ||
| --djdt-health-border-color-2: #ff0000; | ||
| } | ||
|
|
||
| #djDebug[data-theme="dark"] { | ||
|
|
@@ -56,6 +63,13 @@ | |
| --djdt-button-border-color: var(--djdt-table-border-color); | ||
| --djdt-pre-border-color: var(--djdt-table-border-color); | ||
| --djdt-raw-border-color: var(--djdt-table-border-color); | ||
|
|
||
| --djdt-health-background-color-1: #4b3f1b; | ||
| --djdt-health-color-1: #ffe761; | ||
| --djdt-health-border-color-1: #ffcc00; | ||
| --djdt-health-background-color-2: #5a2327; | ||
| --djdt-health-color-2: #ffb3b3; | ||
| --djdt-health-border-color-2: #ff0000; | ||
| } | ||
|
|
||
| /* Debug Toolbar CSS Reset, adapted from Eric Meyer's CSS Reset */ | ||
|
|
@@ -377,6 +391,29 @@ | |
| animation: spin 2s linear infinite; | ||
| } | ||
|
|
||
| /* Panel and Toolbar hidden button health states */ | ||
| #djDebug .djdt-health-1 { | ||
|
Member
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. All browsers support nesting these days. So one |
||
| /* The background can be shadowed by #djDebugToolbar li.djdt-active a:hover */ | ||
| background: var(--djdt-health-background-color-1) !important; | ||
| color: var(--djdt-health-color-1); | ||
| } | ||
|
|
||
| #djDebug .djdt-health-2 { | ||
| /* The background can be shadowed by #djDebugToolbar li.djdt-active a:hover */ | ||
| background: var(--djdt-health-background-color-2) !important; | ||
| color: var(--djdt-health-color-2); | ||
| } | ||
|
|
||
| #djDebug .djdt-toolbarhandle-health-1 { | ||
| /* The border-color is shadowed by the default #djShowToolBarButton border */ | ||
| border-color: var(--djdt-health-border-color-1) !important; | ||
| } | ||
|
|
||
| #djDebug .djdt-toolbarhandle-health-2 { | ||
| /* The border-color is shadowed by the default #djShowToolBarButton border */ | ||
| border-color: var(--djdt-health-border-color-2) !important; | ||
| } | ||
|
|
||
| @keyframes spin { | ||
| 0% { | ||
| transform: rotate(0deg); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,6 +20,7 @@ | |
|
|
||
| from debug_toolbar import APP_NAME, settings as dt_settings | ||
| from debug_toolbar.store import get_store | ||
| from debug_toolbar.utils import HealthLevel | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
@@ -72,6 +73,17 @@ def csp_nonce(self): | |
| """ | ||
| return getattr(self.request, "csp_nonce", None) | ||
|
|
||
| @property | ||
| def health_level(self): | ||
| """ | ||
| Return the maximum health level across all panels. | ||
| This is used to color the toolbar hidden button. | ||
| """ | ||
| if not self.panels: | ||
| return HealthLevel.NONE | ||
|
|
||
| return max(panel.health_level for panel in self.enabled_panels) | ||
|
Comment on lines
+82
to
+85
Member
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. EAFP: Just try to return the max and catch the |
||
|
|
||
| def get_panel_by_id(self, panel_id): | ||
| """ | ||
| Get the panel with the given id, which is the class name by default. | ||
|
|
||
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.
I know we currently have a wild mix, but it's never too late to write docstrings in the present tense imperative mood.