Increase logs #657
Increase logs #657
Conversation
📝 WalkthroughWalkthroughThis pull request refactors several modules and adjusts logging and import organization. In the API layer, the HoprdAPI now accesses all request and response objects through dedicated namespaces with updated type annotations and logging calls. Minor formatting improvements and newline additions were applied to request object files. The core modules update import paths and add logging details, while the Node class now provides a property for contextual log parameters. The subgraph package is reorganized by introducing a new Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant GP as GraphQLProvider
participant HTTP as aiohttp
C->>GP: get(queryKey)
GP->>GP: _initialize_query(queryKey)
GP->>GP: _load_query(query_file)
GP->>HTTP: _execute(query)
HTTP-->>GP: Response or Error
GP->>C: Return data or error
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
ct-app/core/node.py (2)
61-65: Great addition of a structured log parameters property.The new
log_base_paramsproperty creates a consistent dictionary of contextual information (URL, address, peer_id) that's used across all log messages, which improves log analysis and correlation.Consider adding null checks for
self.addressto avoid potential AttributeError ifself.addressis None - though the current implementation withgetattrhandles this well.
69-74: Improved log message for safe address retrieval.The logging now includes the node's contextual information through
log_base_params, making it easier to correlate logs.Consider combining the nested if statements as suggested by static analysis:
- if self._safe_address is None: - if info := await self.api.node_info(): + if self._safe_address is None and (info := await self.api.node_info()):🧰 Tools
🪛 Ruff (0.8.2)
69-70: Use a single
ifstatement instead of nestedifstatementsCombine
ifstatements usingand(SIM102)
ct-app/core/subgraph/graphql_provider.py (1)
65-83: Consider reusing the session and handling non-2xx status codes.
- Initializing a fresh
ClientSessionfor each request adds overhead. Reusing the session for multiple queries often improves performance.- The current implementation does not explicitly handle non-2xx HTTP status codes beyond exceptions. Logging or raising an error for unsuccessful status codes may help with debugging.
ct-app/core/api/hoprd_api.py (1)
55-60: Consider usinglogger.exceptionfor richer tracebacks.Replacing
logger.exceptionwithlogger.errorremoves the stack trace. If you need detailed debugging info forOSErroror unknown exceptions, reverting tologger.exceptionmight be helpful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
ct-app/core/api/hoprd_api.py(9 hunks)ct-app/core/api/request_objects.py(2 hunks)ct-app/core/core.py(3 hunks)ct-app/core/node.py(17 hunks)ct-app/core/subgraph/__init__.py(1 hunks)ct-app/core/subgraph/graphql_provider.py(1 hunks)ct-app/core/subgraph/mode.py(1 hunks)ct-app/core/subgraph/providers.py(2 hunks)ct-app/test/model/test_subgraph_type.py(1 hunks)helm/values-prod-blue.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- ct-app/core/api/request_objects.py
- ct-app/test/model/test_subgraph_type.py
- ct-app/core/subgraph/mode.py
🧰 Additional context used
🪛 Ruff (0.8.2)
ct-app/core/node.py
69-70: Use a single if statement instead of nested if statements
Combine if statements using and
(SIM102)
ct-app/core/subgraph/graphql_provider.py
50-50: Do not use mutable data structures for argument defaults
Replace with None; initialize within function
(B006)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Build and push container image
🔇 Additional comments (30)
helm/values-prod-blue.yaml (1)
13-13: Increased logging for p2p transport module.This change enhances the logging configuration by enabling debug-level logs specifically for the hopr_transport_p2p component while maintaining info-level for other components.
Since this is a production environment, please confirm this increased verbosity is intentional and acceptable from a performance perspective, as debug logs can increase log volume significantly.
ct-app/core/subgraph/__init__.py (2)
2-2: Updated import source for ProviderError.The import for
ProviderErrorhas been moved fromproviderstographql_provider, reflecting the reorganization of the codebase. TheGraphQLProviderclass is now also imported from the same module.
7-14: Reorganized exported entities in__all__.The
__all__list has been updated to includeGraphQLProviderand reintroduceModeandURL, which improves clarity about which elements are part of the public API.ct-app/core/core.py (3)
8-8: Updated import path for GraphQLProvider.The import for
GraphQLProviderhas been changed fromcore.subgraph.providerstocore.subgraph, reflecting the reorganization of the codebase structure.
88-88: Added informative log message for subgraph rotation.A new log message has been added at the info level to indicate when subgraphs are being rotated, improving observability of this operation.
164-164: Improved log formatting with consistent spacing.Added proper spacing between the log message and the dictionary parameters for better readability.
ct-app/core/node.py (21)
84-85: Enhanced log message with contextual information.Updated log message to include the node's base parameters, improving observability.
89-90: Improved debug log with context parameters.The log message now includes node context via
log_base_paramsand has proper whitespace formatting.
103-103: Added context to warning log about node reachability.Enhanced warning message with node identification details from
log_base_params.
105-105: Enhanced address retrieval failure log.Added contextual information to the warning about missing address.
119-121: Improved balance retrieval logging.Added node context to the warning message about balance retrieval failure.
124-125: Enhanced balance retrieval success log.The log now includes both balance information and node context parameters.
152-153: Improved log for channel opening process.Added node context to channel opening log, making it easier to track which node is performing channel operations.
169-170: Added context to incoming channel closure log.Improved log message with node identification information.
187-188: Enhanced pending channel closure log.Added node context to the log message about pending channel closure.
228-229: Improved old channel closure log message.Added context parameters to the log for dangling channel closure.
251-253: Enhanced channel funding log message.Improved the log with additional context about the node performing the operation.
268-269: Minor formatting improvement in peer creation.Reformatted the set comprehension for better readability.
278-279: Added context to peer scanning log.Enhanced the log message about reachable peers with node identification information.
314-315: Improved channel scanning log message.Added node context to the channel scanning log for better observability.
330-330: Extracted channel funds value with more explicit nested access.Improved the code to use dict.get() with nested access pattern for better robustness.
332-333: Enhanced logging for channel funds retrieval.Added node context to the log message about funds in outgoing channels.
343-349: Added important warning for potential inbox overflow.Added a new warning when the message count is a power of 2, which could indicate a full inbox. This is a good defensive check.
Could you confirm that checking for powers of 2 (
count && !(count & (count - 1))) is the correct way to detect a full inbox? This seems like it would trigger for counts of 1, 2, 4, 8, 16, etc. - is this expected?
354-355: Improved error logging for message parsing.Enhanced the exception log with node context for better troubleshooting.
361-364: Improved metrics labeling with consistent formatting.Reformatted the metrics labels for better readability and consistency.
368-368: Minor code style improvement.Simplified the code by using
await MessageQueue().get_async()directly.
385-388: Improved message sending code formatting.Enhanced readability of the message sending code with consistent indentation and spacing.
ct-app/core/subgraph/providers.py (2)
1-1: Looks good!Importing the
GraphQLProviderfrom the new module is straightforward and aligns with the refactored structure.
28-28: Verify that the GraphQL type matches.
'$id_in: [Bytes!] = [""]'suggests a custom scalar or specific schema requirement. Please ensure the subgraph schema includes aBytes!type for this parameter.ct-app/core/api/hoprd_api.py (1)
10-11: Consolidated import structure looks clean.Switching to namespaced imports (
request,response) improves maintainability and readability.
Summary by CodeRabbit
New Features
GraphQLProviderclass for managing GraphQL queries with enhanced error handling and metrics tracking.Refactor
Chores