-
Notifications
You must be signed in to change notification settings - Fork 2
Add opt-in debug logging configuration for Jupyter server #31
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?
Conversation
Adds configurable debug logging to help troubleshoot kernel communication and server issues without impacting production performance. Changes: - Add environment variable-based debug logging configuration - Make Application.log_level conditional (DEBUG or INFO) - Add enhanced logging_config for tornado, jupyter_server, and jupyter_client - Add opt-in Session.debug for ZMQ message flow logging - Add environment variable documentation to README.md All features are opt-in via environment variables: - DEEPNOTE_ENABLE_DEBUG_LOGGING: Enables DEBUG-level logs (default: false/INFO) - DEEPNOTE_ENABLE_ZMQ_DEBUG: Enables ZMQ message debugging (default: false) This ensures production-safe defaults with no performance impact when disabled.
📝 WalkthroughWalkthroughDocumentation and configuration for environment-driven debug logging are added. The README documents two new environment variables: Pre-merge checks✅ Passed checks (3 passed)
Comment |
|
📦 Python package built successfully!
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
=======================================
Coverage 72.88% 72.88%
=======================================
Files 93 93
Lines 5142 5142
Branches 754 754
=======================================
Hits 3748 3748
Misses 1150 1150
Partials 244 244
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
README.md(1 hunks)deepnote_core/resources/jupyter/jupyter_server_config.py(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build and push artifacts for Python 3.12
- GitHub Check: Test - Python 3.13
- GitHub Check: Test - Python 3.11
- GitHub Check: Build and push artifacts for Python 3.11
- GitHub Check: Build and push artifacts for Python 3.13
- GitHub Check: Test - Python 3.10
- GitHub Check: Build and push artifacts for Python 3.10
- GitHub Check: Build and push artifacts for Python 3.9
- GitHub Check: Typecheck - 3.9
🔇 Additional comments (3)
deepnote_core/resources/jupyter/jupyter_server_config.py (2)
13-17: LGTM!Environment variable parsing and log level assignment are correct.
423-431: LGTM!Logging configuration correctly uses the string
log_levelvariable and targets appropriate loggers for Jupyter debugging.README.md (1)
66-90: LGTM!Documentation accurately describes the debug logging features, defaults, and usage. Examples are correct and the performance warning is appropriate.
| # Conditional based on DEEPNOTE_ENABLE_DEBUG_LOGGING environment variable | ||
| c.Application.log_level = 10 if debug_logging_enabled else 20 # DEBUG or INFO |
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.
Inconsistency: log_level variable defined as string but integers used here.
Line 17 defines log_level = "DEBUG" if debug_logging_enabled else "INFO" (string), but this line uses integers directly. Either use the string variable consistently or define an integer variable.
Apply this diff for consistency:
-c.Application.log_level = 10 if debug_logging_enabled else 20 # DEBUG or INFO
+c.Application.log_level = log_level🤖 Prompt for AI Agents
In deepnote_core/resources/jupyter/jupyter_server_config.py around lines 35-36,
the code sets c.Application.log_level using integer literals while earlier (line
17) a string variable log_level = "DEBUG" if debug_logging_enabled else "INFO"
is defined; update this line to use the existing log_level variable consistently
by mapping the string to the corresponding integer (e.g., DEBUG->10, INFO->20)
or change the earlier variable to an integer value and use that variable here so
both places use the same type and value source.
| # Enable ZMQ message flow debugging for troubleshooting kernel communication | ||
| # Set DEEPNOTE_ENABLE_ZMQ_DEBUG=true to enable detailed ZMQ message logging | ||
| if os.getenv("DEEPNOTE_ENABLE_ZMQ_DEBUG", "false").lower() == "true": | ||
| c.Session.debug = True |
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
Optional: Extract to variable for consistency.
This works correctly, but for consistency with lines 16-17, consider extracting the environment check to a variable.
# Debug output in the Session
# Default: False
-# Enable ZMQ message flow debugging for troubleshooting kernel communication
-# Set DEEPNOTE_ENABLE_ZMQ_DEBUG=true to enable detailed ZMQ message logging
-if os.getenv("DEEPNOTE_ENABLE_ZMQ_DEBUG", "false").lower() == "true":
- c.Session.debug = True
+# Enable ZMQ message flow debugging for troubleshooting kernel communication
+# Set DEEPNOTE_ENABLE_ZMQ_DEBUG=true to enable detailed ZMQ message logging
+zmq_debug_enabled = os.getenv("DEEPNOTE_ENABLE_ZMQ_DEBUG", "false").lower() == "true"
+if zmq_debug_enabled:
+ c.Session.debug = TrueCommittable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In deepnote_core/resources/jupyter/jupyter_server_config.py around lines 837 to
840, extract the environment check into a named variable (e.g.,
enable_zmq_debug) for consistency with the earlier pattern (lines 16-17),
assigning it from os.getenv("DEEPNOTE_ENABLE_ZMQ_DEBUG", "false").lower() ==
"true", then use that variable in the if statement to set c.Session.debug =
True; keep the existing behavior and value comparison but centralize the env
read into the variable.
|
🚀 Review App Deployment Started
|
Summary
Adds opt-in debug logging configuration for the Jupyter server to help troubleshoot kernel communication and server issues without impacting production performance.
Changes
DEEPNOTE_ENABLE_DEBUG_LOGGING=trueSession.debugviaDEEPNOTE_ENABLE_ZMQ_DEBUGfor detailed ZMQ message flow loggingEnvironment Variables
DEEPNOTE_ENABLE_DEBUG_LOGGINGfalse(INFO level)DEEPNOTE_ENABLE_ZMQ_DEBUGfalseProduction Safety
Testing
To test debug logging:
```bash
DEEPNOTE_ENABLE_DEBUG_LOGGING=true deepnote-toolkit server
```
To test ZMQ debugging:
```bash
DEEPNOTE_ENABLE_ZMQ_DEBUG=true deepnote-toolkit server
```
Related
This PR provides the logging foundation for the execution tracking PR (coming next) which will help debug stuck executions.
Files Modified
Checklist
Summary by CodeRabbit
Documentation
New Features