Skip to content
This repository was archived by the owner on Jul 4, 2025. It is now read-only.

Conversation

@nguyenhoangthuan99
Copy link
Contributor

@nguyenhoangthuan99 nguyenhoangthuan99 commented Sep 11, 2024

Fix janhq/cortex.cpp#1054
Summary changed:

  • Add file_logger implementation as cortex-cpp did
  • Create an interface SetFileLogger(int max_log_lines, const std::string& log_path) so that when server in cortex-cpp load engines, it can enable file logger by calling this function

@nguyenhoangthuan99 nguyenhoangthuan99 marked this pull request as ready for review September 12, 2024 01:56
@vansangpfiev vansangpfiev changed the title Feat/redirect log feat: redirect log Sep 12, 2024
return false;
}
virtual void SetFileLogger() = 0;
virtual void SetFileLogger(int max_log_lines,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move this function to the end of the class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll remove it

constexpr char log_base_name[] = "logs/cortex";
constexpr char log_folder[] = "logs";
constexpr size_t max_log_file_size = 20000000; // ~20mb
constexpr size_t max_log_file_size = 20000000; // ~20mb

Choose a reason for hiding this comment

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

Question: is this settable via .cortexrc? (and is it per-engine specific?)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

New logger don't need that config, I'll remove max_log_file_size

@dan-menlo
Copy link

@nguyenhoangthuan99 I've approved this PR to unblock you.

However, can I verify my understanding of this PR, and the separation of concerns between the main cortex and llama.cpp-engine

  • cortex.llamacpp has its own file logger (?)
  • cortex.cpp uses SetFileLogger to pass the path to the llama.cpp engine, which then implements its own file logging

I have the following questions:

  • If we run multiple engines, will there be lock problems on the log file (or is it one process with cortex.cpp?)
  • If each engine implements its own logging, will there be a standardized logging format? (e.g. from Trantor verbose, with timestamps)

@nguyenhoangthuan99
Copy link
Contributor Author

nguyenhoangthuan99 commented Sep 12, 2024

@nguyenhoangthuan99 I've approved this PR to unblock you.

However, can I verify my understanding of this PR, and the separation of concerns between the main cortex and llama.cpp-engine

  • cortex.llamacpp has its own file logger (?)
  • cortex.cpp uses SetFileLogger to pass the path to the llama.cpp engine, which then implements its own file logging

I have the following questions:

  • If we run multiple engines, will there be lock problems on the log file (or is it one process with cortex.cpp?)
  • If each engine implements its own logging, will there be a standardized logging format? (e.g. from Trantor verbose, with timestamps)
  • When we run multiple engines, all engines still run in the same process, but they can in different thread, so the lock problem won't appear.
  • The File logger implementation is copied from cortex.cpp so every format still the same, this is a limitation happened when we separate into many repos, some part of code cannot be reused and we have to copy it manually

@nguyenhoangthuan99 nguyenhoangthuan99 merged commit b78d205 into main Sep 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

improve: redirect log to right position

4 participants