Skip to content
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

epic: Cortex.cpp logs #1153

Closed
4 of 6 tasks
dan-homebrew opened this issue Sep 8, 2024 · 5 comments · Fixed by #1173
Closed
4 of 6 tasks

epic: Cortex.cpp logs #1153

dan-homebrew opened this issue Sep 8, 2024 · 5 comments · Fixed by #1173
Assignees
Labels
category: app shell Installer, updaters, distributions type: epic A major feature or initiative

Comments

@dan-homebrew
Copy link
Contributor

dan-homebrew commented Sep 8, 2024

Goal

  • Strong logging infra for Server and CLI
  • Users can find and submit logfiles easily in case of bugs
  • Logs should ideally not have PII for privacy (e.g. if published on Github)

Open Questions

  • Should we have separate logs for Server and CLI? (remove needs for UDS)
  • What is our log pruning strategy?

Tasklist

@dan-homebrew
Copy link
Contributor Author

dan-homebrew commented Sep 8, 2024

@nguyenhoangthuan99 I would like to raise a few issues on the Logs implementation, based on what I saw on Fri's demo:

Issue 1: UDS for combined log file?

  • I am very impressed with the UDS implementation - but does it add too much complexity?
  • I am afraid it will trigger some crazy Windows issue in the future
  • Is it better for us to have two log files (e.g. server and CLI)
  • Note: in Jan, we don't need the CLI log file

Issue 2: Configurable log file?

  • Given that cortex.cpp is typically used in Jan, can we have a configurable log location?
  • I believe we have an open issue for this

Issue 3: Logging parameters

  • We should not be saving log files by date/time (e.g. cortex-20120312-timestamp)
  • We should just have cortex.log (e.g. for server) and cortex-cli.log, that has a truncate length (e.g. 10000 lines)
  • We should research what is the best truncate length for this - there's likely a best practice somewhere
  • We should not end up storing a few GB of logs on a user's device

cc @vansangpfiev @namchuai @louis-jan thoughts pls

@nguyenhoangthuan99
Copy link
Contributor

Issue 1: UDS

  • Initially, UDS was planned as the connection method between cortex-js and cortex-cpp, especially when integrating with Jan, cortex-js, and cortex-cpp. However, now that cortex-js development has stopped, this plan can be reconsidered.
  • The idea of having separate log files for the server and CLI is a better solution, which I support. In the past, we opted for a single log file as cortex-js had done, but the new approach offers clearer separation.

Issue 2: Configurable logfile:

  • We will implement configurable log locations.
  • Previously, there was a discussion about separating logs for the CLI, cortex-cpp server, and engines (llamacpp, tensorrt-llm) or merging them into one cortex.log. However, this may no longer be necessary. A single log file should suffice since Jan users generally won’t need multiple logs. Additionally, the log file contains sufficient information, like filenames and line numbers, to identify issues.

Issue 3: Logging Params

  • We decided to use Trantor (a dependency of Drogon) for logging as it provides all the features we need, such as non-blocking, thread safety, and multi-stream file logging, without impacting system performance.
  • Trantor automatically formats logs for each server session, creating a new log file each time the server starts based on the date and timestamp. This makes debugging easier for general usecases. It also supports setting limits on log file size and the number of log files per session.
  • For Jan and cortex-cpp, which will frequently start and stop the server, we need to implement a custom logger based on Trantor's AsyncFileLogger. This logger will be added to work with cortex-cpp, cortex.llamacpp, and cortex.tensorrt-llm, need to make change to engines repos too.
    cc @vansangpfiev

@vansangpfiev
Copy link
Contributor

I agree

Issue 1: UDS

  • Initially, UDS was planned as the connection method between cortex-js and cortex-cpp, especially when integrating with Jan, cortex-js, and cortex-cpp. However, now that cortex-js development has stopped, this plan can be reconsidered.
  • The idea of having separate log files for the server and CLI is a better solution, which I support. In the past, we opted for a single log file as cortex-js had done, but the new approach offers clearer separation.

Issue 2: Configurable logfile:

  • We will implement configurable log locations.
  • Previously, there was a discussion about separating logs for the CLI, cortex-cpp server, and engines (llamacpp, tensorrt-llm) or merging them into one cortex.log. However, this may no longer be necessary. A single log file should suffice since Jan users generally won’t need multiple logs. Additionally, the log file contains sufficient information, like filenames and line numbers, to identify issues.

Issue 3: Logging Params

  • We decided to use Trantor (a dependency of Drogon) for logging as it provides all the features we need, such as non-blocking, thread safety, and multi-stream file logging, without impacting system performance.
  • Trantor automatically formats logs for each server session, creating a new log file each time the server starts based on the date and timestamp. This makes debugging easier for general usecases. It also supports setting limits on log file size and the number of log files per session.
  • For Jan and cortex-cpp, which will frequently start and stop the server, we need to implement a custom logger based on Trantor's AsyncFileLogger. This logger will be added to work with cortex-cpp, cortex.llamacpp, and cortex.tensorrt-llm, need to make change to engines repos too.
    cc @vansangpfiev

I agree. Just want to confirm that we will add the log configuration into .cortexrc, right?

@namchuai
Copy link
Contributor

namchuai commented Sep 9, 2024

I also agree, just have some thoughts.

  • Even if we separate the log file (server and cli), there is still chances that log can't be write, e.g. when user spawning multiple terminal and some operation takes long time to finish.
  • Separating log is harder to trace the issue. We might need to chronologically read between two log files to understand the issue.

@nguyenhoangthuan99 nguyenhoangthuan99 linked a pull request Sep 9, 2024 that will close this issue
@freelerobot freelerobot added the category: app shell Installer, updaters, distributions label Sep 9, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Completed in Jan & Cortex Sep 10, 2024
@nguyenhoangthuan99 nguyenhoangthuan99 moved this from Completed to In Progress in Jan & Cortex Sep 10, 2024
@dan-homebrew
Copy link
Contributor Author

I also agree, just have some thoughts.

  • Even if we separate the log file (server and cli), there is still chances that log can't be write, e.g. when user spawning multiple terminal and some operation takes long time to finish.
  • Separating log is harder to trace the issue. We might need to chronologically read between two log files to understand the issue.

For logs, do we have a standard Trantor line format (e.g. TIME Log message)?

  • If so, it shouldn't be difficult to do a merge of two logfiles to do analysis

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: app shell Installer, updaters, distributions type: epic A major feature or initiative
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants