Skip to content

Conversation

@ovitrif
Copy link
Collaborator

@ovitrif ovitrif commented Jan 22, 2026

Closes #655

Summary

Both APP and LDK logs now write to the same file in chronological order.

Changes

  • Created shared LogSaver instance used by both APP and LDK loggers
  • Unified log file path: bitkit_<timestamp>.log
  • Logs automatically in chronological order due to shared file
  • File writes serialized via single-threaded coroutine queue
  • APP/LDK tag prefixes distinguish log sources

Preview

Testing

  1. Uninstal previous app, build & open app, reset & restore wallet
  2. Export logs via Settings > Advanced > Lightning Connections > "Export…" button
  3. Extract & open the most recent logs files in a text editor
  4. ✔️ Verify both app and ldk-node logs were recorded in chrono order

Or, as a dev just go to device explorer and the app's data folder and open that file in Android Studio to check.

Copy link
Collaborator Author

@ovitrif ovitrif left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@claude attend review comments.

@ovitrif ovitrif self-assigned this Jan 22, 2026
@claude

This comment has been minimized.

@ovitrif

This comment was marked as resolved.

@ovitrif ovitrif changed the title feat: merge app and ldk logs into unified file feat: save app and ldk logs to same file Jan 22, 2026
@claude

This comment has been minimized.

@ovitrif ovitrif changed the title feat: save app and ldk logs to same file feat: merge app and ldk logs into same file Jan 22, 2026
@ovitrif ovitrif changed the title feat: merge app and ldk logs into same file feat: save app and ldk logs to same file Jan 22, 2026
@ovitrif ovitrif marked this pull request as ready for review January 22, 2026 19:23
ovitrif

This comment was marked as resolved.

@ovitrif ovitrif enabled auto-merge January 22, 2026 19:25
@claude

This comment has been minimized.

@ovitrif ovitrif force-pushed the claude/issue-655-20260122-1114 branch 2 times, most recently from ed51c18 to bfa9a1f Compare January 22, 2026 20:58
github-actions bot and others added 3 commits January 22, 2026 22:46
- Both APP and LDK loggers now write to same file (bitkit_<timestamp>.log)
- Logs are automatically in chronological order due to shared file
- File writes serialized via single-threaded coroutine queue
- APP/LDK tag prefixes distinguish log sources
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ovitrif ovitrif force-pushed the claude/issue-655-20260122-1114 branch from b4989ce to 778ca10 Compare January 22, 2026 21:46

val Logger = AppLogger()

class AppLogger(private val source: LogSource = LogSource.Bitkit) {
Copy link
Member

@jvsena42 jvsena42 Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this source parameter is not used anymore

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: could make LoggerImpl

@ovitrif
Copy link
Collaborator Author

ovitrif commented Jan 23, 2026

@claude address review points of jvsena

@claude

This comment has been minimized.

Co-authored-by: Ovi Trif <ovitrif@users.noreply.github.com>
private const val TAG = "Logger"
private var saver: LogSaver? = null

fun getOrCreateSaver(): LogSaver {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this method is called by two classes on init(). could have a race condittion

Copy link
Member

@jvsena42 jvsena42 Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but as far as I can see, it would just replace the instance. WDYT?

Copy link
Member

@jvsena42 jvsena42 Jan 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Ldk LogSource is not used anymore, could be removed

Comment on lines 61 to 66
fun reset() {
warn("Wiping entire logs directory…", context = TAG)
runCatching { Env.logDir.deleteRecursively() }
saver = null
delegate = runCatching { createDelegate() }.getOrNull()
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on app wipe, reset() could race with getOrCreateSaver()

Copy link
Member

@jvsena42 jvsena42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth take a look on the race condition comments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Merge logs of app and ldk in same files, in chrono order

3 participants