-
Notifications
You must be signed in to change notification settings - Fork 16
Docs: Adding In-App Logging System documentation #393
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
Conversation
WalkthroughAdds comprehensive logging documentation and three new logging configuration fields controlling in-memory buffer size, batch deletion size, and output formatting for the MostroP2P app. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI/Main Isolate
participant Logger as Logger (singleton)
participant Isolate as Background Isolate
participant Storage as File Storage
participant Sender as Log Sender (background)
rect rgb(230,245,255)
UI->>Logger: logger.i / logger.e / logger.d
Logger-->>Logger: cleanMessage() -> sanitize & redact
Logger-->>Storage: append entry (in-memory FIFO + persistent file)
end
rect rgb(245,255,230)
Isolate->>Isolate: generate log
Isolate->>Isolate: IsolateLogOutput -> serialize msg
Isolate->>Logger: send to main-isolate receiver
Logger-->>Storage: append entry
end
rect rgb(255,250,230)
alt buffer > logMaxEntries
Logger->>Logger: remove oldest entries (batch delete logBatchDeleteSize)
end
Sender->>Logger: read batch for upload
Sender->>Storage: mark uploaded / rotate
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
Pre-merge checks✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
🧹 Nitpick comments (2)
docs/LOGGING_IMPLEMENTATION.md (2)
42-49: Optional: Add language identifier to fenced code block.The markdown linter suggests adding a language identifier to the fenced code block. While this is ASCII art rather than code, you could add
textas the language identifier to satisfy the linter.🔎 Suggested fix
-``` +```text Main Isolate: logger (singleton) → MemoryLogOutput → UI (LogsScreen)
1-6: Optional: Clarify planning status upfront.The document doesn't indicate this is planning documentation until line 203. Consider adding a status notice near the top (after line 5) to set expectations immediately.
🔎 Suggested addition
## Overview Implementation of a comprehensive logging system for MostroP2P mobile app with in-memory capture, background isolate support, privacy sanitization, and export capabilities. > **Status**: Phase 1 - Planning Complete > This document describes the planned logging system architecture. Implementation is pending.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/LOGGING_IMPLEMENTATION.mdlib/core/config.dart
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{dart,flutter}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{dart,flutter}: Runflutter analyzeafter any code change - Mandatory before commits to ensure zero linting issues
Runflutter testafter any code change - Mandatory before commits to ensure all unit tests pass
Files:
lib/core/config.dart
**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.dart: Use Riverpod for all state management - encapsulate business logic in Notifiers and access data only through repository classes
All code comments must be in English - use clear, concise English for variable names, function names, and comments
Always checkmountedbefore using BuildContext after async operations to prevent errors on disposed widgets
Useconstconstructors where possible for better performance and immutability
Remove unused imports and dependencies to maintain code cleanliness and reduce build size
**/*.dart: Application code should be organized underlib/, grouped by domain withlib/features/<feature>/structure, shared utilities inlib/shared/, dependency wiring inlib/core/, and services inlib/services/
Persistence, APIs, and background jobs should live inlib/data/andlib/background/; generated localization output must be inlib/generated/and must stay untouched
Applyflutter format .to enforce canonical Dart formatting (two-space indentation, trailing commas) before committing
Resolve every analyzer warning in Dart code
Name Riverpod providers using the<Feature>Provideror<Feature>Notifierconvention
Localize all user-facing strings via ARB files and access them withS.of(context)rather than hard-coded literals
Files:
lib/core/config.dart
🧠 Learnings (5)
📓 Common learnings
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:114-118
Timestamp: 2025-09-17T20:45:07.179Z
Learning: For the MostroP2P/mobile project, grunch prefers to separate documentation PRs from code improvement PRs. Documentation PRs should focus on explaining how the current system works, while code fixes and improvements should be handled in separate PRs.
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
📚 Learning: 2025-09-17T20:45:32.468Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
Applied to files:
docs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-09-17T20:45:07.179Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:114-118
Timestamp: 2025-09-17T20:45:07.179Z
Learning: For the MostroP2P/mobile project, grunch prefers to separate documentation PRs from code improvement PRs. Documentation PRs should focus on explaining how the current system works, while code fixes and improvements should be handled in separate PRs.
Applied to files:
docs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-05-08T16:29:52.154Z
Learnt from: chebizarro
Repo: MostroP2P/mobile PR: 74
File: lib/background/background.dart:74-77
Timestamp: 2025-05-08T16:29:52.154Z
Learning: In the Mostro Mobile background service architecture, events aren't stored by the background process. Instead, the background service only checks if events exist in the eventStore and sends notifications for new ones, while the foreground process is responsible for storing and processing events. This is an intentional design decision to separate concerns.
Applied to files:
docs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-10-14T21:12:06.887Z
Learnt from: Catrya
Repo: MostroP2P/mobile PR: 327
File: lib/features/order/notfiers/abstract_mostro_notifier.dart:141-154
Timestamp: 2025-10-14T21:12:06.887Z
Learning: In the MostroP2P mobile codebase, the notification system uses a two-layer localization pattern: providers/notifiers (without BuildContext access) call `showCustomMessage()` with string keys (e.g., 'orderTimeoutMaker', 'orderCanceled'), and the UI layer's `NotificationListenerWidget` has a switch statement that maps these keys to localized strings using `S.of(context)`. This architectural pattern properly separates concerns while maintaining full localization support for all user-facing messages.
Applied to files:
lib/core/config.dart
🪛 markdownlint-cli2 (0.18.1)
docs/LOGGING_IMPLEMENTATION.md
13-13: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
18-18: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
28-28: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
lib/core/config.dart (1)
46-49: Logger configuration constants look good for Phase 1 planning.The constants are well-defined and align with the documentation in
LOGGING_IMPLEMENTATION.md. The mutablefullLogsInfofield allows runtime configuration switching between simple and pretty-printed logs, which is appropriate for this use case.
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
🧹 Nitpick comments (1)
docs/LOGGING_IMPLEMENTATION.md (1)
42-49: Specify language identifier for fenced code block.The architecture diagram block (line 42) is missing a language identifier. Add one even for pseudo-code or plain-text diagrams to satisfy markdown linting standards.
🔎 Proposed fix
-``` +``` Main Isolate: logger (singleton) → MemoryLogOutput → UI (LogsScreen)(Alternatively, use
```text,```plaintext, or```diagramif your renderer supports it.)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/LOGGING_IMPLEMENTATION.md
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:114-118
Timestamp: 2025-09-17T20:45:07.179Z
Learning: For the MostroP2P/mobile project, grunch prefers to separate documentation PRs from code improvement PRs. Documentation PRs should focus on explaining how the current system works, while code fixes and improvements should be handled in separate PRs.
📚 Learning: 2025-09-17T20:45:32.468Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:176-183
Timestamp: 2025-09-17T20:45:32.468Z
Learning: For PR #306 in MostroP2P/mobile repository: This is a documentation-only PR intended to explain how the current requestId system works, not to fix any bugs or issues in the code. The documentation should accurately reflect existing behavior.
Applied to files:
docs/LOGGING_IMPLEMENTATION.md
📚 Learning: 2025-09-17T20:45:07.179Z
Learnt from: grunch
Repo: MostroP2P/mobile PR: 306
File: docs/architecture/REQUEST_ID_ANALYSIS.md:114-118
Timestamp: 2025-09-17T20:45:07.179Z
Learning: For the MostroP2P/mobile project, grunch prefers to separate documentation PRs from code improvement PRs. Documentation PRs should focus on explaining how the current system works, while code fixes and improvements should be handled in separate PRs.
Applied to files:
docs/LOGGING_IMPLEMENTATION.md
🪛 markdownlint-cli2 (0.18.1)
docs/LOGGING_IMPLEMENTATION.md
13-13: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
18-18: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
23-23: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
28-28: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
42-42: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (1)
- GitHub Check: build
grunch
left a comment
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.
LGTM
Summary
Phase 1 of in-app logging system implementation. This PR establishes the implementation roadmap and adds initial configuration.
Changes
docs/LOGGING_IMPLEMENTATION.md- Complete implementation plan with architecture and phased approachlib/core/config.dart- Logger configuration constants (logMaxEntries, logBatchDeleteSize, fullLogsInfo)Why This Approach?
Singleton logger pattern with centralized output management provides:
Trade-off: Requires updating ~30+ files to replace
Logger()calls, but ensures complete log visibility and privacy by design.Documentation
Full implementation details in
docs/LOGGING_IMPLEMENTATION.mdSummary by CodeRabbit
Documentation
New Features
✏️ Tip: You can customize this high-level summary in your review settings.